Pull request 2075: 1660-refactor-dns

Updates #1660. Squashed commit of the following: commit ed49233 Merge: 85e8252 388583c Author: Ainar Garipov <A.Garipov@AdGuard.COM> Date: Fri Nov 17 16:00:13 2023 +0300 Merge branch 'master' into 1660-refactor-dns commit 85e8252 Author: Ainar Garipov <A.Garipov@AdGuard.COM> Date: Fri Nov 17 14:55:11 2023 +0300 home: imp dns conf commit bd255a7 Author: Ainar Garipov <A.Garipov@AdGuard.COM> Date: Fri Nov 17 14:30:50 2023 +0300 all: imp dns conf
AdguardTeam · Nov 17, 2023 · 1c0bf95 · 1c0bf95
1 parent 388583c
commit 1c0bf95
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 93 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,9 +29,10 @@ NOTE: Add new changes BELOW THIS COMMENT.
 
 #### Configuration changes
 
-- Added new properties `clients.persistent.upstreams_cache_enabled` and
-  `clients.persistent.upstreams_cache_size` which describe cache configuration
-  for each client custom upstream configuration.
+- The property `dns.bogus_nxdomain` is now validated more strictly.
+- Added new properties `clients.persistent.*.upstreams_cache_enabled` and
+  `clients.persistent.*.upstreams_cache_size` that describe cache configuration
+  for each client's custom upstream configuration.
 
 ### Fixed
 

diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go
@@ -334,16 +334,9 @@ func (s *Server) createProxyConfig() (conf proxy.Config, err error) {
 		srvConf.FastestTimeout.Duration,
 	)
 
-	for i, s := range srvConf.BogusNXDomain {
-		var subnet netip.Prefix
-		subnet, err = aghnet.ParseSubnet(s)
-		if err != nil {
-			log.Error("subnet at index %d: %s", i, err)
-
-			continue
-		}
-
-		conf.BogusNXDomain = append(conf.BogusNXDomain, subnet)
+	conf.BogusNXDomain, err = parseBogusNXDOMAIN(srvConf.BogusNXDomain)
+	if err != nil {
+		return proxy.Config{}, fmt.Errorf("bogus_nxdomain: %w", err)
 	}
 
 	err = s.prepareTLS(&conf)
@@ -365,6 +358,21 @@ func (s *Server) createProxyConfig() (conf proxy.Config, err error) {
 	return conf, nil
 }
 
+// parseBogusNXDOMAIN parses the bogus NXDOMAIN strings into valid subnets.
+func parseBogusNXDOMAIN(confBogusNXDOMAIN []string) (subnets []netip.Prefix, err error) {
+	for i, s := range confBogusNXDOMAIN {
+		var subnet netip.Prefix
+		subnet, err = aghnet.ParseSubnet(s)
+		if err != nil {
+			return nil, fmt.Errorf("subnet at index %d: %w", i, err)
+		}
+
+		subnets = append(subnets, subnet)
+	}
+
+	return subnets, nil
+}
+
 const defaultBlockedResponseTTL = 3600
 
 // initDefaultSettings initializes default settings if nothing

diff --git a/internal/home/config.go b/internal/home/config.go
@@ -115,6 +115,8 @@ type configuration struct {
 	// Theme is a UI theme for current user.
 	Theme Theme `yaml:"theme"`
 
+	// TODO(a.garipov): Make DNS and the fields below pointers and validate
+	// and/or reset on explicit nulling.
 	DNS      dnsConfig         `yaml:"dns"`
 	TLS      tlsConfigSettings `yaml:"tls"`
 	QueryLog queryLogConfig    `yaml:"querylog"`
@@ -214,13 +216,13 @@ type dnsConfig struct {
 	// DNS64Prefixes is the list of NAT64 prefixes to be used for DNS64.
 	DNS64Prefixes []netip.Prefix `yaml:"dns64_prefixes"`
 
-	// ServeHTTP3 defines if HTTP/3 is be allowed for incoming requests.
+	// ServeHTTP3 defines if HTTP/3 is allowed for incoming requests.
 	//
 	// TODO(a.garipov): Add to the UI when HTTP/3 support is no longer
 	// experimental.
 	ServeHTTP3 bool `yaml:"serve_http3"`
 
-	// UseHTTP3Upstreams defines if HTTP/3 is be allowed for DNS-over-HTTPS
+	// UseHTTP3Upstreams defines if HTTP/3 is allowed for DNS-over-HTTPS
 	// upstreams.
 	//
 	// TODO(a.garipov): Add to the UI when HTTP/3 support is no longer

diff --git a/internal/home/dns.go b/internal/home/dns.go
@@ -150,7 +150,7 @@ func initDNSServer(
 
 	Context.clients.dnsServer = Context.dnsServer
 
-	dnsConf, err := newServerConfig(tlsConf, httpReg)
+	dnsConf, err := newServerConfig(&config.DNS, config.Clients.Sources, tlsConf, httpReg)
 	if err != nil {
 		closeDNSServer()
 
@@ -223,21 +223,36 @@ func ipsToUDPAddrs(ips []netip.Addr, port uint16) (udpAddrs []*net.UDPAddr) {
 	return udpAddrs
 }
 
+// newServerConfig converts values from the configuration file into the internal
+// DNS server configuration.  All arguments must not be nil.
 func newServerConfig(
+	dnsConf *dnsConfig,
+	clientSrcConf *clientSourcesConfig,
 	tlsConf *tlsConfigSettings,
 	httpReg aghhttp.RegisterFunc,
 ) (newConf *dnsforward.ServerConfig, err error) {
-	dnsConf := config.DNS
 	hosts := aghalg.CoalesceSlice(dnsConf.BindHosts, []netip.Addr{netutil.IPv4Localhost()})
 
+	fwdConf := dnsConf.Config
+	fwdConf.FilterHandler = applyAdditionalFiltering
+	fwdConf.ClientsContainer = &Context.clients
+
 	newConf = &dnsforward.ServerConfig{
-		UDPListenAddrs: ipsToUDPAddrs(hosts, dnsConf.Port),
-		TCPListenAddrs: ipsToTCPAddrs(hosts, dnsConf.Port),
-		Config:         dnsConf.Config,
-		ConfigModified: onConfigModified,
-		HTTPRegister:   httpReg,
-		UseDNS64:       config.DNS.UseDNS64,
-		DNS64Prefixes:  config.DNS.DNS64Prefixes,
+		UDPListenAddrs:         ipsToUDPAddrs(hosts, dnsConf.Port),
+		TCPListenAddrs:         ipsToTCPAddrs(hosts, dnsConf.Port),
+		Config:                 fwdConf,
+		TLSConfig:              newDNSTLSConfig(tlsConf, hosts),
+		TLSAllowUnencryptedDoH: tlsConf.AllowUnencryptedDoH,
+		UpstreamTimeout:        dnsConf.UpstreamTimeout.Duration,
+		TLSv12Roots:            Context.tlsRoots,
+		ConfigModified:         onConfigModified,
+		HTTPRegister:           httpReg,
+		LocalPTRResolvers:      dnsConf.LocalPTRResolvers,
+		UseDNS64:               dnsConf.UseDNS64,
+		DNS64Prefixes:          dnsConf.DNS64Prefixes,
+		UsePrivateRDNS:         dnsConf.UsePrivateRDNS,
+		ServeHTTP3:             dnsConf.ServeHTTP3,
+		UseHTTP3Upstreams:      dnsConf.UseHTTP3Upstreams,
 	}
 
 	var initialAddresses []netip.Addr
@@ -255,79 +270,81 @@ func newServerConfig(
 		AddressUpdater:   &Context.clients,
 		InitialAddresses: initialAddresses,
 		CatchPanics:      true,
-		UseRDNS:          config.Clients.Sources.RDNS,
-		UseWHOIS:         config.Clients.Sources.WHOIS,
+		UseRDNS:          clientSrcConf.RDNS,
+		UseWHOIS:         clientSrcConf.WHOIS,
 	}
 
-	if tlsConf.Enabled {
-		newConf.TLSConfig = tlsConf.TLSConfig
-		newConf.TLSConfig.ServerName = tlsConf.ServerName
-
-		if tlsConf.PortHTTPS != 0 {
-			newConf.HTTPSListenAddrs = ipsToTCPAddrs(hosts, tlsConf.PortHTTPS)
-		}
-
-		if tlsConf.PortDNSOverTLS != 0 {
-			newConf.TLSListenAddrs = ipsToTCPAddrs(hosts, tlsConf.PortDNSOverTLS)
-		}
+	newConf.DNSCryptConfig, err = newDNSCryptConfig(tlsConf, hosts)
+	if err != nil {
+		// Don't wrap the error, because it's already wrapped by
+		// newDNSCryptConfig.
+		return nil, err
+	}
 
-		if tlsConf.PortDNSOverQUIC != 0 {
-			newConf.QUICListenAddrs = ipsToUDPAddrs(hosts, tlsConf.PortDNSOverQUIC)
-		}
+	return newConf, nil
+}
 
-		if tlsConf.PortDNSCrypt != 0 {
-			newConf.DNSCryptConfig, err = newDNSCrypt(hosts, *tlsConf)
-			if err != nil {
-				// Don't wrap the error, because it's already wrapped by
-				// newDNSCrypt.
-				return nil, err
-			}
-		}
+// newDNSTLSConfig converts values from the configuration file into the internal
+// TLS settings for the DNS server.  tlsConf must not be nil.
+func newDNSTLSConfig(conf *tlsConfigSettings, addrs []netip.Addr) (dnsConf dnsforward.TLSConfig) {
+	if !conf.Enabled {
+		return dnsforward.TLSConfig{}
 	}
 
-	newConf.TLSv12Roots = Context.tlsRoots
-	newConf.TLSAllowUnencryptedDoH = tlsConf.AllowUnencryptedDoH
+	dnsConf = conf.TLSConfig
+	dnsConf.ServerName = conf.ServerName
 
-	newConf.FilterHandler = applyAdditionalFiltering
-	newConf.ClientsContainer = &Context.clients
+	if conf.PortHTTPS != 0 {
+		dnsConf.HTTPSListenAddrs = ipsToTCPAddrs(addrs, conf.PortHTTPS)
+	}
 
-	newConf.LocalPTRResolvers = dnsConf.LocalPTRResolvers
-	newConf.UpstreamTimeout = dnsConf.UpstreamTimeout.Duration
+	if conf.PortDNSOverTLS != 0 {
+		dnsConf.TLSListenAddrs = ipsToTCPAddrs(addrs, conf.PortDNSOverTLS)
+	}
 
-	newConf.UsePrivateRDNS = dnsConf.UsePrivateRDNS
-	newConf.ServeHTTP3 = dnsConf.ServeHTTP3
-	newConf.UseHTTP3Upstreams = dnsConf.UseHTTP3Upstreams
+	if conf.PortDNSOverQUIC != 0 {
+		dnsConf.QUICListenAddrs = ipsToUDPAddrs(addrs, conf.PortDNSOverQUIC)
+	}
 
-	return newConf, nil
+	return dnsConf
 }
 
-func newDNSCrypt(hosts []netip.Addr, tlsConf tlsConfigSettings) (dnscc dnsforward.DNSCryptConfig, err error) {
-	if tlsConf.DNSCryptConfigFile == "" {
-		return dnscc, errors.Error("no dnscrypt_config_file")
+// newDNSCryptConfig converts values from the configuration file into the
+// internal DNSCrypt settings for the DNS server.  conf must not be nil.
+func newDNSCryptConfig(
+	conf *tlsConfigSettings,
+	addrs []netip.Addr,
+) (dnsCryptConf dnsforward.DNSCryptConfig, err error) {
+	if !conf.Enabled || conf.PortDNSCrypt == 0 {
+		return dnsforward.DNSCryptConfig{}, nil
 	}
 
-	f, err := os.Open(tlsConf.DNSCryptConfigFile)
+	if conf.DNSCryptConfigFile == "" {
+		return dnsforward.DNSCryptConfig{}, errors.Error("no dnscrypt_config_file")
+	}
+
+	f, err := os.Open(conf.DNSCryptConfigFile)
 	if err != nil {
-		return dnscc, fmt.Errorf("opening dnscrypt config: %w", err)
+		return dnsforward.DNSCryptConfig{}, fmt.Errorf("opening dnscrypt config: %w", err)
 	}
 	defer func() { err = errors.WithDeferred(err, f.Close()) }()
 
 	rc := &dnscrypt.ResolverConfig{}
 	err = yaml.NewDecoder(f).Decode(rc)
 	if err != nil {
-		return dnscc, fmt.Errorf("decoding dnscrypt config: %w", err)
+		return dnsforward.DNSCryptConfig{}, fmt.Errorf("decoding dnscrypt config: %w", err)
 	}
 
 	cert, err := rc.CreateCert()
 	if err != nil {
-		return dnscc, fmt.Errorf("creating dnscrypt cert: %w", err)
+		return dnsforward.DNSCryptConfig{}, fmt.Errorf("creating dnscrypt cert: %w", err)
 	}
 
 	return dnsforward.DNSCryptConfig{
 		ResolverCert:   cert,
 		ProviderName:   rc.ProviderName,
-		UDPListenAddrs: ipsToUDPAddrs(hosts, tlsConf.PortDNSCrypt),
-		TCPListenAddrs: ipsToTCPAddrs(hosts, tlsConf.PortDNSCrypt),
+		UDPListenAddrs: ipsToUDPAddrs(addrs, conf.PortDNSCrypt),
+		TCPListenAddrs: ipsToTCPAddrs(addrs, conf.PortDNSCrypt),
 		Enabled:        true,
 	}, nil
 }
@@ -343,34 +360,36 @@ func getDNSEncryption() (de dnsEncryption) {
 
 	Context.tls.WriteDiskConfig(&tlsConf)
 
-	if tlsConf.Enabled && len(tlsConf.ServerName) != 0 {
-		hostname := tlsConf.ServerName
-		if tlsConf.PortHTTPS != 0 {
-			addr := hostname
-			if p := tlsConf.PortHTTPS; p != defaultPortHTTPS {
-				addr = netutil.JoinHostPort(addr, p)
-			}
-
-			de.https = (&url.URL{
-				Scheme: "https",
-				Host:   addr,
-				Path:   "/dns-query",
-			}).String()
-		}
+	if !tlsConf.Enabled || len(tlsConf.ServerName) == 0 {
+		return dnsEncryption{}
+	}
 
-		if p := tlsConf.PortDNSOverTLS; p != 0 {
-			de.tls = (&url.URL{
-				Scheme: "tls",
-				Host:   netutil.JoinHostPort(hostname, p),
-			}).String()
+	hostname := tlsConf.ServerName
+	if tlsConf.PortHTTPS != 0 {
+		addr := hostname
+		if p := tlsConf.PortHTTPS; p != defaultPortHTTPS {
+			addr = netutil.JoinHostPort(addr, p)
 		}
 
-		if p := tlsConf.PortDNSOverQUIC; p != 0 {
-			de.quic = (&url.URL{
-				Scheme: "quic",
-				Host:   netutil.JoinHostPort(hostname, p),
-			}).String()
-		}
+		de.https = (&url.URL{
+			Scheme: "https",
+			Host:   addr,
+			Path:   "/dns-query",
+		}).String()
+	}
+
+	if p := tlsConf.PortDNSOverTLS; p != 0 {
+		de.tls = (&url.URL{
+			Scheme: "tls",
+			Host:   netutil.JoinHostPort(hostname, p),
+		}).String()
+	}
+
+	if p := tlsConf.PortDNSOverQUIC; p != 0 {
+		de.quic = (&url.URL{
+			Scheme: "quic",
+			Host:   netutil.JoinHostPort(hostname, p),
+		}).String()
 	}
 
 	return de
@@ -455,7 +474,7 @@ func reconfigureDNSServer() (err error) {
 	tlsConf := &tlsConfigSettings{}
 	Context.tls.WriteDiskConfig(tlsConf)
 
-	newConf, err := newServerConfig(tlsConf, httpRegister)
+	newConf, err := newServerConfig(&config.DNS, config.Clients.Sources, tlsConf, httpRegister)
 	if err != nil {
 		return fmt.Errorf("generating forwarding dns server config: %w", err)
 	}