Pull request: 4403-internal-proxy

Updates #4403.

* commit 'd519929988575439c2238924ae5b1d96091445f7':
  dnsforward: imp code, fmt
  Also honor the user-defined UpstreamMode for the internal DNS proxy

internal/dnsforward/config.go

@@ -163,10 +163,10 @@ type TLSConfig struct {
 
 // DNSCryptConfig is the DNSCrypt server configuration struct.
 type DNSCryptConfig struct {
+	ResolverCert   *dnscrypt.Cert
+	ProviderName   string
 	UDPListenAddrs []*net.UDPAddr
 	TCPListenAddrs []*net.TCPAddr
-	ProviderName   string
-	ResolverCert   *dnscrypt.Cert
 	Enabled        bool
 }
 
@@ -214,68 +214,67 @@ var defaultValues = ServerConfig{
 	FilteringConfig: FilteringConfig{BlockedResponseTTL: 3600},
 }
 
-// createProxyConfig creates and validates configuration for the main proxy
+// createProxyConfig creates and validates configuration for the main proxy.
-func (s *Server) createProxyConfig() (proxy.Config, error) {
+func (s *Server) createProxyConfig() (conf proxy.Config, err error) {
-	proxyConfig := proxy.Config{
+	srvConf := s.conf
-		UDPListenAddr:          s.conf.UDPListenAddrs,
+	conf = proxy.Config{
-		TCPListenAddr:          s.conf.TCPListenAddrs,
+		UDPListenAddr:          srvConf.UDPListenAddrs,
-		Ratelimit:              int(s.conf.Ratelimit),
+		TCPListenAddr:          srvConf.TCPListenAddrs,
-		RatelimitWhitelist:     s.conf.RatelimitWhitelist,
+		Ratelimit:              int(srvConf.Ratelimit),
-		RefuseAny:              s.conf.RefuseAny,
+		RatelimitWhitelist:     srvConf.RatelimitWhitelist,
-		TrustedProxies:         s.conf.TrustedProxies,
+		RefuseAny:              srvConf.RefuseAny,
-		CacheMinTTL:            s.conf.CacheMinTTL,
+		TrustedProxies:         srvConf.TrustedProxies,
-		CacheMaxTTL:            s.conf.CacheMaxTTL,
+		CacheMinTTL:            srvConf.CacheMinTTL,
-		CacheOptimistic:        s.conf.CacheOptimistic,
+		CacheMaxTTL:            srvConf.CacheMaxTTL,
-		UpstreamConfig:         s.conf.UpstreamConfig,
+		CacheOptimistic:        srvConf.CacheOptimistic,
+		UpstreamConfig:         srvConf.UpstreamConfig,
 		BeforeRequestHandler:   s.beforeRequestHandler,
 		RequestHandler:         s.handleDNSRequest,
-		EnableEDNSClientSubnet: s.conf.EnableEDNSClientSubnet,
+		EnableEDNSClientSubnet: srvConf.EnableEDNSClientSubnet,
-		MaxGoroutines:          int(s.conf.MaxGoroutines),
+		MaxGoroutines:          int(srvConf.MaxGoroutines),
 	}
 
-	if s.conf.CacheSize != 0 {
+	if srvConf.CacheSize != 0 {
-		proxyConfig.CacheEnabled = true
+		conf.CacheEnabled = true
-		proxyConfig.CacheSizeBytes = int(s.conf.CacheSize)
+		conf.CacheSizeBytes = int(srvConf.CacheSize)
 	}
 
-	proxyConfig.UpstreamMode = proxy.UModeLoadBalance
+	setProxyUpstreamMode(
-	if s.conf.AllServers {
+		&conf,
-		proxyConfig.UpstreamMode = proxy.UModeParallel
+		srvConf.AllServers,
-	} else if s.conf.FastestAddr {
+		srvConf.FastestAddr,
-		proxyConfig.UpstreamMode = proxy.UModeFastestAddr
+		srvConf.FastestTimeout.Duration,
-		proxyConfig.FastestPingTimeout = s.conf.FastestTimeout.Duration
+	)
-	}
 
-	for i, s := range s.conf.BogusNXDomain {
+	for i, s := range srvConf.BogusNXDomain {
-		subnet, err := netutil.ParseSubnet(s)
+		var subnet *net.IPNet
+		subnet, err = netutil.ParseSubnet(s)
 		if err != nil {
 			log.Error("subnet at index %d: %s", i, err)
 
 			continue
 		}
 
-		proxyConfig.BogusNXDomain = append(proxyConfig.BogusNXDomain, subnet)
+		conf.BogusNXDomain = append(conf.BogusNXDomain, subnet)
 	}
 
-	// TLS settings
+	err = s.prepareTLS(&conf)
-	err := s.prepareTLS(&proxyConfig)
 	if err != nil {
-		return proxyConfig, err
+		return conf, fmt.Errorf("validating tls: %w", err)
 	}
 
-	if s.conf.DNSCryptConfig.Enabled {
+	if c := srvConf.DNSCryptConfig; c.Enabled {
-		proxyConfig.DNSCryptUDPListenAddr = s.conf.DNSCryptConfig.UDPListenAddrs
+		conf.DNSCryptUDPListenAddr = c.UDPListenAddrs
-		proxyConfig.DNSCryptTCPListenAddr = s.conf.DNSCryptConfig.TCPListenAddrs
+		conf.DNSCryptTCPListenAddr = c.TCPListenAddrs
-		proxyConfig.DNSCryptProviderName = s.conf.DNSCryptConfig.ProviderName
+		conf.DNSCryptProviderName = c.ProviderName
-		proxyConfig.DNSCryptResolverCert = s.conf.DNSCryptConfig.ResolverCert
+		conf.DNSCryptResolverCert = c.ResolverCert
 	}
 
-	// Validate proxy config
+	if conf.UpstreamConfig == nil || len(conf.UpstreamConfig.Upstreams) == 0 {
-	if proxyConfig.UpstreamConfig == nil || len(proxyConfig.UpstreamConfig.Upstreams) == 0 {
+		return conf, errors.Error("no default upstream servers configured")
-		return proxyConfig, errors.Error("no default upstream servers configured")
 	}
 
-	return proxyConfig, nil
+	return conf, nil
 }
 
 const (
@@ -381,14 +380,45 @@ func (s *Server) prepareUpstreamSettings() error {
 	return nil
 }
 
-// prepareIntlProxy - initializes DNS proxy that we use for internal DNS queries
+// prepareInternalProxy initializes the DNS proxy that is used for internal DNS
-func (s *Server) prepareIntlProxy() {
+// queries, such at client PTR resolving and udpater hostname resolving.
+func (s *Server) prepareInternalProxy() {
+	conf := &proxy.Config{
+		CacheEnabled:   true,
+		CacheSizeBytes: 4096,
+		UpstreamConfig: s.conf.UpstreamConfig,
+		MaxGoroutines:  int(s.conf.MaxGoroutines),
+	}
+
+	srvConf := s.conf
+	setProxyUpstreamMode(
+		conf,
+		srvConf.AllServers,
+		srvConf.FastestAddr,
+		srvConf.FastestTimeout.Duration,
+	)
+
+	// TODO(a.garipov): Make a proper constructor for proxy.Proxy.
 	s.internalProxy = &proxy.Proxy{
-		Config: proxy.Config{
+		Config: *conf,
-			CacheEnabled:   true,
+	}
-			CacheSizeBytes: 4096,
+}
-			UpstreamConfig: s.conf.UpstreamConfig,
+
-		},
+// setProxyUpstreamMode sets the upstream mode and related settings in conf
+// based on provided parameters.
+func setProxyUpstreamMode(
+	conf *proxy.Config,
+	allServers bool,
+	fastestAddr bool,
+	fastestTimeout time.Duration,
+) {
+	if allServers {
+		conf.UpstreamMode = proxy.UModeParallel
+	} else if fastestAddr {
+		conf.UpstreamMode = proxy.UModeFastestAddr
+		conf.FastestPingTimeout = fastestTimeout
+	} else {
+		conf.UpstreamMode = proxy.UModeLoadBalance
 	}
 }
 

internal/dnsforward/dnsforward.go

@@ -49,11 +49,12 @@ type hostToIPTable = map[string]net.IP
 // Server is the main way to start a DNS server.
 //
 // Example:
-//  s := dnsforward.Server{}
+//
-//  err := s.Start(nil) // will start a DNS server listening on default port 53, in a goroutine
+//	s := dnsforward.Server{}
-//  err := s.Reconfigure(ServerConfig{UDPListenAddr: &net.UDPAddr{Port: 53535}}) // will reconfigure running DNS server to listen on UDP port 53535
+//	err := s.Start(nil) // will start a DNS server listening on default port 53, in a goroutine
-//  err := s.Stop() // will stop listening on port 53535 and cancel all goroutines
+//	err := s.Reconfigure(ServerConfig{UDPListenAddr: &net.UDPAddr{Port: 53535}}) // will reconfigure running DNS server to listen on UDP port 53535
-//  err := s.Start(nil) // will start listening again, on port 53535, in a goroutine
+//	err := s.Stop() // will stop listening on port 53535 and cancel all goroutines
+//	err := s.Start(nil) // will start listening again, on port 53535, in a goroutine
 //
 // The zero Server is empty and ready for use.
 type Server struct {
@@ -176,18 +177,6 @@ func NewServer(p DNSCreateParams) (s *Server, err error) {
 	return s, nil
 }
 
-// NewCustomServer creates a new instance of *Server with custom internal proxy.
-func NewCustomServer(internalProxy *proxy.Proxy) *Server {
-	s := &Server{
-		recDetector: newRecursionDetector(0, 1),
-	}
-	if internalProxy != nil {
-		s.internalProxy = internalProxy
-	}
-
-	return s
-}
-
 // Close gracefully closes the server.  It is safe for concurrent use.
 //
 // TODO(e.burkov): A better approach would be making Stop method waiting for all
@@ -488,7 +477,7 @@ func (s *Server) Prepare(config *ServerConfig) error {
 
 	// Prepare a DNS proxy instance that we use for internal DNS queries
 	// --
-	s.prepareIntlProxy()
+	s.prepareInternalProxy()
 
 	s.access, err = newAccessCtx(s.conf.AllowedClients, s.conf.DisallowedClients, s.conf.BlockedHosts)
 	if err != nil {

internal/dnsforward/dnsforward_test.go

@@ -1217,13 +1217,17 @@ func TestServer_Exchange(t *testing.T) {
 	errUpstream := aghtest.NewErrorUpstream()
 	nonPtrUpstream := aghtest.NewBlockUpstream("some-host", true)
 
-	srv := NewCustomServer(&proxy.Proxy{
+	srv := &Server{
-		Config: proxy.Config{
+		recDetector: newRecursionDetector(0, 1),
-			UpstreamConfig: &proxy.UpstreamConfig{
+		internalProxy: &proxy.Proxy{
-				Upstreams: []upstream.Upstream{extUpstream},
+			Config: proxy.Config{
+				UpstreamConfig: &proxy.UpstreamConfig{
+					Upstreams: []upstream.Upstream{extUpstream},
+				},
 			},
 		},
-	})
+	}
+
 	srv.conf.ResolveClients = true
 	srv.conf.UsePrivateRDNS = true
 

internal/dnsforward/http.go

@@ -711,11 +711,16 @@ func (s *Server) handleTestUpstreamDNS(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+// handleDoH is the DNS-over-HTTPs handler.
+//
 // Control flow:
-// web
+//
-//  -> dnsforward.handleDoH -> dnsforward.ServeHTTP
+//	HTTP server
-//  -> proxy.ServeHTTP -> proxy.handleDNSRequest
+//	-> dnsforward.handleDoH
-//  -> dnsforward.handleDNSRequest
+//	-> dnsforward.ServeHTTP
+//	-> proxy.ServeHTTP
+//	-> proxy.handleDNSRequest
+//	-> dnsforward.handleDNSRequest
 func (s *Server) handleDoH(w http.ResponseWriter, r *http.Request) {
 	if !s.conf.TLSAllowUnencryptedDoH && r.TLS == nil {
 		aghhttp.Error(r, w, http.StatusNotFound, "Not Found")

internal/dnsforward/svcbmsg.go

@@ -157,10 +157,10 @@ var svcbKeyHandlers = map[string]svcbKeyHandler{
 // Firstly, the parsing of non-contiguous values isn't supported.  Secondly, the
 // parsing of value-lists is not supported either.
 //
-//   ipv4hint=127.0.0.1             // Supported.
+//	ipv4hint=127.0.0.1             // Supported.
-//   ipv4hint="127.0.0.1"           // Unsupported.
+//	ipv4hint="127.0.0.1"           // Unsupported.
-//   ipv4hint=127.0.0.1,127.0.0.2   // Unsupported.
+//	ipv4hint=127.0.0.1,127.0.0.2   // Unsupported.
-//   ipv4hint="127.0.0.1,127.0.0.2" // Unsupported.
+//	ipv4hint="127.0.0.1,127.0.0.2" // Unsupported.
 //
 // TODO(a.garipov): Support all of these.
 func (s *Server) genAnswerSVCB(req *dns.Msg, svcb *rules.DNSSVCB) (ans *dns.SVCB) {

internal/home/dns.go

@@ -280,10 +280,10 @@ func newDNSCrypt(hosts []net.IP, tlsConf tlsConfigSettings) (dnscc dnsforward.DN
 	}
 
 	return dnsforward.DNSCryptConfig{
-		UDPListenAddrs: ipsToUDPAddrs(hosts, tlsConf.PortDNSCrypt),
-		TCPListenAddrs: ipsToTCPAddrs(hosts, tlsConf.PortDNSCrypt),
 		ResolverCert:   cert,
 		ProviderName:   rc.ProviderName,
+		UDPListenAddrs: ipsToUDPAddrs(hosts, tlsConf.PortDNSCrypt),
+		TCPListenAddrs: ipsToTCPAddrs(hosts, tlsConf.PortDNSCrypt),
 		Enabled:        true,
 	}, nil
 }

internal/version/version.go

@@ -73,8 +73,7 @@ const (
 
 // fmtModule returns formatted information about module.  The result looks like:
 //
-//   github.com/Username/module@v1.2.3 (sum: someHASHSUM=)
+//	github.com/Username/module@v1.2.3 (sum: someHASHSUM=)
-//
 func fmtModule(m *debug.Module) (formatted string) {
 	if m == nil {
 		return ""
@@ -118,18 +117,18 @@ const (
 
 // Verbose returns formatted build information.  Output example:
 //
-//   AdGuard Home
+//	AdGuard Home
-//   Version: v0.105.3
+//	Version: v0.105.3
-//   Channel: development
+//	Channel: development
-//   Go version: go1.15.3
+//	Go version: go1.15.3
-//   Build time: 2021-03-30T16:26:08Z+0300
+//	Build time: 2021-03-30T16:26:08Z+0300
-//   GOOS: darwin
+//	GOOS: darwin
-//   GOARCH: amd64
+//	GOARCH: amd64
-//   Race: false
+//	Race: false
-//   Main module:
+//	Main module:
-//           ...
+//	        ...
-//   Dependencies:
+//	Dependencies:
-//           ...
+//	        ...
 //
 // TODO(e.burkov): Make it write into passed io.Writer.
 func Verbose() (v string) {