From 916e0d0fc6b84b1be09ed1bb16b8de09943a9921 Mon Sep 17 00:00:00 2001 From: Christopher Koch Date: Sun, 13 Jan 2019 15:52:18 -0500 Subject: dhcpv4: conform to RFC 2131 with respect to options. Removes AddOption and GetOption. RFC 2131 specifies that options may only appear once (Section 4.1). If an option does appear more than once, its byte values must be concatenated. RFC 3396 further specifies that to send options longer than 255 bytes, one option may be split into multiple option codes, which must be concatenated back together by the receiver. Both of these are concerned with the byte representation of options. Fact is, based on both RFCs one can say that an option may only appear once, but may be composed of multiple values. Because an option may appear only once logically in any case, we remove the AddOption and GetOption functions and leave only UpdateOption and GetOneOption. Also remove all additions & checks of the End option - the marshaling and unmarshaling code is exclusively responsible for that now. --- dhcpv4/bsdp/bsdp_test.go | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) (limited to 'dhcpv4/bsdp/bsdp_test.go') diff --git a/dhcpv4/bsdp/bsdp_test.go b/dhcpv4/bsdp/bsdp_test.go index cb1a456..2caa6e5 100644 --- a/dhcpv4/bsdp/bsdp_test.go +++ b/dhcpv4/bsdp/bsdp_test.go @@ -37,7 +37,7 @@ func TestParseBootImageListFromAck(t *testing.T) { }, } ack, _ := dhcpv4.New() - ack.AddOption(&OptVendorSpecificInformation{ + ack.UpdateOption(&OptVendorSpecificInformation{ []dhcpv4.Option{&OptBootImageList{expectedBootImages}}, }) @@ -69,7 +69,6 @@ func TestNewInformList_NoReplyPort(t *testing.T) { require.True(t, m.Options.Has(dhcpv4.OptionVendorSpecificInformation)) require.True(t, m.Options.Has(dhcpv4.OptionParameterRequestList)) require.True(t, m.Options.Has(dhcpv4.OptionMaximumDHCPMessageSize)) - require.True(t, m.Options.Has(dhcpv4.OptionEnd)) opt := m.GetOneOption(dhcpv4.OptionVendorSpecificInformation) require.NotNil(t, opt, "vendor opts not present") @@ -108,8 +107,7 @@ func newAck(hwAddr net.HardwareAddr, transactionID [4]byte) *dhcpv4.DHCPv4 { ack.TransactionID = transactionID ack.HWType = iana.HWTypeEthernet ack.ClientHWAddr = hwAddr - ack.AddOption(&dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeAck}) - ack.AddOption(&dhcpv4.OptionGeneric{OptionCode: dhcpv4.OptionEnd}) + ack.UpdateOption(&dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeAck}) return ack } @@ -127,7 +125,7 @@ func TestInformSelectForAck_Broadcast(t *testing.T) { } ack := newAck(hwAddr, tid) ack.SetBroadcast() - ack.AddOption(&dhcpv4.OptServerIdentifier{ServerID: serverID}) + ack.UpdateOption(&dhcpv4.OptServerIdentifier{ServerID: serverID}) m, err := InformSelectForAck(*ack, 0, bootImage) require.NoError(t, err) @@ -143,7 +141,6 @@ func TestInformSelectForAck_Broadcast(t *testing.T) { require.True(t, m.Options.Has(dhcpv4.OptionDHCPMessageType)) opt := m.GetOneOption(dhcpv4.OptionDHCPMessageType) require.Equal(t, dhcpv4.MessageTypeInform, opt.(*dhcpv4.OptMessageType).MessageType) - require.True(t, m.Options.Has(dhcpv4.OptionEnd)) // Validate vendor opts. require.True(t, m.Options.Has(dhcpv4.OptionVendorSpecificInformation)) @@ -186,7 +183,7 @@ func TestInformSelectForAck_BadReplyPort(t *testing.T) { } ack := newAck(hwAddr, tid) ack.SetBroadcast() - ack.AddOption(&dhcpv4.OptServerIdentifier{ServerID: serverID}) + ack.UpdateOption(&dhcpv4.OptServerIdentifier{ServerID: serverID}) _, err := InformSelectForAck(*ack, 11223, bootImage) require.Error(t, err, "expect error for > 1024 replyPort") @@ -206,7 +203,7 @@ func TestInformSelectForAck_ReplyPort(t *testing.T) { } ack := newAck(hwAddr, tid) ack.SetBroadcast() - ack.AddOption(&dhcpv4.OptServerIdentifier{ServerID: serverID}) + ack.UpdateOption(&dhcpv4.OptServerIdentifier{ServerID: serverID}) replyPort := uint16(999) m, err := InformSelectForAck(*ack, replyPort, bootImage) @@ -281,9 +278,6 @@ func TestNewReplyForInformList(t *testing.T) { RequireHasOption(t, ack.Options, &dhcpv4.OptClassIdentifier{Identifier: AppleVendorID}) require.NotNil(t, ack.GetOneOption(dhcpv4.OptionVendorSpecificInformation)) - // Ensure options terminated with End option. - require.Equal(t, &dhcpv4.OptionGeneric{OptionCode: dhcpv4.OptionEnd}, ack.Options[len(ack.Options)-1]) - // Vendor-specific options. vendorOpts := ack.GetOneOption(dhcpv4.OptionVendorSpecificInformation).(*OptVendorSpecificInformation) RequireHasOption(t, vendorOpts.Options, &OptMessageType{Type: MessageTypeList}) @@ -363,9 +357,6 @@ func TestNewReplyForInformSelect(t *testing.T) { RequireHasOption(t, ack.Options, &dhcpv4.OptClassIdentifier{Identifier: AppleVendorID}) require.NotNil(t, ack.GetOneOption(dhcpv4.OptionVendorSpecificInformation)) - // Ensure options are terminated with End option. - require.Equal(t, &dhcpv4.OptionGeneric{OptionCode: dhcpv4.OptionEnd}, ack.Options[len(ack.Options)-1]) - vendorOpts := ack.GetOneOption(dhcpv4.OptionVendorSpecificInformation).(*OptVendorSpecificInformation) RequireHasOption(t, vendorOpts.Options, &OptMessageType{Type: MessageTypeSelect}) RequireHasOption(t, vendorOpts.Options, &OptSelectedBootImageID{ID: images[0].ID}) @@ -424,7 +415,7 @@ func TestMessageTypeForPacket(t *testing.T) { t.Run(tt.tcName, func(t *testing.T) { pkt, _ = dhcpv4.New() for _, opt := range tt.opts { - pkt.AddOption(opt) + pkt.UpdateOption(opt) } gotMessageType = MessageTypeFromPacket(pkt) require.Equal(t, tt.wantMessageType, gotMessageType) -- cgit v1.2.3