From 827fc455b1b727ecec9cc27bca0ecc62e6d478b4 Mon Sep 17 00:00:00 2001 From: Stanislav Chzhen Date: Mon, 30 Jun 2025 19:20:40 +0300 Subject: [PATCH] Pull request 2428: 7856-fix-dnscrypt-config Updates #7856. Squashed commit of the following: commit cc4406f9d9c2fb6e2c0f7c669dfd98b4bc101f8e Author: Stanislav Chzhen Date: Mon Jun 30 19:08:55 2025 +0300 all: fix chlog commit 6045a091e225cb06cbb50b0196fa077e0e04db49 Author: Stanislav Chzhen Date: Mon Jun 30 19:07:43 2025 +0300 all: fix chlog commit 34a3193ad2be4af7300a2a105a45cd3dba441052 Merge: 4c43bd91a 5e4458223 Author: Stanislav Chzhen Date: Mon Jun 30 19:06:03 2025 +0300 Merge branch 'master' into 7856-fix-dnscrypt-config commit 4c43bd91addbbbdfbd9a212f689b599adb202167 Author: Stanislav Chzhen Date: Thu Jun 26 13:57:49 2025 +0300 dnsforward: imp docs commit af87eb8441bc01e96fcc65d1bb8643c28cb5d273 Author: Stanislav Chzhen Date: Wed Jun 25 20:07:47 2025 +0300 all: fix dnscrypt config --- CHANGELOG.md | 6 ++++ internal/dnsforward/config.go | 59 ++++++++++++++++++++++++----------- internal/home/dns.go | 42 ++++++++++++------------- 3 files changed, 67 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e50c4db2..f4d32be6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,12 @@ See also the [v0.107.64 GitHub milestone][ms-v0.107.64]. NOTE: Add new changes BELOW THIS COMMENT. --> +### Fixed + +- Validation process for DNSCrypt settings ([#7856]) + +[#7856]: https://github.com/AdguardTeam/AdGuardHome/issues/7856 + diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go index 3217d2fd..198b7702 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,23 @@ 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. It is not + // nil if there is at least one UDP or TCP address present. + 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. It is not empty if + // there is at least one UDP or TCP address present. + ProviderName string } // ServerConfig represents server configuration. @@ -234,7 +248,6 @@ type ServerConfig struct { TLSConf *TLSConfig Config - DNSCryptConfig TLSAllowUnencryptedDoH bool // UpstreamTimeout is the timeout for querying upstream servers. @@ -351,13 +364,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 +614,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 +639,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 +660,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 }