all: remove private rdns logic

go.mod

@@ -3,8 +3,8 @@ module github.com/AdguardTeam/AdGuardHome
 go 1.21.8
 
 require (
-	github.com/AdguardTeam/dnsproxy v0.67.0
-	github.com/AdguardTeam/golibs v0.21.0
+	github.com/AdguardTeam/dnsproxy v0.67.1-0.20240403090357-a2c0e321a217
+	github.com/AdguardTeam/golibs v0.22.0
 	github.com/AdguardTeam/urlfilter v0.18.0
 	github.com/NYTimes/gziphandler v1.1.1
 	github.com/ameshkov/dnscrypt/v2 v2.2.7
@@ -49,6 +49,7 @@ require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect
 	github.com/google/pprof v0.0.0-20240227163752-401108e1b7e7 // indirect
+	github.com/jessevdk/go-flags v1.5.0 // indirect
 	github.com/mdlayher/socket v0.5.0 // indirect
 	github.com/onsi/ginkgo/v2 v2.16.0 // indirect
 	github.com/patrickmn/go-cache v2.1.0+incompatible // indirect

go.sum

@@ -1,7 +1,11 @@
 github.com/AdguardTeam/dnsproxy v0.67.0 h1:7oKfcA8sm9d1N4qvhsNmQWBX4+fs3sX4cAnERmBXEbw=
 github.com/AdguardTeam/dnsproxy v0.67.0/go.mod h1:XLfD6IpSplUZZ+f5vhWSJW1mp4wm+KkHWiMo9w7U1Ls=
+github.com/AdguardTeam/dnsproxy v0.67.1-0.20240403090357-a2c0e321a217 h1:ryczFRf8y6PEzCjgy/S3Ptg4Ea1TUYFyiEZnoEyEV7s=
+github.com/AdguardTeam/dnsproxy v0.67.1-0.20240403090357-a2c0e321a217/go.mod h1:5wIQueGTDX1Uk4GYevRh7HCtsCUR/U9lxf478+STOZI=
 github.com/AdguardTeam/golibs v0.21.0 h1:0swWyNaHTmT7aMwffKd9d54g4wBd8Oaj0fl+5l/PRdE=
 github.com/AdguardTeam/golibs v0.21.0/go.mod h1:/votX6WK1PdcZ3T2kBOPjPCGmfhlKixhI6ljYrFRPvI=
+github.com/AdguardTeam/golibs v0.22.0 h1:wvT/UFIT8XIBfMabnD3LcDRiorx8J0lc3A/bzD6OX7c=
+github.com/AdguardTeam/golibs v0.22.0/go.mod h1:/votX6WK1PdcZ3T2kBOPjPCGmfhlKixhI6ljYrFRPvI=
 github.com/AdguardTeam/urlfilter v0.18.0 h1:ZZzwODC/ADpjJSODxySrrUnt/fvOCfGFaCW6j+wsGfQ=
 github.com/AdguardTeam/urlfilter v0.18.0/go.mod h1:IXxBwedLiZA2viyHkaFxY/8mjub0li2PXRg8a3d9Z1s=
 github.com/NYTimes/gziphandler v1.1.1 h1:ZUDjpQae29j0ryrS0u/B8HZfJBtBQHjqw2rQ2cqUQ3I=
@@ -58,6 +62,8 @@ github.com/hugelgupf/socketpair v0.0.0-20190730060125-05d35a94e714/go.mod h1:2Go
 github.com/insomniacslk/dhcp v0.0.0-20240227161007-c728f5dd21c8 h1:V3plQrMHRWOB5zMm3yNqvBxDQVW1+/wHBSok5uPdmVs=
 github.com/insomniacslk/dhcp v0.0.0-20240227161007-c728f5dd21c8/go.mod h1:izxuNQZeFrbx2nK2fAyN5iNUB34Fe9j0nK4PwLzAkKw=
 github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
+github.com/jessevdk/go-flags v1.5.0 h1:1jKYvbxEjfUl0fmqTCOfonvskHHXMjBySTLW4y9LFvc=
+github.com/jessevdk/go-flags v1.5.0/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4=
 github.com/josharian/native v1.0.0/go.mod h1:7X/raswPFr05uY3HiLlYeyQntB6OO7E/d2Cu7qoaN2w=
 github.com/josharian/native v1.1.1-0.20230202152459-5c7d0dd6ab86 h1:elKwZS1OcdQ0WwEDBeqxKwb7WB62QX8bvZ/FJnVXIfk=
 github.com/josharian/native v1.1.1-0.20230202152459-5c7d0dd6ab86/go.mod h1:aFAMtuldEgx/4q7iSGazk22+IcgvtiC+HIimFO9XlS8=
@@ -158,6 +164,7 @@ golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20201015000850-e3ed0017c211/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210315160823-c6e025ad8005/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220209214540-3681064d5158/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.4.1-0.20230131160137-e7d7f63158de/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=

internal/dnsforward/config.go

@@ -235,9 +235,18 @@ type DNSCryptConfig struct {
 // ServerConfig represents server configuration.
 // The zero ServerConfig is empty and ready for use.
 type ServerConfig struct {
-	UDPListenAddrs []*net.UDPAddr        // UDP listen address
-	TCPListenAddrs []*net.TCPAddr        // TCP listen address
-	UpstreamConfig *proxy.UpstreamConfig // Upstream DNS servers config
+	// UDPListenAddrs is the list of addresses to listen for DNS-over-UDP.
+	UDPListenAddrs []*net.UDPAddr
+
+	// TCPListenAddrs is the list of addresses to listen for DNS-over-TCP.
+	TCPListenAddrs []*net.TCPAddr
+
+	// UpstreamConfig is the general configuration of upstream DNS servers.
+	UpstreamConfig *proxy.UpstreamConfig
+
+	// PrivateRDNSUpstreamConfig is the configuration of upstream DNS servers
+	// for private reverse DNS.
+	PrivateRDNSUpstreamConfig *proxy.UpstreamConfig
 
 	// AddrProcConf defines the configuration for the client IP processor.
 	// If nil, [client.EmptyAddrProc] is used.
@@ -306,24 +315,28 @@ func (s *Server) newProxyConfig() (conf *proxy.Config, err error) {
 	trustedPrefixes := netutil.UnembedPrefixes(srvConf.TrustedProxies)
 
 	conf = &proxy.Config{
-		HTTP3:                  srvConf.ServeHTTP3,
-		Ratelimit:              int(srvConf.Ratelimit),
-		RatelimitSubnetLenIPv4: srvConf.RatelimitSubnetLenIPv4,
-		RatelimitSubnetLenIPv6: srvConf.RatelimitSubnetLenIPv6,
-		RatelimitWhitelist:     srvConf.RatelimitWhitelist,
-		RefuseAny:              srvConf.RefuseAny,
-		TrustedProxies:         netutil.SliceSubnetSet(trustedPrefixes),
-		CacheMinTTL:            srvConf.CacheMinTTL,
-		CacheMaxTTL:            srvConf.CacheMaxTTL,
-		CacheOptimistic:        srvConf.CacheOptimistic,
-		UpstreamConfig:         srvConf.UpstreamConfig,
-		BeforeRequestHandler:   s.beforeRequestHandler,
-		RequestHandler:         s.handleDNSRequest,
-		HTTPSServerName:        aghhttp.UserAgent(),
-		EnableEDNSClientSubnet: srvConf.EDNSClientSubnet.Enabled,
-		MaxGoroutines:          srvConf.MaxGoroutines,
-		UseDNS64:               srvConf.UseDNS64,
-		DNS64Prefs:             srvConf.DNS64Prefixes,
+		HTTP3:                     srvConf.ServeHTTP3,
+		Ratelimit:                 int(srvConf.Ratelimit),
+		RatelimitSubnetLenIPv4:    srvConf.RatelimitSubnetLenIPv4,
+		RatelimitSubnetLenIPv6:    srvConf.RatelimitSubnetLenIPv6,
+		RatelimitWhitelist:        srvConf.RatelimitWhitelist,
+		RefuseAny:                 srvConf.RefuseAny,
+		TrustedProxies:            netutil.SliceSubnetSet(trustedPrefixes),
+		CacheMinTTL:               srvConf.CacheMinTTL,
+		CacheMaxTTL:               srvConf.CacheMaxTTL,
+		CacheOptimistic:           srvConf.CacheOptimistic,
+		UpstreamConfig:            srvConf.UpstreamConfig,
+		PrivateRDNSUpstreamConfig: srvConf.PrivateRDNSUpstreamConfig,
+		BeforeRequestHandler:      s.beforeRequestHandler,
+		RequestHandler:            s.handleDNSRequest,
+		HTTPSServerName:           aghhttp.UserAgent(),
+		EnableEDNSClientSubnet:    srvConf.EDNSClientSubnet.Enabled,
+		MaxGoroutines:             srvConf.MaxGoroutines,
+		UseDNS64:                  srvConf.UseDNS64,
+		DNS64Prefs:                srvConf.DNS64Prefixes,
+		UsePrivateRDNS:            srvConf.UsePrivateRDNS,
+		PrivateSubnets:            s.privateNets,
+		// TODO(e.burkov):  !! add message constructor
 	}
 
 	if srvConf.EDNSClientSubnet.UseCustom {

internal/dnsforward/dns64_test.go

@@ -63,6 +63,7 @@ func newRR(t *testing.T, name string, qtype uint16, ttl uint32, val any) (rr dns
 	return rr
 }
 
+// TODO(e.burkov):  !! add ErrNoUptreams case
 func TestServer_HandleDNSRequest_dns64(t *testing.T) {
 	const (
 		ipv4Domain    = "ipv4.only."

internal/dnsforward/dnsforward.go

@@ -135,12 +135,6 @@ type Server struct {
 	// WHOIS, etc.
 	addrProc client.AddressProcessor
 
-	// localResolvers is a DNS proxy instance used to resolve PTR records for
-	// addresses considered private as per the [privateNets].
-	//
-	// TODO(e.burkov):  Remove once the local resolvers logic moved to dnsproxy.
-	localResolvers *proxy.Proxy
-
 	// sysResolvers used to fetch system resolvers to use by default for private
 	// PTR resolving.
 	sysResolvers SystemResolvers
@@ -158,12 +152,6 @@ type Server struct {
 	// [upstream.Resolver] interface.
 	bootResolvers []*upstream.UpstreamResolver
 
-	// 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
-
 	// dns64Pref is the NAT64 prefix used for DNS64 response mapping.  The major
 	// part of DNS64 happens inside the [proxy] package, but there still are
 	// some places where response mapping is needed (e.g. DHCP).
@@ -212,14 +200,6 @@ type DNSCreateParams struct {
 	LocalDomain string
 }
 
-const (
-	// recursionTTL is the time recursive request is cached for.
-	recursionTTL = 1 * time.Second
-	// cachedRecurrentReqNum is the maximum number of cached recurrent
-	// requests.
-	cachedRecurrentReqNum = 1000
-)
-
 // NewServer creates a new instance of the dnsforward.Server
 // Note: this function must be called only once
 //
@@ -256,7 +236,6 @@ func NewServer(p DNSCreateParams) (s *Server, err error) {
 		// TODO(e.burkov):  Use some case-insensitive string comparison.
 		localDomainSuffix: strings.ToLower(localDomainSuffix),
 		etcHosts:          etcHosts,
-		recDetector:       newRecursionDetector(recursionTTL, cachedRecurrentReqNum),
 		clientIDCache: cache.New(cache.Config{
 			EnableLRU: true,
 			MaxCount:  defaultClientIDCacheCount,
@@ -366,6 +345,7 @@ func (s *Server) Exchange(ip netip.Addr) (host string, ttl time.Duration, err er
 	s.serverLock.RLock()
 	defer s.serverLock.RUnlock()
 
+	// TODO(e.burkov):  Migrate to netip.Addr already.
 	arpa, err := netutil.IPToReversedAddr(ip.AsSlice())
 	if err != nil {
 		return "", 0, fmt.Errorf("reversing ip: %w", err)
@@ -386,25 +366,23 @@ func (s *Server) Exchange(ip netip.Addr) (host string, ttl time.Duration, err er
 	}
 
 	dctx := &proxy.DNSContext{
-		Proto: "udp",
-		Req:   req,
+		Proto:           "udp",
+		Req:             req,
+		IsPrivateClient: true,
 	}
 
-	var resolver *proxy.Proxy
 	var errMsg string
 	if s.privateNets.Contains(ip) {
 		if !s.conf.UsePrivateRDNS {
 			return "", 0, nil
 		}
 
-		resolver = s.localResolvers
 		errMsg = "resolving a private address: %w"
-		s.recDetector.add(*req)
+		dctx.RequestedPrivateRDNS = netip.PrefixFrom(ip, ip.BitLen())
 	} else {
-		resolver = s.internalProxy
 		errMsg = "resolving an address: %w"
 	}
-	if err = resolver.Resolve(dctx); err != nil {
+	if err = s.internalProxy.Resolve(dctx); err != nil {
 		return "", 0, fmt.Errorf(errMsg, err)
 	}
 
@@ -541,35 +519,6 @@ func (err *LocalResolversError) Unwrap() error {
 	return err.Err
 }
 
-// setupLocalResolvers initializes and sets the resolvers for local addresses.
-// It assumes s.serverLock is locked or s not running.  It returns the upstream
-// configuration used for private PTR resolving, or nil if it's disabled.  Note,
-// that it's safe to put nil into [proxy.Config.PrivateRDNSUpstreamConfig].
-func (s *Server) setupLocalResolvers(boot upstream.Resolver) (uc *proxy.UpstreamConfig, err error) {
-	if !s.conf.UsePrivateRDNS {
-		// It's safe to put nil into [proxy.Config.PrivateRDNSUpstreamConfig].
-		return nil, nil
-	}
-
-	uc, err = s.prepareLocalResolvers(boot)
-	if err != nil {
-		// Don't wrap the error because it's informative enough as is.
-		return nil, err
-	}
-
-	localResolvers, err := proxy.New(&proxy.Config{
-		UpstreamConfig: uc,
-	})
-	if err != nil {
-		return nil, &LocalResolversError{Err: err}
-	}
-
-	s.localResolvers = localResolvers
-
-	// TODO(e.burkov):  Should we also consider the DNS64 usage?
-	return uc, nil
-}
-
 // Prepare initializes parameters of s using data from conf.  conf must not be
 // nil.
 func (s *Server) Prepare(conf *ServerConfig) (err error) {
@@ -586,7 +535,7 @@ func (s *Server) Prepare(conf *ServerConfig) (err error) {
 
 	s.initDefaultSettings()
 
-	boot, err := s.prepareInternalDNS()
+	err = s.prepareInternalDNS()
 	if err != nil {
 		// Don't wrap the error, because it's informative enough as is.
 		return err
@@ -608,12 +557,6 @@ func (s *Server) Prepare(conf *ServerConfig) (err error) {
 		return fmt.Errorf("preparing access: %w", err)
 	}
 
-	// TODO(e.burkov):  Remove once the local resolvers logic moved to dnsproxy.
-	proxyConfig.PrivateRDNSUpstreamConfig, err = s.setupLocalResolvers(boot)
-	if err != nil {
-		return fmt.Errorf("setting up resolvers: %w", err)
-	}
-
 	proxyConfig.Fallbacks, err = s.setupFallbackDNS()
 	if err != nil {
 		return fmt.Errorf("setting up fallback dns servers: %w", err)
@@ -626,8 +569,6 @@ func (s *Server) Prepare(conf *ServerConfig) (err error) {
 
 	s.dnsProxy = dnsProxy
 
-	s.recDetector.clear()
-
 	s.setupAddrProc()
 
 	s.registerHandlers()
@@ -638,10 +579,10 @@ func (s *Server) Prepare(conf *ServerConfig) (err error) {
 // prepareInternalDNS initializes the internal state of s before initializing
 // the primary DNS proxy instance.  It assumes s.serverLock is locked or the
 // Server not running.
-func (s *Server) prepareInternalDNS() (boot upstream.Resolver, err error) {
+func (s *Server) prepareInternalDNS() (err error) {
 	err = s.prepareIpsetListSettings()
 	if err != nil {
-		return nil, fmt.Errorf("preparing ipset settings: %w", err)
+		return fmt.Errorf("preparing ipset settings: %w", err)
 	}
 
 	s.bootstrap, s.bootResolvers, err = s.createBootstrap(s.conf.BootstrapDNS, &upstream.Options{
@@ -650,21 +591,28 @@ func (s *Server) prepareInternalDNS() (boot upstream.Resolver, err error) {
 	})
 	if err != nil {
 		// Don't wrap the error, because it's informative enough as is.
-		return nil, err
+		return err
 	}
 
 	err = s.prepareUpstreamSettings(s.bootstrap)
 	if err != nil {
 		// Don't wrap the error, because it's informative enough as is.
-		return s.bootstrap, err
+		return err
+	}
+
+	if s.conf.UsePrivateRDNS {
+		s.conf.PrivateRDNSUpstreamConfig, err = s.prepareLocalResolvers(s.bootstrap)
+		if err != nil {
+			return fmt.Errorf("setting up resolvers: %w", err)
+		}
 	}
 
 	err = s.prepareInternalProxy()
 	if err != nil {
-		return s.bootstrap, fmt.Errorf("preparing internal proxy: %w", err)
+		return fmt.Errorf("preparing internal proxy: %w", err)
 	}
 
-	return s.bootstrap, nil
+	return nil
 }
 
 // setupFallbackDNS initializes the fallback DNS servers.
@@ -743,10 +691,15 @@ func validateBlockingMode(
 func (s *Server) prepareInternalProxy() (err error) {
 	srvConf := s.conf
 	conf := &proxy.Config{
-		CacheEnabled:   true,
-		CacheSizeBytes: 4096,
-		UpstreamConfig: srvConf.UpstreamConfig,
-		MaxGoroutines:  s.conf.MaxGoroutines,
+		CacheEnabled:              true,
+		CacheSizeBytes:            4096,
+		PrivateRDNSUpstreamConfig: srvConf.PrivateRDNSUpstreamConfig,
+		UpstreamConfig:            srvConf.UpstreamConfig,
+		MaxGoroutines:             srvConf.MaxGoroutines,
+		UseDNS64:                  srvConf.UseDNS64,
+		DNS64Prefs:                srvConf.DNS64Prefixes,
+		UsePrivateRDNS:            srvConf.UsePrivateRDNS,
+		// TODO(e.burkov):  !! add message constructor
 	}
 
 	err = setProxyUpstreamMode(conf, srvConf.UpstreamMode, srvConf.FastestTimeout.Duration)
@@ -782,11 +735,6 @@ func (s *Server) stopLocked() (err error) {
 		}
 	}
 
-	logCloserErr(s.internalProxy.UpstreamConfig, "dnsforward: closing internal resolvers: %s")
-	if s.localResolvers != nil {
-		logCloserErr(s.localResolvers.UpstreamConfig, "dnsforward: closing local resolvers: %s")
-	}
-
 	for _, b := range s.bootResolvers {
 		logCloserErr(b, "dnsforward: closing bootstrap %s: %s", b.Address())
 	}

internal/dnsforward/http.go

@@ -352,7 +352,11 @@ func (req *jsonDNSConfig) validateUpstreamDNSServers(privateNets netutil.SubnetS
 	}
 
 	if req.LocalPTRUpstreams != nil {
-		err = ValidateUpstreamsPrivate(*req.LocalPTRUpstreams, privateNets)
+		var uc *proxy.UpstreamConfig
+		uc, err = proxy.ParseUpstreamsConfig(*req.LocalPTRUpstreams, &upstream.Options{})
+		if err == nil {
+			err = proxy.ValidatePrivateConfig(uc, privateNets)
+		}
 		if err != nil {
 			return fmt.Errorf("private upstream servers: %w", err)
 		}