From 5ae826d8a99c0906a09e595459fe8418926c63fc Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Fri, 14 Oct 2022 20:14:07 +0300 Subject: [PATCH] home: refactor override --- internal/dnsforward/config.go | 30 +++++++++++++----------------- internal/home/config.go | 27 +++++++++++++++++++++++++++ internal/home/home.go | 23 +++++------------------ internal/home/web.go | 7 ++----- 4 files changed, 47 insertions(+), 40 deletions(-) diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go index 9994f1cb..caad6547 100644 --- a/internal/dnsforward/config.go +++ b/internal/dnsforward/config.go @@ -12,7 +12,6 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" - "github.com/AdguardTeam/AdGuardHome/internal/aghtls" "github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/AdguardTeam/dnsproxy/proxy" "github.com/AdguardTeam/dnsproxy/upstream" @@ -166,10 +165,9 @@ type TLSConfig struct { // DNS names from certificate (SAN) or CN value from Subject dnsNames []string - // OverrideTLSCiphers holds the cipher names. If the slice is empty - // default set of ciphers are used for https listener, else this is - // considered. - OverrideTLSCiphers []string `yaml:"override_tls_ciphers" json:"-"` + // OverrideTLSCiphers, when set, contains the names of the cipher suites to + // use. If the slice is empty, the default safe suites are used. + OverrideTLSCiphers []string `yaml:"override_tls_ciphers,omitempty" json:"-"` } // DNSCryptConfig is the DNSCrypt server configuration struct. @@ -198,7 +196,9 @@ type ServerConfig struct { UpstreamTimeout time.Duration TLSv12Roots *x509.CertPool // list of root CAs for TLSv1.2 - TLSCiphers []uint16 // list of TLS ciphers to use + + // TLSCiphers are the IDs of TLS cipher suites to use. + TLSCiphers []uint16 // Called when the configuration is changed by HTTP request ConfigModified func() @@ -353,17 +353,13 @@ func UpstreamHTTPVersions(http3 bool) (v []upstream.HTTPVersion) { // prepareUpstreamSettings - prepares upstream DNS server settings func (s *Server) prepareUpstreamSettings() error { - // We're setting a customized set of RootCAs - // The reason is that Go default mechanism of loading TLS roots - // does not always work properly on some routers so we're - // loading roots manually and pass it here. - // See "util.LoadSystemRootCAs" + // We're setting a customized set of RootCAs. The reason is that Go default + // mechanism of loading TLS roots does not always work properly on some + // routers so we're loading roots manually and pass it here. + // + // See [aghtls.SystemRootCAs]. upstream.RootCAs = s.conf.TLSv12Roots - - // See util.InitTLSCiphers -- removed unsafe ciphers - if len(s.conf.TLSCiphers) > 0 { - upstream.CipherSuites = s.conf.TLSCiphers - } + upstream.CipherSuites = s.conf.TLSCiphers // Load upstreams either from the file, or from the settings var upstreams []string @@ -499,7 +495,7 @@ func (s *Server) prepareTLS(proxyConfig *proxy.Config) error { proxyConfig.TLSConfig = &tls.Config{ GetCertificate: s.onGetCertificate, - CipherSuites: aghtls.SaferCipherSuites(), + CipherSuites: s.conf.TLSCiphers, MinVersion: tls.VersionTLS12, } diff --git a/internal/home/config.go b/internal/home/config.go index 7df4f853..c7198d93 100644 --- a/internal/home/config.go +++ b/internal/home/config.go @@ -9,6 +9,7 @@ import ( "sync" "github.com/AdguardTeam/AdGuardHome/internal/aghalg" + "github.com/AdguardTeam/AdGuardHome/internal/aghtls" "github.com/AdguardTeam/AdGuardHome/internal/dhcpd" "github.com/AdguardTeam/AdGuardHome/internal/dnsforward" "github.com/AdguardTeam/AdGuardHome/internal/filtering" @@ -380,6 +381,7 @@ func parseConfig() (err error) { // we add support for HTTP/3 for web admin interface. addPorts(udpPorts, udpPort(config.TLS.PortDNSOverQUIC)) } + if err = tcpPorts.Validate(); err != nil { return fmt.Errorf("validating tcp ports: %w", err) } else if err = udpPorts.Validate(); err != nil { @@ -394,6 +396,11 @@ func parseConfig() (err error) { config.DNS.UpstreamTimeout = timeutil.Duration{Duration: dnsforward.DefaultTimeout} } + err = setContextTLSCipherIDs() + if err != nil { + return err + } + return nil } @@ -496,3 +503,23 @@ func (c *configuration) write() (err error) { return nil } + +// setContextTLSCipherIDs sets the TLS cipher suite IDs to use. +func setContextTLSCipherIDs() (err error) { + if len(config.TLS.OverrideTLSCiphers) == 0 { + log.Info("tls: using default ciphers") + + Context.tlsCipherIDs = aghtls.SaferCipherSuites() + + return nil + } + + log.Info("tls: overriding ciphers: %s", config.TLS.OverrideTLSCiphers) + + Context.tlsCipherIDs, err = aghtls.ParseCiphers(config.TLS.OverrideTLSCiphers) + if err != nil { + return fmt.Errorf("parsing override ciphers: %w", err) + } + + return nil +} diff --git a/internal/home/home.go b/internal/home/home.go index c8bd992e..5d3582e9 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -84,6 +84,10 @@ type homeContext struct { transport *http.Transport client *http.Client appSignalChannel chan os.Signal // Channel for receiving OS signals by the console app + + // tlsCipherIDs are the ID of the cipher suites that AdGuard Home must use. + tlsCipherIDs []uint16 + // runningAsService flag is set to true when options are passed from the service runner runningAsService bool } @@ -153,7 +157,7 @@ func setupContext(opts options) { Proxy: getHTTPProxy, TLSClientConfig: &tls.Config{ RootCAs: Context.tlsRoots, - CipherSuites: aghtls.SaferCipherSuites(), + CipherSuites: Context.tlsCipherIDs, MinVersion: tls.VersionTLS12, }, } @@ -386,11 +390,6 @@ func initWeb(opts options, clientBuildFS fs.FS) (web *Web, err error) { } } - tlsCiphers, err := getTLSCiphers() - if err != nil { - return nil, err - } - webConf := webConfig{ firstRun: Context.firstRun, BindHost: config.BindHost, @@ -405,7 +404,6 @@ func initWeb(opts options, clientBuildFS fs.FS) (web *Web, err error) { clientBetaFS: clientBetaFS, serveHTTP3: config.DNS.ServeHTTP3, - tlsCiphers: tlsCiphers, } web = newWeb(&webConf) @@ -916,14 +914,3 @@ type jsonError struct { // Message is the error message, an opaque string. Message string `json:"message"` } - -// getTLSCiphers check for overridden tls ciphers, if the slice is -// empty, then default safe ciphers are used -func getTLSCiphers() (cipherIds []uint16, err error) { - if len(config.TLS.OverrideTLSCiphers) == 0 { - return aghtls.SaferCipherSuites(), nil - } else { - log.Info("Overriding TLS Ciphers : %s", config.TLS.OverrideTLSCiphers) - return aghtls.ParseCiphers(config.TLS.OverrideTLSCiphers) - } -} diff --git a/internal/home/web.go b/internal/home/web.go index 86700357..7836355f 100644 --- a/internal/home/web.go +++ b/internal/home/web.go @@ -33,9 +33,6 @@ const ( ) type webConfig struct { - // Ciphers that are used for https listener - tlsCiphers []uint16 - clientFS fs.FS clientBetaFS fs.FS @@ -300,7 +297,7 @@ func (web *Web) tlsServerLoop() { TLSConfig: &tls.Config{ Certificates: []tls.Certificate{web.httpsServer.cert}, RootCAs: Context.tlsRoots, - CipherSuites: web.conf.tlsCiphers, + CipherSuites: Context.tlsCipherIDs, MinVersion: tls.VersionTLS12, }, Handler: withMiddlewares(Context.mux, limitRequestBody), @@ -334,7 +331,7 @@ func (web *Web) mustStartHTTP3(address string) { TLSConfig: &tls.Config{ Certificates: []tls.Certificate{web.httpsServer.cert}, RootCAs: Context.tlsRoots, - CipherSuites: web.conf.tlsCiphers, + CipherSuites: Context.tlsCipherIDs, MinVersion: tls.VersionTLS12, }, Handler: withMiddlewares(Context.mux, limitRequestBody),