From e87114a6e449d7a2e458b5529923e5668dfa3a11 Mon Sep 17 00:00:00 2001 From: Christopher Koch Date: Fri, 28 Dec 2018 20:19:14 -0800 Subject: dhcpv4: simplify option parsing. option's codes and lengths were being parsed twice: once in ParseOption and once in each option type's Parse implementation. Consolidate such that it only happens once. Additionally, only pass data to options that they should parse -- we know the length before the Parse function is called, so the option only gets to see the data it needs to see. Also, use uio.Lexer to simplify parsing code in general. Easier to read and reason about. --- dhcpv4/bsdp/boot_image.go | 45 +++++++--------- dhcpv4/bsdp/boot_image_test.go | 33 ++++++------ dhcpv4/bsdp/bsdp.go | 6 +-- dhcpv4/bsdp/bsdp_option_boot_image_list.go | 34 +++--------- dhcpv4/bsdp/bsdp_option_boot_image_list_test.go | 38 ------------- dhcpv4/bsdp/bsdp_option_default_boot_image_id.go | 23 +++----- .../bsdp/bsdp_option_default_boot_image_id_test.go | 13 ++--- dhcpv4/bsdp/bsdp_option_generic.go | 19 ++----- dhcpv4/bsdp/bsdp_option_generic_test.go | 10 +--- dhcpv4/bsdp/bsdp_option_machine_name.go | 19 ++----- dhcpv4/bsdp/bsdp_option_machine_name_test.go | 17 +----- dhcpv4/bsdp/bsdp_option_message_type.go | 25 +++------ dhcpv4/bsdp/bsdp_option_message_type_test.go | 17 +----- dhcpv4/bsdp/bsdp_option_reply_port.go | 20 ++----- dhcpv4/bsdp/bsdp_option_reply_port_test.go | 11 ++-- dhcpv4/bsdp/bsdp_option_selected_boot_image_id.go | 23 +++----- .../bsdp_option_selected_boot_image_id_test.go | 12 ++--- dhcpv4/bsdp/bsdp_option_server_identifier.go | 18 ++----- dhcpv4/bsdp/bsdp_option_server_identifier_test.go | 8 +-- dhcpv4/bsdp/bsdp_option_server_priority.go | 20 ++----- dhcpv4/bsdp/bsdp_option_server_priority_test.go | 12 ++--- dhcpv4/bsdp/bsdp_option_version.go | 19 ++----- dhcpv4/bsdp/bsdp_option_version_test.go | 14 +---- dhcpv4/bsdp/option_vendor_specific_information.go | 62 ++++------------------ .../option_vendor_specific_information_test.go | 20 ++----- dhcpv4/bsdp/types.go | 4 +- 26 files changed, 121 insertions(+), 421 deletions(-) (limited to 'dhcpv4/bsdp') diff --git a/dhcpv4/bsdp/boot_image.go b/dhcpv4/bsdp/boot_image.go index 88a9404..fa9b1a6 100644 --- a/dhcpv4/bsdp/boot_image.go +++ b/dhcpv4/bsdp/boot_image.go @@ -3,6 +3,8 @@ package bsdp import ( "encoding/binary" "fmt" + + "github.com/u-root/u-root/pkg/uio" ) // BootImageType represents the different BSDP boot image types. @@ -60,16 +62,14 @@ func (b BootImageID) String() string { return s + " " + t + " image" } -// BootImageIDFromBytes deserializes a collection of 4 bytes to a BootImageID. -func BootImageIDFromBytes(bytes []byte) (*BootImageID, error) { - if len(bytes) < 4 { - return nil, fmt.Errorf("not enough bytes to serialize BootImageID") - } - return &BootImageID{ - IsInstall: bytes[0]&0x80 != 0, - ImageType: BootImageType(bytes[0] & 0x7f), - Index: binary.BigEndian.Uint16(bytes[2:]), - }, nil +// Unmarshal reads b's binary representation from buf. +func (b *BootImageID) Unmarshal(buf *uio.Lexer) error { + byte0 := buf.Read8() + _ = buf.Read8() + b.IsInstall = byte0&0x80 != 0 + b.ImageType = BootImageType(byte0 & 0x7f) + b.Index = buf.Read16() + return buf.Error() } // BootImage describes a boot image - contains the boot image ID and the name. @@ -87,25 +87,16 @@ func (b *BootImage) ToBytes() []byte { } // String converts a BootImage to a human-readable representation. -func (b *BootImage) String() string { +func (b BootImage) String() string { return fmt.Sprintf("%v %v", b.Name, b.ID.String()) } -// BootImageFromBytes returns a deserialized BootImage struct from bytes. -func BootImageFromBytes(bytes []byte) (*BootImage, error) { - // Should at least contain 4 bytes of BootImageID + byte for length of - // boot image name. - if len(bytes) < 5 { - return nil, fmt.Errorf("not enough bytes to serialize BootImage") - } - imageID, err := BootImageIDFromBytes(bytes[:4]) - if err != nil { - return nil, err - } - nameLength := int(bytes[4]) - if 5+nameLength > len(bytes) { - return nil, fmt.Errorf("not enough bytes for BootImage") +// Unmarshal reads data from buf into b. +func (b *BootImage) Unmarshal(buf *uio.Lexer) error { + if err := (&b.ID).Unmarshal(buf); err != nil { + return err } - name := string(bytes[5 : 5+nameLength]) - return &BootImage{ID: *imageID, Name: name}, nil + nameLength := buf.Read8() + b.Name = string(buf.Consume(int(nameLength))) + return buf.Error() } diff --git a/dhcpv4/bsdp/boot_image_test.go b/dhcpv4/bsdp/boot_image_test.go index 004aa30..d8e3aeb 100644 --- a/dhcpv4/bsdp/boot_image_test.go +++ b/dhcpv4/bsdp/boot_image_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "github.com/u-root/u-root/pkg/uio" ) func TestBootImageIDToBytes(t *testing.T) { @@ -28,25 +29,23 @@ func TestBootImageIDFromBytes(t *testing.T) { ImageType: BootImageTypeMacOSX, Index: 0x1000, } - newBootImage, err := BootImageIDFromBytes(b.ToBytes()) - require.NoError(t, err) - require.Equal(t, b, *newBootImage) + var newBootImage BootImageID + require.NoError(t, uio.FromBigEndian(&newBootImage, b.ToBytes())) + require.Equal(t, b, newBootImage) b = BootImageID{ IsInstall: true, ImageType: BootImageTypeMacOSX, Index: 0x1011, } - newBootImage, err = BootImageIDFromBytes(b.ToBytes()) - require.NoError(t, err) - require.Equal(t, b, *newBootImage) + require.NoError(t, uio.FromBigEndian(&newBootImage, b.ToBytes())) + require.Equal(t, b, newBootImage) } func TestBootImageIDFromBytesFail(t *testing.T) { serialized := []byte{0x81, 0, 0x10} // intentionally left short - deserialized, err := BootImageIDFromBytes(serialized) - require.Nil(t, deserialized) - require.Error(t, err) + var deserialized BootImageID + require.Error(t, uio.FromBigEndian(&deserialized, serialized)) } func TestBootImageIDString(t *testing.T) { @@ -97,8 +96,8 @@ func TestBootImageFromBytes(t *testing.T) { 7, // len(Name) 98, 115, 100, 112, 45, 50, 49, // byte-encoding of Name } - b, err := BootImageFromBytes(input) - require.NoError(t, err) + var b BootImage + require.NoError(t, uio.FromBigEndian(&b, input)) expectedBootImage := BootImage{ ID: BootImageID{ IsInstall: false, @@ -107,15 +106,14 @@ func TestBootImageFromBytes(t *testing.T) { }, Name: "bsdp-21", } - require.Equal(t, expectedBootImage, *b) + require.Equal(t, expectedBootImage, b) } func TestBootImageFromBytesOnlyBootImageID(t *testing.T) { // Only a BootImageID, nothing else. input := []byte{0x1, 0, 0x10, 0x10} - b, err := BootImageFromBytes(input) - require.Nil(t, b) - require.Error(t, err) + var b BootImage + require.Error(t, uio.FromBigEndian(&b, input)) } func TestBootImageFromBytesShortBootImage(t *testing.T) { @@ -124,9 +122,8 @@ func TestBootImageFromBytesShortBootImage(t *testing.T) { 7, // len(Name) 98, 115, 100, 112, 45, 50, // Name bytes (intentionally off-by-one) } - b, err := BootImageFromBytes(input) - require.Nil(t, b) - require.Error(t, err) + var b BootImage + require.Error(t, uio.FromBigEndian(&b, input)) } func TestBootImageString(t *testing.T) { diff --git a/dhcpv4/bsdp/bsdp.go b/dhcpv4/bsdp/bsdp.go index 3f97602..6bc0cfd 100644 --- a/dhcpv4/bsdp/bsdp.go +++ b/dhcpv4/bsdp/bsdp.go @@ -23,7 +23,7 @@ const AppleVendorID = "AAPLBSDPC" type ReplyConfig struct { ServerIP net.IP ServerHostname, BootFileName string - ServerPriority int + ServerPriority uint16 Images []BootImage DefaultImage, SelectedImage *BootImage } @@ -36,7 +36,7 @@ func ParseBootImageListFromAck(ack dhcpv4.DHCPv4) ([]BootImage, error) { if opt == nil { return nil, errors.New("ParseBootImageListFromAck: could not find vendor-specific option") } - vendorOpt, err := ParseOptVendorSpecificInformation(opt.ToBytes()) + vendorOpt, err := ParseOptVendorSpecificInformation(opt.ToBytes()[2:]) if err != nil { return nil, err } @@ -60,7 +60,7 @@ func MessageTypeFromPacket(packet *dhcpv4.DHCPv4) *MessageType { err error ) for _, opt := range packet.GetOption(dhcpv4.OptionVendorSpecificInformation) { - if vendorOpts, err = ParseOptVendorSpecificInformation(opt.ToBytes()); err == nil { + if vendorOpts, err = ParseOptVendorSpecificInformation(opt.ToBytes()[2:]); err == nil { if o := vendorOpts.GetOneOption(OptionMessageType); o != nil { if optMessageType, ok := o.(*OptMessageType); ok { return &optMessageType.Type diff --git a/dhcpv4/bsdp/bsdp_option_boot_image_list.go b/dhcpv4/bsdp/bsdp_option_boot_image_list.go index 6417221..d018655 100644 --- a/dhcpv4/bsdp/bsdp_option_boot_image_list.go +++ b/dhcpv4/bsdp/bsdp_option_boot_image_list.go @@ -1,9 +1,8 @@ package bsdp import ( - "fmt" - "github.com/insomniacslk/dhcp/dhcpv4" + "github.com/u-root/u-root/pkg/uio" ) // Implements the BSDP option listing the boot images. @@ -17,34 +16,15 @@ type OptBootImageList struct { // ParseOptBootImageList constructs an OptBootImageList struct from a sequence // of bytes and returns it, or an error. func ParseOptBootImageList(data []byte) (*OptBootImageList, error) { - // Should have at least code + length - if len(data) < 2 { - return nil, dhcpv4.ErrShortByteStream - } - code := dhcpv4.OptionCode(data[0]) - if code != OptionBootImageList { - return nil, fmt.Errorf("expected option %v, got %v instead", OptionBootImageList, code) - } - length := int(data[1]) - if len(data) < length+2 { - return nil, fmt.Errorf("expected length %d, got %d instead", length, len(data)) - } + buf := uio.NewBigEndianBuffer(data) - // Offset from code + length byte var bootImages []BootImage - idx := 2 - for { - if idx >= length+2 { - break - } - image, err := BootImageFromBytes(data[idx:]) - if err != nil { - return nil, fmt.Errorf("parsing bytes stream: %v", err) + for buf.Has(5) { + var image BootImage + if err := (&image).Unmarshal(buf); err != nil { + return nil, err } - bootImages = append(bootImages, *image) - - // 4 bytes of BootImageID, 1 byte of name length, name - idx += 4 + 1 + len(image.Name) + bootImages = append(bootImages, image) } return &OptBootImageList{bootImages}, nil diff --git a/dhcpv4/bsdp/bsdp_option_boot_image_list_test.go b/dhcpv4/bsdp/bsdp_option_boot_image_list_test.go index d2784ae..0819d64 100644 --- a/dhcpv4/bsdp/bsdp_option_boot_image_list_test.go +++ b/dhcpv4/bsdp/bsdp_option_boot_image_list_test.go @@ -45,8 +45,6 @@ func TestOptBootImageListInterfaceMethods(t *testing.T) { func TestParseOptBootImageList(t *testing.T) { data := []byte{ - 9, // code - 22, // length // boot image 1 0x1, 0x0, 0x03, 0xe9, // ID 6, // name length @@ -78,25 +76,8 @@ func TestParseOptBootImageList(t *testing.T) { } require.Equal(t, &OptBootImageList{expectedBootImages}, o) - // Short byte stream - data = []byte{9} - _, err = ParseOptBootImageList(data) - require.Error(t, err, "should get error from short byte stream") - - // Wrong code - data = []byte{54, 1, 1} - _, err = ParseOptBootImageList(data) - require.Error(t, err, "should get error from wrong code") - - // Bad length - data = []byte{9, 10, 1, 1, 1} - _, err = ParseOptBootImageList(data) - require.Error(t, err, "should get error from bad length") - // Error parsing boot image (malformed) data = []byte{ - 9, // code - 22, // length // boot image 1 0x1, 0x0, 0x03, 0xe9, // ID 4, // name length @@ -108,25 +89,6 @@ func TestParseOptBootImageList(t *testing.T) { } _, err = ParseOptBootImageList(data) require.Error(t, err, "should get error from bad boot image") - - // Should not get error parsing boot image with excess length. - data = []byte{ - 9, // code - 22, // length - // boot image 1 - 0x1, 0x0, 0x03, 0xe9, // ID - 6, // name length - 'b', 's', 'd', 'p', '-', '1', - // boot image 2 - 0x80, 0x0, 0x23, 0x31, // ID - 6, // name length - 'b', 's', 'd', 'p', '-', '2', - - // Simulate another option after boot image list - 7, 4, 0x80, 0x0, 0x23, 0x32, - } - _, err = ParseOptBootImageList(data) - require.NoError(t, err, "should not get error from options after boot image list") } func TestOptBootImageListString(t *testing.T) { diff --git a/dhcpv4/bsdp/bsdp_option_default_boot_image_id.go b/dhcpv4/bsdp/bsdp_option_default_boot_image_id.go index 4c87df8..52f7780 100644 --- a/dhcpv4/bsdp/bsdp_option_default_boot_image_id.go +++ b/dhcpv4/bsdp/bsdp_option_default_boot_image_id.go @@ -4,12 +4,13 @@ import ( "fmt" "github.com/insomniacslk/dhcp/dhcpv4" + "github.com/u-root/u-root/pkg/uio" ) +// OptDefaultBootImageID contains the selected boot image ID. +// // Implements the BSDP option default boot image ID, which tells the client // which image is the default boot image if one is not selected. - -// OptDefaultBootImageID contains the selected boot image ID. type OptDefaultBootImageID struct { ID BootImageID } @@ -17,22 +18,12 @@ type OptDefaultBootImageID struct { // ParseOptDefaultBootImageID constructs an OptDefaultBootImageID struct from a sequence of // bytes and returns it, or an error. func ParseOptDefaultBootImageID(data []byte) (*OptDefaultBootImageID, error) { - if len(data) < 6 { - return nil, dhcpv4.ErrShortByteStream - } - code := dhcpv4.OptionCode(data[0]) - if code != OptionDefaultBootImageID { - return nil, fmt.Errorf("expected option %v, got %v instead", OptionDefaultBootImageID, code) - } - length := int(data[1]) - if length != 4 { - return nil, fmt.Errorf("expected length 4, got %d instead", length) - } - id, err := BootImageIDFromBytes(data[2:6]) - if err != nil { + var o OptDefaultBootImageID + buf := uio.NewBigEndianBuffer(data) + if err := o.ID.Unmarshal(buf); err != nil { return nil, err } - return &OptDefaultBootImageID{*id}, nil + return &o, buf.FinError() } // Code returns the option code. diff --git a/dhcpv4/bsdp/bsdp_option_default_boot_image_id_test.go b/dhcpv4/bsdp/bsdp_option_default_boot_image_id_test.go index e062e2d..ad29c30 100644 --- a/dhcpv4/bsdp/bsdp_option_default_boot_image_id_test.go +++ b/dhcpv4/bsdp/bsdp_option_default_boot_image_id_test.go @@ -17,24 +17,17 @@ func TestOptDefaultBootImageIDInterfaceMethods(t *testing.T) { func TestParseOptDefaultBootImageID(t *testing.T) { b := BootImageID{IsInstall: true, ImageType: BootImageTypeMacOSX, Index: 1001} - bootImageBytes := b.ToBytes() - data := append([]byte{byte(OptionDefaultBootImageID), 4}, bootImageBytes...) - o, err := ParseOptDefaultBootImageID(data) + o, err := ParseOptDefaultBootImageID(b.ToBytes()) require.NoError(t, err) require.Equal(t, &OptDefaultBootImageID{b}, o) // Short byte stream - data = []byte{byte(OptionDefaultBootImageID), 4} + data := []byte{} _, err = ParseOptDefaultBootImageID(data) require.Error(t, err, "should get error from short byte stream") - // Wrong code - data = []byte{54, 2, 1, 0, 0, 0} - _, err = ParseOptDefaultBootImageID(data) - require.Error(t, err, "should get error from wrong code") - // Bad length - data = []byte{byte(OptionDefaultBootImageID), 5, 1, 0, 0, 0, 0} + data = []byte{1, 0, 0, 0, 0} _, err = ParseOptDefaultBootImageID(data) require.Error(t, err, "should get error from bad length") } diff --git a/dhcpv4/bsdp/bsdp_option_generic.go b/dhcpv4/bsdp/bsdp_option_generic.go index 6a51e29..6702e9c 100644 --- a/dhcpv4/bsdp/bsdp_option_generic.go +++ b/dhcpv4/bsdp/bsdp_option_generic.go @@ -16,21 +16,8 @@ type OptGeneric struct { // ParseOptGeneric parses a bytestream and creates a new OptGeneric from it, // or an error. -func ParseOptGeneric(data []byte) (*OptGeneric, error) { - if len(data) == 0 { - return nil, dhcpv4.ErrZeroLengthByteStream - } - var ( - length int - optionData []byte - ) - code := dhcpv4.OptionCode(data[0]) - length = int(data[1]) - if len(data) < length+2 { - return nil, fmt.Errorf("invalid data length: declared %v, actual %v", length, len(data)) - } - optionData = data[2 : length+2] - return &OptGeneric{OptionCode: code, Data: optionData}, nil +func ParseOptGeneric(code dhcpv4.OptionCode, data []byte) (*OptGeneric, error) { + return &OptGeneric{OptionCode: code, Data: data}, nil } // Code returns the generic option code. @@ -45,7 +32,7 @@ func (o OptGeneric) ToBytes() []byte { // String returns a human-readable representation of a generic option. func (o OptGeneric) String() string { - code, ok := OptionCodeToString[o.Code()] + code, ok := optionCodeToString[o.Code()] if !ok { code = "Unknown" } diff --git a/dhcpv4/bsdp/bsdp_option_generic_test.go b/dhcpv4/bsdp/bsdp_option_generic_test.go index 27436dd..eae77e1 100644 --- a/dhcpv4/bsdp/bsdp_option_generic_test.go +++ b/dhcpv4/bsdp/bsdp_option_generic_test.go @@ -7,19 +7,11 @@ import ( ) func TestParseOptGeneric(t *testing.T) { - // Empty bytestream produces error - _, err := ParseOptGeneric([]byte{}) - require.Error(t, err, "error from empty bytestream") - // Good parse - o, err := ParseOptGeneric([]byte{1, 1, 1}) + o, err := ParseOptGeneric(OptionMessageType, []byte{1}) require.NoError(t, err) require.Equal(t, OptionMessageType, o.Code()) require.Equal(t, MessageTypeList, MessageType(o.Data[0])) - - // Bad parse - o, err = ParseOptGeneric([]byte{1, 2, 1}) - require.Error(t, err, "invalid length") } func TestOptGenericCode(t *testing.T) { diff --git a/dhcpv4/bsdp/bsdp_option_machine_name.go b/dhcpv4/bsdp/bsdp_option_machine_name.go index dc05378..cffba2e 100644 --- a/dhcpv4/bsdp/bsdp_option_machine_name.go +++ b/dhcpv4/bsdp/bsdp_option_machine_name.go @@ -1,15 +1,13 @@ package bsdp import ( - "fmt" - "github.com/insomniacslk/dhcp/dhcpv4" ) +// OptMachineName represents a BSDP message type. +// // Implements the BSDP option machine name, which gives the Netboot server's // machine name. - -// OptMachineName represents a BSDP message type. type OptMachineName struct { Name string } @@ -17,18 +15,7 @@ type OptMachineName struct { // ParseOptMachineName constructs an OptMachineName struct from a sequence of // bytes and returns it, or an error. func ParseOptMachineName(data []byte) (*OptMachineName, error) { - if len(data) < 2 { - return nil, dhcpv4.ErrShortByteStream - } - code := dhcpv4.OptionCode(data[0]) - if code != OptionMachineName { - return nil, fmt.Errorf("expected option %v, got %v instead", OptionMachineName, code) - } - length := int(data[1]) - if len(data) < length+2 { - return nil, fmt.Errorf("expected length %d, got %d instead", length, len(data)) - } - return &OptMachineName{Name: string(data[2 : length+2])}, nil + return &OptMachineName{Name: string(data)}, nil } // Code returns the option code. diff --git a/dhcpv4/bsdp/bsdp_option_machine_name_test.go b/dhcpv4/bsdp/bsdp_option_machine_name_test.go index 9019020..712bc49 100644 --- a/dhcpv4/bsdp/bsdp_option_machine_name_test.go +++ b/dhcpv4/bsdp/bsdp_option_machine_name_test.go @@ -15,25 +15,10 @@ func TestOptMachineNameInterfaceMethods(t *testing.T) { } func TestParseOptMachineName(t *testing.T) { - data := []byte{130, 7, 's', 'o', 'm', 'e', 'b', 'o', 'x'} + data := []byte{'s', 'o', 'm', 'e', 'b', 'o', 'x'} o, err := ParseOptMachineName(data) require.NoError(t, err) require.Equal(t, &OptMachineName{"somebox"}, o) - - // Short byte stream - data = []byte{130} - _, err = ParseOptMachineName(data) - require.Error(t, err, "should get error from short byte stream") - - // Wrong code - data = []byte{54, 1, 1} - _, err = ParseOptMachineName(data) - require.Error(t, err, "should get error from wrong code") - - // Bad length - data = []byte{130, 5, 1} - _, err = ParseOptMachineName(data) - require.Error(t, err, "should get error from bad length") } func TestOptMachineNameString(t *testing.T) { diff --git a/dhcpv4/bsdp/bsdp_option_message_type.go b/dhcpv4/bsdp/bsdp_option_message_type.go index c11b56b..8c3c3d4 100644 --- a/dhcpv4/bsdp/bsdp_option_message_type.go +++ b/dhcpv4/bsdp/bsdp_option_message_type.go @@ -4,12 +4,13 @@ import ( "fmt" "github.com/insomniacslk/dhcp/dhcpv4" + "github.com/u-root/u-root/pkg/uio" ) +// MessageType represents the different BSDP message types. +// // Implements the BSDP option message type. Can be one of LIST, SELECT, or // FAILED. - -// MessageType represents the different BSDP message types. type MessageType byte // BSDP Message types - e.g. LIST, SELECT, FAILED @@ -20,14 +21,14 @@ const ( ) func (m MessageType) String() string { - if s, ok := MessageTypeToString[m]; ok { + if s, ok := messageTypeToString[m]; ok { return s } return "Unknown" } -// MessageTypeToString maps each BSDP message type to a human-readable string. -var MessageTypeToString = map[MessageType]string{ +// messageTypeToString maps each BSDP message type to a human-readable string. +var messageTypeToString = map[MessageType]string{ MessageTypeList: "LIST", MessageTypeSelect: "SELECT", MessageTypeFailed: "FAILED", @@ -41,18 +42,8 @@ type OptMessageType struct { // ParseOptMessageType constructs an OptMessageType struct from a sequence of // bytes and returns it, or an error. func ParseOptMessageType(data []byte) (*OptMessageType, error) { - if len(data) < 3 { - return nil, dhcpv4.ErrShortByteStream - } - code := dhcpv4.OptionCode(data[0]) - if code != OptionMessageType { - return nil, fmt.Errorf("expected option %v, got %v instead", OptionMessageType, code) - } - length := int(data[1]) - if length != 1 { - return nil, fmt.Errorf("expected length 1, got %d instead", length) - } - return &OptMessageType{Type: MessageType(data[2])}, nil + buf := uio.NewBigEndianBuffer(data) + return &OptMessageType{Type: MessageType(buf.Read8())}, buf.FinError() } // Code returns the option code. diff --git a/dhcpv4/bsdp/bsdp_option_message_type_test.go b/dhcpv4/bsdp/bsdp_option_message_type_test.go index 9b7564f..41652be 100644 --- a/dhcpv4/bsdp/bsdp_option_message_type_test.go +++ b/dhcpv4/bsdp/bsdp_option_message_type_test.go @@ -14,25 +14,10 @@ func TestOptMessageTypeInterfaceMethods(t *testing.T) { } func TestParseOptMessageType(t *testing.T) { - data := []byte{1, 1, 1} // DISCOVER + data := []byte{1} // DISCOVER o, err := ParseOptMessageType(data) require.NoError(t, err) require.Equal(t, &OptMessageType{MessageTypeList}, o) - - // Short byte stream - data = []byte{1, 1} - _, err = ParseOptMessageType(data) - require.Error(t, err, "should get error from short byte stream") - - // Wrong code - data = []byte{54, 1, 1} - _, err = ParseOptMessageType(data) - require.Error(t, err, "should get error from wrong code") - - // Bad length - data = []byte{1, 5, 1} - _, err = ParseOptMessageType(data) - require.Error(t, err, "should get error from bad length") } func TestOptMessageTypeString(t *testing.T) { diff --git a/dhcpv4/bsdp/bsdp_option_reply_port.go b/dhcpv4/bsdp/bsdp_option_reply_port.go index f1cc49f..da5e9c4 100644 --- a/dhcpv4/bsdp/bsdp_option_reply_port.go +++ b/dhcpv4/bsdp/bsdp_option_reply_port.go @@ -5,14 +5,15 @@ import ( "fmt" "github.com/insomniacslk/dhcp/dhcpv4" + "github.com/u-root/u-root/pkg/uio" ) +// OptReplyPort represents a BSDP protocol version. +// // Implements the BSDP option reply port. This is used when BSDP responses // should be sent to a reply port other than the DHCP default. The macOS GUI // "Startup Disk Select" sends this option since it's operating in an // unprivileged context. - -// OptReplyPort represents a BSDP protocol version. type OptReplyPort struct { Port uint16 } @@ -20,19 +21,8 @@ type OptReplyPort struct { // ParseOptReplyPort constructs an OptReplyPort struct from a sequence of // bytes and returns it, or an error. func ParseOptReplyPort(data []byte) (*OptReplyPort, error) { - if len(data) < 4 { - return nil, dhcpv4.ErrShortByteStream - } - code := dhcpv4.OptionCode(data[0]) - if code != OptionReplyPort { - return nil, fmt.Errorf("expected option %v, got %v instead", OptionReplyPort, code) - } - length := int(data[1]) - if length != 2 { - return nil, fmt.Errorf("expected length 2, got %d instead", length) - } - port := binary.BigEndian.Uint16(data[2:4]) - return &OptReplyPort{port}, nil + buf := uio.NewBigEndianBuffer(data) + return &OptReplyPort{buf.Read16()}, buf.FinError() } // Code returns the option code. diff --git a/dhcpv4/bsdp/bsdp_option_reply_port_test.go b/dhcpv4/bsdp/bsdp_option_reply_port_test.go index c9906ff..719bbc8 100644 --- a/dhcpv4/bsdp/bsdp_option_reply_port_test.go +++ b/dhcpv4/bsdp/bsdp_option_reply_port_test.go @@ -14,23 +14,18 @@ func TestOptReplyPortInterfaceMethods(t *testing.T) { } func TestParseOptReplyPort(t *testing.T) { - data := []byte{byte(OptionReplyPort), 2, 0, 1} + data := []byte{0, 1} o, err := ParseOptReplyPort(data) require.NoError(t, err) require.Equal(t, &OptReplyPort{1}, o) // Short byte stream - data = []byte{byte(OptionReplyPort), 2} + data = []byte{} _, err = ParseOptReplyPort(data) require.Error(t, err, "should get error from short byte stream") - // Wrong code - data = []byte{54, 2, 1, 0} - _, err = ParseOptReplyPort(data) - require.Error(t, err, "should get error from wrong code") - // Bad length - data = []byte{byte(OptionReplyPort), 4, 1, 0} + data = []byte{1} _, err = ParseOptReplyPort(data) require.Error(t, err, "should get error from bad length") } diff --git a/dhcpv4/bsdp/bsdp_option_selected_boot_image_id.go b/dhcpv4/bsdp/bsdp_option_selected_boot_image_id.go index 5b00ded..52b6eab 100644 --- a/dhcpv4/bsdp/bsdp_option_selected_boot_image_id.go +++ b/dhcpv4/bsdp/bsdp_option_selected_boot_image_id.go @@ -4,12 +4,13 @@ import ( "fmt" "github.com/insomniacslk/dhcp/dhcpv4" + "github.com/u-root/u-root/pkg/uio" ) +// OptSelectedBootImageID contains the selected boot image ID. +// // Implements the BSDP option selected boot image ID, which tells the server // which boot image has been selected by the client. - -// OptSelectedBootImageID contains the selected boot image ID. type OptSelectedBootImageID struct { ID BootImageID } @@ -17,22 +18,12 @@ type OptSelectedBootImageID struct { // ParseOptSelectedBootImageID constructs an OptSelectedBootImageID struct from a sequence of // bytes and returns it, or an error. func ParseOptSelectedBootImageID(data []byte) (*OptSelectedBootImageID, error) { - if len(data) < 6 { - return nil, dhcpv4.ErrShortByteStream - } - code := dhcpv4.OptionCode(data[0]) - if code != OptionSelectedBootImageID { - return nil, fmt.Errorf("expected option %v, got %v instead", OptionSelectedBootImageID, code) - } - length := int(data[1]) - if length != 4 { - return nil, fmt.Errorf("expected length 4, got %d instead", length) - } - id, err := BootImageIDFromBytes(data[2:6]) - if err != nil { + var o OptSelectedBootImageID + buf := uio.NewBigEndianBuffer(data) + if err := o.ID.Unmarshal(buf); err != nil { return nil, err } - return &OptSelectedBootImageID{*id}, nil + return &o, buf.FinError() } // Code returns the option code. diff --git a/dhcpv4/bsdp/bsdp_option_selected_boot_image_id_test.go b/dhcpv4/bsdp/bsdp_option_selected_boot_image_id_test.go index 9529e41..a55fd9f 100644 --- a/dhcpv4/bsdp/bsdp_option_selected_boot_image_id_test.go +++ b/dhcpv4/bsdp/bsdp_option_selected_boot_image_id_test.go @@ -17,24 +17,18 @@ func TestOptSelectedBootImageIDInterfaceMethods(t *testing.T) { func TestParseOptSelectedBootImageID(t *testing.T) { b := BootImageID{IsInstall: true, ImageType: BootImageTypeMacOSX, Index: 1001} - bootImageBytes := b.ToBytes() - data := append([]byte{byte(OptionSelectedBootImageID), 4}, bootImageBytes...) + data := b.ToBytes() o, err := ParseOptSelectedBootImageID(data) require.NoError(t, err) require.Equal(t, &OptSelectedBootImageID{b}, o) // Short byte stream - data = []byte{byte(OptionSelectedBootImageID), 4} + data = []byte{} _, err = ParseOptSelectedBootImageID(data) require.Error(t, err, "should get error from short byte stream") - // Wrong code - data = []byte{54, 2, 1, 0, 0, 0} - _, err = ParseOptSelectedBootImageID(data) - require.Error(t, err, "should get error from wrong code") - // Bad length - data = []byte{byte(OptionSelectedBootImageID), 5, 1, 0, 0, 0, 0} + data = []byte{1, 0, 0, 0, 0} _, err = ParseOptSelectedBootImageID(data) require.Error(t, err, "should get error from bad length") } diff --git a/dhcpv4/bsdp/bsdp_option_server_identifier.go b/dhcpv4/bsdp/bsdp_option_server_identifier.go index 252a0aa..26ec37a 100644 --- a/dhcpv4/bsdp/bsdp_option_server_identifier.go +++ b/dhcpv4/bsdp/bsdp_option_server_identifier.go @@ -5,6 +5,7 @@ import ( "net" "github.com/insomniacslk/dhcp/dhcpv4" + "github.com/u-root/u-root/pkg/uio" ) // OptServerIdentifier represents an option encapsulating the server identifier. @@ -15,21 +16,8 @@ type OptServerIdentifier struct { // ParseOptServerIdentifier returns a new OptServerIdentifier from a byte // stream, or error if any. func ParseOptServerIdentifier(data []byte) (*OptServerIdentifier, error) { - if len(data) < 2 { - return nil, dhcpv4.ErrShortByteStream - } - code := dhcpv4.OptionCode(data[0]) - if code != OptionServerIdentifier { - return nil, fmt.Errorf("expected code %v, got %v", OptionServerIdentifier, code) - } - length := int(data[1]) - if length != 4 { - return nil, fmt.Errorf("unexpected length: expected 4, got %v", length) - } - if len(data) < 6 { - return nil, dhcpv4.ErrShortByteStream - } - return &OptServerIdentifier{ServerID: net.IP(data[2 : 2+length])}, nil + buf := uio.NewBigEndianBuffer(data) + return &OptServerIdentifier{ServerID: net.IP(buf.CopyN(net.IPv4len))}, buf.FinError() } // Code returns the option code. diff --git a/dhcpv4/bsdp/bsdp_option_server_identifier_test.go b/dhcpv4/bsdp/bsdp_option_server_identifier_test.go index 5267caa..d832c40 100644 --- a/dhcpv4/bsdp/bsdp_option_server_identifier_test.go +++ b/dhcpv4/bsdp/bsdp_option_server_identifier_test.go @@ -26,15 +26,9 @@ func TestParseOptServerIdentifier(t *testing.T) { require.Error(t, err, "empty byte stream") o, err = ParseOptServerIdentifier([]byte{3, 4, 192}) - require.Error(t, err, "short byte stream") - - o, err = ParseOptServerIdentifier([]byte{3, 3, 192, 168, 0, 1}) require.Error(t, err, "wrong IP length") - o, err = ParseOptServerIdentifier([]byte{53, 4, 192, 168, 1}) - require.Error(t, err, "wrong option code") - - o, err = ParseOptServerIdentifier([]byte{3, 4, 192, 168, 0, 1}) + o, err = ParseOptServerIdentifier([]byte{192, 168, 0, 1}) require.NoError(t, err) require.Equal(t, net.IP{192, 168, 0, 1}, o.ServerID) } diff --git a/dhcpv4/bsdp/bsdp_option_server_priority.go b/dhcpv4/bsdp/bsdp_option_server_priority.go index 1952b7e..66bfa44 100644 --- a/dhcpv4/bsdp/bsdp_option_server_priority.go +++ b/dhcpv4/bsdp/bsdp_option_server_priority.go @@ -5,31 +5,19 @@ import ( "fmt" "github.com/insomniacslk/dhcp/dhcpv4" + "github.com/u-root/u-root/pkg/uio" ) -// This option implements the server identifier option -// https://tools.ietf.org/html/rfc2132 - // OptServerPriority represents an option encapsulating the server priority. type OptServerPriority struct { - Priority int + Priority uint16 } // ParseOptServerPriority returns a new OptServerPriority from a byte stream, or // error if any. func ParseOptServerPriority(data []byte) (*OptServerPriority, error) { - if len(data) < 4 { - return nil, dhcpv4.ErrShortByteStream - } - code := dhcpv4.OptionCode(data[0]) - if code != OptionServerPriority { - return nil, fmt.Errorf("expected code %v, got %v", OptionServerPriority, code) - } - length := int(data[1]) - if length != 2 { - return nil, fmt.Errorf("unexpected length: expected 2, got %v", length) - } - return &OptServerPriority{Priority: int(binary.BigEndian.Uint16(data[2:4]))}, nil + buf := uio.NewBigEndianBuffer(data) + return &OptServerPriority{Priority: buf.Read16()}, buf.FinError() } // Code returns the option code. diff --git a/dhcpv4/bsdp/bsdp_option_server_priority_test.go b/dhcpv4/bsdp/bsdp_option_server_priority_test.go index d12ad55..cbcef1d 100644 --- a/dhcpv4/bsdp/bsdp_option_server_priority_test.go +++ b/dhcpv4/bsdp/bsdp_option_server_priority_test.go @@ -22,16 +22,10 @@ func TestParseOptServerPriority(t *testing.T) { o, err = ParseOptServerPriority([]byte{}) require.Error(t, err, "empty byte stream") - o, err = ParseOptServerPriority([]byte{4, 2, 1}) + o, err = ParseOptServerPriority([]byte{1}) require.Error(t, err, "short byte stream") - o, err = ParseOptServerPriority([]byte{4, 3, 1, 1}) - require.Error(t, err, "wrong priority length") - - o, err = ParseOptServerPriority([]byte{53, 2, 168, 1}) - require.Error(t, err, "wrong option code") - - o, err = ParseOptServerPriority([]byte{4, 2, 0, 100}) + o, err = ParseOptServerPriority([]byte{0, 100}) require.NoError(t, err) - require.Equal(t, 100, o.Priority) + require.Equal(t, uint16(100), o.Priority) } diff --git a/dhcpv4/bsdp/bsdp_option_version.go b/dhcpv4/bsdp/bsdp_option_version.go index 8431a94..38158d7 100644 --- a/dhcpv4/bsdp/bsdp_option_version.go +++ b/dhcpv4/bsdp/bsdp_option_version.go @@ -4,10 +4,9 @@ import ( "fmt" "github.com/insomniacslk/dhcp/dhcpv4" + "github.com/u-root/u-root/pkg/uio" ) -// Implements the BSDP option version. Can be one of 1.0 or 1.1 - // Specific versions. var ( Version1_0 = []byte{1, 0} @@ -15,6 +14,8 @@ var ( ) // OptVersion represents a BSDP protocol version. +// +// Implements the BSDP option version. Can be one of 1.0 or 1.1 type OptVersion struct { Version []byte } @@ -22,18 +23,8 @@ type OptVersion struct { // ParseOptVersion constructs an OptVersion struct from a sequence of // bytes and returns it, or an error. func ParseOptVersion(data []byte) (*OptVersion, error) { - if len(data) < 4 { - return nil, dhcpv4.ErrShortByteStream - } - code := dhcpv4.OptionCode(data[0]) - if code != OptionVersion { - return nil, fmt.Errorf("expected option %v, got %v instead", OptionVersion, code) - } - length := int(data[1]) - if length != 2 { - return nil, fmt.Errorf("expected length 2, got %d instead", length) - } - return &OptVersion{data[2:4]}, nil + buf := uio.NewBigEndianBuffer(data) + return &OptVersion{buf.CopyN(2)}, buf.FinError() } // Code returns the option code. diff --git a/dhcpv4/bsdp/bsdp_option_version_test.go b/dhcpv4/bsdp/bsdp_option_version_test.go index c6a6afc..d28f243 100644 --- a/dhcpv4/bsdp/bsdp_option_version_test.go +++ b/dhcpv4/bsdp/bsdp_option_version_test.go @@ -14,25 +14,15 @@ func TestOptVersionInterfaceMethods(t *testing.T) { } func TestParseOptVersion(t *testing.T) { - data := []byte{2, 2, 1, 1} + data := []byte{1, 1} o, err := ParseOptVersion(data) require.NoError(t, err) require.Equal(t, &OptVersion{Version1_1}, o) // Short byte stream - data = []byte{2, 2} + data = []byte{2} _, err = ParseOptVersion(data) require.Error(t, err, "should get error from short byte stream") - - // Wrong code - data = []byte{54, 2, 1, 0} - _, err = ParseOptVersion(data) - require.Error(t, err, "should get error from wrong code") - - // Bad length - data = []byte{2, 4, 1, 0} - _, err = ParseOptVersion(data) - require.Error(t, err, "should get error from bad length") } func TestOptVersionString(t *testing.T) { diff --git a/dhcpv4/bsdp/option_vendor_specific_information.go b/dhcpv4/bsdp/option_vendor_specific_information.go index e735b57..1bd41a7 100644 --- a/dhcpv4/bsdp/option_vendor_specific_information.go +++ b/dhcpv4/bsdp/option_vendor_specific_information.go @@ -1,8 +1,6 @@ package bsdp import ( - "errors" - "fmt" "strings" "github.com/insomniacslk/dhcp/dhcpv4" @@ -11,20 +9,17 @@ import ( // OptVendorSpecificInformation encapsulates the BSDP-specific options used for // the protocol. type OptVendorSpecificInformation struct { - Options []dhcpv4.Option + Options dhcpv4.Options } // parseOption is similar to dhcpv4.ParseOption, except that it switches based // on the BSDP specific options. -func parseOption(data []byte) (dhcpv4.Option, error) { - if len(data) == 0 { - return nil, dhcpv4.ErrZeroLengthByteStream - } +func parseOption(code dhcpv4.OptionCode, data []byte) (dhcpv4.Option, error) { var ( opt dhcpv4.Option err error ) - switch dhcpv4.OptionCode(data[0]) { + switch code { case OptionBootImageList: opt, err = ParseOptBootImageList(data) case OptionDefaultBootImageID: @@ -44,7 +39,7 @@ func parseOption(data []byte) (dhcpv4.Option, error) { case OptionVersion: opt, err = ParseOptVersion(data) default: - opt, err = ParseOptGeneric(data) + opt, err = ParseOptGeneric(code, data) } if err != nil { return nil, err @@ -55,39 +50,10 @@ func parseOption(data []byte) (dhcpv4.Option, error) { // ParseOptVendorSpecificInformation constructs an OptVendorSpecificInformation struct from a sequence of // bytes and returns it, or an error. func ParseOptVendorSpecificInformation(data []byte) (*OptVendorSpecificInformation, error) { - // Should at least have code + length - if len(data) < 2 { - return nil, dhcpv4.ErrShortByteStream - } - code := dhcpv4.OptionCode(data[0]) - if code != dhcpv4.OptionVendorSpecificInformation { - return nil, fmt.Errorf("expected option %v, got %v instead", dhcpv4.OptionVendorSpecificInformation, code) - } - length := int(data[1]) - if len(data) < length+2 { - return nil, fmt.Errorf("expected length 2, got %d instead", length) - } - - options := make([]dhcpv4.Option, 0, 10) - idx := 2 - for { - if idx == len(data) { - break - } - // This should never happen. - if idx > len(data) { - return nil, errors.New("read past the end of options") - } - opt, err := parseOption(data[idx:]) - if err != nil { - return nil, err - } - options = append(options, opt) - - // Account for code + length bytes - idx += 2 + opt.Length() + options, err := dhcpv4.OptionsFromBytesWithParser(data, parseOption, false /* don't check for OptionEnd tag */) + if err != nil { + return nil, err } - return &OptVendorSpecificInformation{options}, nil } @@ -133,20 +99,10 @@ func (o *OptVendorSpecificInformation) Length() int { // GetOption returns all suboptions that match the given OptionCode code. func (o *OptVendorSpecificInformation) GetOption(code dhcpv4.OptionCode) []dhcpv4.Option { - var opts []dhcpv4.Option - for _, opt := range o.Options { - if opt.Code() == code { - opts = append(opts, opt) - } - } - return opts + return o.Options.GetOption(code) } // GetOneOption returns the first suboption that matches the OptionCode code. func (o *OptVendorSpecificInformation) GetOneOption(code dhcpv4.OptionCode) dhcpv4.Option { - opts := o.GetOption(code) - if len(opts) == 0 { - return nil - } - return opts[0] + return o.Options.GetOneOption(code) } diff --git a/dhcpv4/bsdp/option_vendor_specific_information_test.go b/dhcpv4/bsdp/option_vendor_specific_information_test.go index 8a4368f..8aa2bdf 100644 --- a/dhcpv4/bsdp/option_vendor_specific_information_test.go +++ b/dhcpv4/bsdp/option_vendor_specific_information_test.go @@ -34,19 +34,11 @@ func TestParseOptVendorSpecificInformation(t *testing.T) { o *OptVendorSpecificInformation err error ) - o, err = ParseOptVendorSpecificInformation([]byte{}) - require.Error(t, err, "empty byte stream") - o, err = ParseOptVendorSpecificInformation([]byte{1, 2}) require.Error(t, err, "short byte stream") - o, err = ParseOptVendorSpecificInformation([]byte{53, 2, 1, 1}) - require.Error(t, err, "wrong option code") - // Good byte stream data := []byte{ - 43, // code - 7, // length 1, 1, 1, // List option 2, 2, 1, 1, // Version option } @@ -59,13 +51,13 @@ func TestParseOptVendorSpecificInformation(t *testing.T) { }, } require.Equal(t, 2, len(o.Options), "number of parsed suboptions") - require.Equal(t, expected.Options[0].Code(), o.Options[0].Code()) - require.Equal(t, expected.Options[1].Code(), o.Options[1].Code()) + typ := o.GetOneOption(OptionMessageType) + version := o.GetOneOption(OptionVersion) + require.Equal(t, expected.Options[0].Code(), typ.Code()) + require.Equal(t, expected.Options[1].Code(), version.Code()) // Short byte stream (length and data mismatch) data = []byte{ - 43, // code - 7, // length 1, 1, 1, // List option 2, 2, 1, // Version option } @@ -74,8 +66,6 @@ func TestParseOptVendorSpecificInformation(t *testing.T) { // Bad option data = []byte{ - 43, // code - 7, // length 1, 1, 1, // List option 2, 2, 1, // Version option 5, 3, 1, 1, 1, // Reply port option @@ -85,8 +75,6 @@ func TestParseOptVendorSpecificInformation(t *testing.T) { // Boot images + default. data = []byte{ - 43, // code - 7, // length 1, 1, 1, // List option 2, 2, 1, 1, // Version option 5, 2, 1, 1, // Reply port option diff --git a/dhcpv4/bsdp/types.go b/dhcpv4/bsdp/types.go index ac4b05b..aa9a824 100644 --- a/dhcpv4/bsdp/types.go +++ b/dhcpv4/bsdp/types.go @@ -26,9 +26,9 @@ const ( OptionMachineName dhcpv4.OptionCode = 130 ) -// OptionCodeToString maps BSDP OptionCodes to human-readable strings +// optionCodeToString maps BSDP OptionCodes to human-readable strings // describing what they are. -var OptionCodeToString = map[dhcpv4.OptionCode]string{ +var optionCodeToString = map[dhcpv4.OptionCode]string{ OptionMessageType: "BSDP Message Type", OptionVersion: "BSDP Version", OptionServerIdentifier: "BSDP Server Identifier", -- cgit v1.2.3