From fac574d3241546da868cf05f90ce5e4cdd1b4532 Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Mon, 13 Sep 2021 20:05:41 +0300 Subject: [PATCH] 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 Date: Mon Sep 13 19:57:09 2021 +0300 dhcpd: imp tests commit 1789171283280b6934eed87137a693cd310a9c0a Author: Eugene Burkov Date: Mon Sep 13 19:04:12 2021 +0300 dhcpd: fix ip version commit 07108a95a2026592e72cabecbf6275b6dd50c18a Author: Eugene Burkov Date: Mon Sep 13 18:56:21 2021 +0300 all: imp log of changes commit 461441b3709bf1383abebffa4067ea89f4763d79 Author: Eugene Burkov Date: Mon Sep 13 18:48:55 2021 +0300 dhcpd: imp code & docs, log changes commit 723f818baeadb9f0805cad96351a3b117155a103 Author: Eugene Burkov Date: Mon Sep 13 17:27:30 2021 +0300 dhcpd: add default options commit 575e9d01cf95a564aed31d26a6cc9376850d321a Author: Eugene Burkov Date: Wed Sep 8 13:05:01 2021 +0300 dhcpd: imp options logic --- CHANGELOG.md | 4 +- internal/dhcpd/options.go | 134 ------------------- internal/dhcpd/options_test.go | 109 ---------------- internal/dhcpd/options_unix.go | 192 ++++++++++++++++++++++++++++ internal/dhcpd/options_unix_test.go | 177 +++++++++++++++++++++++++ internal/dhcpd/server.go | 7 - internal/dhcpd/v4.go | 63 ++++----- internal/dhcpd/v4_test.go | 15 ++- 8 files changed, 415 insertions(+), 286 deletions(-) delete mode 100644 internal/dhcpd/options.go delete mode 100644 internal/dhcpd/options_test.go create mode 100644 internal/dhcpd/options_unix.go create mode 100644 internal/dhcpd/options_unix_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 095b471f..d69aaee9 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/internal/dhcpd/options.go b/internal/dhcpd/options.go deleted file mode 100644 index bd3f77a1..00000000 --- a/internal/dhcpd/options.go +++ /dev/null @@ -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 -} diff --git a/internal/dhcpd/options_test.go b/internal/dhcpd/options_test.go deleted file mode 100644 index 63f736d1..00000000 --- a/internal/dhcpd/options_test.go +++ /dev/null @@ -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) - }) - } -} diff --git a/internal/dhcpd/options_unix.go b/internal/dhcpd/options_unix.go new file mode 100644 index 00000000..820eabd1 --- /dev/null +++ b/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 +} diff --git a/internal/dhcpd/options_unix_test.go b/internal/dhcpd/options_unix_test.go new file mode 100644 index 00000000..5c936b00 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/internal/dhcpd/server.go b/internal/dhcpd/server.go index d326e812..83127aa0 100644 --- a/internal/dhcpd/server.go +++ b/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 -} diff --git a/internal/dhcpd/v4.go b/internal/dhcpd/v4.go index 3e9a954c..b13be453 100644 --- a/internal/dhcpd/v4.go +++ b/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) - copy(resp.YourIPAddr, l.IP) + resp.YourIPAddr = netutil.CloneIP(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 { - resp.Options[opt.code] = opt.data + // Update values for each explicitly configured parameter requested by + // 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 -// according to RFC-2131. +// send writes resp for peer to conn considering the req's parameters according +// 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() - - 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) - } + s.options = prepareOptions(s.conf) return s, nil } diff --git a/internal/dhcpd/v4_test.go b/internal/dhcpd/v4_test.go index bbc1dcea..51cd82a3 100644 --- a/internal/dhcpd/v4_test.go +++ b/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) {