Merge branch 'master' into lint

9 files changed, 140 insertions, 1 deletions

diff --git a/dhcpv4/dhcpv4.go b/dhcpv4/dhcpv4.go
index b894fbb..e2fd190 100644
--- a/dhcpv4/dhcpv4.go
+++ b/dhcpv4/dhcpv4.go
@@ -231,6 +231,7 @@ func NewInform(hwaddr net.HardwareAddr, localIP net.IP, modifiers ...Modifier) (
 }
 
 // NewRequestFromOffer builds a DHCPv4 request from an offer.
+// It assumes the SELECTING state by default, see Section 4.3.2 in RFC 2131 for more details.
 func NewRequestFromOffer(offer *DHCPv4, modifiers ...Modifier) (*DHCPv4, error) {
 	return New(PrependModifiers(modifiers,
 		WithReply(offer),
@@ -248,6 +249,21 @@ func NewRequestFromOffer(offer *DHCPv4, modifiers ...Modifier) (*DHCPv4, error)
 	)...)
 }
 
+// NewRenewFromOffer builds a DHCPv4 RENEW-style request from an offer. RENEW requests have minor
+// changes to their options compared to SELECT requests as specified by RFC 2131, section 4.3.2.
+func NewRenewFromOffer(offer *DHCPv4, modifiers ...Modifier) (*DHCPv4, error) {
+	return NewRequestFromOffer(offer, PrependModifiers(modifiers,
+		// The server identifier option must not be filled in
+		WithoutOption(OptionServerIdentifier),
+		// The requested IP address must not be filled in
+		WithoutOption(OptionRequestedIPAddress),
+		// The client IP must be filled in with the IP offered to the client
+		WithClientIP(offer.YourIPAddr),
+		// The renewal request must use unicast
+		WithBroadcast(false),
+	)...)
+}
+
 // NewReplyFromRequest builds a DHCPv4 reply from a request.
 func NewReplyFromRequest(request *DHCPv4, modifiers ...Modifier) (*DHCPv4, error) {
 	return New(PrependModifiers(modifiers,
@@ -382,6 +398,13 @@ func (d *DHCPv4) GetOneOption(code OptionCode) []byte {
 	return d.Options.Get(code)
 }
 
+// DeleteOption deletes an existing option with the given option code.
+func (d *DHCPv4) DeleteOption(code OptionCode) {
+	if d.Options != nil {
+		d.Options.Del(code)
+	}
+}
+
 // UpdateOption replaces an existing option with the same option code with the
 // given one, adding it if not already present.
 func (d *DHCPv4) UpdateOption(opt Option) {
diff --git a/dhcpv4/dhcpv4_test.go b/dhcpv4/dhcpv4_test.go
index 5d60b52..eff7a71 100644
--- a/dhcpv4/dhcpv4_test.go
+++ b/dhcpv4/dhcpv4_test.go
@@ -266,6 +266,43 @@ func TestDHCPv4NewRequestFromOfferWithModifier(t *testing.T) {
 	require.Equal(t, MessageTypeRequest, req.MessageType())
 }
 
+func TestDHCPv4NewRenewFromOffer(t *testing.T) {
+	offer, err := New()
+	require.NoError(t, err)
+	offer.SetBroadcast()
+	offer.UpdateOption(OptMessageType(MessageTypeOffer))
+	offer.UpdateOption(OptServerIdentifier(net.IPv4(192, 168, 0, 1)))
+	offer.UpdateOption(OptRequestedIPAddress(net.IPv4(192, 168, 0, 1)))
+	offer.YourIPAddr = net.IPv4(192, 168, 0, 1)
+
+	// RFC 2131: RENEW-style requests will be unicast
+	var req *DHCPv4
+	req, err = NewRenewFromOffer(offer)
+	require.NoError(t, err)
+	require.Equal(t, MessageTypeRequest, req.MessageType())
+	require.Nil(t, req.GetOneOption(OptionServerIdentifier))
+	require.Nil(t, req.GetOneOption(OptionRequestedIPAddress))
+	require.Equal(t, offer.YourIPAddr, req.ClientIPAddr)
+	require.True(t, req.IsUnicast())
+	require.False(t, req.IsBroadcast())
+	// Renewals should behave identically to initial requests regarding requested options
+	require.True(t, req.IsOptionRequested(OptionRouter))
+	require.True(t, req.IsOptionRequested(OptionSubnetMask))
+	require.True(t, req.IsOptionRequested(OptionDomainName))
+	require.True(t, req.IsOptionRequested(OptionDomainNameServer))
+}
+
+func TestDHCPv4NewRenewFromOfferWithModifier(t *testing.T) {
+	offer, err := New()
+	require.NoError(t, err)
+	offer.UpdateOption(OptMessageType(MessageTypeOffer))
+	userClass := WithUserClass("linuxboot", false)
+	req, err := NewRenewFromOffer(offer, userClass)
+	require.NoError(t, err)
+	require.Equal(t, MessageTypeRequest, req.MessageType())
+	require.Contains(t, req.UserClass(), "linuxboot")
+}
+
 func TestNewReplyFromRequest(t *testing.T) {
 	discover, err := New()
 	require.NoError(t, err)
diff --git a/dhcpv4/modifiers.go b/dhcpv4/modifiers.go
index 0ab35bc..68da298 100644
--- a/dhcpv4/modifiers.go
+++ b/dhcpv4/modifiers.go
@@ -99,6 +99,13 @@ func WithOption(opt Option) Modifier {
 	}
 }
 
+// WithoutOption removes the DHCPv4 option with the given code
+func WithoutOption(code OptionCode) Modifier {
+	return func(d *DHCPv4) {
+		d.DeleteOption(code)
+	}
+}
+
 // WithUserClass adds a user class option to the packet.
 // The rfc parameter allows you to specify if the userclass should be
 // rfc compliant or not. More details in issue #113
diff --git a/dhcpv4/modifiers_test.go b/dhcpv4/modifiers_test.go
index 274b409..57d8ff0 100644
--- a/dhcpv4/modifiers_test.go
+++ b/dhcpv4/modifiers_test.go
@@ -44,6 +44,17 @@ func TestWithOptionModifier(t *testing.T) {
 	require.Equal(t, "slackware.it", dnOpt)
 }
 
+func TestWithoutOptionModifier(t *testing.T) {
+	d, err := New(
+		WithOption(OptDomainName("slackware.it")),
+		WithoutOption(OptionDomainName),
+	)
+	require.NoError(t, err)
+
+	require.False(t, d.Options.Has(OptionDomainName))
+	require.Equal(t, "", d.DomainName())
+}
+
 func TestUserClassModifier(t *testing.T) {
 	d, err := New(WithUserClass("linuxboot", false))
 	require.NoError(t, err)
diff --git a/dhcpv4/nclient4/client.go b/dhcpv4/nclient4/client.go
index 02bd1f6..d40e1a9 100644
--- a/dhcpv4/nclient4/client.go
+++ b/dhcpv4/nclient4/client.go
@@ -493,6 +493,7 @@ func (e *ErrNak) Error() string {
 }
 
 // RequestFromOffer sends a Request message and waits for an response.
+// It assumes the SELECTING state by default, see Section 4.3.2 in RFC 2131 for more details.
 func (c *Client) RequestFromOffer(ctx context.Context, offer *dhcpv4.DHCPv4, modifiers ...dhcpv4.Modifier) (*Lease, error) {
 	// TODO(chrisko): should this be unicast to the server?
 	request, err := dhcpv4.NewRequestFromOffer(offer, dhcpv4.PrependModifiers(modifiers,
diff --git a/dhcpv4/nclient4/lease.go b/dhcpv4/nclient4/lease.go
index 184fae2..1895dd0 100644
--- a/dhcpv4/nclient4/lease.go
+++ b/dhcpv4/nclient4/lease.go
@@ -3,6 +3,7 @@
 package nclient4
 
 import (
+	"context"
 	"fmt"
 	"net"
 	"time"
@@ -36,3 +37,41 @@ func (c *Client) Release(lease *Lease, modifiers ...dhcpv4.Modifier) error {
 	}
 	return err
 }
+
+// Renew sends a DHCPv4 request to the server to renew the given lease. The renewal information is
+// sourced from the initial offer in the lease, and the ACK of the lease is updated to the ACK of
+// the latest renewal. This avoids issues with DHCP servers that omit information needed to build a
+// completely new lease from their renewal ACK (such as the Windows DHCP Server).
+func (c *Client) Renew(ctx context.Context, lease *Lease, modifiers ...dhcpv4.Modifier) error {
+	if lease == nil {
+		return fmt.Errorf("lease is nil")
+	}
+
+	request, err := dhcpv4.NewRenewFromOffer(lease.Offer, dhcpv4.PrependModifiers(modifiers,
+		dhcpv4.WithOption(dhcpv4.OptMaxMessageSize(MaxMessageSize)))...)
+	if err != nil {
+		return fmt.Errorf("unable to create a request: %w", err)
+	}
+
+	// Servers are supposed to only respond to Requests containing their server identifier,
+	// but sometimes non-compliant servers respond anyway.
+	// Clients are not required to validate this field, but servers are required to
+	// include the server identifier in their Offer per RFC 2131 Section 4.3.1 Table 3.
+	response, err := c.SendAndRead(ctx, c.serverAddr, request, IsAll(
+		IsCorrectServer(lease.Offer.ServerIdentifier()),
+		IsMessageType(dhcpv4.MessageTypeAck, dhcpv4.MessageTypeNak)))
+	if err != nil {
+		return fmt.Errorf("got an error while processing the request: %w", err)
+	}
+	if response.MessageType() == dhcpv4.MessageTypeNak {
+		return &ErrNak{
+			Offer: lease.Offer,
+			Nak:   response,
+		}
+	}
+
+	// Update the ACK of the lease with the ACK of the latest renewal
+	lease.ACK = response
+
+	return nil
+}
diff --git a/dhcpv4/nclient4/lease_test.go b/dhcpv4/nclient4/lease_test.go
index 361847f..d27eeca 100644
--- a/dhcpv4/nclient4/lease_test.go
+++ b/dhcpv4/nclient4/lease_test.go
@@ -1,4 +1,4 @@
-// this tests nclient4 with lease and release
+// this tests nclient4 with lease, renew and release
 
 package nclient4
 
@@ -236,6 +236,14 @@ func (sll *testServerLeaseList) runTest(t *testing.T) {
 		sll.lastTestSvrErrLock.RLock()
 		keepgoing := chkerr(err, sll.lastTestSvrErr, l.ShouldFail, t)
 		sll.lastTestSvrErrLock.RUnlock()
+
+		if keepgoing {
+			err = clnt.Renew(context.Background(), lease)
+			sll.lastTestSvrErrLock.RLock()
+			keepgoing = chkerr(err, sll.lastTestSvrErr, l.ShouldFail, t)
+			sll.lastTestSvrErrLock.RUnlock()
+		}
+
 		if keepgoing {
 			err = clnt.Release(lease)
 			//this sleep is to make sure release is handled by server
diff --git a/dhcpv4/options.go b/dhcpv4/options.go
index fdc79ae..9d404b4 100644
--- a/dhcpv4/options.go
+++ b/dhcpv4/options.go
@@ -81,6 +81,11 @@ func (o Options) Has(opcode OptionCode) bool {
 	return ok
 }
 
+// Del deletes the option matching the option code.
+func (o Options) Del(opcode OptionCode) {
+	delete(o, opcode.Code())
+}
+
 // Update updates the existing options with the passed option, adding it
 // at the end if not present already
 func (o Options) Update(option Option) {
diff --git a/dhcpv4/server4/server_test.go b/dhcpv4/server4/server_test.go
index b3f6d9d..7be868c 100644
--- a/dhcpv4/server4/server_test.go
+++ b/dhcpv4/server4/server_test.go
@@ -115,6 +115,14 @@ func TestServer(t *testing.T) {
 		require.Equal(t, xid, p.TransactionID)
 		require.Equal(t, ifaces[0].HardwareAddr, p.ClientHWAddr)
 	}
+
+	err = c.Renew(context.Background(), lease, modifiers...)
+	require.NoError(t, err)
+	require.NotNil(t, lease.Offer, lease.ACK)
+	for _, p := range []*dhcpv4.DHCPv4{lease.Offer, lease.ACK} {
+		require.Equal(t, xid, p.TransactionID)
+		require.Equal(t, ifaces[0].HardwareAddr, p.ClientHWAddr)
+	}
 }
 
 func TestBadAddrFamily(t *testing.T) {