From b296fa224653c1ae0003709708039a08f40842eb Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Mon, 18 Oct 2021 15:29:29 +0300 Subject: [PATCH 1/3] Pull request: 3744 DNS server DHCP option Merge in DNS/adguard-home from 3744-options-priority to master Updates #3744. Squashed commit of the following: commit 30f1d483bebd92348250573d2edd708247081b45 Author: Eugene Burkov Date: Mon Oct 18 15:22:49 2021 +0300 dhcpd: imp tests more commit 9a8194e2f259ac7a88b23a1480c74decfef587b3 Author: Eugene Burkov Date: Mon Oct 18 15:09:20 2021 +0300 dhcpd: imp tests commit d915e0b407adcfd24df6e28be22f095909749aa3 Author: Eugene Burkov Date: Mon Oct 18 14:46:20 2021 +0300 dhcpd: fix options priority --- CHANGELOG.md | 2 + internal/dhcpd/v4.go | 7 +- internal/dhcpd/v4_test.go | 145 ++++++++++++++++++++++++++++---------- 3 files changed, 113 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b017cf0..d4de38dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,6 +114,7 @@ In this release, the schema version has changed from 10 to 12. ### Fixed +- Incorrect assignment of explicitly configured DHCP options ([#3744]). - Occasional panic during shutdown ([#3655]). - Addition of IPs into only one as opposed to all matching ipsets on Linux ([#3638]). @@ -204,6 +205,7 @@ In this release, the schema version has changed from 10 to 12. [#3607]: https://github.com/AdguardTeam/AdGuardHome/issues/3607 [#3638]: https://github.com/AdguardTeam/AdGuardHome/issues/3638 [#3655]: https://github.com/AdguardTeam/AdGuardHome/issues/3655 +[#3744]: https://github.com/AdguardTeam/AdGuardHome/issues/3744 diff --git a/internal/dhcpd/v4.go b/internal/dhcpd/v4.go index d7e4dfae..8ddfe98b 100644 --- a/internal/dhcpd/v4.go +++ b/internal/dhcpd/v4.go @@ -900,9 +900,10 @@ func (s *v4Server) process(req, resp *dhcpv4.DHCPv4) int { 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) { + // Update the value of Domain Name Server option separately from others if + // not assigned yet since its value is set after server's creating. + if requested.Has(dhcpv4.OptionDomainNameServer) && + !resp.Options.Has(dhcpv4.OptionDomainNameServer) { resp.UpdateOption(dhcpv4.OptDNS(s.conf.dnsIPAddrs...)) } diff --git a/internal/dhcpd/v4_test.go b/internal/dhcpd/v4_test.go index d20e715c..9bed3c60 100644 --- a/internal/dhcpd/v4_test.go +++ b/internal/dhcpd/v4_test.go @@ -5,8 +5,10 @@ package dhcpd import ( "net" + "strings" "testing" + "github.com/AdguardTeam/golibs/stringutil" "github.com/insomniacslk/dhcp/dhcpv4" "github.com/mdlayher/raw" "github.com/stretchr/testify/assert" @@ -16,17 +18,34 @@ import ( func notify4(flags uint32) { } -func TestV4_AddRemove_static(t *testing.T) { - s, err := v4Create(V4ServerConf{ +// defaultV4ServerConf returns the default configuration for *v4Server to use in +// tests. +func defaultV4ServerConf() (conf V4ServerConf) { + return V4ServerConf{ Enabled: true, RangeStart: net.IP{192, 168, 10, 100}, RangeEnd: net.IP{192, 168, 10, 200}, GatewayIP: net.IP{192, 168, 10, 1}, SubnetMask: net.IP{255, 255, 255, 0}, notify: notify4, - }) + } +} + +// defaultSrv prepares the default DHCPServer to use in tests. The underlying +// type of s is *v4Server. +func defaultSrv(t *testing.T) (s DHCPServer) { + t.Helper() + + var err error + s, err = v4Create(defaultV4ServerConf()) require.NoError(t, err) + return s +} + +func TestV4_AddRemove_static(t *testing.T) { + s := defaultSrv(t) + ls := s.GetLeases(LeasesStatic) assert.Empty(t, ls) @@ -37,7 +56,7 @@ func TestV4_AddRemove_static(t *testing.T) { IP: net.IP{192, 168, 10, 150}, } - err = s.AddStaticLease(l) + err := s.AddStaticLease(l) require.NoError(t, err) err = s.AddStaticLease(l) @@ -65,15 +84,7 @@ func TestV4_AddRemove_static(t *testing.T) { } func TestV4_AddReplace(t *testing.T) { - sIface, err := v4Create(V4ServerConf{ - Enabled: true, - RangeStart: net.IP{192, 168, 10, 100}, - RangeEnd: net.IP{192, 168, 10, 200}, - GatewayIP: net.IP{192, 168, 10, 1}, - SubnetMask: net.IP{255, 255, 255, 0}, - notify: notify4, - }) - require.NoError(t, err) + sIface := defaultSrv(t) s, ok := sIface.(*v4Server) require.True(t, ok) @@ -89,7 +100,7 @@ func TestV4_AddReplace(t *testing.T) { }} for i := range dynLeases { - err = s.addLease(&dynLeases[i]) + err := s.addLease(&dynLeases[i]) require.NoError(t, err) } @@ -104,7 +115,7 @@ func TestV4_AddReplace(t *testing.T) { }} for _, l := range stLeases { - err = s.AddStaticLease(l) + err := s.AddStaticLease(l) require.NoError(t, err) } @@ -118,17 +129,80 @@ func TestV4_AddReplace(t *testing.T) { } } -func TestV4StaticLease_Get(t *testing.T) { - var err error - sIface, err := v4Create(V4ServerConf{ - Enabled: true, - RangeStart: net.IP{192, 168, 10, 100}, - RangeEnd: net.IP{192, 168, 10, 200}, - GatewayIP: net.IP{192, 168, 10, 1}, - SubnetMask: net.IP{255, 255, 255, 0}, - notify: notify4, +func TestV4Server_Process_optionsPriority(t *testing.T) { + defaultIP := net.IP{192, 168, 1, 1} + knownIP := net.IP{1, 2, 3, 4} + + // prepareSrv creates a *v4Server and sets the opt6IPs in the initial + // configuration of the server as the value for DHCP option 6. + prepareSrv := func(t *testing.T, opt6IPs []net.IP) (s *v4Server) { + t.Helper() + + conf := defaultV4ServerConf() + if len(opt6IPs) > 0 { + b := &strings.Builder{} + stringutil.WriteToBuilder(b, "6 ips ", opt6IPs[0].String()) + for _, ip := range opt6IPs[1:] { + stringutil.WriteToBuilder(b, ",", ip.String()) + } + conf.Options = []string{b.String()} + } + + ss, err := v4Create(conf) + require.NoError(t, err) + + var ok bool + s, ok = ss.(*v4Server) + require.True(t, ok) + + s.conf.dnsIPAddrs = []net.IP{defaultIP} + + return s + } + + // checkResp creates a discovery message with DHCP option 6 requested amd + // asserts the response to contain wantIPs in this option. + checkResp := func(t *testing.T, s *v4Server, wantIPs []net.IP) { + t.Helper() + + mac := net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA} + req, err := dhcpv4.NewDiscovery(mac, dhcpv4.WithRequestedOptions( + dhcpv4.OptionDomainNameServer, + )) + require.NoError(t, err) + + var resp *dhcpv4.DHCPv4 + resp, err = dhcpv4.NewReplyFromRequest(req) + require.NoError(t, err) + + res := s.process(req, resp) + require.Equal(t, 1, res) + + o := resp.GetOneOption(dhcpv4.OptionDomainNameServer) + require.NotEmpty(t, o) + + wantData := []byte{} + for _, ip := range wantIPs { + wantData = append(wantData, ip...) + } + assert.Equal(t, o, wantData) + } + + t.Run("default", func(t *testing.T) { + s := prepareSrv(t, nil) + + checkResp(t, s, []net.IP{defaultIP}) }) - require.NoError(t, err) + + t.Run("explicitly_configured", func(t *testing.T) { + s := prepareSrv(t, []net.IP{knownIP, knownIP}) + + checkResp(t, s, []net.IP{knownIP, knownIP}) + }) +} + +func TestV4StaticLease_Get(t *testing.T) { + sIface := defaultSrv(t) s, ok := sIface.(*v4Server) require.True(t, ok) @@ -140,7 +214,7 @@ func TestV4StaticLease_Get(t *testing.T) { HWAddr: net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA}, IP: net.IP{192, 168, 10, 150}, } - err = s.AddStaticLease(l) + err := s.AddStaticLease(l) require.NoError(t, err) var req, resp *dhcpv4.DHCPv4 @@ -208,19 +282,14 @@ func TestV4StaticLease_Get(t *testing.T) { } func TestV4DynamicLease_Get(t *testing.T) { + conf := defaultV4ServerConf() + conf.Options = []string{ + "81 hex 303132", + "82 ip 1.2.3.4", + } + var err error - sIface, err := v4Create(V4ServerConf{ - Enabled: true, - RangeStart: net.IP{192, 168, 10, 100}, - RangeEnd: net.IP{192, 168, 10, 200}, - GatewayIP: net.IP{192, 168, 10, 1}, - SubnetMask: net.IP{255, 255, 255, 0}, - notify: notify4, - Options: []string{ - "81 hex 303132", - "82 ip 1.2.3.4", - }, - }) + sIface, err := v4Create(conf) require.NoError(t, err) s, ok := sIface.(*v4Server) From d7aafa7dc6e89d71b613c28689a1ea5f31e09238 Mon Sep 17 00:00:00 2001 From: Dmitry Seregin Date: Tue, 19 Oct 2021 19:28:18 +0300 Subject: [PATCH 2/3] Pull request #1329: 3529 validate dhcpv4 Merge in DNS/adguard-home from 3529-validate-dhcpv4 to master Squashed commit of the following: commit 2f2455aa13a41398cd2846f31be96da9d34ba95d Author: Dmitriy Seregin Date: Tue Oct 19 19:18:12 2021 +0300 dhcpv4: better test && fix changelog commit ec4ff9180e8390fb739b3be0fc76fd2c715fe691 Author: Dmitriy Seregin Date: Mon Oct 18 19:08:44 2021 +0300 dhcpv4: better tests commit e0e2f27b7a063ed84af170b16c3f87636cb738d2 Author: Dmitriy Seregin Date: Mon Oct 18 18:55:47 2021 +0300 dhcpv4: better tests commit 73e1d08e1265e336ee6339d5021f90883fe3e395 Author: Dmitriy Seregin Date: Mon Oct 18 18:47:21 2021 +0300 dhcpv4: better tests commit f636fc316123f26b6e2930afb4b22c18024ec93d Author: Dmitriy Seregin Date: Mon Oct 18 18:47:07 2021 +0300 all: updated golibs commit 86dd107a1d483ac24bd8c26422324eb8b9c3d086 Merge: 51aaf6d9 b296fa22 Author: Dmitriy Seregin Date: Mon Oct 18 17:18:17 2021 +0300 Merge branch 'master' into 3529-validate-dhcpv4 commit 51aaf6d9eb5fbe2b4304254dc6782305a19c53fa Author: Dmitriy Seregin Date: Mon Oct 18 17:18:02 2021 +0300 dhcpv4: better changelog commit 720b896bb595c57fab6d376f88c8a4b1d131db40 Author: Dmitriy Seregin Date: Mon Oct 18 17:14:25 2021 +0300 dhcpv4: better tests commit 1098beffca8d5feb2ec104d26419210962c9a97d Author: Dmitriy Seregin Date: Mon Oct 18 12:08:26 2021 +0300 dhcp: changelog commit d1f6c89d68657431fb261658133c67e9e3135c1c Author: Dmitriy Seregin Date: Mon Oct 18 12:03:06 2021 +0300 dhcpv4: fixed tests commit 8b6713468fc04321c5238300df90bbb2d67ee679 Merge: 9991e9cb 3fa38fb4 Author: Dmitriy Seregin Date: Mon Oct 18 11:57:57 2021 +0300 Merge branch 'master' into 3529-validate-dhcpv4 commit 9991e9cbee7dc87d8fa1d7e86e6cc7e09ab6938c Author: Dmitriy Seregin Date: Mon Oct 18 11:55:40 2021 +0300 dhcpv4: added tests commit 5798a80de6c060365c1c647326d46cc13ccf28cb Author: Dmitriy Seregin Date: Mon Oct 18 11:46:03 2021 +0300 dhcpv4: validate subnet mask and ip range --- CHANGELOG.md | 3 + client/src/__locales/en.json | 12 +- .../components/Settings/Dhcp/FormDHCPv4.js | 28 ++++- .../Settings/Dhcp/StaticLeases/Form.js | 8 +- .../Settings/Dhcp/StaticLeases/Modal.js | 8 ++ .../Settings/Dhcp/StaticLeases/index.js | 6 + client/src/components/Settings/Dhcp/index.js | 2 + client/src/helpers/helpers.js | 14 +++ client/src/helpers/validators.js | 113 +++++++++++++++++- go.mod | 2 +- go.sum | 4 +- internal/dhcpd/dhcpd_test.go | 44 +++++++ internal/dhcpd/v4.go | 25 ++++ 13 files changed, 258 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4de38dd..99821349 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,8 @@ and this project adheres to ### Changed +- DHCP gateway address, subnet mask, IP address range, and leases validations + ([#3529]). - The `systemd` service script will now create the `/var/log` directory when it doesn't exist ([#3579]). - Items in allowed clients, disallowed clients, and blocked hosts lists are now @@ -196,6 +198,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 +[#3529]: https://github.com/AdguardTeam/AdGuardHome/issues/3529 [#3538]: https://github.com/AdguardTeam/AdGuardHome/issues/3538 [#3551]: https://github.com/AdguardTeam/AdGuardHome/issues/3551 [#3564]: https://github.com/AdguardTeam/AdGuardHome/issues/3564 diff --git a/client/src/__locales/en.json b/client/src/__locales/en.json index f3d8abcd..6db195fe 100644 --- a/client/src/__locales/en.json +++ b/client/src/__locales/en.json @@ -37,6 +37,9 @@ "dhcp_ipv6_settings": "DHCP IPv6 Settings", "form_error_required": "Required field", "form_error_ip4_format": "Invalid IPv4 format", + "form_error_ip4_range_start_format": "Invalid range start IPv4 format", + "form_error_ip4_range_end_format": "Invalid range end IPv4 format", + "form_error_ip4_gateway_format": "Invalid gateway IPv4 format", "form_error_ip6_format": "Invalid IPv6 format", "form_error_ip_format": "Invalid IP format", "form_error_mac_format": "Invalid MAC format", @@ -45,7 +48,14 @@ "form_error_subnet": "Subnet \"{{cidr}}\" does not contain the IP address \"{{ip}}\"", "form_error_positive": "Must be greater than 0", "form_error_negative": "Must be equal to 0 or greater", - "range_end_error": "Must be greater than range start", + "out_of_range_error": "Must be out of range \"{{start}}\"-\"{{end}}\"", + "in_range_error": "Must be in range \"{{start}}\"-\"{{end}}\"", + "lower_range_start_error": "Must be lower than range start", + "lower_range_end_error": "Must be lower than range end", + "greater_range_start_error": "Must be greater than range start", + "greater_range_end_error": "Must be greater than range end", + "subnet_error": "Addresses must be in one subnet", + "gateway_or_subnet_invalid": "Subnet mask invalid", "dhcp_form_gateway_input": "Gateway IP", "dhcp_form_subnet_input": "Subnet mask", "dhcp_form_range_title": "Range of IP addresses", diff --git a/client/src/components/Settings/Dhcp/FormDHCPv4.js b/client/src/components/Settings/Dhcp/FormDHCPv4.js index 873e7696..cb371f9f 100644 --- a/client/src/components/Settings/Dhcp/FormDHCPv4.js +++ b/client/src/components/Settings/Dhcp/FormDHCPv4.js @@ -13,6 +13,9 @@ import { validateIpv4, validateRequiredValue, validateIpv4RangeEnd, + validateGatewaySubnetMask, + validateIpForGatewaySubnetMask, + validateNotInRange, } from '../../../helpers/validators'; const FormDHCPv4 = ({ @@ -54,7 +57,11 @@ const FormDHCPv4 = ({ type="text" className="form-control" placeholder={t(ipv4placeholders.gateway_ip)} - validate={[validateIpv4, validateRequired]} + validate={[ + validateIpv4, + validateRequired, + validateNotInRange, + ]} disabled={!isInterfaceIncludesIpv4} /> @@ -66,7 +73,11 @@ const FormDHCPv4 = ({ type="text" className="form-control" placeholder={t(ipv4placeholders.subnet_mask)} - validate={[validateIpv4, validateRequired]} + validate={[ + validateIpv4, + validateRequired, + validateGatewaySubnetMask, + ]} disabled={!isInterfaceIncludesIpv4} /> @@ -84,7 +95,11 @@ const FormDHCPv4 = ({ type="text" className="form-control" placeholder={t(ipv4placeholders.range_start)} - validate={[validateIpv4]} + validate={[ + validateIpv4, + validateGatewaySubnetMask, + validateIpForGatewaySubnetMask, + ]} disabled={!isInterfaceIncludesIpv4} /> @@ -95,7 +110,12 @@ const FormDHCPv4 = ({ type="text" className="form-control" placeholder={t(ipv4placeholders.range_end)} - validate={[validateIpv4, validateIpv4RangeEnd]} + validate={[ + validateIpv4, + validateIpv4RangeEnd, + validateGatewaySubnetMask, + validateIpForGatewaySubnetMask, + ]} disabled={!isInterfaceIncludesIpv4} /> diff --git a/client/src/components/Settings/Dhcp/StaticLeases/Form.js b/client/src/components/Settings/Dhcp/StaticLeases/Form.js index e857144d..7e9c641b 100644 --- a/client/src/components/Settings/Dhcp/StaticLeases/Form.js +++ b/client/src/components/Settings/Dhcp/StaticLeases/Form.js @@ -10,6 +10,7 @@ import { validateMac, validateRequiredValue, validateIpv4InCidr, + validateInRange, } from '../../../../helpers/validators'; import { FORM_NAME } from '../../../../helpers/constants'; import { toggleLeaseModal } from '../../../../actions'; @@ -53,7 +54,12 @@ const Form = ({ type="text" className="form-control" placeholder={t('form_enter_subnet_ip', { cidr })} - validate={[validateRequiredValue, validateIpv4, validateIpv4InCidr]} + validate={[ + validateRequiredValue, + validateIpv4, + validateIpv4InCidr, + validateInRange, + ]} />
diff --git a/client/src/components/Settings/Dhcp/StaticLeases/Modal.js b/client/src/components/Settings/Dhcp/StaticLeases/Modal.js index b65c298e..8ad0f009 100644 --- a/client/src/components/Settings/Dhcp/StaticLeases/Modal.js +++ b/client/src/components/Settings/Dhcp/StaticLeases/Modal.js @@ -11,6 +11,8 @@ const Modal = ({ handleSubmit, processingAdding, cidr, + rangeStart, + rangeEnd, }) => { const dispatch = useDispatch(); @@ -38,10 +40,14 @@ const Modal = ({ ip: '', hostname: '', cidr, + rangeStart, + rangeEnd, }} onSubmit={handleSubmit} processingAdding={processingAdding} cidr={cidr} + rangeStart={rangeStart} + rangeEnd={rangeEnd} />
@@ -53,6 +59,8 @@ Modal.propTypes = { handleSubmit: PropTypes.func.isRequired, processingAdding: PropTypes.bool.isRequired, cidr: PropTypes.string.isRequired, + rangeStart: PropTypes.string, + rangeEnd: PropTypes.string, }; export default withTranslation()(Modal); diff --git a/client/src/components/Settings/Dhcp/StaticLeases/index.js b/client/src/components/Settings/Dhcp/StaticLeases/index.js index 6e12f30e..bdd7cec5 100644 --- a/client/src/components/Settings/Dhcp/StaticLeases/index.js +++ b/client/src/components/Settings/Dhcp/StaticLeases/index.js @@ -22,6 +22,8 @@ const StaticLeases = ({ processingDeleting, staticLeases, cidr, + rangeStart, + rangeEnd, }) => { const [t] = useTranslation(); const dispatch = useDispatch(); @@ -100,6 +102,8 @@ const StaticLeases = ({ handleSubmit={handleSubmit} processingAdding={processingAdding} cidr={cidr} + rangeStart={rangeStart} + rangeEnd={rangeEnd} /> ); @@ -111,6 +115,8 @@ StaticLeases.propTypes = { processingAdding: PropTypes.bool.isRequired, processingDeleting: PropTypes.bool.isRequired, cidr: PropTypes.string.isRequired, + rangeStart: PropTypes.string, + rangeEnd: PropTypes.string, }; cellWrap.propTypes = { diff --git a/client/src/components/Settings/Dhcp/index.js b/client/src/components/Settings/Dhcp/index.js index 844e662e..6208d4a6 100644 --- a/client/src/components/Settings/Dhcp/index.js +++ b/client/src/components/Settings/Dhcp/index.js @@ -275,6 +275,8 @@ const Dhcp = () => { processingAdding={processingAdding} processingDeleting={processingDeleting} cidr={cidr} + rangeStart={dhcp?.values?.v4?.range_start} + rangeEnd={dhcp?.values?.v4?.range_end} />