Skip to content

Commit

Permalink
Add relaxed padding setting to the nclient4
Browse files Browse the repository at this point in the history
Signed-off-by: Bogdan-Denisenko <[email protected]>
  • Loading branch information
Bogdan-Denisenko committed Aug 2, 2024
1 parent ddd8a41 commit 79d51f4
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 9 deletions.
10 changes: 9 additions & 1 deletion dhcpv4/dhcpv4.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,15 @@ func NewReleaseFromACK(ack *DHCPv4, modifiers ...Modifier) (*DHCPv4, error) {

// FromBytes decodes a DHCPv4 packet from a sequence of bytes, and returns an
// error if the packet is not valid.
// Octets after End option are checked against the pad option.
func FromBytes(q []byte) (*DHCPv4, error) {
return FromBytesWithRelaxedPadding(q, false)
}

// FromBytesWithRelaxedPadding decodes a DHCPv4 packet from a sequence of bytes, and returns an
// error if the packet is not valid.
// Octets after the End option are checked or not according to pad option.
func FromBytesWithRelaxedPadding(q []byte, relaxedPadding bool) (*DHCPv4, error) {
var p DHCPv4
buf := uio.NewBigEndianBuffer(q)

Expand Down Expand Up @@ -353,7 +361,7 @@ func FromBytes(q []byte) (*DHCPv4, error) {
}

p.Options = make(Options)
if err := p.Options.fromBytesCheckEnd(buf.Data(), true); err != nil {
if err := p.Options.fromBytesWithRelaxedPadding(buf.Data(), true, relaxedPadding); err != nil {
return nil, err
}
return &p, nil
Expand Down
17 changes: 16 additions & 1 deletion dhcpv4/nclient4/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ type Client struct {
// bufferCap is the channel capacity for each TransactionID.
bufferCap int

// relaxedPadding is a flag that allows you to relax one of the packet parsing requirements.
// The RFC says that octets after the End option SHOULD be pad options. Not MUST.
// So we need a way to accept packets not padded with zeros.
relaxedPadding bool

// serverAddr is the UDP address to send all packets to.
//
// This may be an actual broadcast address, or a unicast address.
Expand Down Expand Up @@ -267,7 +272,7 @@ func (c *Client) receiveLoop() {
return
}

msg, err := dhcpv4.FromBytes(b[:n])
msg, err := dhcpv4.FromBytesWithRelaxedPadding(b[:n], c.relaxedPadding)
if err != nil {
// Not a valid DHCP packet; keep listening.
continue
Expand Down Expand Up @@ -396,6 +401,16 @@ func WithServerAddr(n *net.UDPAddr) ClientOpt {
}
}

// WithRelaxedPadding is an option that allows you to relax one of the packet parsing requirements.
// The RFC says that octets after the End option SHOULD be pad options. Not MUST.
// So we need a way to accept packets not padded with zeros.
func WithRelaxedPadding() ClientOpt {
return func(c *Client) (err error) {
c.relaxedPadding = true
return
}
}

// Matcher matches DHCP packets.
type Matcher func(*dhcpv4.DHCPv4) bool

Expand Down
64 changes: 64 additions & 0 deletions dhcpv4/nclient4/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,40 @@ func serveAndClient(ctx context.Context, responses [][]*dhcpv4.DHCPv4, opts ...C
return mc, serverConn
}

func serveAndClientWithBytesResp(response []byte, opts ...ClientOpt) *Client {
// Fake PacketConn connection.
clientRawConn, serverRawConn, err := socketpair.PacketSocketPair()
if err != nil {
panic(err)
}

clientConn := NewBroadcastUDPConn(clientRawConn, &net.UDPAddr{Port: ClientPort})
serverConn := NewBroadcastUDPConn(serverRawConn, &net.UDPAddr{Port: ServerPort})

o := []ClientOpt{WithRetry(1), WithTimeout(2 * time.Second)}
o = append(o, opts...)
mc, err := NewWithConn(clientConn, net.HardwareAddr{0xa, 0xb, 0xc, 0xd, 0xe, 0xf}, o...)
if err != nil {
panic(err)
}

// Fake server.
go func() {
b := make([]byte, 4096)
_, peer, err := serverConn.ReadFrom(b)
if err != nil {
panic(err)
}

_, err = serverConn.WriteTo(response, peer)
if err != nil {
panic(err)
}
}()

return mc
}

func ComparePacket(got *dhcpv4.DHCPv4, want *dhcpv4.DHCPv4) error {
if got == nil && got == want {
return nil
Expand Down Expand Up @@ -281,6 +315,36 @@ func TestSimpleSendAndReadDiscardGarbage(t *testing.T) {
}
}

func TestSendAndReadWithRelaxedPadding(t *testing.T) {
pkt := newPacket(dhcpv4.OpcodeBootRequest, [4]byte{0x33, 0x33, 0x33, 0x33})

// Both the server and client only get 2 seconds.
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()

response, err := dhcpv4.NewReplyFromRequest(pkt)
if err != nil {
t.Errorf("NewReplyFromRequest(%v) = %v, want nil", pkt, err)
}

bytes := response.ToBytes()

// Add garbage to the end.
bytes[len(bytes)-1] = 0x01

mc := serveAndClientWithBytesResp(bytes, WithRelaxedPadding())
defer mc.Close()

rcvd, err := mc.SendAndRead(ctx, DefaultServers, pkt, nil)
if err != nil {
t.Errorf("SendAndRead(%v) = %v, want nil", pkt, err)
}

if err := ComparePacket(rcvd, response); err != nil {
t.Errorf("got unexpected packets: %v", err)
}
}

func TestMultipleSendAndRead(t *testing.T) {
for _, tt := range []struct {
desc string
Expand Down
10 changes: 7 additions & 3 deletions dhcpv4/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func (o Options) ToBytes() []byte {
//
// Returns an error if any invalid option or length is found.
func (o Options) FromBytes(data []byte) error {
return o.fromBytesCheckEnd(data, false)
return o.fromBytesWithRelaxedPadding(data, false, false)
}

const (
Expand All @@ -115,9 +115,9 @@ const (
optEnd = 255
)

// FromBytesCheckEnd parses Options from byte sequences using the
// fromBytesWithRelaxedPadding parses Options from byte sequences using the
// parsing function that is passed in as a paremeter
func (o Options) fromBytesCheckEnd(data []byte, checkEndOption bool) error {
func (o Options) fromBytesWithRelaxedPadding(data []byte, checkEndOption bool, relaxedPadding bool) error {
if len(data) == 0 {
return nil
}
Expand Down Expand Up @@ -161,6 +161,10 @@ func (o Options) fromBytesCheckEnd(data []byte, checkEndOption bool) error {
return io.ErrUnexpectedEOF
}

if relaxedPadding {
return nil
}

// Any bytes left must be padding.
var pad uint8
for buf.Len() >= 1 {
Expand Down
15 changes: 11 additions & 4 deletions dhcpv4/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,10 @@ func TestOptionsMarshal(t *testing.T) {

func TestOptionsUnmarshal(t *testing.T) {
for i, tt := range []struct {
input []byte
want Options
wantError bool
input []byte
relaxedPadding bool
want Options
wantError bool
}{
{
// Buffer missing data.
Expand Down Expand Up @@ -259,6 +260,12 @@ func TestOptionsUnmarshal(t *testing.T) {
input: []byte{byte(OptionEnd), 3},
wantError: true,
},
{
// Option present after the End if relaxedPadding.
input: []byte{byte(OptionEnd), 3},
relaxedPadding: true,
want: Options{},
},
{
input: []byte{byte(OptionEnd)},
want: Options{},
Expand Down Expand Up @@ -306,7 +313,7 @@ func TestOptionsUnmarshal(t *testing.T) {
} {
t.Run(fmt.Sprintf("Test %02d", i), func(t *testing.T) {
opt := make(Options)
err := opt.fromBytesCheckEnd(tt.input, true)
err := opt.fromBytesWithRelaxedPadding(tt.input, true, tt.relaxedPadding)
if tt.wantError {
require.Error(t, err)
} else {
Expand Down

0 comments on commit 79d51f4

Please sign in to comment.