From af87eb8441bc01e96fcc65d1bb8643c28cb5d273 Mon Sep 17 00:00:00 2001
From: Stanislav Chzhen <s.chzhen@adguard.com>
Date: Wed, 25 Jun 2025 20:07:47 +0300
Subject: [PATCH] all: fix dnscrypt config

---
 CHANGELOG.md                  |  4 +++
 internal/dnsforward/config.go | 57 +++++++++++++++++++++++------------
 internal/home/dns.go          | 42 +++++++++++++-------------
 3 files changed, 63 insertions(+), 40 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1a8f508b..e4cffea1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -24,10 +24,14 @@ NOTE: Add new changes BELOW THIS COMMENT.
 
 ### Fixed
 
+- Validation process for DNSCrypt settings ([#7856])
+
 - The hostnames of DHCP clients with multiple labels not being recognized.
 
 - Status reported by the systemd service implementation in cases of auto-restart after a failed start.
 
+[#7856]: https://github.com/AdguardTeam/AdGuardHome/issues/7856
+
 [go-1.24.4]: https://groups.google.com/g/golang-announce/c/ufZ8WpEsA3A
 
 <!--
diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go
index 3217d2fd..00e61861 100644
--- a/internal/dnsforward/config.go
+++ b/internal/dnsforward/config.go
@@ -167,10 +167,14 @@ type EDNSClientSubnet struct {
 	UseCustom bool `yaml:"use_custom"`
 }
 
-// TLSConfig contains the TLS configuration settings for DNS-over-HTTPS (DoH),
-// DNS-over-TLS (DoT), DNS-over-QUIC (DoQ), and Discovery of Designated
-// Resolvers (DDR).
+// TLSConfig contains the TLS configuration settings for DNSCrypt,
+// DNS-over-HTTPS (DoH), DNS-over-TLS (DoT), DNS-over-QUIC (DoQ), and Discovery
+// of Designated Resolvers (DDR).
 type TLSConfig struct {
+	// DNSCryptConf contains the configuration settings for a DNSCrypt server.
+	// It is nil if the DNSCrypt server is disabled.
+	DNSCryptConf *DNSCryptConfig
+
 	// Cert is the TLS certificate used for TLS connections.  It is nil if
 	// encryption is disabled.
 	Cert *tls.Certificate
@@ -197,13 +201,21 @@ type TLSConfig struct {
 	StrictSNICheck bool
 }
 
-// DNSCryptConfig is the DNSCrypt server configuration struct.
+// DNSCryptConfig contains the configuration settings for a DNSCrypt server.
 type DNSCryptConfig struct {
-	ResolverCert   *dnscrypt.Cert
-	ProviderName   string
+	// ResolverCert is the certificate used for DNSCrypt connections.
+	ResolverCert *dnscrypt.Cert
+
+	// UDPListenAddrs are the addresses to listen on for DNSCrypt UDP
+	// connections.
 	UDPListenAddrs []*net.UDPAddr
+
+	// TCPListenAddrs are the addresses to listen on for DNSCrypt TCP
+	// connections.
 	TCPListenAddrs []*net.TCPAddr
-	Enabled        bool
+
+	// ProviderName is the name of the DNSCrypt provider.
+	ProviderName string
 }
 
 // ServerConfig represents server configuration.
@@ -234,7 +246,6 @@ type ServerConfig struct {
 	TLSConf *TLSConfig
 
 	Config
-	DNSCryptConfig
 	TLSAllowUnencryptedDoH bool
 
 	// UpstreamTimeout is the timeout for querying upstream servers.
@@ -351,13 +362,6 @@ func (s *Server) newProxyConfig() (conf *proxy.Config, err error) {
 		return nil, fmt.Errorf("validating plain: %w", err)
 	}
 
-	if c := srvConf.DNSCryptConfig; c.Enabled {
-		conf.DNSCryptUDPListenAddr = c.UDPListenAddrs
-		conf.DNSCryptTCPListenAddr = c.TCPListenAddrs
-		conf.DNSCryptProviderName = c.ProviderName
-		conf.DNSCryptResolverCert = c.ResolverCert
-	}
-
 	conf, err = prepareCacheConfig(conf,
 		srvConf.CacheSize,
 		srvConf.CacheMinTTL,
@@ -608,8 +612,23 @@ func (conf *ServerConfig) ourAddrsSet() (m addrPortSet, err error) {
 	}
 }
 
+// prepareDNSCrypt sets up the DNSCrypt configuration for the DNS proxy.
+func (s *Server) prepareDNSCrypt(proxyConf *proxy.Config) {
+	dnsCryptConf := s.conf.TLSConf.DNSCryptConf
+	if dnsCryptConf == nil {
+		return
+	}
+
+	proxyConf.DNSCryptUDPListenAddr = dnsCryptConf.UDPListenAddrs
+	proxyConf.DNSCryptTCPListenAddr = dnsCryptConf.TCPListenAddrs
+	proxyConf.DNSCryptProviderName = dnsCryptConf.ProviderName
+	proxyConf.DNSCryptResolverCert = dnsCryptConf.ResolverCert
+}
+
 // prepareTLS sets up the TLS configuration for the DNS proxy.
-func (s *Server) prepareTLS(proxyConfig *proxy.Config) (err error) {
+func (s *Server) prepareTLS(proxyConf *proxy.Config) (err error) {
+	s.prepareDNSCrypt(proxyConf)
+
 	if s.conf.TLSConf.Cert == nil {
 		return
 	}
@@ -618,8 +637,8 @@ func (s *Server) prepareTLS(proxyConfig *proxy.Config) (err error) {
 		return nil
 	}
 
-	proxyConfig.TLSListenAddr = s.conf.TLSConf.TLSListenAddrs
-	proxyConfig.QUICListenAddr = s.conf.TLSConf.QUICListenAddrs
+	proxyConf.TLSListenAddr = s.conf.TLSConf.TLSListenAddrs
+	proxyConf.QUICListenAddr = s.conf.TLSConf.QUICListenAddrs
 
 	cert, err := x509.ParseCertificate(s.conf.TLSConf.Cert.Certificate[0])
 	if err != nil {
@@ -639,7 +658,7 @@ func (s *Server) prepareTLS(proxyConfig *proxy.Config) (err error) {
 		}
 	}
 
-	proxyConfig.TLSConfig = &tls.Config{
+	proxyConf.TLSConfig = &tls.Config{
 		GetCertificate: s.onGetCertificate,
 		CipherSuites:   s.conf.TLSCiphers,
 		MinVersion:     tls.VersionTLS12,
diff --git a/internal/home/dns.go b/internal/home/dns.go
index 9afc6263..dea65bf7 100644
--- a/internal/home/dns.go
+++ b/internal/home/dns.go
@@ -295,13 +295,6 @@ func newServerConfig(
 		UseWHOIS:         clientSrcConf.WHOIS,
 	}
 
-	newConf.DNSCryptConfig, err = newDNSCryptConfig(tlsConf, hosts)
-	if err != nil {
-		// Don't wrap the error, because it's already wrapped by
-		// newDNSCryptConfig.
-		return nil, err
-	}
-
 	return newConf, nil
 }
 
@@ -315,7 +308,14 @@ func newDNSTLSConfig(
 		return &dnsforward.TLSConfig{}, nil
 	}
 
+	dnsCryptConf, err := newDNSCryptConfig(conf, addrs)
+	if err != nil {
+		// Don't wrap the error, because it's informative enough as is.
+		return nil, err
+	}
+
 	dnsConf = &dnsforward.TLSConfig{
+		DNSCryptConf:   dnsCryptConf,
 		ServerName:     conf.ServerName,
 		StrictSNICheck: conf.StrictSNICheck,
 	}
@@ -334,15 +334,16 @@ func newDNSTLSConfig(
 
 	cert, err := tls.X509KeyPair(conf.CertificateChainData, conf.PrivateKeyData)
 	if err != nil {
-		const format = "parsing tls key pair: %w"
-		if conf.AllowUnencryptedDoH {
+		err = fmt.Errorf("parsing tls key pair: %w", err)
+		if conf.AllowUnencryptedDoH || dnsCryptConf != nil {
 			// TODO(s.chzhen):  Use [slog.Logger].
-			log.Info("warning: %s: %s", format, err)
+			log.Info("warning: %s", err)
 
 			return dnsConf, nil
 		}
 
-		return nil, fmt.Errorf(format, err)
+		// Don't wrap the error, because it's already annotated.
+		return nil, err
 	}
 
 	dnsConf.Cert = &cert
@@ -355,38 +356,37 @@ func newDNSTLSConfig(
 func newDNSCryptConfig(
 	conf *tlsConfigSettings,
 	addrs []netip.Addr,
-) (dnsCryptConf dnsforward.DNSCryptConfig, err error) {
-	if !conf.Enabled || conf.PortDNSCrypt == 0 {
-		return dnsforward.DNSCryptConfig{}, nil
+) (dnsCryptConf *dnsforward.DNSCryptConfig, err error) {
+	if conf.PortDNSCrypt == 0 {
+		return nil, nil
 	}
 
 	if conf.DNSCryptConfigFile == "" {
-		return dnsforward.DNSCryptConfig{}, errors.Error("no dnscrypt_config_file")
+		return nil, fmt.Errorf("dnscrypt_config_file: %w", errors.ErrEmptyValue)
 	}
 
 	f, err := os.Open(conf.DNSCryptConfigFile)
 	if err != nil {
-		return dnsforward.DNSCryptConfig{}, fmt.Errorf("opening dnscrypt config: %w", err)
+		return nil, 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 dnsforward.DNSCryptConfig{}, fmt.Errorf("decoding dnscrypt config: %w", err)
+		return nil, fmt.Errorf("decoding dnscrypt config: %w", err)
 	}
 
 	cert, err := rc.CreateCert()
 	if err != nil {
-		return dnsforward.DNSCryptConfig{}, fmt.Errorf("creating dnscrypt cert: %w", err)
+		return nil, fmt.Errorf("creating dnscrypt cert: %w", err)
 	}
 
-	return dnsforward.DNSCryptConfig{
+	return &dnsforward.DNSCryptConfig{
 		ResolverCert:   cert,
-		ProviderName:   rc.ProviderName,
 		UDPListenAddrs: ipsToUDPAddrs(addrs, conf.PortDNSCrypt),
 		TCPListenAddrs: ipsToTCPAddrs(addrs, conf.PortDNSCrypt),
-		Enabled:        true,
+		ProviderName:   rc.ProviderName,
 	}, nil
 }