Pull request: 3538 dhcp options

Merge in DNS/adguard-home from 3538-dhcp-options to master

Closes #3538.
Updates #3366.

Squashed commit of the following:

commit 8b8cd118834aaf393fd13daf2afc9867794ee102
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Sep 13 19:57:09 2021 +0300

    dhcpd: imp tests

commit 1789171283280b6934eed87137a693cd310a9c0a
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Sep 13 19:04:12 2021 +0300

    dhcpd: fix ip version

commit 07108a95a2026592e72cabecbf6275b6dd50c18a
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Sep 13 18:56:21 2021 +0300

    all: imp log of changes

commit 461441b3709bf1383abebffa4067ea89f4763d79
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Sep 13 18:48:55 2021 +0300

    dhcpd: imp code & docs, log changes

commit 723f818baeadb9f0805cad96351a3b117155a103
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Sep 13 17:27:30 2021 +0300

    dhcpd: add default options

commit 575e9d01cf95a564aed31d26a6cc9376850d321a
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Sep 8 13:05:01 2021 +0300

    dhcpd: imp options logic

CHANGELOG.md

@@ -117,7 +117,8 @@ In this release, the schema version has changed from 10 to 12.
 
 - Panic when upstream server responds with empty question section ([#3551]).
 - 9GAG blocking ([#3564]).
-- DHCP now follows RFCs more closely ([#3443]).
+- DHCP now follows RFCs more closely when it comes to response sending and
+  option selection ([#3443], [#3538]).
 - Occasional panics when reading old statistics databases ([#3506]).
 - `reload` service action on macOS and FreeBSD ([#3457]).
 - Inaccurate using of service actions in the installation script ([#3450]).
@@ -190,6 +191,7 @@ In this release, the schema version has changed from 10 to 12.
 [#3450]: https://github.com/AdguardTeam/AdGuardHome/issues/3450
 [#3457]: https://github.com/AdguardTeam/AdGuardHome/issues/3457
 [#3506]: https://github.com/AdguardTeam/AdGuardHome/issues/3506
+[#3538]: https://github.com/AdguardTeam/AdGuardHome/issues/3538
 [#3551]: https://github.com/AdguardTeam/AdGuardHome/issues/3551
 [#3564]: https://github.com/AdguardTeam/AdGuardHome/issues/3564
 [#3568]: https://github.com/AdguardTeam/AdGuardHome/issues/3568

internal/dhcpd/options.go

@@ -1,134 +0,0 @@
-package dhcpd
-
-import (
-	"encoding/hex"
-	"fmt"
-	"net"
-	"strconv"
-	"strings"
-
-	"github.com/AdguardTeam/golibs/errors"
-)
-
-// hexDHCPOptionParserHandler parses a DHCP option as a hex-encoded string.
-// For example:
-//
-//   252 hex 736f636b733a2f2f70726f78792e6578616d706c652e6f7267
-//
-func hexDHCPOptionParserHandler(s string) (data []byte, err error) {
-	data, err = hex.DecodeString(s)
-	if err != nil {
-		return nil, fmt.Errorf("decoding hex: %w", err)
-	}
-
-	return data, nil
-}
-
-// ipDHCPOptionParserHandler parses a DHCP option as a single IP address.
-// For example:
-//
-//   6 ip 192.168.1.1
-//
-func ipDHCPOptionParserHandler(s string) (data []byte, err error) {
-	ip := net.ParseIP(s)
-	if ip == nil {
-		return nil, errors.Error("invalid ip")
-	}
-
-	// Most DHCP options require IPv4, so do not put the 16-byte
-	// version if we can.  Otherwise, the clients will receive weird
-	// data that looks like four IPv4 addresses.
-	//
-	// See https://github.com/AdguardTeam/AdGuardHome/issues/2688.
-	if ip4 := ip.To4(); ip4 != nil {
-		data = ip4
-	} else {
-		data = ip
-	}
-
-	return data, nil
-}
-
-// textDHCPOptionParserHandler parses a DHCP option as a simple UTF-8 encoded
-// text.  For example:
-//
-//   252 text http://192.168.1.1/wpad.dat
-//
-func ipsDHCPOptionParserHandler(s string) (data []byte, err error) {
-	ipStrs := strings.Split(s, ",")
-	for i, ipStr := range ipStrs {
-		var ipData []byte
-		ipData, err = ipDHCPOptionParserHandler(ipStr)
-		if err != nil {
-			return nil, fmt.Errorf("parsing ip at index %d: %w", i, err)
-		}
-
-		data = append(data, ipData...)
-	}
-
-	return data, nil
-}
-
-// ipsDHCPOptionParserHandler parses a DHCP option as a comma-separates list of
-// IP addresses.  For example:
-//
-//   6 ips 192.168.1.1,192.168.1.2
-//
-func textDHCPOptionParserHandler(s string) (data []byte, err error) {
-	return []byte(s), nil
-}
-
-// dhcpOptionParserHandler is a parser for a single dhcp option type.
-type dhcpOptionParserHandler func(s string) (data []byte, err error)
-
-// dhcpOptionParser parses DHCP options.
-type dhcpOptionParser struct {
-	handlers map[string]dhcpOptionParserHandler
-}
-
-// newDHCPOptionParser returns a new dhcpOptionParser.
-func newDHCPOptionParser() (p *dhcpOptionParser) {
-	return &dhcpOptionParser{
-		handlers: map[string]dhcpOptionParserHandler{
-			"hex":  hexDHCPOptionParserHandler,
-			"ip":   ipDHCPOptionParserHandler,
-			"ips":  ipsDHCPOptionParserHandler,
-			"text": textDHCPOptionParserHandler,
-		},
-	}
-}
-
-// parse parses an option.  See the handlers' documentation for more info.
-func (p *dhcpOptionParser) parse(s string) (code uint8, data []byte, err error) {
-	defer func() { err = errors.Annotate(err, "invalid option string %q: %w", s) }()
-
-	s = strings.TrimSpace(s)
-	parts := strings.SplitN(s, " ", 3)
-	if len(parts) < 3 {
-		return 0, nil, errors.Error("need at least three fields")
-	}
-
-	codeStr := parts[0]
-	typ := parts[1]
-	val := parts[2]
-
-	var code64 uint64
-	code64, err = strconv.ParseUint(codeStr, 10, 8)
-	if err != nil {
-		return 0, nil, fmt.Errorf("parsing option code: %w", err)
-	}
-
-	code = uint8(code64)
-
-	h, ok := p.handlers[typ]
-	if !ok {
-		return 0, nil, fmt.Errorf("unknown option type %q", typ)
-	}
-
-	data, err = h(val)
-	if err != nil {
-		return 0, nil, err
-	}
-
-	return code, data, nil
-}

internal/dhcpd/options_test.go

@@ -1,109 +0,0 @@
-package dhcpd
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-func TestDHCPOptionParser(t *testing.T) {
-	testCases := []struct {
-		name       string
-		in         string
-		wantErrMsg string
-		wantData   []byte
-		wantCode   uint8
-	}{{
-		name:       "hex_success",
-		in:         "6 hex c0a80101c0a80102",
-		wantErrMsg: "",
-		wantData:   []byte{0xC0, 0xA8, 0x01, 0x01, 0xC0, 0xA8, 0x01, 0x02},
-		wantCode:   6,
-	}, {
-		name:       "ip_success",
-		in:         "6 ip 1.2.3.4",
-		wantErrMsg: "",
-		wantData:   []byte{0x01, 0x02, 0x03, 0x04},
-		wantCode:   6,
-	}, {
-		name:       "ip_success_v6",
-		in:         "6 ip ::1234",
-		wantErrMsg: "",
-		wantData: []byte{
-			0x00, 0x00, 0x00, 0x00,
-			0x00, 0x00, 0x00, 0x00,
-			0x00, 0x00, 0x00, 0x00,
-			0x00, 0x00, 0x12, 0x34,
-		},
-		wantCode: 6,
-	}, {
-		name:       "ips_success",
-		in:         "6 ips 192.168.1.1,192.168.1.2",
-		wantErrMsg: "",
-		wantData:   []byte{0xC0, 0xA8, 0x01, 0x01, 0xC0, 0xA8, 0x01, 0x02},
-		wantCode:   6,
-	}, {
-		name:       "text_success",
-		in:         "252 text http://192.168.1.1/",
-		wantErrMsg: "",
-		wantData:   []byte("http://192.168.1.1/"),
-		wantCode:   252,
-	}, {
-		name:       "bad_parts",
-		in:         "6 ip",
-		wantErrMsg: `invalid option string "6 ip": need at least three fields`,
-		wantCode:   0,
-		wantData:   nil,
-	}, {
-		name: "bad_code",
-		in:   "256 ip 1.1.1.1",
-		wantErrMsg: `invalid option string "256 ip 1.1.1.1": parsing option code: ` +
-			`strconv.ParseUint: parsing "256": value out of range`,
-		wantCode: 0,
-		wantData: nil,
-	}, {
-		name:       "bad_type",
-		in:         "6 bad 1.1.1.1",
-		wantErrMsg: `invalid option string "6 bad 1.1.1.1": unknown option type "bad"`,
-		wantCode:   0,
-		wantData:   nil,
-	}, {
-		name: "hex_error",
-		in:   "6 hex ZZZ",
-		wantErrMsg: `invalid option string "6 hex ZZZ": decoding hex: ` +
-			`encoding/hex: invalid byte: U+005A 'Z'`,
-		wantData: nil,
-		wantCode: 0,
-	}, {
-		name:       "ip_error",
-		in:         "6 ip 1.2.3.x",
-		wantErrMsg: `invalid option string "6 ip 1.2.3.x": invalid ip`,
-		wantData:   nil,
-		wantCode:   0,
-	}, {
-		name: "ips_error",
-		in:   "6 ips 192.168.1.1,192.168.1.x",
-		wantErrMsg: `invalid option string "6 ips 192.168.1.1,192.168.1.x": ` +
-			`parsing ip at index 1: invalid ip`,
-		wantData: nil,
-		wantCode: 0,
-	}}
-
-	p := newDHCPOptionParser()
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			code, data, err := p.parse(tc.in)
-			if tc.wantErrMsg == "" {
-				assert.Nil(t, err)
-			} else {
-				require.NotNil(t, err)
-				assert.Equal(t, tc.wantErrMsg, err.Error())
-			}
-
-			assert.Equal(t, tc.wantCode, code)
-			assert.Equal(t, tc.wantData, data)
-		})
-	}
-}

internal/dhcpd/options_unix.go

@@ -0,0 +1,192 @@
+//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris
+// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris
+
+package dhcpd
+
+import (
+	"encoding/hex"
+	"fmt"
+	"net"
+	"strconv"
+	"strings"
+
+	"github.com/AdguardTeam/golibs/errors"
+	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/netutil"
+	"github.com/insomniacslk/dhcp/dhcpv4"
+)
+
+// The aliases for DHCP option types available for explicit declaration.
+const (
+	hexTyp  = "hex"
+	ipTyp   = "ip"
+	ipsTyp  = "ips"
+	textTyp = "text"
+)
+
+// parseDHCPOptionHex parses a DHCP option as a hex-encoded string.  For
+// example:
+//
+//   252 hex 736f636b733a2f2f70726f78792e6578616d706c652e6f7267
+//
+func parseDHCPOptionHex(s string) (val dhcpv4.OptionValue, err error) {
+	var data []byte
+	data, err = hex.DecodeString(s)
+	if err != nil {
+		return nil, fmt.Errorf("decoding hex: %w", err)
+	}
+
+	return dhcpv4.OptionGeneric{Data: data}, nil
+}
+
+// parseDHCPOptionIP parses a DHCP option as a single IP address.  For example:
+//
+//   6 ip 192.168.1.1
+//
+func parseDHCPOptionIP(s string) (val dhcpv4.OptionValue, err error) {
+	var ip net.IP
+	// All DHCPv4 options require IPv4, so don't put the 16-byte version.
+	// Otherwise, the clients will receive weird data that looks like four
+	// IPv4 addresses.
+	//
+	// See https://github.com/AdguardTeam/AdGuardHome/issues/2688.
+	if ip, err = netutil.ParseIPv4(s); err != nil {
+		return nil, err
+	}
+
+	return dhcpv4.IP(ip), nil
+}
+
+// parseDHCPOptionIPs parses a DHCP option as a comma-separates list of IP
+// addresses.  For example:
+//
+//   6 ips 192.168.1.1,192.168.1.2
+//
+func parseDHCPOptionIPs(s string) (val dhcpv4.OptionValue, err error) {
+	var ips dhcpv4.IPs
+	var ip net.IP
+	for i, ipStr := range strings.Split(s, ",") {
+		// See notes in the ipDHCPOptionParserHandler.
+		if ip, err = netutil.ParseIPv4(ipStr); err != nil {
+			return nil, fmt.Errorf("parsing ip at index %d: %w", i, err)
+		}
+
+		ips = append(ips, ip)
+	}
+
+	return ips, nil
+}
+
+// parseDHCPOptionText parses a DHCP option as a simple UTF-8 encoded
+// text.  For example:
+//
+//   252 text http://192.168.1.1/wpad.dat
+//
+func parseDHCPOptionText(s string) (val dhcpv4.OptionValue) {
+	return dhcpv4.OptionGeneric{Data: []byte(s)}
+}
+
+// parseDHCPOption parses an option.  See the documentation of parseDHCPOption*
+// for more info.
+func parseDHCPOption(s string) (opt dhcpv4.Option, err error) {
+	defer func() { err = errors.Annotate(err, "invalid option string %q: %w", s) }()
+
+	s = strings.TrimSpace(s)
+	parts := strings.SplitN(s, " ", 3)
+	if len(parts) < 3 {
+		return opt, errors.Error("need at least three fields")
+	}
+
+	var code64 uint64
+	code64, err = strconv.ParseUint(parts[0], 10, 8)
+	if err != nil {
+		return opt, fmt.Errorf("parsing option code: %w", err)
+	}
+
+	var optVal dhcpv4.OptionValue
+	switch typ, val := parts[1], parts[2]; typ {
+	case hexTyp:
+		optVal, err = parseDHCPOptionHex(val)
+	case ipTyp:
+		optVal, err = parseDHCPOptionIP(val)
+	case ipsTyp:
+		optVal, err = parseDHCPOptionIPs(val)
+	case textTyp:
+		optVal = parseDHCPOptionText(val)
+	default:
+		return opt, fmt.Errorf("unknown option type %q", typ)
+	}
+
+	if err != nil {
+		return opt, err
+	}
+
+	return dhcpv4.Option{
+		Code:  dhcpv4.GenericOptionCode(code64),
+		Value: optVal,
+	}, nil
+}
+
+// prepareOptions builds the set of DHCP options according to host requirements
+// document and values from conf.
+func prepareOptions(conf V4ServerConf) (opts dhcpv4.Options) {
+	opts = dhcpv4.Options{
+		// Set default values for host configuration parameters listed
+		// in Appendix A of RFC-2131.  Those parameters, if requested by
+		// client, should be returned with values defined by Host
+		// Requirements Document.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc2131#appendix-A.
+		//
+		// See also https://datatracker.ietf.org/doc/html/rfc1122,
+		// https://datatracker.ietf.org/doc/html/rfc1123, and
+		// https://datatracker.ietf.org/doc/html/rfc2132.
+
+		// IP-Layer Per Host
+
+		dhcpv4.OptionNonLocalSourceRouting.Code(): []byte{0},
+		// Set the current recommended default time to live for the
+		// Internet Protocol which is 64, see
+		// https://datatracker.ietf.org/doc/html/rfc1700.
+		dhcpv4.OptionDefaultIPTTL.Code(): []byte{64},
+
+		// IP-Layer Per Interface
+
+		dhcpv4.OptionPerformMaskDiscovery.Code():   []byte{0},
+		dhcpv4.OptionMaskSupplier.Code():           []byte{0},
+		dhcpv4.OptionPerformRouterDiscovery.Code(): []byte{1},
+		// The all-routers address is preferred wherever possible, see
+		// https://datatracker.ietf.org/doc/html/rfc1256#section-5.1.
+		dhcpv4.OptionRouterSolicitationAddress.Code(): net.IPv4allrouter.To4(),
+		dhcpv4.OptionBroadcastAddress.Code():          net.IPv4bcast.To4(),
+
+		// Link-Layer Per Interface
+
+		dhcpv4.OptionTrailerEncapsulation.Code():  []byte{0},
+		dhcpv4.OptionEthernetEncapsulation.Code(): []byte{0},
+
+		// TCP Per Host
+
+		dhcpv4.OptionTCPKeepaliveInterval.Code(): dhcpv4.Duration(0).ToBytes(),
+		dhcpv4.OptionTCPKeepaliveGarbage.Code():  []byte{0},
+
+		// Values From Configuration
+
+		dhcpv4.OptionRouter.Code():     netutil.CloneIP(conf.subnet.IP),
+		dhcpv4.OptionSubnetMask.Code(): dhcpv4.IPMask(conf.subnet.Mask).ToBytes(),
+	}
+
+	// Set values for explicitly configured options.
+	for i, o := range conf.Options {
+		opt, err := parseDHCPOption(o)
+		if err != nil {
+			log.Error("dhcpv4: bad option string at index %d: %s", i, err)
+
+			continue
+		}
+
+		opts.Update(opt)
+	}
+
+	return opts
+}

internal/dhcpd/options_unix_test.go

@@ -0,0 +1,177 @@
+//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris
+// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris
+
+package dhcpd
+
+import (
+	"fmt"
+	"net"
+	"testing"
+
+	"github.com/insomniacslk/dhcp/dhcpv4"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseOpt(t *testing.T) {
+	testCases := []struct {
+		name       string
+		in         string
+		wantErrMsg string
+		wantOpt    dhcpv4.Option
+	}{{
+		name:       "hex_success",
+		in:         "6 hex c0a80101c0a80102",
+		wantErrMsg: "",
+		wantOpt: dhcpv4.OptDNS(
+			net.IP{0xC0, 0xA8, 0x01, 0x01},
+			net.IP{0xC0, 0xA8, 0x01, 0x02},
+		),
+	}, {
+		name:       "ip_success",
+		in:         "6 ip 1.2.3.4",
+		wantErrMsg: "",
+		wantOpt: dhcpv4.OptDNS(
+			net.IP{0x01, 0x02, 0x03, 0x04},
+		),
+	}, {
+		name:       "ip_fail_v6",
+		in:         "6 ip ::1234",
+		wantErrMsg: "invalid option string \"6 ip ::1234\": bad ipv4 address \"::1234\"",
+		wantOpt:    dhcpv4.Option{},
+	}, {
+		name:       "ips_success",
+		in:         "6 ips 192.168.1.1,192.168.1.2",
+		wantErrMsg: "",
+		wantOpt: dhcpv4.OptDNS(
+			net.IP{0xC0, 0xA8, 0x01, 0x01},
+			net.IP{0xC0, 0xA8, 0x01, 0x02},
+		),
+	}, {
+		name:       "text_success",
+		in:         "252 text http://192.168.1.1/",
+		wantErrMsg: "",
+		wantOpt: dhcpv4.OptGeneric(
+			dhcpv4.GenericOptionCode(252),
+			[]byte("http://192.168.1.1/"),
+		),
+	}, {
+		name:       "bad_parts",
+		in:         "6 ip",
+		wantErrMsg: `invalid option string "6 ip": need at least three fields`,
+		wantOpt:    dhcpv4.Option{},
+	}, {
+		name: "bad_code",
+		in:   "256 ip 1.1.1.1",
+		wantErrMsg: `invalid option string "256 ip 1.1.1.1": parsing option code: ` +
+			`strconv.ParseUint: parsing "256": value out of range`,
+		wantOpt: dhcpv4.Option{},
+	}, {
+		name:       "bad_type",
+		in:         "6 bad 1.1.1.1",
+		wantErrMsg: `invalid option string "6 bad 1.1.1.1": unknown option type "bad"`,
+		wantOpt:    dhcpv4.Option{},
+	}, {
+		name: "hex_error",
+		in:   "6 hex ZZZ",
+		wantErrMsg: `invalid option string "6 hex ZZZ": decoding hex: ` +
+			`encoding/hex: invalid byte: U+005A 'Z'`,
+		wantOpt: dhcpv4.Option{},
+	}, {
+		name:       "ip_error",
+		in:         "6 ip 1.2.3.x",
+		wantErrMsg: "invalid option string \"6 ip 1.2.3.x\": bad ipv4 address \"1.2.3.x\"",
+		wantOpt:    dhcpv4.Option{},
+	}, {
+		name: "ips_error",
+		in:   "6 ips 192.168.1.1,192.168.1.x",
+		wantErrMsg: "invalid option string \"6 ips 192.168.1.1,192.168.1.x\": " +
+			"parsing ip at index 1: bad ipv4 address \"192.168.1.x\"",
+		wantOpt: dhcpv4.Option{},
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			opt, err := parseDHCPOption(tc.in)
+			if tc.wantErrMsg != "" {
+				require.Error(t, err)
+				assert.Equal(t, tc.wantErrMsg, err.Error())
+
+				return
+			}
+
+			require.NoError(t, err)
+
+			assert.Equal(t, tc.wantOpt.Code.Code(), opt.Code.Code())
+			assert.Equal(t, tc.wantOpt.Value.ToBytes(), opt.Value.ToBytes())
+		})
+	}
+}
+
+func TestPrepareOptions(t *testing.T) {
+	allDefault := dhcpv4.Options{
+		dhcpv4.OptionNonLocalSourceRouting.Code():     []byte{0},
+		dhcpv4.OptionDefaultIPTTL.Code():              []byte{64},
+		dhcpv4.OptionPerformMaskDiscovery.Code():      []byte{0},
+		dhcpv4.OptionMaskSupplier.Code():              []byte{0},
+		dhcpv4.OptionPerformRouterDiscovery.Code():    []byte{1},
+		dhcpv4.OptionRouterSolicitationAddress.Code(): []byte{224, 0, 0, 2},
+		dhcpv4.OptionBroadcastAddress.Code():          []byte{255, 255, 255, 255},
+		dhcpv4.OptionTrailerEncapsulation.Code():      []byte{0},
+		dhcpv4.OptionEthernetEncapsulation.Code():     []byte{0},
+		dhcpv4.OptionTCPKeepaliveInterval.Code():      []byte{0, 0, 0, 0},
+		dhcpv4.OptionTCPKeepaliveGarbage.Code():       []byte{0},
+	}
+	oneIP, otherIP := net.IP{1, 2, 3, 4}, net.IP{5, 6, 7, 8}
+
+	testCases := []struct {
+		name   string
+		opts   []string
+		checks dhcpv4.Options
+	}{{
+		name:   "all_default",
+		checks: allDefault,
+	}, {
+		name: "configured_ip",
+		opts: []string{
+			fmt.Sprintf("%d ip %s", dhcpv4.OptionBroadcastAddress, oneIP),
+		},
+		checks: dhcpv4.Options{
+			dhcpv4.OptionBroadcastAddress.Code(): oneIP,
+		},
+	}, {
+		name: "configured_ips",
+		opts: []string{
+			fmt.Sprintf("%d ips %s,%s", dhcpv4.OptionDomainNameServer, oneIP, otherIP),
+		},
+		checks: dhcpv4.Options{
+			dhcpv4.OptionDomainNameServer.Code(): append(oneIP, otherIP...),
+		},
+	}, {
+		name: "configured_bad",
+		opts: []string{
+			"20 hex",
+			"23 hex abc",
+			"32 ips 1,2,3,4",
+			"28 256.256.256.256",
+		},
+		checks: allDefault,
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			opts := prepareOptions(V4ServerConf{
+				// Just to avoid nil pointer dereference.
+				subnet:  &net.IPNet{},
+				Options: tc.opts,
+			})
+			for c, v := range tc.checks {
+				optVal := opts.Get(dhcpv4.GenericOptionCode(c))
+				require.NotNil(t, optVal)
+
+				assert.Len(t, optVal, len(v))
+				assert.Equal(t, v, optVal)
+			}
+		})
+	}
+}

internal/dhcpd/server.go

@@ -72,8 +72,6 @@ type V4ServerConf struct {
 	// gateway.
 	subnet *net.IPNet
 
-	options []dhcpOption
-
 	// notify is a way to signal to other components that leases have
 	// change.  notify must be called outside of locked sections, since the
 	// clients might want to get the new data.
@@ -104,8 +102,3 @@ type V6ServerConf struct {
 	// Server calls this function when leases data changes
 	notify func(uint32)
 }
-
-type dhcpOption struct {
-	code uint8
-	data []byte
-}

internal/dhcpd/v4.go

@@ -40,6 +40,9 @@ type v4Server struct {
 
 	// leasesLock protects leases, leaseHosts, and leasedOffsets.
 	leasesLock sync.Mutex
+
+	// options holds predefined DHCP options to return to clients.
+	options dhcpv4.Options
 }
 
 // WriteDiskConfig4 - write configuration
@@ -831,6 +834,9 @@ func (s *v4Server) processRelease(req, resp *dhcpv4.DHCPv4) (err error) {
 func (s *v4Server) process(req, resp *dhcpv4.DHCPv4) int {
 	var err error
 
+	// Include server's identifier option since any reply should contain it.
+	//
+	// See https://datatracker.ietf.org/doc/html/rfc2131#page-29.
 	resp.UpdateOption(dhcpv4.OptServerIdentifier(s.conf.dnsIPAddrs[0]))
 
 	// TODO(a.garipov): Refactor this into handlers.
@@ -873,17 +879,29 @@ func (s *v4Server) process(req, resp *dhcpv4.DHCPv4) int {
 	}
 
 	if l != nil {
-		resp.YourIPAddr = make([]byte, 4)
+		resp.YourIPAddr = netutil.CloneIP(l.IP)
-		copy(resp.YourIPAddr, l.IP)
 	}
 
+	// Set IP address lease time for all DHCPOFFER messages and DHCPACK
+	// messages replied for DHCPREQUEST.
+	//
+	// TODO(e.burkov):  Inspect why this is always set to configured value.
 	resp.UpdateOption(dhcpv4.OptIPAddressLeaseTime(s.conf.leaseTime))
-	resp.UpdateOption(dhcpv4.OptRouter(s.conf.subnet.IP))
-	resp.UpdateOption(dhcpv4.OptSubnetMask(s.conf.subnet.Mask))
-	resp.UpdateOption(dhcpv4.OptDNS(s.conf.dnsIPAddrs...))
 
-	for _, opt := range s.conf.options {
+	// Update values for each explicitly configured parameter requested by
-		resp.Options[opt.code] = opt.data
+	// client.
+	//
+	// See https://datatracker.ietf.org/doc/html/rfc2131#section-4.3.1.
+	requested := req.ParameterRequestList()
+	for _, code := range requested {
+		if configured := s.options; configured.Has(code) {
+			resp.UpdateOption(dhcpv4.OptGeneric(code, configured.Get(code)))
+		}
+	}
+	// Update the value of Domain Name Server option separately from others
+	// since its value is set after server's creating.
+	if requested.Has(dhcpv4.OptionDomainNameServer) {
+		resp.UpdateOption(dhcpv4.OptDNS(s.conf.dnsIPAddrs...))
 	}
 
 	return 1
@@ -897,7 +915,8 @@ func (s *v4Server) packetHandler(conn net.PacketConn, peer net.Addr, req *dhcpv4
 	log.Debug("dhcpv4: received message: %s", req.Summary())
 
 	switch req.MessageType() {
-	case dhcpv4.MessageTypeDiscover,
+	case
+		dhcpv4.MessageTypeDiscover,
 		dhcpv4.MessageTypeRequest,
 		dhcpv4.MessageTypeDecline,
 		dhcpv4.MessageTypeRelease:
@@ -931,13 +950,13 @@ func (s *v4Server) packetHandler(conn net.PacketConn, peer net.Addr, req *dhcpv4
 	s.send(peer, conn, req, resp)
 }
 
-// send writes resp for peer to conn considering the req's fields and options
+// send writes resp for peer to conn considering the req's parameters according
-// according to RFC-2131.
+// to RFC-2131.
 //
 // See https://datatracker.ietf.org/doc/html/rfc2131#section-4.1.
 func (s *v4Server) send(peer net.Addr, conn net.PacketConn, req, resp *dhcpv4.DHCPv4) {
 	var unicast bool
-	if giaddr, unspec := req.GatewayIPAddr, netutil.IPv4Zero(); !giaddr.Equal(unspec) {
+	if giaddr := req.GatewayIPAddr; giaddr != nil && !giaddr.IsUnspecified() {
 		// Send any return messages to the server port on the BOOTP
 		// relay agent whose address appears in giaddr.
 		peer = &net.UDPAddr{
@@ -947,7 +966,7 @@ func (s *v4Server) send(peer net.Addr, conn net.PacketConn, req, resp *dhcpv4.DH
 		unicast = true
 	} else if mtype := resp.MessageType(); mtype == dhcpv4.MessageTypeNak {
 		// Broadcast any DHCPNAK messages to 0xffffffff.
-	} else if ciaddr := req.ClientIPAddr; !ciaddr.Equal(unspec) {
+	} else if ciaddr := req.ClientIPAddr; ciaddr != nil && !ciaddr.IsUnspecified() {
 		// Unicast DHCPOFFER and DHCPACK messages to the address in
 		// ciaddr.
 		peer = &net.UDPAddr{
@@ -1104,25 +1123,7 @@ func v4Create(conf V4ServerConf) (srv DHCPServer, err error) {
 		s.conf.leaseTime = time.Second * time.Duration(conf.LeaseDuration)
 	}
 
-	p := newDHCPOptionParser()
+	s.options = prepareOptions(s.conf)
-
-	for i, o := range conf.Options {
-		var code uint8
-		var data []byte
-		code, data, err = p.parse(o)
-		if err != nil {
-			log.Error("dhcpv4: bad option string at index %d: %s", i, err)
-
-			continue
-		}
-
-		opt := dhcpOption{
-			code: code,
-			data: data,
-		}
-
-		s.conf.options = append(s.conf.options, opt)
-	}
 
 	return s, nil
 }

internal/dhcpd/v4_test.go

@@ -148,7 +148,9 @@ func TestV4StaticLease_Get(t *testing.T) {
 	mac := net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA}
 
 	t.Run("discover", func(t *testing.T) {
-		req, err = dhcpv4.NewDiscovery(mac)
+		req, err = dhcpv4.NewDiscovery(mac, dhcpv4.WithRequestedOptions(
+			dhcpv4.OptionDomainNameServer,
+		))
 		require.NoError(t, err)
 
 		resp, err = dhcpv4.NewReplyFromRequest(req)
@@ -231,7 +233,10 @@ func TestV4DynamicLease_Get(t *testing.T) {
 	mac := net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA}
 
 	t.Run("discover", func(t *testing.T) {
-		req, err = dhcpv4.NewDiscovery(mac)
+		req, err = dhcpv4.NewDiscovery(mac, dhcpv4.WithRequestedOptions(
+			dhcpv4.OptionFQDN,
+			dhcpv4.OptionRelayAgentInformation,
+		))
 		require.NoError(t, err)
 
 		resp, err = dhcpv4.NewReplyFromRequest(req)
@@ -257,9 +262,11 @@ func TestV4DynamicLease_Get(t *testing.T) {
 
 		assert.Equal(t, s.conf.subnet.Mask, resp.SubnetMask())
 		assert.Equal(t, s.conf.leaseTime.Seconds(), resp.IPAddressLeaseTime(-1).Seconds())
-		assert.Equal(t, []byte("012"), resp.Options[uint8(dhcpv4.OptionFQDN)])
+		assert.Equal(t, []byte("012"), resp.Options.Get(dhcpv4.OptionFQDN))
 
-		assert.Equal(t, net.IP{1, 2, 3, 4}, net.IP(resp.RelayAgentInfo().ToBytes()))
+		rai := resp.RelayAgentInfo()
+		require.NotNil(t, rai)
+		assert.Equal(t, net.IP{1, 2, 3, 4}, net.IP(rai.ToBytes()))
 	})
 
 	t.Run("request", func(t *testing.T) {