switch to encoding DHCP options instead of appending

dhcpv4/client.go

@@ -42,6 +42,7 @@ type RequestConfig struct {
 	ClientHardwareAddr [6]byte
 	// Optional hostname to request.
 	Hostname string
+	ClientID string
 }
 
 func (c *Client) BeginRequest(xid uint32, cfg RequestConfig) error {
@@ -108,24 +109,29 @@ func (c *Client) Encapsulate(carrierFrame []byte, frameOffset int) (int, error)
 	if err != nil {
 		return 0, err
 	}
+	opts := frm.OptionsPayload()
+	if len(opts) < 255 {
+		return 0, errors.New("too short packet for options")
+	}
 
-	// var options []Option
-	// var nextState ClientState
-	optBuf := c.auxbuf[:0]
 	var nextState ClientState
+	var numOpts int
 	switch c.state {
 	case StateInit:
 		// Send out discover.
-		optBuf = AppendOption(optBuf, OptMessageType, byte(MsgDiscover))
+		n, _ := EncodeOption(opts[numOpts:], OptMessageType, byte(MsgDiscover))
-		optBuf = AppendOption(optBuf, OptParameterRequestList, defaultParamReqList...)
+		numOpts += n
-		optBuf = AppendOption(optBuf, OptClientIdentifier, c.clientMAC[:]...)
+		n, _ = EncodeOption(opts[numOpts:], OptParameterRequestList, defaultParamReqList...)
+		numOpts += n
 		maxlen := len(dst)
 		if maxlen > math.MaxUint16 {
 			maxlen = math.MaxUint16
 		}
-		optBuf = AppendOption(optBuf, OptMaximumMessageSize, byte(maxlen>>8), byte(maxlen))
+		n, _ = EncodeOption16(opts[numOpts:], OptMaximumMessageSize, uint16(maxlen))
+		numOpts += n
 		if c.reqIP != [4]byte{} {
-			optBuf = AppendOption(optBuf, OptRequestedIPaddress, c.reqIP[:]...)
+			n, _ = EncodeOption(opts[numOpts:], OptRequestedIPaddress, c.reqIP[:]...)
+			numOpts += n
 		}
 		nextState = StateSelecting
 
@@ -134,27 +140,33 @@ func (c *Client) Encapsulate(carrierFrame []byte, frameOffset int) (int, error)
 			return 0, nil // Offer not yet received.
 		}
 		// Send out request, we know we've received an offer by now.
-		optBuf = AppendOption(optBuf, OptMessageType, byte(MsgRequest))
+		n, _ := EncodeOption(opts[numOpts:], OptMessageType, byte(MsgRequest))
-		optBuf = AppendOption(optBuf, OptRequestedIPaddress, c.offer[:]...)
+		numOpts += n
-		optBuf = AppendOption(optBuf, OptServerIdentification, c.svip[:]...)
+		n, _ = EncodeOption(opts[numOpts:], OptRequestedIPaddress, c.offer[:]...)
+		numOpts += n
+		n, _ = EncodeOption(opts[numOpts:], OptServerIdentification, c.svip[:]...)
+		numOpts += n
 		nextState = StateRequesting
 
 	default:
 		return 0, errors.New("unhandled state")
 	}
+	n, _ := EncodeOption(opts[numOpts:], OptClientIdentifier, c.clientMAC[:]...)
+	numOpts += n
 	if len(c.reqHostname) > 0 {
-		optBuf = AppendOptionString(optBuf, OptHostName, c.reqHostname)
+		n, err := EncodeOptionString(opts[numOpts:], OptHostName, c.reqHostname)
-	}
+		numOpts += n
-	optBuf = append(optBuf, 0xff) // End mark.
+		if err != nil {
-	options := frm.OptionsPayload()
+			return 0, err
-	if len(optBuf) > len(options) {
+		}
-		return 0, errors.New("DHCPv4 short buffer for options")
 	}
+
+	opts[numOpts] = byte(OptEnd)
+	numOpts++
 	c.setHeader(frm)
-	n := copy(options, optBuf)
 	c.setIP(carrierFrame, frameOffset)
 	c.state = nextState
-	return optionsOffset + n, nil
+	return optionsOffset + numOpts, nil
 }
 
 func (c *Client) Demux(carrierData []byte, frameOffset int) error {

dhcpv4/definitions.go

@@ -35,18 +35,19 @@ func (state ClientState) HasIP() bool {
 	return state == StateBound || state == StateRenewing || state == StateRebinding
 }
 
-func AppendOption(dst []byte, opt OptNum, data ...byte) []byte {
+func EncodeOptionString(dst []byte, opt OptNum, data string) (int, error) {
-	if len(data) > 255 {
+	bdata := unsafe.Slice(unsafe.StringData(data), len(data))
-		panic("option data too long")
+	return EncodeOption(dst, opt, bdata...)
-	}
-	dst = append(dst, byte(opt), byte(len(data)))
-	dst = append(dst, data...)
-	return dst
 }
 
-func AppendOptionString(dst []byte, opt OptNum, data string) []byte {
+func EncodeOption16(dst []byte, opt OptNum, v uint16) (int, error) {
-	bdata := unsafe.Slice(unsafe.StringData(data), len(data))
+	// See binary.BigEndian.PutUint16()
-	return AppendOption(dst, opt, bdata...)
+	return EncodeOption(dst, opt, byte(v>>8), byte(v))
+}
+
+func EncodeOption32(dst []byte, opt OptNum, v uint32) (int, error) {
+	// See binary.BigEndian.PutUint32()
+	return EncodeOption(dst, opt, byte(v>>24), byte(v>>16), byte(v>>8), byte(v))
 }
 
 func EncodeOption(dst []byte, opt OptNum, data ...byte) (int, error) {
@@ -66,6 +67,8 @@ type OptNum uint8
 
 // DHCP options. Taken from https://help.sonicwall.com/help/sw/eng/6800/26/2/3/content/Network_DHCP_Server.042.12.htm.
 const (
+	OptEnd OptNum = 255 // end options
+
 	OptWordAligned                 OptNum = 0  // word-aligned
 	OptSubnetMask                  OptNum = 1  // subnet mask
 	OptTimeOffset                  OptNum = 2  // Time offset in seconds from UTC
@@ -128,6 +131,7 @@ const (
 	OptRebindingTimeValue          OptNum = 59 // DHCP rebinding (T2) time
 	OptClientIdentifier            OptNum = 60 // Client identifier
 	OptClientIdentifier1           OptNum = 61 // Client identifier(1)
+
 )
 
 type Op byte

dhcpv4/dhcp_test.go

@@ -78,3 +78,65 @@ func TestClientServer(t *testing.T) {
 	}
 	assertClState(StateBound)
 }
+
+func TestExample(t *testing.T) {
+	const (
+		xid        = 1
+		offerLease = 9001
+	)
+	var cl Client
+	clientHwaddr := [6]byte{0, 0, 0, 0, 0, 1}
+	clientReqAddr := [4]byte{192, 168, 1, 2}
+	clientHostname := "client"
+	serverIP := [4]byte{192, 168, 1, 1}
+	subnetMask := [4]byte{255, 255, 255, 0}
+	routerAddr := [4]byte{192, 168, 1, 0}
+	dnsAddr := [4]byte{192, 168, 1, 255}
+	cl.BeginRequest(xid, RequestConfig{
+		RequestedAddr:      clientReqAddr,
+		ClientHardwareAddr: clientHwaddr,
+		Hostname:           clientHostname,
+	})
+	buf := make([]byte, 2048)
+	n, err := cl.Encapsulate(buf, 0)
+	if err != nil {
+		t.Fatal(err)
+	} else if n <= 0 {
+		t.Fatal("no data sent out by client after starting request")
+	}
+
+	dfrm, _ := NewFrame(buf)
+	dfrm.ClearHeader()
+	dfrm.SetOp(OpReply)
+	dfrm.SetHardware(1, 6, 0)
+	dfrm.SetFlags(0)
+	dfrm.SetXID(xid)
+	dfrm.SetSecs(1)
+	*dfrm.YIAddr() = clientReqAddr
+	copy(dfrm.CHAddr()[:], clientHwaddr[:])
+	dfrm.SetMagicCookie(MagicCookie)
+	ntot := 0
+	nopt, _ := EncodeOption(buf[optionsOffset+ntot:], OptMessageType, byte(MsgOffer))
+	ntot += nopt
+	nopt, _ = EncodeOption(buf[optionsOffset+ntot:], OptServerIdentification, serverIP[:]...)
+	ntot += nopt
+	nopt, _ = EncodeOption32(buf[optionsOffset+ntot:], OptServerIdentification, offerLease)
+	ntot += nopt
+	nopt, _ = EncodeOption(buf[optionsOffset+ntot:], OptSubnetMask, subnetMask[:]...)
+	ntot += nopt
+	nopt, _ = EncodeOption(buf[optionsOffset+ntot:], OptRouter, routerAddr[:]...)
+	ntot += nopt
+	nopt, _ = EncodeOption(buf[optionsOffset+ntot:], OptDNSServers, dnsAddr[:]...)
+	ntot += nopt
+	nopt, _ = EncodeOption(buf[optionsOffset+ntot:], OptEnd, dnsAddr[:]...)
+	ntot += nopt
+
+	err = cl.Demux(buf[:optionsOffset+ntot], 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// frame := buf[:optionsOffset]
+	// frame = AppendOption(frame, OptMessageType, byte(MsgOffer))
+	// frame = AppendOption(frame, OptServerIdentification, serverIP[:]...)
+	// frame = AppendOption(frame, OptIPAddressLeaseTime, serverIP[:]...)
+}

dhcpv4/server.go

@@ -136,8 +136,14 @@ func (sv *Server) Demux(carrierData []byte, frameOffset int) error {
 		sv.pending++
 
 	case MsgRequest:
-		if client.state != StateSelecting && client.state != StateRequesting {
+		if !clientExists {
+			err = errors.New("request for non existing client?")
+		} else if dfrm.XID() != client.xid {
+			err = errors.New("unexpected XID for client")
+		} else if client.state != StateSelecting && client.state != StateRequesting {
 			err = errors.New("DHCP request unexpected state")
+		}
+		if err != nil {
 			break
 		}
 		client.state = StateRequesting
@@ -151,16 +157,15 @@ func (sv *Server) Demux(carrierData []byte, frameOffset int) error {
 	}
 	sv.hosts[clientIDRaw] = client
 	return nil
-	// n := copy(dfrm.OptionsPayload(), optBuf)
 }
 
 func (sv *Server) Encapsulate(carrierData []byte, frameOffset int) (int, error) {
 	carrierIsIP := frameOffset >= 28
 	dfrm, err := NewFrame(carrierData[frameOffset:])
-	optBuf := dfrm.OptionsPayload()[:0]
+	optBuf := dfrm.OptionsPayload()[:]
 	if err != nil {
 		return 0, err
-	} else if cap(optBuf) < 255 {
+	} else if len(optBuf) < 255 {
 		return 0, errOptionNotFit
 	}
 	if sv.pending == 0 {
@@ -181,15 +186,27 @@ func (sv *Server) Encapsulate(carrierData []byte, frameOffset int) (int, error)
 		return 0, nil // Nothing to do.
 	}
 	futureState := ClientState(0)
+	var nopt int
 	switch client.state {
 	case StateInit:
 		futureState = StateSelecting
-		optBuf = AppendOption(optBuf, OptMessageType, byte(MsgOffer))
+		nopt, err = EncodeOption(optBuf[nopt:], OptMessageType, byte(MsgOffer))
 	case StateRequesting:
 		futureState = StateBound
-		optBuf = AppendOption(optBuf, OptMessageType, byte(MsgAck))
+		nopt, err = EncodeOption(optBuf[nopt:], OptMessageType, byte(MsgAck))
 		*dfrm.CIAddr() = client.addr
 	}
+	if err != nil {
+		return 0, err
+	}
+	n, _ := EncodeOption(optBuf[nopt:], OptServerIdentification, sv.siaddr[:]...)
+	nopt += n
+	if sv.gwaddr != [4]byte{} {
+		n, _ = EncodeOption(optBuf[nopt:], OptRouter, sv.gwaddr[:]...)
+		nopt += n
+	}
+	optBuf[nopt] = byte(OptEnd)
+	nopt++
 
 	dfrm.ClearHeader()
 	dfrm.SetOp(OpReply)
@@ -208,12 +225,13 @@ func (sv *Server) Encapsulate(carrierData []byte, frameOffset int) (int, error)
 			return 0, err
 		}
 	}
+
 	client.state = futureState
 
 	// Set server state.
 	sv.hosts[clientID] = client
 	sv.pending--
-	return optionsOffset + len(optBuf), nil
+	return optionsOffset + nopt, nil
 }
 
 func (sv *Server) getClient(clientID [36]byte) (serverEntry, bool) {