From 1fb043768e4334fa9827ab710d24ce0e489e4bf8 Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Fri, 2 Sep 2022 16:56:59 +0300 Subject: [PATCH] Pull request: 4865-dhcp-rewrites Updates #4865. Squashed commit of the following: commit b26575b72299126f2ce7535104800cc6750698f3 Author: Ainar Garipov Date: Fri Sep 2 16:47:25 2022 +0300 dnsforward: imp code, docs, logs commit c60942c1432175866ac1d182709de33429534de0 Author: Ainar Garipov Date: Fri Sep 2 16:24:44 2022 +0300 dnsforward: process unknown queries in dhcp domain --- CHANGELOG.md | 4 ++ internal/dnsforward/config.go | 2 +- internal/dnsforward/dns.go | 113 +++++++++++++++++++------------- internal/dnsforward/dns_test.go | 4 +- 4 files changed, 74 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4d95c70..84057c5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,9 @@ and this project adheres to ### Changed +- When the DHCP server is enabled, queries for domain names under + `dhcp.local_domain_name` not pointing to real DHCP client hostnames are now + processed by filters ([#4865]). - The DHCPREQUEST handling is now closer to the [RFC 2131][rfc-2131] ([#4863]). - The internal DNS client, used to resolve hostnames of external clients and also during automatic updates, now respects the upstream mode settings for the @@ -57,6 +60,7 @@ and this project adheres to [#4745]: https://github.com/AdguardTeam/AdGuardHome/issues/4745 [#4850]: https://github.com/AdguardTeam/AdGuardHome/issues/4850 [#4863]: https://github.com/AdguardTeam/AdGuardHome/issues/4863 +[#4865]: https://github.com/AdguardTeam/AdGuardHome/issues/4865 [rfc-2131]: https://datatracker.ietf.org/doc/html/rfc2131 diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go index 3c2c74ee..19ec710e 100644 --- a/internal/dnsforward/config.go +++ b/internal/dnsforward/config.go @@ -381,7 +381,7 @@ func (s *Server) prepareUpstreamSettings() error { } // prepareInternalProxy initializes the DNS proxy that is used for internal DNS -// queries, such at client PTR resolving and udpater hostname resolving. +// queries, such at client PTR resolving and updater hostname resolving. func (s *Server) prepareInternalProxy() { conf := &proxy.Config{ CacheEnabled: true, diff --git a/internal/dnsforward/dns.go b/internal/dnsforward/dns.go index fe4f1b31..3f5642e9 100644 --- a/internal/dnsforward/dns.go +++ b/internal/dnsforward/dns.go @@ -250,11 +250,11 @@ func (s *Server) onDHCPLeaseChanged(flags int) { s.setTableIPToHost(ipToHost) } -// processDDRQuery responds to SVCB query for a special use domain name -// ‘_dns.resolver.arpa’. The result contains different types of encryption -// supported by current user configuration. +// processDDRQuery responds to Discovery of Designated Resolvers (DDR) SVCB +// queries. The response contains different types of encryption supported by +// current user configuration. // -// See https://www.ietf.org/archive/id/draft-ietf-add-ddr-06.html. +// See https://www.ietf.org/archive/id/draft-ietf-add-ddr-10.html. func (s *Server) processDDRQuery(dctx *dnsContext) (rc resultCode) { pctx := dctx.proxyCtx q := pctx.Req.Question[0] @@ -279,11 +279,13 @@ func (s *Server) processDDRQuery(dctx *dnsContext) (rc resultCode) { return resultCodeSuccess } -// makeDDRResponse creates DDR answer according to server configuration. The -// contructed SVCB resource records have the priority of 1 for each entry, -// similar to examples provided by https://www.ietf.org/archive/id/draft-ietf-add-ddr-06.html. +// makeDDRResponse creates a DDR answer based on the server configuration. The +// constructed SVCB resource records have the priority of 1 for each entry, +// similar to examples provided by the [draft standard]. // // TODO(a.meshkov): Consider setting the priority values based on the protocol. +// +// [draft standard]: https://www.ietf.org/archive/id/draft-ietf-add-ddr-10.html. func (s *Server) makeDDRResponse(req *dns.Msg) (resp *dns.Msg) { resp = s.makeResponse(req) // TODO(e.burkov): Think about storing the FQDN version of the server's @@ -357,10 +359,10 @@ func (s *Server) processDetermineLocal(dctx *dnsContext) (rc resultCode) { return rc } -// hostToIP tries to get an IP leased by DHCP and returns the copy of address -// since the data inside the internal table may be changed while request +// dhcpHostToIP tries to get an IP leased by DHCP and returns the copy of +// address since the data inside the internal table may be changed while request // processing. It's safe for concurrent use. -func (s *Server) hostToIP(host string) (ip net.IP, ok bool) { +func (s *Server) dhcpHostToIP(host string) (ip net.IP, ok bool) { s.tableHostToIPLock.Lock() defer s.tableHostToIPLock.Unlock() @@ -385,46 +387,32 @@ func (s *Server) hostToIP(host string) (ip net.IP, ok bool) { // // TODO(a.garipov): Adapt to AAAA as well. func (s *Server) processDHCPHosts(dctx *dnsContext) (rc resultCode) { - if !s.dhcpServer.Enabled() { - return resultCodeSuccess - } - - req := dctx.proxyCtx.Req - q := req.Question[0] - - // Go on processing the AAAA request despite the fact that we don't support - // it yet. The expected behavior here is to respond with an empty answer - // and not NXDOMAIN. - if q.Qtype != dns.TypeA && q.Qtype != dns.TypeAAAA { - return resultCodeSuccess - } - - reqHost := strings.ToLower(q.Name[:len(q.Name)-1]) - // TODO(a.garipov): Move everything related to DHCP local domain to the DHCP - // server. - if !strings.HasSuffix(reqHost, s.localDomainSuffix) { - return resultCodeSuccess - } - pctx := dctx.proxyCtx + req := pctx.Req + q := req.Question[0] + reqHost, ok := s.isDHCPClientHostQ(q) + if !ok { + return resultCodeSuccess + } + if !dctx.isLocalClient { - log.Debug("dns: %q requests for internal host", pctx.Addr) + log.Debug("dns: %q requests for dhcp host %q", pctx.Addr, reqHost) pctx.Res = s.genNXDomain(req) // Do not even put into query log. return resultCodeFinish } - ip, ok := s.hostToIP(reqHost) + ip, ok := s.dhcpHostToIP(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. - pctx.Res = s.genNXDomain(req) + // Go on and process them with filters, including dnsrewrite ones, and + // possibly route them to a domain-specific upstream. + log.Debug("dns: no dhcp record for %q", reqHost) - return resultCodeFinish + return resultCodeSuccess } - log.Debug("dns: internal record: %s -> %s", q.Name, ip) + log.Debug("dns: dhcp record for %q is %s", reqHost, ip) resp := s.makeResponse(req) if q.Qtype == dns.TypeA { @@ -502,9 +490,9 @@ func (s *Server) processRestrictLocal(dctx *dnsContext) (rc resultCode) { return resultCodeSuccess } -// ipToHost tries to get a hostname leased by DHCP. It's safe for concurrent -// use. -func (s *Server) ipToHost(ip net.IP) (host string, ok bool) { +// ipToDHCPHost tries to get a hostname leased by DHCP. It's safe for +// concurrent use. +func (s *Server) ipToDHCPHost(ip net.IP) (host string, ok bool) { s.tableIPToHostLock.Lock() defer s.tableIPToHostLock.Unlock() @@ -527,8 +515,8 @@ func (s *Server) ipToHost(ip net.IP) (host string, ok bool) { return host, true } -// Respond to PTR requests if the target IP is leased by our DHCP server and the -// requestor is inside the local network. +// processDHCPAddrs responds to PTR requests if the target IP is leased by the +// DHCP server. func (s *Server) processDHCPAddrs(dctx *dnsContext) (rc resultCode) { pctx := dctx.proxyCtx if pctx.Res != nil { @@ -540,12 +528,12 @@ func (s *Server) processDHCPAddrs(dctx *dnsContext) (rc resultCode) { return resultCodeSuccess } - host, ok := s.ipToHost(ip) + host, ok := s.ipToDHCPHost(ip) if !ok { return resultCodeSuccess } - log.Debug("dns: reverse-lookup: %s -> %s", ip, host) + log.Debug("dns: dhcp reverse record for %s is %q", ip, host) req := pctx.Req resp := s.makeResponse(req) @@ -639,14 +627,25 @@ func ipStringFromAddr(addr net.Addr) (ipStr string) { // processUpstream passes request to upstream servers and handles the response. func (s *Server) processUpstream(dctx *dnsContext) (rc resultCode) { pctx := dctx.proxyCtx + req := pctx.Req + q := req.Question[0] if pctx.Res != nil { // The response has already been set. return resultCodeSuccess + } else if reqHost, ok := s.isDHCPClientHostQ(q); ok { + // A DHCP client hostname query that hasn't been handled or filtered. + // Respond with an NXDOMAIN. + // + // TODO(a.garipov): Route such queries to a custom upstream for the + // local domain name if there is one. + log.Debug("dns: dhcp client hostname %q was not filtered", reqHost) + pctx.Res = s.genNXDomain(req) + + return resultCodeFinish } s.setCustomUpstream(pctx, dctx.clientID) - req := pctx.Req origReqAD := false if s.conf.EnableDNSSEC { if req.AuthenticatedData { @@ -679,6 +678,28 @@ func (s *Server) processUpstream(dctx *dnsContext) (rc resultCode) { return resultCodeSuccess } +// isDHCPClientHostQ returns true if q is from a request for a DHCP client +// hostname. If ok is true, reqHost contains the requested hostname. +func (s *Server) isDHCPClientHostQ(q dns.Question) (reqHost string, ok bool) { + if !s.dhcpServer.Enabled() { + return "", false + } + + // Include AAAA here, because despite the fact that we don't support it yet, + // the expected behavior here is to respond with an empty answer and not + // NXDOMAIN. + if qt := q.Qtype; qt != dns.TypeA && qt != dns.TypeAAAA { + return "", false + } + + reqHost = strings.ToLower(q.Name[:len(q.Name)-1]) + if strings.HasSuffix(reqHost, s.localDomainSuffix) { + return reqHost, true + } + + return "", false +} + // setCustomUpstream sets custom upstream settings in pctx, if necessary. func (s *Server) setCustomUpstream(pctx *proxy.DNSContext, clientID string) { customUpsByClient := s.conf.GetCustomUpstreamByClient diff --git a/internal/dnsforward/dns_test.go b/internal/dnsforward/dns_test.go index 25e28afd..218edb1a 100644 --- a/internal/dnsforward/dns_test.go +++ b/internal/dnsforward/dns_test.go @@ -248,7 +248,7 @@ func TestServer_ProcessDHCPHosts_localRestriction(t *testing.T) { name: "local_client_unknown_host", host: "wronghost.lan", wantIP: nil, - wantRes: resultCodeFinish, + wantRes: resultCodeSuccess, isLocalCli: true, }, { name: "external_client_known_host", @@ -358,7 +358,7 @@ func TestServer_ProcessDHCPHosts(t *testing.T) { host: "example-new.lan", suffix: defaultLocalDomainSuffix, wantIP: nil, - wantRes: resultCodeFinish, + wantRes: resultCodeSuccess, qtyp: dns.TypeA, }, { name: "success_internal_aaaa",