From 2f824820d38be947fc9bc6509ee327b265494398 Mon Sep 17 00:00:00 2001 From: Damyan Yordanov Date: Wed, 26 Mar 2025 19:10:55 +0100 Subject: [PATCH 1/3] Support microsoft-conform user class option in DHCPv6 Signed-off-by: Damyan Yordanov --- dhcpv6/option_userclass.go | 20 +++++++++++++++----- dhcpv6/option_userclass_test.go | 16 ++++++++++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/dhcpv6/option_userclass.go b/dhcpv6/option_userclass.go index 8eeb3d704..cebd7b8a8 100644 --- a/dhcpv6/option_userclass.go +++ b/dhcpv6/option_userclass.go @@ -38,16 +38,26 @@ func (op *OptUserClass) String() string { return fmt.Sprintf("%s: [%s]", op.Code(), strings.Join(ucStrings, ", ")) } -// FromBytes builds an OptUserClass structure from a sequence of bytes. The -// input data does not include option code and length bytes. +// FromBytes builds an OptUserClass structure from a sequence of bytes. +// The input data does not include option code and length bytes. func (op *OptUserClass) FromBytes(data []byte) error { if len(data) == 0 { return fmt.Errorf("%w: user class option must not be empty", uio.ErrBufferTooShort) } buf := uio.NewBigEndianBuffer(data) + var userClasses [][]byte for buf.Has(2) { - len := buf.Read16() - op.UserClasses = append(op.UserClasses, buf.CopyN(int(len))) + length := buf.Read16() + if int(length) > buf.Len() { + break + } + uc := buf.CopyN(int(length)) + userClasses = append(userClasses, uc) } - return buf.FinError() + if len(userClasses) > 0 { + op.UserClasses = userClasses + } else { + op.UserClasses = [][]byte{data} + } + return nil } diff --git a/dhcpv6/option_userclass_test.go b/dhcpv6/option_userclass_test.go index ff2926bb1..dd4a2753e 100644 --- a/dhcpv6/option_userclass_test.go +++ b/dhcpv6/option_userclass_test.go @@ -77,3 +77,19 @@ func TestOptUserClassString(t *testing.T) { "String() should contain the list of user classes", ) } + +func TestOptUserClassMicrosoftFormat(t *testing.T) { + data := []byte{ + 'l', 'i', 'n', 'u', 'x', 'b', 'o', 'o', 't', + } + var opt OptUserClass + err := opt.FromBytes(data) + require.NoError(t, err) + + require.Contains( + t, + opt.String(), + "User Class: [linuxboot]", + "String() should contain the list of user classes", + ) +} From 211748466e72451cf187299883ddb5f0ec250739 Mon Sep 17 00:00:00 2001 From: Damyan Yordanov Date: Fri, 20 Mar 2026 12:03:28 +0100 Subject: [PATCH 2/3] Implement review findings 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 --- dhcpv6/option_userclass.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dhcpv6/option_userclass.go b/dhcpv6/option_userclass.go index cebd7b8a8..aaaef0e7c 100644 --- a/dhcpv6/option_userclass.go +++ b/dhcpv6/option_userclass.go @@ -40,23 +40,32 @@ func (op *OptUserClass) String() string { // FromBytes builds an OptUserClass structure from a sequence of bytes. // The input data does not include option code and length bytes. +// It supports both RFC 8415-compliant options (each user class preceded by a +// 16-bit length field) and MS-compatible, non-RFC-compliant options (a single +// user class as raw bytes without a length prefix). func (op *OptUserClass) FromBytes(data []byte) error { if len(data) == 0 { return fmt.Errorf("%w: user class option must not be empty", uio.ErrBufferTooShort) } buf := uio.NewBigEndianBuffer(data) var userClasses [][]byte + var bufferTooShort bool for buf.Has(2) { length := buf.Read16() if int(length) > buf.Len() { + bufferTooShort = true break } uc := buf.CopyN(int(length)) userClasses = append(userClasses, uc) } if len(userClasses) > 0 { + if bufferTooShort { + return fmt.Errorf("%w: user class option too short", uio.ErrBufferTooShort) + } op.UserClasses = userClasses } else { + // MS-compatible format: treat entire data as a single user class op.UserClasses = [][]byte{data} } return nil From 974dd10a4584b2144adf7dcfb88aa79e611520f3 Mon Sep 17 00:00:00 2001 From: Damyan Yordanov Date: Mon, 23 Mar 2026 14:36:20 +0100 Subject: [PATCH 3/3] Improve packet handling and adding a test 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 --- dhcpv6/option_userclass.go | 18 ++++++++++++------ dhcpv6/option_userclass_test.go | 9 +++++++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/dhcpv6/option_userclass.go b/dhcpv6/option_userclass.go index aaaef0e7c..eaf05d868 100644 --- a/dhcpv6/option_userclass.go +++ b/dhcpv6/option_userclass.go @@ -40,9 +40,12 @@ func (op *OptUserClass) String() string { // FromBytes builds an OptUserClass structure from a sequence of bytes. // The input data does not include option code and length bytes. -// It supports both RFC 8415-compliant options (each user class preceded by a -// 16-bit length field) and MS-compatible, non-RFC-compliant options (a single -// user class as raw bytes without a length prefix). +// It first attempts to parse the data as RFC 8415-compliant (each user class +// preceded by a 16-bit length field). If no complete user class can be +// extracted, it falls back to treating the entire data as a single user class +// (MS-compatible, non-RFC-compliant behavior). Note that ToBytes always +// serializes in RFC 8415-compliant format regardless of how the data was +// originally parsed. func (op *OptUserClass) FromBytes(data []byte) error { if len(data) == 0 { return fmt.Errorf("%w: user class option must not be empty", uio.ErrBufferTooShort) @@ -64,9 +67,12 @@ func (op *OptUserClass) FromBytes(data []byte) error { return fmt.Errorf("%w: user class option too short", uio.ErrBufferTooShort) } op.UserClasses = userClasses - } else { - // MS-compatible format: treat entire data as a single user class - op.UserClasses = [][]byte{data} + return buf.FinError() } + // MS-compatible fallback: no valid length-prefixed classes found, + // treat entire data as a single user class. + // Note: when no valid classes were parsed, we cannot distinguish corrupt + // RFC data from genuine MS-format data, so we treat it as MS-compatible. + op.UserClasses = [][]byte{data} return nil } diff --git a/dhcpv6/option_userclass_test.go b/dhcpv6/option_userclass_test.go index dd4a2753e..15d24ba12 100644 --- a/dhcpv6/option_userclass_test.go +++ b/dhcpv6/option_userclass_test.go @@ -78,6 +78,15 @@ func TestOptUserClassString(t *testing.T) { ) } +func TestOptUserClassTruncatedAfterValid(t *testing.T) { + // Valid entry "ABC" followed by a truncated entry (declared length 5, only 1 byte) + data := []byte{0, 3, 'A', 'B', 'C', 0, 5, 'X'} + var opt OptUserClass + err := opt.FromBytes(data) + require.Error(t, err) + require.True(t, errors.Is(err, uio.ErrBufferTooShort), "expected ErrBufferTooShort, got %v", err) +} + func TestOptUserClassMicrosoftFormat(t *testing.T) { data := []byte{ 'l', 'i', 'n', 'u', 'x', 'b', 'o', 'o', 't',