Skip to content

Support microsoft-conform user class option in DHCPv6 - #559

Open
damyan wants to merge 3 commits into
insomniacslk:masterfrom
damyan:fix/fix-non-rfc-conform-optusrclass-v6
Open

Support microsoft-conform user class option in DHCPv6#559
damyan wants to merge 3 commits into
insomniacslk:masterfrom
damyan:fix/fix-non-rfc-conform-optusrclass-v6

Conversation

@damyan

@damyan damyan commented Mar 26, 2025

Copy link
Copy Markdown

This pull request addresses an issue in the DHCPv6 User Class option (OptUserClass) where the FromBytes method failed to parse data in the Microsoft-compatible format. The existing implementation adhered strictly to the RFC 8415 specification, which requires each user class to be preceded by a 16-bit length field. However, Microsoft DHCPv6 implementations deviate from this by providing a single user class as raw bytes without a length prefix. This mismatch caused a "buffer too short" error when parsing Microsoft-formatted data, as observed in the otherwise failing test TestOptUserClassBroken.

Changes Made

  • Updated FromBytes Method:
    • The method now attempts to parse the data as multiple user classes with length prefixes (per RFC standards).
    • If a length field exceeds the remaining buffer size, parsing halts at that point.
    • If no valid user classes are parsed (e.g., due to missing or invalid length prefixes), the entire data buffer is treated as a single user class, aligning with Microsoft’s format.
    • This dual-parsing approach ensures compatibility with both RFC-compliant and Microsoft-specific inputs without raising errors.

Why This Fix Is Necessary

  • Interoperability: Supporting both RFC and Microsoft formats is essential for seamless interaction with a wide range of DHCPv6 clients, including those in Microsoft environments.
  • Test Reliability: The previous implementation caused TestOptUserClassBroken to fail unnecessarily. This change eliminates the error while maintaining functionality for standard inputs.

@damyan
damyan force-pushed the fix/fix-non-rfc-conform-optusrclass-v6 branch 2 times, most recently from a882567 to 7980a6f Compare March 27, 2025 07:37
@pmazzini

Copy link
Copy Markdown
Collaborator

Related for DHCPv4: #114

@damyan
damyan force-pushed the fix/fix-non-rfc-conform-optusrclass-v6 branch from 25d0e90 to 1159fb8 Compare August 22, 2025 13:20

@insomniacslk insomniacslk left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! Just a couple of requests (commented inline), otherwise it looks good to me

Comment thread dhcpv6/option_userclass.go
Comment thread dhcpv6/option_userclass.go
damyan added a commit to ironcore-dev/FeDHCP that referenced this pull request Jan 19, 2026
Go for private vesions until
insomniacslk/dhcp#559 is merged
Signed-off-by: Damyan Yordanov <damyan.yordanov@sap.com>
@damyan
damyan force-pushed the fix/fix-non-rfc-conform-optusrclass-v6 branch from 06ef025 to 2f82482 Compare March 20, 2026 11:00
Document the dual-parsing behavior in the FromBytes doc comment.

Add a bufferTooShort flag so that a truncated length field returns
`ErrBufferTooShort` for RFC-compliant payloads.

Signed-off-by: Damyan Yordanov <damyan.yordanov@sap.com>
@damyan

damyan commented Mar 20, 2026

Copy link
Copy Markdown
Author

Rebased the branch and implemented the review findings.

Do not reinterpret corrupt data (first buffer exceeds the buffer) as MS
format

Catch trailing bytes in the normal RFC 8415 case

Add a test for buffer too short

Signed-off-by: Damyan Yordanov <damyan.yordanov@sap.com>
@damyan
damyan force-pushed the fix/fix-non-rfc-conform-optusrclass-v6 branch from 34add2e to 974dd10 Compare March 23, 2026 13:40
maxmoehl pushed a commit to ironcore-dev/FeDHCP that referenced this pull request Mar 26, 2026
Go for private vesions until
insomniacslk/dhcp#559 is merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants