From a06543a1d52daad75984c8480f69629734b1f800 Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Thu, 29 Apr 2021 16:00:07 +0300 Subject: [PATCH] Pull request: all: do not check local domains when dhcp srv is off Updates #3028. Squashed commit of the following: commit 49d3ca5c9de0468ccb1792e9de263fd66e30d79c Author: Ainar Garipov Date: Thu Apr 29 15:35:32 2021 +0300 all: do not check local domains when dhcp srv is off --- CHANGELOG.md | 2 ++ internal/dhcpd/dhcpd.go | 6 ++++++ internal/dnsforward/dns.go | 4 ++++ internal/dnsforward/dns_test.go | 4 +++- internal/dnsforward/dnsforward_test.go | 16 +++++++++++----- internal/home/home.go | 4 ++++ 6 files changed, 30 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d7d372d..cebe2054 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,11 +19,13 @@ and this project adheres to ### Fixed +- Local domain name handling when the DHCP server is disabled ([#3028]). - Normalization of perviously-saved invalid static DHCP leases ([#3027]). - Validation of IPv6 addresses with zones in system resolvers ([#3022]). [#3022]: https://github.com/AdguardTeam/AdGuardHome/issues/3022 [#3027]: https://github.com/AdguardTeam/AdGuardHome/issues/3027 +[#3028]: https://github.com/AdguardTeam/AdGuardHome/issues/3028 diff --git a/internal/dhcpd/dhcpd.go b/internal/dhcpd/dhcpd.go index 3d2b8a8b..59be06a0 100644 --- a/internal/dhcpd/dhcpd.go +++ b/internal/dhcpd/dhcpd.go @@ -133,6 +133,7 @@ type Server struct { // ServerInterface is an interface for servers. type ServerInterface interface { + Enabled() (ok bool) Leases(flags int) []Lease SetOnLeaseChanged(onLeaseChanged OnLeaseChangedT) } @@ -207,6 +208,11 @@ func Create(conf ServerConfig) *Server { return s } +// Enabled returns true when the server is enabled. +func (s *Server) Enabled() (ok bool) { + return s.conf.Enabled +} + // server calls this function after DB is updated func (s *Server) onNotify(flags uint32) { if flags == LeaseChangedDBStore { diff --git a/internal/dnsforward/dns.go b/internal/dnsforward/dns.go index 36cc2025..431c43c3 100644 --- a/internal/dnsforward/dns.go +++ b/internal/dnsforward/dns.go @@ -249,6 +249,10 @@ func (s *Server) hostToIP(host string) (ip net.IP, ok bool) { // // TODO(a.garipov): Adapt to AAAA as well. func (s *Server) processInternalHosts(dctx *dnsContext) (rc resultCode) { + if !s.dhcpServer.Enabled() { + return resultCodeSuccess + } + req := dctx.proxyCtx.Req q := req.Question[0] diff --git a/internal/dnsforward/dns_test.go b/internal/dnsforward/dns_test.go index 04f065dd..9e3d2f95 100644 --- a/internal/dnsforward/dns_test.go +++ b/internal/dnsforward/dns_test.go @@ -90,6 +90,7 @@ func TestServer_ProcessInternalHosts_localRestriction(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { s := &Server{ + dhcpServer: &testDHCP{}, localDomainSuffix: defaultLocalDomainSuffix, tableHostToIP: hostToIPTable{ "example": knownIP, @@ -201,6 +202,7 @@ func TestServer_ProcessInternalHosts(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { s := &Server{ + dhcpServer: &testDHCP{}, localDomainSuffix: tc.suffix, tableHostToIP: hostToIPTable{ "example": knownIP, @@ -318,7 +320,7 @@ func TestLocalRestriction(t *testing.T) { } t.Run(tc.name, func(t *testing.T) { err = s.handleDNSRequest(nil, pctx) - require.Nil(t, err) + require.NoError(t, err) require.NotNil(t, pctx.Res) require.Len(t, pctx.Res.Answer, tc.wantLen) if tc.wantLen > 0 { diff --git a/internal/dnsforward/dnsforward_test.go b/internal/dnsforward/dnsforward_test.go index cdc41300..fdd703e1 100644 --- a/internal/dnsforward/dnsforward_test.go +++ b/internal/dnsforward/dnsforward_test.go @@ -75,6 +75,7 @@ func createTestServer( require.NotNil(t, snd) s, err = NewServer(DNSCreateParams{ + DHCPServer: &testDHCP{}, DNSFilter: f, SubnetDetector: snd, }) @@ -736,6 +737,7 @@ func TestBlockedCustomIP(t *testing.T) { var s *Server s, err = NewServer(DNSCreateParams{ + DHCPServer: &testDHCP{}, DNSFilter: dnsfilter.New(&dnsfilter.Config{}, filters), SubnetDetector: snd, }) @@ -873,6 +875,7 @@ func TestRewrite(t *testing.T) { var s *Server s, err = NewServer(DNSCreateParams{ + DHCPServer: &testDHCP{}, DNSFilter: f, SubnetDetector: snd, }) @@ -1016,11 +1019,13 @@ func TestMatchDNSName(t *testing.T) { type testDHCP struct{} +func (d *testDHCP) Enabled() (ok bool) { return true } + func (d *testDHCP) Leases(flags int) []dhcpd.Lease { l := dhcpd.Lease{ - IP: net.IP{127, 0, 0, 1}, + IP: net.IP{192, 168, 12, 34}, HWAddr: net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA}, - Hostname: "localhost", + Hostname: "myhost", } return []dhcpd.Lease{l} @@ -1056,7 +1061,7 @@ func TestPTRResponseFromDHCPLeases(t *testing.T) { }) addr := s.dnsProxy.Addr(proxy.ProtoUDP) - req := createTestMessageWithType("1.0.0.127.in-addr.arpa.", dns.TypePTR) + req := createTestMessageWithType("34.12.168.192.in-addr.arpa.", dns.TypePTR) resp, err := dns.Exchange(req, addr.String()) require.NoError(t, err) @@ -1064,11 +1069,11 @@ func TestPTRResponseFromDHCPLeases(t *testing.T) { require.Len(t, resp.Answer, 1) assert.Equal(t, dns.TypePTR, resp.Answer[0].Header().Rrtype) - assert.Equal(t, "1.0.0.127.in-addr.arpa.", resp.Answer[0].Header().Name) + assert.Equal(t, "34.12.168.192.in-addr.arpa.", resp.Answer[0].Header().Name) ptr, ok := resp.Answer[0].(*dns.PTR) require.True(t, ok) - assert.Equal(t, "localhost.", ptr.Ptr) + assert.Equal(t, "myhost.", ptr.Ptr) } func TestPTRResponseFromHosts(t *testing.T) { @@ -1098,6 +1103,7 @@ func TestPTRResponseFromHosts(t *testing.T) { var s *Server s, err = NewServer(DNSCreateParams{ + DHCPServer: &testDHCP{}, DNSFilter: dnsfilter.New(&c, nil), SubnetDetector: snd, }) diff --git a/internal/home/home.go b/internal/home/home.go index b647b10c..a072062b 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -184,6 +184,10 @@ func setupConfig(args options) { Context.dhcpServer = dhcpd.Create(config.DHCP) if Context.dhcpServer == nil { + // TODO(a.garipov): There are a lot of places in the code right + // now which assume that the DHCP server can be nil despite this + // condition. Inspect them and perhaps rewrite them to use + // Enabled() instead. log.Fatalf("can't initialize dhcp module") }