From dd202a1a5ffb745305467d94d9d22f1694e1551d Mon Sep 17 00:00:00 2001 From: Andreas Auernhammer Date: Sat, 13 Jan 2018 04:42:11 +0100 Subject: [PATCH] restrict TLS cipher suites of the server (#5245) This change restircts the supported cipher suites of the minio server. The server only supports AEAD ciphers (Chacha20Poly1305 and AES-GCM) The supported cipher suites are: - 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_ECDSA_WITH_AES_128_GCM_SHA256 - tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 - tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 Fixes #5244 and #5291 --- pkg/http/server.go | 35 ++++++++++++++++-- pkg/http/server_test.go | 82 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/pkg/http/server.go b/pkg/http/server.go index ef48bf097..6db3fbc01 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -25,6 +25,7 @@ import ( "time" humanize "github.com/dustin/go-humanize" + "github.com/minio/minio-go/pkg/set" ) const ( @@ -64,16 +65,19 @@ type Server struct { // Start - start HTTP server func (srv *Server) Start() (err error) { // Take a copy of server fields. - tlsConfig := srv.TLSConfig + var tlsConfig *tls.Config + if srv.TLSConfig != nil { + tlsConfig = srv.TLSConfig.Clone() + } readTimeout := srv.ReadTimeout writeTimeout := srv.WriteTimeout - handler := srv.Handler + handler := srv.Handler // if srv.Handler holds non-synced state -> possible data race - addrs := srv.Addrs + addrs := set.CreateStringSet(srv.Addrs...).ToSlice() // copy and remove duplicates tcpKeepAliveTimeout := srv.TCPKeepAliveTimeout updateBytesReadFunc := srv.UpdateBytesReadFunc updateBytesWrittenFunc := srv.UpdateBytesWrittenFunc - errorLogFunc := srv.ErrorLogFunc + errorLogFunc := srv.ErrorLogFunc // if srv.ErrorLogFunc holds non-synced state -> possible data race // Create new HTTP listener. var listener *httpListener @@ -118,9 +122,12 @@ func (srv *Server) Start() (err error) { // Shutdown - shuts down HTTP server. func (srv *Server) Shutdown() error { + srv.listenerMutex.Lock() if srv.listener == nil { + srv.listenerMutex.Unlock() return errors.New("server not initialized") } + srv.listenerMutex.Unlock() if atomic.AddUint32(&srv.inShutdown, 1) > 1 { // shutdown in progress @@ -149,12 +156,32 @@ func (srv *Server) Shutdown() error { } } +// Secure Go implementations of modern TLS ciphers +// The following ciphers are excluded because: +// - RC4 ciphers: RC4 is broken +// - 3DES ciphers: Because of the 64 bit blocksize of DES (Sweet32) +// - CBC-SHA256 ciphers: No countermeasures against Lucky13 timing attack +// - CBC-SHA ciphers: Legacy ciphers (SHA-1) and non-constant time +// implementation of CBC. +// (CBC-SHA ciphers can be enabled again if required) +// - RSA key exchange ciphers: Disabled because of dangerous PKCS1-v1.5 RSA +// padding scheme. See Bleichenbacher attacks. +var defaultCipherSuites = []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_ECDSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, +} + // NewServer - creates new HTTP server using given arguments. func NewServer(addrs []string, handler http.Handler, certificate *tls.Certificate) *Server { var tlsConfig *tls.Config if certificate != nil { tlsConfig = &tls.Config{ PreferServerCipherSuites: true, + CipherSuites: defaultCipherSuites, MinVersion: tls.VersionTLS12, NextProtos: []string{"http/1.1", "h2"}, } diff --git a/pkg/http/server_test.go b/pkg/http/server_test.go index 2b1e82ff5..ef18a88fd 100644 --- a/pkg/http/server_test.go +++ b/pkg/http/server_test.go @@ -22,6 +22,7 @@ import ( "net/http" "reflect" "testing" + "time" ) func TestNewServer(t *testing.T) { @@ -97,3 +98,84 @@ func TestNewServer(t *testing.T) { } } } + +func TestServerTLSCiphers(t *testing.T) { + var unsupportedCipherSuites = []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, // Go stack contains (some) countermeasures against timing attacks (Lucky13) + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, // No countermeasures against timing attacks + tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, // Go stack contains (some) countermeasures against timing attacks (Lucky13) + tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, // Broken cipher + tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, // Sweet32 + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, // Go stack contains (some) countermeasures against timing attacks (Lucky13) + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, // No countermeasures against timing attacks + tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, // Go stack contains (some) countermeasures against timing attacks (Lucky13) + tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA, // Broken cipher + + // all RSA-PKCS1-v1.5 ciphers are disabled - danger of Bleichenbacher attack variants + tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, // Sweet32 + tls.TLS_RSA_WITH_AES_128_CBC_SHA, // Go stack contains (some) countermeasures against timing attacks (Lucky13) + tls.TLS_RSA_WITH_AES_128_CBC_SHA256, // No countermeasures against timing attacks + tls.TLS_RSA_WITH_AES_256_CBC_SHA, // Go stack contains (some) countermeasures against timing attacks (Lucky13) + tls.TLS_RSA_WITH_RC4_128_SHA, // Broken cipher + + tls.TLS_RSA_WITH_AES_128_GCM_SHA256, // Disabled because of RSA-PKCS1-v1.5 - AES-GCM is considered secure. + tls.TLS_RSA_WITH_AES_256_GCM_SHA384, // Disabled because of RSA-PKCS1-v1.5 - AES-GCM is considered secure. + } + + certificate, err := getTLSCert() + if err != nil { + t.Fatalf("Unable to parse private/certificate data. %v\n", err) + } + + testCases := []struct { + ciphers []uint16 + resetServerCiphers bool + expectErr bool + }{ + {nil, false, false}, + {defaultCipherSuites, false, false}, + {unsupportedCipherSuites, false, true}, + {nil, true, false}, + } + + for i, testCase := range testCases { + func() { + addr := "127.0.0.1:" + getNextPort() + + server := NewServer([]string{addr}, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, "Hello, world") + }), + &certificate) + if testCase.resetServerCiphers { + // Use Go default ciphers. + server.TLSConfig.CipherSuites = nil + } + + go func() { + server.Start() + }() + defer server.Shutdown() + + client := http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + CipherSuites: testCase.ciphers, + }, + }, + } + + // There is no guaranteed way to know whether the HTTP server is started successfully. + // The only option is to connect and check. Hence below sleep is used as workaround. + time.Sleep(1 * time.Second) + + _, err := client.Get("https://" + addr) + expectErr := (err != nil) + + if expectErr != testCase.expectErr { + t.Fatalf("test %v: error: expected: %v, got: %v", i+1, testCase.expectErr, expectErr) + } + }() + } +}