diff --git a/go.sum b/go.sum index a5b49b33426..0c0d69764ef 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -github.com/AdguardTeam/dnsproxy v0.46.6-0.20230125113741-98cb8a899e49 h1:TDZsKB8BrKA2na6p5l20BvEu3MmgOWhIfTANz5laFuE= -github.com/AdguardTeam/dnsproxy v0.46.6-0.20230125113741-98cb8a899e49/go.mod h1:ZEkTmTJ2XInT3aVy0mHtEnSWSclpHHj/9hfNXDuAk5k= +github.com/AdguardTeam/dnsproxy v0.46.5 h1:TiJZhwaIDDaKkqEfJ9AD9aroFjcHN8oEbKB8WfTjSIs= +github.com/AdguardTeam/dnsproxy v0.46.5/go.mod h1:yKBVgFlE6CqTQtye++3e7SATaMPc4Ixij+KkHsM6HhM= github.com/AdguardTeam/golibs v0.4.0/go.mod h1:skKsDKIBB7kkFflLJBpfGX+G8QFTx0WKUzB6TIgtUj4= github.com/AdguardTeam/golibs v0.10.4/go.mod h1:rSfQRGHIdgfxriDDNgNJ7HmE5zRoURq8R+VdR81Zuzw= github.com/AdguardTeam/golibs v0.11.4 h1:IltyvxwCTN+xxJF5sh6VadF8Zfbf8elgCm9dgijSVzM= diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go index c30d0d898cd..b48eb776ccb 100644 --- a/internal/dnsforward/config.go +++ b/internal/dnsforward/config.go @@ -5,6 +5,7 @@ import ( "crypto/x509" "fmt" "net" + "net/netip" "os" "sort" "strings" @@ -225,7 +226,7 @@ type ServerConfig struct { LocalPTRResolvers []string // DNS64Prefixes is a slice of NAT64 prefixes to be used for DNS64. - DNS64Prefixes []string + DNS64Prefixes []netip.Prefix // ResolveClients signals if the RDNS should resolve clients' addresses. ResolveClients bool @@ -271,6 +272,8 @@ func (s *Server) createProxyConfig() (conf proxy.Config, err error) { RequestHandler: s.handleDNSRequest, EnableEDNSClientSubnet: srvConf.EnableEDNSClientSubnet, MaxGoroutines: int(srvConf.MaxGoroutines), + UseDNS64: srvConf.UseDNS64, + DNS64Prefs: srvConf.DNS64Prefixes, } if srvConf.CacheSize != 0 { diff --git a/internal/dnsforward/dns.go b/internal/dnsforward/dns.go index 1411a0f4a96..b1a9f6533e4 100644 --- a/internal/dnsforward/dns.go +++ b/internal/dnsforward/dns.go @@ -10,6 +10,8 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/dhcpd" "github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/AdguardTeam/dnsproxy/proxy" + "github.com/AdguardTeam/dnsproxy/upstream" + "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/golibs/stringutil" @@ -419,7 +421,7 @@ func (s *Server) processDHCPHosts(dctx *dnsContext) (rc resultCode) { } resp.Answer = append(resp.Answer, a) case dns.TypeAAAA: - if len(s.dns64Prefs) > 0 { + if s.dns64Pref != (netip.Prefix{}) { // Respond with DNS64-mapped address for IPv4 host if DNS64 is // enabled. aaaa := &dns.AAAA{ @@ -468,15 +470,6 @@ func (s *Server) processRestrictLocal(dctx *dnsContext) (rc resultCode) { return resultCodeSuccess } - if s.shouldStripDNS64(ip) { - // Strip the prefix from the address to get the original IPv4. - ip = ip[nat64PrefixLen:] - - // Treat a DNS64-prefixed address as a locally served one since those - // queries should never be sent to the global DNS. - dctx.unreversedReqIP = ip - } - // 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 // shouldn't be accessible externally. @@ -671,11 +664,16 @@ func (s *Server) processUpstream(dctx *dnsContext) (rc resultCode) { return resultCodeError } - if dctx.err = prx.Resolve(pctx); dctx.err != nil { - return resultCodeError - } + if err := prx.Resolve(pctx); err != nil { + if errors.Is(err, upstream.ErrNoUpstreams) { + // Do not even put into querylog. + pctx.Res = s.genNXDomain(req) + + return resultCodeFinish + } + + dctx.err = err - if s.performDNS64(prx, dctx) == resultCodeError { return resultCodeError } @@ -770,6 +768,8 @@ func (s *Server) filterAfterResponse(dctx *dnsContext, pctx *proxy.DNSContext) ( // Check the response only if it's from an upstream. Don't check the // response if the protection is disabled since dnsrewrite rules aren't // applied to it anyway. + // + // TODO(e.burkov): Reconsider the latter. if !dctx.protectionEnabled || !dctx.responseFromUpstream || s.dnsFilter == nil { return resultCodeSuccess } diff --git a/internal/dnsforward/dns64.go b/internal/dnsforward/dns64.go index d6ea9c8fa5b..23b0febb4a8 100644 --- a/internal/dnsforward/dns64.go +++ b/internal/dnsforward/dns64.go @@ -1,34 +1,10 @@ package dnsforward import ( - "fmt" "net" "net/netip" "github.com/AdguardTeam/dnsproxy/proxy" - "github.com/AdguardTeam/golibs/log" - "github.com/AdguardTeam/golibs/mathutil" - "github.com/AdguardTeam/golibs/netutil" - "github.com/miekg/dns" -) - -const ( - // maxNAT64PrefixBitLen is the maximum length of a NAT64 prefix in bits. - // See https://datatracker.ietf.org/doc/html/rfc6147#section-5.2. - maxNAT64PrefixBitLen = 96 - - // nat64PrefixLen is the length of a NAT64 prefix in bytes. - nat64PrefixLen = net.IPv6len - net.IPv4len - - // maxDNS64SynTTL is the maximum TTL for synthesized DNS64 responses with no - // SOA records in seconds. - // - // If the SOA RR was not delivered with the negative response to the AAAA - // query, then the DNS64 SHOULD use the TTL of the original A RR or 600 - // seconds, whichever is shorter. - // - // See https://datatracker.ietf.org/doc/html/rfc6147#section-5.1.7. - maxDNS64SynTTL uint32 = 600 ) // setupDNS64 initializes DNS64 settings, the NAT64 prefixes in particular. If @@ -38,227 +14,22 @@ const ( // is specified explicitly. Each prefix also validated to be a valid IPv6 // CIDR with a maximum length of 96 bits. The first specified prefix is then // used to synthesize AAAA records. -func (s *Server) setupDNS64() (err error) { +func (s *Server) setupDNS64() { if !s.conf.UseDNS64 { - return nil - } - - l := len(s.conf.DNS64Prefixes) - if l == 0 { - s.dns64Prefs = []netip.Prefix{dns64WellKnownPref} - - return nil - } - - prefs := make([]netip.Prefix, 0, l) - for i, pref := range s.conf.DNS64Prefixes { - var p netip.Prefix - p, err = netip.ParsePrefix(pref) - if err != nil { - return fmt.Errorf("prefix at index %d: %w", i, err) - } - - addr := p.Addr() - if !addr.Is6() { - return fmt.Errorf("prefix at index %d: %q is not an IPv6 prefix", i, pref) - } - - if p.Bits() > maxNAT64PrefixBitLen { - return fmt.Errorf("prefix at index %d: %q is too long for DNS64", i, pref) - } - - prefs = append(prefs, p.Masked()) - } - - s.dns64Prefs = prefs - - return nil -} - -// checkDNS64 checks if DNS64 should be performed. It returns a DNS64 request -// to resolve or nil if DNS64 is not desired. It also filters resp to not -// contain any NAT64 excluded addresses in the answer section, if needed. Both -// req and resp must not be nil. -// -// See https://datatracker.ietf.org/doc/html/rfc6147. -func (s *Server) checkDNS64(req, resp *dns.Msg) (dns64Req *dns.Msg) { - if len(s.dns64Prefs) == 0 { - return nil - } - - q := req.Question[0] - if q.Qtype != dns.TypeAAAA || q.Qclass != dns.ClassINET { - // DNS64 operation for classes other than IN is undefined, and a DNS64 - // MUST behave as though no DNS64 function is configured. - return nil - } - - rcode := resp.Rcode - if rcode == dns.RcodeNameError { - // A result with RCODE=3 (Name Error) is handled according to normal DNS - // operation (which is normally to return the error to the client). - return nil - } - - if rcode == dns.RcodeSuccess { - // If resolver receives an answer with at least one AAAA record - // containing an address outside any of the excluded range(s), then it - // by default SHOULD build an answer section for a response including - // only the AAAA record(s) that do not contain any of the addresses - // inside the excluded ranges. - var hasAnswers bool - if resp.Answer, hasAnswers = s.filterNAT64Answers(resp.Answer); hasAnswers { - return nil - } - - // Any other RCODE is treated as though the RCODE were 0 and the answer - // section were empty. - } - - return &dns.Msg{ - MsgHdr: dns.MsgHdr{ - Id: dns.Id(), - RecursionDesired: req.RecursionDesired, - AuthenticatedData: req.AuthenticatedData, - CheckingDisabled: req.CheckingDisabled, - }, - Question: []dns.Question{{ - Name: req.Question[0].Name, - Qtype: dns.TypeA, - Qclass: dns.ClassINET, - }}, - } -} - -// filterNAT64Answers filters out AAAA records that are within one of NAT64 -// exclusion prefixes. hasAnswers is true if the filtered slice contains at -// least a single AAAA answer not within the prefixes or a CNAME. -func (s *Server) filterNAT64Answers(rrs []dns.RR) (filtered []dns.RR, hasAnswers bool) { - filtered = make([]dns.RR, 0, len(rrs)) - for _, ans := range rrs { - switch ans := ans.(type) { - case *dns.AAAA: - addr, err := netutil.IPToAddrNoMapped(ans.AAAA) - if err != nil { - log.Error("dnsforward: bad AAAA record: %s", err) - - continue - } - - if s.withinDNS64(addr) { - // Filter the record. - continue - } - - filtered, hasAnswers = append(filtered, ans), true - case *dns.CNAME, *dns.DNAME: - // If the response contains a CNAME or a DNAME, then the CNAME or - // DNAME chain is followed until the first terminating A or AAAA - // record is reached. - // - // Just treat CNAME and DNAME responses as passable answers since - // AdGuard Home doesn't follow any of these chains except the - // dnsrewrite-defined ones. - filtered, hasAnswers = append(filtered, ans), true - default: - filtered = append(filtered, ans) - } - } - - return filtered, hasAnswers -} - -// synthDNS64 synthesizes a DNS64 response using the original response as a -// basis and modifying it with data from resp. It returns true if the response -// was actually modified. -func (s *Server) synthDNS64(origReq, origResp, resp *dns.Msg) (ok bool) { - if len(resp.Answer) == 0 { - // If there is an empty answer, then the DNS64 responds to the original - // querying client with the answer the DNS64 received to the original - // (initiator's) query. - return false - } - - // The Time to Live (TTL) field is set to the minimum of the TTL of the - // original A RR and the SOA RR for the queried domain. If the original - // response contains no SOA records, the minimum of the TTL of the original - // A RR and [maxDNS64SynTTL] should be used. See [maxDNS64SynTTL]. - soaTTL := maxDNS64SynTTL - for _, rr := range origResp.Ns { - if hdr := rr.Header(); hdr.Rrtype == dns.TypeSOA && hdr.Name == origReq.Question[0].Name { - soaTTL = hdr.Ttl - - break - } + return } - newAns := make([]dns.RR, 0, len(resp.Answer)) - for _, ans := range resp.Answer { - rr := s.synthRR(ans, soaTTL) - if rr == nil { - // The error should have already been logged. - return false - } + if len(s.conf.DNS64Prefixes) == 0 { + // dns64WellKnownPref is the default prefix to use in an algorithmic + // mapping for DNS64. + // + // See https://datatracker.ietf.org/doc/html/rfc6052#section-2.1. + dns64WellKnownPref := netip.MustParsePrefix("64:ff9b::/96") - newAns = append(newAns, rr) - } - - origResp.Answer = newAns - origResp.Ns = resp.Ns - origResp.Extra = resp.Extra - - return true -} - -// dns64WellKnownPref is the default prefix to use in an algorithmic mapping for -// DNS64. See https://datatracker.ietf.org/doc/html/rfc6052#section-2.1. -var dns64WellKnownPref = netip.MustParsePrefix("64:ff9b::/96") - -// withinDNS64 checks if ip is within one of the configured DNS64 prefixes. -// -// TODO(e.burkov): We actually using bytes of only the first prefix from the -// set to construct the answer, so consider using some implementation of a -// prefix set for the rest. -func (s *Server) withinDNS64(ip netip.Addr) (ok bool) { - for _, n := range s.dns64Prefs { - if n.Contains(ip) { - return true - } - } - - return false -} - -// shouldStripDNS64 returns true if DNS64 is enabled and ip has either one of -// custom DNS64 prefixes or the Well-Known one. This is intended to be used -// with PTR requests. -// -// The requirement is to match any Pref64::/n used at the site, and not merely -// the locally configured Pref64::/n. This is because end clients could ask for -// a PTR record matching an address received through a different (site-provided) -// DNS64. -// -// See https://datatracker.ietf.org/doc/html/rfc6147#section-5.3.1. -func (s *Server) shouldStripDNS64(ip net.IP) (ok bool) { - if len(s.dns64Prefs) == 0 { - return false - } - - addr, err := netutil.IPToAddr(ip, netutil.AddrFamilyIPv6) - if err != nil { - return false - } - - switch { - case s.withinDNS64(addr): - log.Debug("dnsforward: %s is within DNS64 custom prefix set", ip) - case dns64WellKnownPref.Contains(addr): - log.Debug("dnsforward: %s is within DNS64 well-known prefix", ip) - default: - return false + s.dns64Pref = dns64WellKnownPref + } else { + s.dns64Pref = s.conf.DNS64Prefixes[0] } - - return true } // mapDNS64 maps ip to IPv6 address using configured DNS64 prefix. ip must be a @@ -267,79 +38,12 @@ func (s *Server) shouldStripDNS64(ip net.IP) (ok bool) { func (s *Server) mapDNS64(ip netip.Addr) (mapped net.IP) { // Don't mask the address here since it should have already been masked on // initialization stage. - pref := s.dns64Prefs[0].Addr().As16() + pref := s.dns64Pref.Masked().Addr().As16() ipData := ip.As4() mapped = make(net.IP, net.IPv6len) - copy(mapped[:nat64PrefixLen], pref[:]) - copy(mapped[nat64PrefixLen:], ipData[:]) + copy(mapped[:proxy.NAT64PrefixLength], pref[:]) + copy(mapped[proxy.NAT64PrefixLength:], ipData[:]) return mapped } - -// performDNS64 processes the current state of dctx assuming that it has already -// been tried to resolve, checks if it contains any acceptable response, and if -// it doesn't, performs DNS64 request and the following synthesis. It returns -// the [resultCodeError] if there was an error set to dctx. -func (s *Server) performDNS64(prx *proxy.Proxy, dctx *dnsContext) (rc resultCode) { - pctx := dctx.proxyCtx - req := pctx.Req - - dns64Req := s.checkDNS64(req, pctx.Res) - if dns64Req == nil { - return resultCodeSuccess - } - - log.Debug("dnsforward: received an empty AAAA response, checking DNS64") - - origReq := pctx.Req - origResp := pctx.Res - origUps := pctx.Upstream - - pctx.Req = dns64Req - defer func() { pctx.Req = origReq }() - - if dctx.err = prx.Resolve(pctx); dctx.err != nil { - return resultCodeError - } - - dns64Resp := pctx.Res - pctx.Res = origResp - if dns64Resp != nil && s.synthDNS64(origReq, pctx.Res, dns64Resp) { - log.Debug("dnsforward: synthesized AAAA response for %q", origReq.Question[0].Name) - } else { - pctx.Upstream = origUps - } - - return resultCodeSuccess -} - -// synthRR synthesizes a DNS64 resource record in compliance with RFC 6147. If -// rr is not an A record, it's returned as is. A records are modified to become -// a DNS64-synthesized AAAA records, and the TTL is set according to the -// original TTL of a record and soaTTL. It returns nil on invalid A records. -func (s *Server) synthRR(rr dns.RR, soaTTL uint32) (result dns.RR) { - aResp, ok := rr.(*dns.A) - if !ok { - return rr - } - - addr, err := netutil.IPToAddr(aResp.A, netutil.AddrFamilyIPv4) - if err != nil { - log.Error("dnsforward: bad A record: %s", err) - - return nil - } - - aaaa := &dns.AAAA{ - Hdr: dns.RR_Header{ - Name: aResp.Hdr.Name, - Rrtype: dns.TypeAAAA, - Class: aResp.Hdr.Class, - Ttl: mathutil.Min(aResp.Hdr.Ttl, soaTTL), - }, - AAAA: s.mapDNS64(addr), - } - - return aaaa -} diff --git a/internal/dnsforward/dns64_test.go b/internal/dnsforward/dns64_test.go index 12925504e0f..f3679b517ff 100644 --- a/internal/dnsforward/dns64_test.go +++ b/internal/dnsforward/dns64_test.go @@ -15,6 +15,16 @@ import ( "github.com/stretchr/testify/require" ) +// maxDNS64SynTTL is the maximum TTL for synthesized DNS64 responses with no SOA +// records in seconds. +// +// If the SOA RR was not delivered with the negative response to the AAAA query, +// then the DNS64 SHOULD use the TTL of the original A RR or 600 seconds, +// whichever is shorter. +// +// See https://datatracker.ietf.org/doc/html/rfc6147#section-5.1.7. +const maxDNS64SynTTL uint32 = 600 + // newRR is a helper that creates a new dns.RR with the given name, qtype, ttl // and value. It fails the test if the qtype is not supported or the type of // value doesn't match the qtype. diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go index 4ff9fc02001..3d9bdba1fe6 100644 --- a/internal/dnsforward/dnsforward.go +++ b/internal/dnsforward/dnsforward.go @@ -80,10 +80,15 @@ type Server struct { privateNets netutil.SubnetSet localResolvers *proxy.Proxy sysResolvers aghnet.SystemResolvers - recDetector *recursionDetector - // dns64Prefix is the set of NAT64 prefixes used for DNS64 handling. - dns64Prefs []netip.Prefix + // recDetector is a cache for recursive requests. It is used to detect + // and prevent recursive requests only for private upstreams. + // + // See https://github.com/adguardTeam/adGuardHome/issues/3185#issuecomment-851048135. + recDetector *recursionDetector + + // dns64Prefs is the NAT64 prefix used for DNS64 response mapping. + dns64Pref netip.Prefix // anonymizer masks the client's IP addresses if needed. anonymizer *aghnet.IPMut @@ -477,6 +482,8 @@ func (s *Server) Prepare(conf *ServerConfig) (err error) { return fmt.Errorf("preparing proxy: %w", err) } + s.setupDNS64() + err = s.prepareInternalProxy() if err != nil { return fmt.Errorf("preparing internal proxy: %w", err) @@ -493,18 +500,18 @@ func (s *Server) Prepare(conf *ServerConfig) (err error) { s.registerHandlers() - err = s.setupDNS64() - if err != nil { - return fmt.Errorf("preparing DNS64: %w", err) - } - - s.dnsProxy = &proxy.Proxy{Config: proxyConfig} - + // TODO(e.burkov): Remove once the local resolvers logic moved to dnsproxy. err = s.setupResolvers(s.conf.LocalPTRResolvers) if err != nil { return fmt.Errorf("setting up resolvers: %w", err) } + if s.conf.UsePrivateRDNS { + proxyConfig.PrivateRDNSUpstreamConfig = s.localResolvers.UpstreamConfig + } + + s.dnsProxy = &proxy.Proxy{Config: proxyConfig} + s.recDetector.clear() return nil diff --git a/internal/dnsforward/dnsforward_test.go b/internal/dnsforward/dnsforward_test.go index 56c21516904..912398269ba 100644 --- a/internal/dnsforward/dnsforward_test.go +++ b/internal/dnsforward/dnsforward_test.go @@ -89,9 +89,14 @@ func createTestServer( s.serverLock.Lock() defer s.serverLock.Unlock() + // TODO(e.burkov): Try to move it higher. if localUps != nil { - s.localResolvers.UpstreamConfig.Upstreams = []upstream.Upstream{localUps} + ups := []upstream.Upstream{localUps} + s.localResolvers.UpstreamConfig.Upstreams = ups s.conf.UsePrivateRDNS = true + s.dnsProxy.PrivateRDNSUpstreamConfig = &proxy.UpstreamConfig{ + Upstreams: ups, + } } return s diff --git a/internal/home/config.go b/internal/home/config.go index f2fc98f5a5d..aa7ebf728d7 100644 --- a/internal/home/config.go +++ b/internal/home/config.go @@ -188,7 +188,7 @@ type dnsConfig struct { UseDNS64 bool `yaml:"use_dns64"` // DNS64Prefixes is the list of NAT64 prefixes to be used for DNS64. - DNS64Prefixes []string `yaml:"dns64_prefixes"` + DNS64Prefixes []netip.Prefix `yaml:"dns64_prefixes"` // ServeHTTP3 defines if HTTP/3 is be allowed for incoming requests. //