From 14d8f58592cbb192216cfe98e28e9b38e40a97c5 Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Tue, 28 Jun 2022 19:09:26 +0300 Subject: [PATCH] Pull request: 4699 dhcp ptr Merge in DNS/adguard-home from 4699-dhcp-ptr to master Closes #4699. Squashed commit of the following: commit 0a8e2b3e22b7fad28a53db65031cc39d8755ecf4 Author: Eugene Burkov Date: Tue Jun 28 18:40:53 2022 +0300 dnsforward: imp naming again commit 0b0884a8305f18f7f69560b86be8837933e220e9 Author: Eugene Burkov Date: Tue Jun 28 18:26:58 2022 +0300 dnsforward: imp naming commit e193f53d9a1dd76d41396c06e2ec5a1e7d176557 Author: Eugene Burkov Date: Tue Jun 28 17:26:00 2022 +0300 all: imp chlog commit 8ac9f84f086d9cb0b0f9da72bfc51f9b70a3dab7 Author: Eugene Burkov Date: Tue Jun 28 17:18:48 2022 +0300 all: log changes commit 7cdc175d02b6eacfcb6ba62a5424d11e2561a879 Author: Eugene Burkov Date: Tue Jun 28 17:03:52 2022 +0300 dnsforward: add tld to dhcp leased hostnames --- CHANGELOG.md | 3 ++ internal/dnsforward/dns.go | 27 +++++------ internal/dnsforward/dns_test.go | 66 +++++++++++++------------- internal/dnsforward/dnsforward.go | 15 +----- internal/dnsforward/dnsforward_test.go | 8 ++-- 5 files changed, 55 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ac34558..0490a5f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,11 +32,14 @@ and this project adheres to ### Fixed +- PTR requests for addresses leased by DHCP will now be resolved into hostnames + under `dhcp.local_domain_name` ([#4699]). - Broken service installation on OpenWrt ([#4677]). [#2993]: https://github.com/AdguardTeam/AdGuardHome/issues/2993 [#3057]: https://github.com/AdguardTeam/AdGuardHome/issues/3057 [#4677]: https://github.com/AdguardTeam/AdGuardHome/issues/4677 +[#4699]: https://github.com/AdguardTeam/AdGuardHome/issues/4699 [ddr-draft-06]: https://www.ietf.org/archive/id/draft-ietf-add-ddr-06.html diff --git a/internal/dnsforward/dns.go b/internal/dnsforward/dns.go index 3c444e2c..f6008e36 100644 --- a/internal/dnsforward/dns.go +++ b/internal/dnsforward/dns.go @@ -100,9 +100,9 @@ func (s *Server) handleDNSRequest(_ *proxy.Proxy, d *proxy.DNSContext) error { s.processInitial, s.processDDRQuery, s.processDetermineLocal, - s.processInternalHosts, + s.processDHCPHosts, s.processRestrictLocal, - s.processInternalIPAddrs, + s.processDHCPAddrs, s.processFilteringBeforeRequest, s.processLocalPTR, s.processUpstream, @@ -230,12 +230,10 @@ func (s *Server) onDHCPLeaseChanged(flags int) { ) } - lowhost := strings.ToLower(l.Hostname) + lowhost := strings.ToLower(l.Hostname + "." + s.localDomainSuffix) + ip := netutil.CloneIP(l.IP) - ipToHost.Set(l.IP, lowhost) - - ip := make(net.IP, 4) - copy(ip, l.IP.To4()) + ipToHost.Set(ip, lowhost) hostToIP[lowhost] = ip } @@ -376,11 +374,11 @@ func (s *Server) hostToIP(host string) (ip net.IP, ok bool) { return ip, true } -// processInternalHosts respond to A requests if the target hostname is known to +// processDHCPHosts respond to A requests if the target hostname is known to // the server. // // TODO(a.garipov): Adapt to AAAA as well. -func (s *Server) processInternalHosts(dctx *dnsContext) (rc resultCode) { +func (s *Server) processDHCPHosts(dctx *dnsContext) (rc resultCode) { if !s.dhcpServer.Enabled() { return resultCodeSuccess } @@ -395,11 +393,10 @@ func (s *Server) processInternalHosts(dctx *dnsContext) (rc resultCode) { return resultCodeSuccess } - reqHost := strings.ToLower(q.Name) + reqHost := strings.ToLower(q.Name[:len(q.Name)-1]) // TODO(a.garipov): Move everything related to DHCP local domain to the DHCP // server. - host := strings.TrimSuffix(reqHost, s.localDomainSuffix) - if host == reqHost { + if !strings.HasSuffix(reqHost, s.localDomainSuffix) { return resultCodeSuccess } @@ -412,7 +409,7 @@ func (s *Server) processInternalHosts(dctx *dnsContext) (rc resultCode) { return resultCodeFinish } - ip, ok := s.hostToIP(host) + ip, ok := s.hostToIP(reqHost) if !ok { // TODO(e.burkov): Inspect special cases when user want to apply some // rules handled by other processors to the hosts with TLD. @@ -469,7 +466,7 @@ func (s *Server) processRestrictLocal(ctx *dnsContext) (rc resultCode) { // Restrict an access to local addresses for external clients. We also // assume that all the DHCP leases we give are locally-served or at least - // don't need to be inaccessible externally. + // don't need to be accessible externally. if !s.privateNets.Contains(ip) { log.Debug("dns: addr %s is not from locally-served network", ip) @@ -526,7 +523,7 @@ func (s *Server) ipToHost(ip net.IP) (host string, ok bool) { // Respond to PTR requests if the target IP is leased by our DHCP server and the // requestor is inside the local network. -func (s *Server) processInternalIPAddrs(ctx *dnsContext) (rc resultCode) { +func (s *Server) processDHCPAddrs(ctx *dnsContext) (rc resultCode) { d := ctx.proxyCtx if d.Res != nil { return resultCodeSuccess diff --git a/internal/dnsforward/dns_test.go b/internal/dnsforward/dns_test.go index 1e0537dc..25e28afd 100644 --- a/internal/dnsforward/dns_test.go +++ b/internal/dnsforward/dns_test.go @@ -229,7 +229,7 @@ func TestServer_ProcessDetermineLocal(t *testing.T) { } } -func TestServer_ProcessInternalHosts_localRestriction(t *testing.T) { +func TestServer_ProcessDHCPHosts_localRestriction(t *testing.T) { knownIP := net.IP{1, 2, 3, 4} testCases := []struct { @@ -270,7 +270,7 @@ func TestServer_ProcessInternalHosts_localRestriction(t *testing.T) { dhcpServer: &testDHCP{}, localDomainSuffix: defaultLocalDomainSuffix, tableHostToIP: hostToIPTable{ - "example": knownIP, + "example." + defaultLocalDomainSuffix: knownIP, }, } @@ -292,7 +292,7 @@ func TestServer_ProcessInternalHosts_localRestriction(t *testing.T) { isLocalClient: tc.isLocalCli, } - res := s.processInternalHosts(dctx) + res := s.processDHCPHosts(dctx) require.Equal(t, tc.wantRes, res) pctx := dctx.proxyCtx if tc.wantRes == resultCodeFinish { @@ -318,10 +318,10 @@ func TestServer_ProcessInternalHosts_localRestriction(t *testing.T) { } } -func TestServer_ProcessInternalHosts(t *testing.T) { +func TestServer_ProcessDHCPHosts(t *testing.T) { const ( examplecom = "example.com" - examplelan = "example.lan" + examplelan = "example." + defaultLocalDomainSuffix ) knownIP := net.IP{1, 2, 3, 4} @@ -370,41 +370,41 @@ func TestServer_ProcessInternalHosts(t *testing.T) { }, { name: "success_custom_suffix", host: "example.custom", - suffix: ".custom.", + suffix: "custom", wantIP: knownIP, wantRes: resultCodeSuccess, qtyp: dns.TypeA, }} for _, tc := range testCases { + s := &Server{ + dhcpServer: &testDHCP{}, + localDomainSuffix: tc.suffix, + tableHostToIP: hostToIPTable{ + "example." + tc.suffix: knownIP, + }, + } + + req := &dns.Msg{ + MsgHdr: dns.MsgHdr{ + Id: 1234, + }, + Question: []dns.Question{{ + Name: dns.Fqdn(tc.host), + Qtype: tc.qtyp, + Qclass: dns.ClassINET, + }}, + } + + dctx := &dnsContext{ + proxyCtx: &proxy.DNSContext{ + Req: req, + }, + isLocalClient: true, + } + t.Run(tc.name, func(t *testing.T) { - s := &Server{ - dhcpServer: &testDHCP{}, - localDomainSuffix: tc.suffix, - tableHostToIP: hostToIPTable{ - "example": knownIP, - }, - } - - req := &dns.Msg{ - MsgHdr: dns.MsgHdr{ - Id: 1234, - }, - Question: []dns.Question{{ - Name: dns.Fqdn(tc.host), - Qtype: tc.qtyp, - Qclass: dns.ClassINET, - }}, - } - - dctx := &dnsContext{ - proxyCtx: &proxy.DNSContext{ - Req: req, - }, - isLocalClient: true, - } - - res := s.processInternalHosts(dctx) + res := s.processDHCPHosts(dctx) pctx := dctx.proxyCtx assert.Equal(t, tc.wantRes, res) if tc.wantRes == resultCodeFinish { diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go index c0cd0e55..9d0e8854 100644 --- a/internal/dnsforward/dnsforward.go +++ b/internal/dnsforward/dnsforward.go @@ -107,7 +107,7 @@ type Server struct { // when no suffix is provided. // // See the documentation for Server.localDomainSuffix. -const defaultLocalDomainSuffix = ".lan." +const defaultLocalDomainSuffix = "lan" // DNSCreateParams are parameters to create a new server. type DNSCreateParams struct { @@ -120,17 +120,6 @@ type DNSCreateParams struct { LocalDomain string } -// domainNameToSuffix converts a domain name into a local domain suffix. -func domainNameToSuffix(tld string) (suffix string) { - l := len(tld) + 2 - b := make([]byte, l) - b[0] = '.' - copy(b[1:], tld) - b[l-1] = '.' - - return string(b) -} - const ( // recursionTTL is the time recursive request is cached for. recursionTTL = 1 * time.Second @@ -151,7 +140,7 @@ func NewServer(p DNSCreateParams) (s *Server, err error) { return nil, fmt.Errorf("local domain: %w", err) } - localDomainSuffix = domainNameToSuffix(p.LocalDomain) + localDomainSuffix = p.LocalDomain } if p.Anonymizer == nil { diff --git a/internal/dnsforward/dnsforward_test.go b/internal/dnsforward/dnsforward_test.go index 36761f41..d6cad013 100644 --- a/internal/dnsforward/dnsforward_test.go +++ b/internal/dnsforward/dnsforward_test.go @@ -1016,10 +1016,13 @@ func (d *testDHCP) Leases(flags dhcpd.GetLeasesFlags) (leases []*dhcpd.Lease) { func (d *testDHCP) SetOnLeaseChanged(onLeaseChanged dhcpd.OnLeaseChangedT) {} func TestPTRResponseFromDHCPLeases(t *testing.T) { + const localDomain = "lan" + s, err := NewServer(DNSCreateParams{ DNSFilter: filtering.New(&filtering.Config{}, nil), DHCPServer: &testDHCP{}, PrivateNets: netutil.SubnetSetFunc(netutil.IsLocallyServed), + LocalDomain: localDomain, }) require.NoError(t, err) @@ -1033,14 +1036,13 @@ func TestPTRResponseFromDHCPLeases(t *testing.T) { err = s.Start() require.NoError(t, err) - t.Cleanup(s.Close) addr := s.dnsProxy.Addr(proxy.ProtoUDP) req := createTestMessageWithType("34.12.168.192.in-addr.arpa.", dns.TypePTR) resp, err := dns.Exchange(req, addr.String()) - require.NoError(t, err) + require.NoErrorf(t, err, "%s", addr) require.Len(t, resp.Answer, 1) @@ -1049,7 +1051,7 @@ func TestPTRResponseFromDHCPLeases(t *testing.T) { ptr, ok := resp.Answer[0].(*dns.PTR) require.True(t, ok) - assert.Equal(t, "myhost.", ptr.Ptr) + assert.Equal(t, dns.Fqdn("myhost."+localDomain), ptr.Ptr) } func TestPTRResponseFromHosts(t *testing.T) {