diff --git a/config/setup/tls.go b/config/setup/tls.go index 7efc6825..20963e76 100644 --- a/config/setup/tls.go +++ b/config/setup/tls.go @@ -6,7 +6,6 @@ import ( "strconv" "strings" - "github.com/mholt/caddy/app" "github.com/mholt/caddy/middleware" ) @@ -40,7 +39,6 @@ func TLS(c *Controller) (middleware.Middleware, error) { value, ok := supportedProtocols[strings.ToLower(args[0])] if !ok { return nil, c.Errf("Wrong protocol name or protocol not supported '%s'", c.Val()) - } c.TLS.ProtocolMinVersion = value value, ok = supportedProtocols[strings.ToLower(args[1])] @@ -50,13 +48,10 @@ func TLS(c *Controller) (middleware.Middleware, error) { c.TLS.ProtocolMaxVersion = value case "ciphers": for c.NextArg() { - value, ok := supportedCiphers[strings.ToUpper(c.Val())] + value, ok := supportedCiphersMap[strings.ToUpper(c.Val())] if !ok { return nil, c.Errf("Wrong cipher name or cipher not supported '%s'", c.Val()) } - if _, ok := http2CipherSuites[value]; app.Http2 && !ok { - return nil, c.Errf("Cipher suite %s is not allowed for HTTP/2", c.Val()) - } c.TLS.Ciphers = append(c.TLS.Ciphers, value) } case "cache": @@ -65,7 +60,7 @@ func TLS(c *Controller) (middleware.Middleware, error) { } size, err := strconv.Atoi(c.Val()) if err != nil { - return nil, c.Errf("Cache parameter must be an number '%s': %v", c.Val(), err) + return nil, c.Errf("Cache parameter must be a number '%s': %v", c.Val(), err) } c.TLS.CacheSize = size default: @@ -76,19 +71,16 @@ func TLS(c *Controller) (middleware.Middleware, error) { // If no ciphers provided, use all that Caddy supports for the protocol if len(c.TLS.Ciphers) == 0 { - for _, v := range supportedCiphers { - if _, ok := http2CipherSuites[v]; !app.Http2 || ok { - c.TLS.Ciphers = append(c.TLS.Ciphers, v) - } - } + c.TLS.Ciphers = supportedCiphers } - // If no ProtocolMin provided, set default MinVersion to TLSv1.1 for security reasons + // Not a cipher suite, but still important for mitigating protocol downgrade attacks + c.TLS.Ciphers = append(c.TLS.Ciphers, tls.TLS_FALLBACK_SCSV) + + // Set default protocol min and max versions - must balance compatibility and security if c.TLS.ProtocolMinVersion == 0 { - c.TLS.ProtocolMinVersion = tls.VersionTLS11 + c.TLS.ProtocolMinVersion = tls.VersionTLS10 } - - //If no ProtocolMax provided, use crypto/tls default MaxVersion(tls1.2) if c.TLS.ProtocolMaxVersion == 0 { c.TLS.ProtocolMaxVersion = tls.VersionTLS12 } @@ -98,6 +90,9 @@ func TLS(c *Controller) (middleware.Middleware, error) { c.TLS.CacheSize = 64 } + // Prefer server cipher suites + c.TLS.PreferServerCipherSuites = true + return nil, nil } @@ -111,11 +106,17 @@ var supportedProtocols = map[string]uint16{ "tls1.2": tls.VersionTLS12, } -// Map of supported ciphers. +// Map of supported ciphers, used only for parsing config. // // Note that, at time of writing, HTTP/2 blacklists 276 cipher suites, // including all but two of the suites below (the two GCM suites). -var supportedCiphers = map[string]uint16{ +// See https://http2.github.io/http2-spec/#BadCipherSuites +// +// TLS_FALLBACK_SCSV is not in this list because we manually ensure +// it is always added (even though it is not technically a cipher suite). +// +// This map, like any map, is NOT ORDERED. Do not range over this map. +var supportedCiphersMap = map[string]uint16{ "ECDHE-RSA-AES128-GCM-SHA256": tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, "ECDHE-ECDSA-AES128-GCM-SHA256": tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, "ECDHE-RSA-AES128-CBC-SHA": tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, @@ -128,9 +129,21 @@ var supportedCiphers = map[string]uint16{ "RSA-3DES-EDE-CBC-SHA": tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, } -// Set of cipher suites not blacklisted by HTTP/2 spec. -// See https://http2.github.io/http2-spec/#BadCipherSuites -var http2CipherSuites = map[uint16]struct{}{ - tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256: struct{}{}, - tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256: struct{}{}, +// List of supported cipher suites in descending order of preference. +// Ordering is very important! Getting the wrong order will break +// mainstream clients, especially with HTTP/2. +// +// Note that TLS_FALLBACK_SCSV is not in this list since it is always +// added manually. +var supportedCiphers = []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + tls.TLS_RSA_WITH_AES_256_CBC_SHA, + tls.TLS_RSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, + tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, } diff --git a/config/setup/tls_test.go b/config/setup/tls_test.go index 0daf7f60..3f841ab8 100644 --- a/config/setup/tls_test.go +++ b/config/setup/tls_test.go @@ -3,8 +3,6 @@ package setup import ( "crypto/tls" "testing" - - "github.com/mholt/caddy/app" ) func TestTLSParseBasic(t *testing.T) { @@ -12,9 +10,10 @@ func TestTLSParseBasic(t *testing.T) { _, err := TLS(c) if err != nil { - t.Error("Expected no errors, but had an error") + t.Errorf("Expected no errors, got: %v", err) } + // Basic checks if c.TLS.Certificate != "cert.pem" { t.Errorf("Expected certificate arg to be 'cert.pem', was '%s'", c.TLS.Certificate) } @@ -24,31 +23,49 @@ func TestTLSParseBasic(t *testing.T) { if !c.TLS.Enabled { t.Error("Expected TLS Enabled=true, but was false") } -} -func TestTLSParseNoOptional(t *testing.T) { - c := newTestController(`tls cert.crt cert.key`) - - _, err := TLS(c) - if err != nil { - t.Errorf("Expected no errors, got: %v", err) + // Security defaults + if c.TLS.ProtocolMinVersion != tls.VersionTLS10 { + t.Errorf("Expected 'tls1.0 (0x0301)' as ProtocolMinVersion, got %#v", c.TLS.ProtocolMinVersion) } - - if len(c.TLS.Ciphers) != len(supportedCiphers) { - t.Errorf("Expected %v Ciphers, got %v", len(supportedCiphers), len(c.TLS.Ciphers)) - } - - if c.TLS.ProtocolMinVersion != tls.VersionTLS11 { - t.Errorf("Expected 'tls1.1 (0x0302)' as ProtocolMinVersion, got %#v", c.TLS.ProtocolMinVersion) - } - if c.TLS.ProtocolMaxVersion != tls.VersionTLS12 { t.Errorf("Expected 'tls1.2 (0x0303)' as ProtocolMaxVersion, got %v", c.TLS.ProtocolMaxVersion) } - if c.TLS.CacheSize != 64 { t.Errorf("Expected CacheSize 64, got %v", c.TLS.CacheSize) } + + // Cipher checks + expectedCiphers := []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + tls.TLS_RSA_WITH_AES_256_CBC_SHA, + tls.TLS_RSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, + tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, + tls.TLS_FALLBACK_SCSV, + } + + // Ensure count is correct (plus one for TLS_FALLBACK_SCSV) + if len(c.TLS.Ciphers) != len(supportedCiphers)+1 { + t.Errorf("Expected %v Ciphers (including TLS_FALLBACK_SCSV), got %v", + len(supportedCiphers)+1, len(c.TLS.Ciphers)) + } + + // Ensure ordering is correct + for i, actual := range c.TLS.Ciphers { + if actual != expectedCiphers[i] { + t.Errorf("Expected cipher in position %d to be %0x, got %0x", i, expectedCiphers[i], actual) + } + } + + if !c.TLS.PreferServerCipherSuites { + t.Error("Expected PreferServerCipherSuites = true, but was false") + } } func TestTLSParseIncompleteParams(t *testing.T) { @@ -88,8 +105,8 @@ func TestTLSParseWithOptionalParams(t *testing.T) { t.Errorf("Expected 'tls1.2 (0x0302)' as ProtocolMaxVersion, got %#v", c.TLS.ProtocolMaxVersion) } - if len(c.TLS.Ciphers) != 3 { - t.Errorf("Expected 3 Ciphers, got %v", len(c.TLS.Ciphers)) + if len(c.TLS.Ciphers)-1 != 3 { + t.Errorf("Expected 3 Ciphers (not including TLS_FALLBACK_SCSV), got %v", len(c.TLS.Ciphers)) } if c.TLS.CacheSize != 128 { @@ -127,43 +144,3 @@ func TestTLSParseWithWrongOptionalParams(t *testing.T) { t.Errorf("Expected errors, but no error returned") } } - -func TestTLSParseWithHTTP2Requirements(t *testing.T) { - params := `tls cert.crt cert.key` - c := newTestController(params) - - // With HTTP2, cipher suites should be limited - app.Http2 = true - _, err := TLS(c) - if err != nil { - t.Errorf("Expected no errors, got: %v", err) - } - if len(c.TLS.Ciphers) != len(http2CipherSuites) { - t.Errorf("With HTTP/2 on, expected %d supported ciphers, got %d", - len(http2CipherSuites), len(c.TLS.Ciphers)) - } - - params = `tls cert.crt cert.key { - ciphers RSA-AES128-CBC-SHA - }` - c = newTestController(params) - // Should not be able to specify a blacklisted cipher suite with HTTP2 on - _, err = TLS(c) - if err == nil { - t.Error("Expected an error because cipher suite is invalid for HTTP/2") - } - - params = `tls cert.crt cert.key` - c = newTestController(params) - - // Without HTTP2, cipher suites should not be as restricted - app.Http2 = false - _, err = TLS(c) - if err != nil { - t.Errorf("Expected no errors, got: %v", err) - } - if len(c.TLS.Ciphers) != len(supportedCiphers) { - t.Errorf("With HTTP/2 off, expected %d supported ciphers, got %d", - len(supportedCiphers), len(c.TLS.Ciphers)) - } -} diff --git a/server/config.go b/server/config.go index 502ba423..6e1711fc 100644 --- a/server/config.go +++ b/server/config.go @@ -55,13 +55,14 @@ func (c Config) Address() string { // TLSConfig describes how TLS should be configured and used, // if at all. A certificate and key are both required. -// Ciphers, Protocols and CacheSize are optional +// The rest is optional. type TLSConfig struct { - Enabled bool - Certificate string - Key string - Ciphers []uint16 - ProtocolMinVersion uint16 - ProtocolMaxVersion uint16 - CacheSize int + Enabled bool + Certificate string + Key string + Ciphers []uint16 + ProtocolMinVersion uint16 + ProtocolMaxVersion uint16 + CacheSize int + PreferServerCipherSuites bool } diff --git a/server/server.go b/server/server.go index 8290e9f5..ecc2bc6b 100644 --- a/server/server.go +++ b/server/server.go @@ -137,7 +137,7 @@ func ListenAndServeTLSWithSNI(srv *http.Server, tlsConfigs []TLSConfig) error { config.MinVersion = tlsConfigs[0].ProtocolMinVersion config.MaxVersion = tlsConfigs[0].ProtocolMaxVersion config.CipherSuites = tlsConfigs[0].Ciphers - config.PreferServerCipherSuites = true + config.PreferServerCipherSuites = tlsConfigs[0].PreferServerCipherSuites conn, err := net.Listen("tcp", addr) if err != nil {