diff --git a/CHANGELOG.md b/CHANGELOG.md index 02f1daed..a8d30018 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,7 +60,13 @@ In this release, the schema version has changed from 12 to 13. - Go 1.16 support. +### Security + +- Weaker cipher suites that use the CBC (cipher block chaining) mode of + operation have been disabled (#2993). + [#1730]: https://github.com/AdguardTeam/AdGuardHome/issues/1730 +[#2993]: https://github.com/AdguardTeam/AdGuardHome/issues/2993 [#3057]: https://github.com/AdguardTeam/AdGuardHome/issues/3057 [#3367]: https://github.com/AdguardTeam/AdGuardHome/issues/3367 diff --git a/internal/aghtls/aghtls.go b/internal/aghtls/aghtls.go new file mode 100644 index 00000000..89de2524 --- /dev/null +++ b/internal/aghtls/aghtls.go @@ -0,0 +1,31 @@ +// Package aghtls contains utilities for work with TLS. +package aghtls + +import "crypto/tls" + +// SaferCipherSuites returns a set of default cipher suites with vulnerable and +// weak cipher suites removed. +func SaferCipherSuites() (safe []uint16) { + suites := tls.CipherSuites() + for _, s := range suites { + switch s.ID { + case + tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, + tls.TLS_RSA_WITH_AES_128_CBC_SHA, + tls.TLS_RSA_WITH_AES_256_CBC_SHA, + tls.TLS_RSA_WITH_AES_128_CBC_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256: + // Less safe 3DES and CBC suites, go on. + default: + safe = append(safe, s.ID) + } + } + + return safe +} diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go index 1678f18a..cb93e278 100644 --- a/internal/dnsforward/config.go +++ b/internal/dnsforward/config.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghtls" "github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/AdguardTeam/dnsproxy/proxy" "github.com/AdguardTeam/dnsproxy/upstream" @@ -426,6 +427,7 @@ func (s *Server) prepareTLS(proxyConfig *proxy.Config) error { proxyConfig.TLSConfig = &tls.Config{ GetCertificate: s.onGetCertificate, + CipherSuites: aghtls.SaferCipherSuites(), MinVersion: tls.VersionTLS12, } diff --git a/internal/home/dns.go b/internal/home/dns.go index 3ac7a9ab..4278a321 100644 --- a/internal/home/dns.go +++ b/internal/home/dns.go @@ -211,7 +211,6 @@ func generateServerConfig() (newConf dnsforward.ServerConfig, err error) { } newConf.TLSv12Roots = Context.tlsRoots - newConf.TLSCiphers = Context.tlsCiphers newConf.TLSAllowUnencryptedDoH = tlsConf.AllowUnencryptedDoH newConf.FilterHandler = applyAdditionalFiltering diff --git a/internal/home/home.go b/internal/home/home.go index 90674d48..021751e8 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -22,6 +22,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghalg" "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/aghos" + "github.com/AdguardTeam/AdGuardHome/internal/aghtls" "github.com/AdguardTeam/AdGuardHome/internal/dhcpd" "github.com/AdguardTeam/AdGuardHome/internal/dnsforward" "github.com/AdguardTeam/AdGuardHome/internal/filtering" @@ -80,7 +81,6 @@ type homeContext struct { disableUpdate bool // If set, don't check for updates controlLock sync.Mutex tlsRoots *x509.CertPool // list of root CAs for TLSv1.2 - tlsCiphers []uint16 // list of TLS ciphers to use transport *http.Transport client *http.Client appSignalChannel chan os.Signal // Channel for receiving OS signals by the console app @@ -145,13 +145,13 @@ func setupContext(args options) { initConfig() Context.tlsRoots = LoadSystemRootCAs() - Context.tlsCiphers = InitTLSCiphers() Context.transport = &http.Transport{ DialContext: customDialContext, Proxy: getHTTPProxy, TLSClientConfig: &tls.Config{ - RootCAs: Context.tlsRoots, - MinVersion: tls.VersionTLS12, + RootCAs: Context.tlsRoots, + CipherSuites: aghtls.SaferCipherSuites(), + MinVersion: tls.VersionTLS12, }, } Context.client = &http.Client{ @@ -182,7 +182,7 @@ func setupContext(args options) { // logIfUnsupported logs a formatted warning if the error is one of the // unsupported errors and returns nil. If err is nil, logIfUnsupported returns -// nil. Otherise, it returns err. +// nil. Otherwise, it returns err. func logIfUnsupported(msg string, err error) (outErr error) { if errors.As(err, new(*aghos.UnsupportedError)) { log.Debug(msg, err) diff --git a/internal/home/tls.go b/internal/home/tls.go index a201be14..eeffe3b6 100644 --- a/internal/home/tls.go +++ b/internal/home/tls.go @@ -26,7 +26,6 @@ import ( "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/google/go-cmp/cmp" - "golang.org/x/sys/cpu" ) var tlsWebHandlersRegistered = false @@ -731,52 +730,3 @@ func LoadSystemRootCAs() (roots *x509.CertPool) { return nil } - -// InitTLSCiphers performs the same work as initDefaultCipherSuites() from -// crypto/tls/common.go but don't uses lots of other default ciphers. -func InitTLSCiphers() (ciphers []uint16) { - // Check the cpu flags for each platform that has optimized GCM - // implementations. The worst case is when all these variables are - // false. - var ( - hasGCMAsmAMD64 = cpu.X86.HasAES && cpu.X86.HasPCLMULQDQ - hasGCMAsmARM64 = cpu.ARM64.HasAES && cpu.ARM64.HasPMULL - // Keep in sync with crypto/aes/cipher_s390x.go. - hasGCMAsmS390X = cpu.S390X.HasAES && - cpu.S390X.HasAESCBC && - cpu.S390X.HasAESCTR && - (cpu.S390X.HasGHASH || cpu.S390X.HasAESGCM) - - hasGCMAsm = hasGCMAsmAMD64 || hasGCMAsmARM64 || hasGCMAsmS390X - ) - - if hasGCMAsm { - // If AES-GCM hardware is provided then prioritize AES-GCM - // cipher suites. - ciphers = []uint16{ - tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, - tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, - tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, - } - } else { - // Without AES-GCM hardware, we put the ChaCha20-Poly1305 cipher - // suites first. - ciphers = []uint16{ - tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, - tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, - tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, - tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - } - } - - return append( - ciphers, - tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, - tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, - ) -} diff --git a/internal/home/web.go b/internal/home/web.go index 54fb1324..99b993bb 100644 --- a/internal/home/web.go +++ b/internal/home/web.go @@ -10,6 +10,7 @@ import ( "time" "github.com/AdguardTeam/AdGuardHome/internal/aghnet" + "github.com/AdguardTeam/AdGuardHome/internal/aghtls" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" @@ -34,14 +35,13 @@ const ( ) type webConfig struct { + clientFS fs.FS + clientBetaFS fs.FS + BindHost net.IP BindPort int BetaBindPort int PortHTTPS int - firstRun bool - - clientFS fs.FS - clientBetaFS fs.FS // ReadTimeout is an option to pass to http.Server for setting an // appropriate field. @@ -54,6 +54,8 @@ type webConfig struct { // WriteTimeout is an option to pass to http.Server for setting an // appropriate field. WriteTimeout time.Duration + + firstRun bool } // HTTPSServer - HTTPS Server @@ -263,9 +265,9 @@ func (web *Web) tlsServerLoop() { Addr: address, TLSConfig: &tls.Config{ Certificates: []tls.Certificate{web.httpsServer.cert}, - MinVersion: tls.VersionTLS12, RootCAs: Context.tlsRoots, - CipherSuites: Context.tlsCiphers, + CipherSuites: aghtls.SaferCipherSuites(), + MinVersion: tls.VersionTLS12, }, Handler: withMiddlewares(Context.mux, limitRequestBody), ReadTimeout: web.conf.ReadTimeout,