diff options
author | Nick Thomas <nick@gitlab.com> | 2019-05-28 12:46:50 +0300 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2019-05-28 12:46:50 +0300 |
commit | 80fa0bb4e200a6b3b9194766dd209de28d1cf08a (patch) | |
tree | c559fced12a012af3f680512e3869b2e4454176c | |
parent | ef7fff4fa64c9cb3ca57faef3f26fa59f4f51ecb (diff) | |
parent | 1050f11598642b017486fc655561399d3766efb5 (diff) |
Merge branch '187-tls-version' into 'master'
Provide ability to disable old TLS versions
Closes #187
See merge request gitlab-org/gitlab-pages!146
-rw-r--r-- | README.md | 8 | ||||
-rw-r--r-- | acceptance_test.go | 88 | ||||
-rw-r--r-- | app.go | 2 | ||||
-rw-r--r-- | app_config.go | 2 | ||||
-rw-r--r-- | helpers_test.go | 10 | ||||
-rw-r--r-- | internal/tlsconfig/tlsconfig.go | 105 | ||||
-rw-r--r-- | internal/tlsconfig/tlsconfig_go1_12.go | 17 | ||||
-rw-r--r-- | internal/tlsconfig/tlsconfig_go1_12_test.go | 42 | ||||
-rw-r--r-- | internal/tlsconfig/tlsconfig_test.go | 71 | ||||
-rw-r--r-- | main.go | 15 | ||||
-rw-r--r-- | server.go | 25 |
11 files changed, 337 insertions, 48 deletions
@@ -226,6 +226,14 @@ to work. However, if it's running on a private network, this may allow websites on the public Internet to access its contents *via* your user's browsers - assuming they know the URL beforehand. +### SSL/TLS versions + +GitLab Pages defaults to TLS 1.2 as the minimum supported TLS version. This can be +configured by using the `-tls-min-version` and `-tls-max-version` options. Accepted +values are `ssl3`, `tls1.0`, `tls1.1`, `tls1.2`, and `tls1.3` (if supported). When `tls1.3` +is used GitLab Pages will add `tls13=1` to `GODEBUG` to enable TLS 1.3. +See https://golang.org/src/crypto/tls/tls.go for more. + ### Configuration The daemon can be configured with any combination of these methods: diff --git a/acceptance_test.go b/acceptance_test.go index f68b31ef..b22d5e97 100644 --- a/acceptance_test.go +++ b/acceptance_test.go @@ -1063,15 +1063,17 @@ func TestAcceptsSupportedCiphers(t *testing.T) { teardown := RunPagesProcess(t, *pagesBinary, listeners, "") defer teardown() - 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_ECDSA_WITH_AES_128_GCM_SHA256, - tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, - tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + tlsConfig := &tls.Config{ + CipherSuites: []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, + }, } - client, cleanup := ClientWithCiphers(ciphers) + client, cleanup := ClientWithConfig(tlsConfig) defer cleanup() rsp, err := client.Get(httpsListener.URL("/")) @@ -1088,11 +1090,13 @@ func TestRejectsUnsupportedCiphers(t *testing.T) { teardown := RunPagesProcess(t, *pagesBinary, listeners, "") defer teardown() - ciphers := []uint16{ - tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, - tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, + tlsConfig := &tls.Config{ + CipherSuites: []uint16{ + tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, + tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, + }, } - client, cleanup := ClientWithCiphers(ciphers) + client, cleanup := ClientWithConfig(tlsConfig) defer cleanup() rsp, err := client.Get(httpsListener.URL("/")) @@ -1110,11 +1114,13 @@ func TestEnableInsecureCiphers(t *testing.T) { teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-insecure-ciphers") defer teardown() - ciphers := []uint16{ - tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, - tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, + tlsConfig := &tls.Config{ + CipherSuites: []uint16{ + tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, + tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, + }, } - client, cleanup := ClientWithCiphers(ciphers) + client, cleanup := ClientWithConfig(tlsConfig) defer cleanup() rsp, err := client.Get(httpsListener.URL("/")) @@ -1125,3 +1131,53 @@ func TestEnableInsecureCiphers(t *testing.T) { require.NoError(t, err) } + +func TestTLSVersions(t *testing.T) { + skipUnlessEnabled(t) + + tests := map[string]struct { + tlsMin string + tlsMax string + tlsClient uint16 + expectError bool + }{ + "client version not supported": {tlsMin: "tls1.1", tlsMax: "tls1.2", tlsClient: tls.VersionTLS10, expectError: true}, + "client version supported": {tlsMin: "tls1.1", tlsMax: "tls1.2", tlsClient: tls.VersionTLS12, expectError: false}, + "client and server using default settings": {tlsMin: "", tlsMax: "", tlsClient: 0, expectError: false}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + args := []string{} + if tc.tlsMin != "" { + args = append(args, "-tls-min-version", tc.tlsMin) + } + if tc.tlsMax != "" { + args = append(args, "-tls-max-version", tc.tlsMax) + } + + teardown := RunPagesProcess(t, *pagesBinary, listeners, "", args...) + defer teardown() + + tlsConfig := &tls.Config{} + if tc.tlsClient != 0 { + tlsConfig.MinVersion = tc.tlsClient + tlsConfig.MaxVersion = tc.tlsClient + } + client, cleanup := ClientWithConfig(tlsConfig) + defer cleanup() + + rsp, err := client.Get(httpsListener.URL("/")) + + if rsp != nil { + rsp.Body.Close() + } + + if tc.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} @@ -258,7 +258,7 @@ func (a *theApp) Run() { wg.Add(1) go func(fd uintptr) { defer wg.Done() - err := listenAndServeTLS(fd, a.RootCertificate, a.RootKey, a.ServeHTTP, a.ServeTLS, a.HTTP2, a.InsecureCiphers, limiter) + err := listenAndServeTLS(fd, a.RootCertificate, a.RootKey, a.ServeHTTP, a.ServeTLS, a.InsecureCiphers, a.TLSMinVersion, a.TLSMaxVersion, a.HTTP2, limiter) if err != nil { fatal(err) } diff --git a/app_config.go b/app_config.go index b2b885f9..0f075136 100644 --- a/app_config.go +++ b/app_config.go @@ -18,6 +18,8 @@ type appConfig struct { ListenAdminUnix uintptr ListenAdminHTTPS uintptr InsecureCiphers bool + TLSMinVersion uint16 + TLSMaxVersion uint16 HTTP2 bool RedirectHTTP bool diff --git a/helpers_test.go b/helpers_test.go index 2e565300..61fa5279 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -329,13 +329,9 @@ func GetRedirectPageWithCookie(t *testing.T, spec ListenSpec, host, urlsuffix st return TestHTTPSClient.Transport.RoundTrip(req) } -func ClientWithCiphers(ciphers []uint16) (*http.Client, func()) { - tr := &http.Transport{ - TLSClientConfig: &tls.Config{ - RootCAs: TestCertPool, - CipherSuites: ciphers, - }, - } +func ClientWithConfig(tlsConfig *tls.Config) (*http.Client, func()) { + tlsConfig.RootCAs = TestCertPool + tr := &http.Transport{TLSClientConfig: tlsConfig} client := &http.Client{Transport: tr} return client, tr.CloseIdleConnections diff --git a/internal/tlsconfig/tlsconfig.go b/internal/tlsconfig/tlsconfig.go new file mode 100644 index 00000000..1c9db71e --- /dev/null +++ b/internal/tlsconfig/tlsconfig.go @@ -0,0 +1,105 @@ +package tlsconfig + +import ( + "crypto/tls" + "fmt" + "os" + "sort" + "strings" +) + +// GetCertificateFunc returns the certificate to be used for given domain +type GetCertificateFunc func(*tls.ClientHelloInfo) (*tls.Certificate, error) + +var ( + preferredCipherSuites = []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, + } + + // AllTLSVersions has all supported flag values + AllTLSVersions = map[string]uint16{ + "": 0, // Default value in tls.Config + "ssl3": tls.VersionSSL30, + "tls1.0": tls.VersionTLS10, + "tls1.1": tls.VersionTLS11, + "tls1.2": tls.VersionTLS12, + } +) + +// FlagUsage returns string with explanation how to use the CLI flag +func FlagUsage(minOrMax string) string { + versions := []string{} + + for version := range AllTLSVersions { + if version != "" { + versions = append(versions, fmt.Sprintf("%q", version)) + } + } + sort.Strings(versions) + + return fmt.Sprintf("Specifies the "+minOrMax+"imum SSL/TLS version, supported values are %s", strings.Join(versions, ", ")) +} + +// Create returns tls.Config for given app configuration +func Create(cert, key []byte, getCertificate GetCertificateFunc, insecureCiphers bool, tlsMinVersion uint16, tlsMaxVersion uint16) (*tls.Config, error) { + tlsConfig := &tls.Config{GetCertificate: getCertificate} + + err := configureCertificate(tlsConfig, cert, key) + if err != nil { + return nil, err + } + + if !insecureCiphers { + configureTLSCiphers(tlsConfig) + } + + tlsConfig.MinVersion = tlsMinVersion + tlsConfig.MaxVersion = tlsMaxVersion + + return tlsConfig, nil +} + +// ValidateTLSVersions returns error if the provided TLS versions config values are not valid +func ValidateTLSVersions(min, max string) error { + tlsMin, tlsMinOk := AllTLSVersions[min] + tlsMax, tlsMaxOk := AllTLSVersions[max] + + if !tlsMinOk { + return fmt.Errorf("Invalid minimum TLS version: %s", min) + } + if !tlsMaxOk { + return fmt.Errorf("Invalid maximum TLS version: %s", max) + } + if tlsMin > tlsMax && tlsMax > 0 { + return fmt.Errorf("Invalid maximum TLS version: %s; Should be at least %s", max, min) + } + + // At this point values are validated so if we have tls1.3 + // accepted we are on Go 1.12+ so let's enable it too. + if min == "tls1.3" || max == "tls1.3" { + os.Setenv("GODEBUG", os.Getenv("GODEBUG")+",tls13=1") + } + + return nil +} + +func configureCertificate(tlsConfig *tls.Config, cert, key []byte) error { + certificate, err := tls.X509KeyPair(cert, key) + if err != nil { + return err + } + + tlsConfig.Certificates = []tls.Certificate{certificate} + + return nil +} + +func configureTLSCiphers(tlsConfig *tls.Config) { + tlsConfig.PreferServerCipherSuites = true + tlsConfig.CipherSuites = preferredCipherSuites +} diff --git a/internal/tlsconfig/tlsconfig_go1_12.go b/internal/tlsconfig/tlsconfig_go1_12.go new file mode 100644 index 00000000..f92bdf09 --- /dev/null +++ b/internal/tlsconfig/tlsconfig_go1_12.go @@ -0,0 +1,17 @@ +// +build go1.12 + +package tlsconfig + +import ( + "crypto/tls" +) + +func init() { + AllTLSVersions["tls1.3"] = tls.VersionTLS13 + + preferredCipherSuites = append(preferredCipherSuites, + tls.TLS_AES_128_GCM_SHA256, + tls.TLS_AES_256_GCM_SHA384, + tls.TLS_CHACHA20_POLY1305_SHA256, + ) +} diff --git a/internal/tlsconfig/tlsconfig_go1_12_test.go b/internal/tlsconfig/tlsconfig_go1_12_test.go new file mode 100644 index 00000000..56b58c9d --- /dev/null +++ b/internal/tlsconfig/tlsconfig_go1_12_test.go @@ -0,0 +1,42 @@ +// +build go1.12 + +package tlsconfig + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestEnableTLS13(t *testing.T) { + tests := map[string]struct { + tlsMin string + tlsMax string + enableTLS13 bool + }{ + "ask for minimum TLS 1.3": {tlsMin: "tls1.3", tlsMax: "", enableTLS13: true}, + "ask for maximim TLS 1.3": {tlsMin: "", tlsMax: "tls1.3", enableTLS13: true}, + "do not ask for TLS 1.3": {tlsMin: "tls1.2", tlsMax: "tls1.2", enableTLS13: false}, + } + + // Store original GODEBUG value + godebug := os.Getenv("GODEBUG") + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + err := ValidateTLSVersions(tc.tlsMin, tc.tlsMax) + require.NoError(t, err) + + if tc.enableTLS13 { + assert.Regexp(t, "tls13=1", os.Getenv("GODEBUG")) + } else { + assert.NotRegexp(t, "tls13=1", godebug) + } + }) + + // Restore original GODEBUG value + os.Setenv("GODEBUG", godebug) + } +} diff --git a/internal/tlsconfig/tlsconfig_test.go b/internal/tlsconfig/tlsconfig_test.go new file mode 100644 index 00000000..b4cf87ad --- /dev/null +++ b/internal/tlsconfig/tlsconfig_test.go @@ -0,0 +1,71 @@ +package tlsconfig + +import ( + "crypto/tls" + "testing" + + "github.com/stretchr/testify/require" +) + +var cert = []byte(`-----BEGIN CERTIFICATE----- +MIIBhTCCASugAwIBAgIQIRi6zePL6mKjOipn+dNuaTAKBggqhkjOPQQDAjASMRAw +DgYDVQQKEwdBY21lIENvMB4XDTE3MTAyMDE5NDMwNloXDTE4MTAyMDE5NDMwNlow +EjEQMA4GA1UEChMHQWNtZSBDbzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABD0d +7VNhbWvZLWPuj/RtHFjvtJBEwOkhbN/BnnE8rnZR8+sbwnc/KhCk3FhnpHZnQz7B +5aETbbIgmuvewdjvSBSjYzBhMA4GA1UdDwEB/wQEAwICpDATBgNVHSUEDDAKBggr +BgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MCkGA1UdEQQiMCCCDmxvY2FsaG9zdDo1 +NDUzgg4xMjcuMC4wLjE6NTQ1MzAKBggqhkjOPQQDAgNIADBFAiEA2zpJEPQyz6/l +Wf86aX6PepsntZv2GYlA5UpabfT2EZICICpJ5h/iI+i341gBmLiAFQOyTDT+/wQc +6MF9+Yw1Yy0t +-----END CERTIFICATE-----`) + +var key = []byte(`-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIIrYSSNQFaA2Hwf1duRSxKtLYX5CB04fSeQ6tF1aY/PuoAoGCCqGSM49 +AwEHoUQDQgAEPR3tU2Fta9ktY+6P9G0cWO+0kETA6SFs38GecTyudlHz6xvCdz8q +EKTcWGekdmdDPsHloRNtsiCa697B2O9IFA== +-----END EC PRIVATE KEY-----`) + +var getCertificate = func(ch *tls.ClientHelloInfo) (*tls.Certificate, error) { + return nil, nil +} + +func TestValidateTLSVersions(t *testing.T) { + tests := map[string]struct { + tlsMin string + tlsMax string + err string + }{ + "invalid minimum TLS version": {tlsMin: "tls123", tlsMax: "", err: "Invalid minimum TLS version: tls123"}, + "invalid maximum TLS version": {tlsMin: "", tlsMax: "tls123", err: "Invalid maximum TLS version: tls123"}, + "TLS versions conflict": {tlsMin: "tls1.2", tlsMax: "tls1.1", err: "Invalid maximum TLS version: tls1.1; Should be at least tls1.2"}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + err := ValidateTLSVersions(tc.tlsMin, tc.tlsMax) + require.EqualError(t, err, tc.err) + }) + } +} + +func TestInvalidKeyPair(t *testing.T) { + _, err := Create([]byte(``), []byte(``), getCertificate, false, tls.VersionTLS11, tls.VersionTLS12) + require.EqualError(t, err, "tls: failed to find any PEM data in certificate input") +} + +func TestInsecureCihers(t *testing.T) { + tlsConfig, err := Create(cert, key, getCertificate, true, tls.VersionTLS11, tls.VersionTLS12) + require.NoError(t, err) + require.False(t, tlsConfig.PreferServerCipherSuites) + require.Empty(t, tlsConfig.CipherSuites) +} + +func TestCreate(t *testing.T) { + tlsConfig, err := Create(cert, key, getCertificate, false, tls.VersionTLS11, tls.VersionTLS12) + require.NoError(t, err) + require.IsType(t, getCertificate, tlsConfig.GetCertificate) + require.True(t, tlsConfig.PreferServerCipherSuites) + require.Equal(t, preferredCipherSuites, tlsConfig.CipherSuites) + require.Equal(t, tls.VersionTLS11, tlsConfig.MinVersion) + require.Equal(t, tls.VersionTLS12, tlsConfig.MaxVersion) +} @@ -10,6 +10,8 @@ import ( "github.com/namsral/flag" log "github.com/sirupsen/logrus" + + "gitlab.com/gitlab-org/gitlab-pages/internal/tlsconfig" ) // VERSION stores the information about the semantic version of application @@ -52,6 +54,8 @@ var ( redirectURI = flag.String("auth-redirect-uri", "", "GitLab application redirect URI") maxConns = flag.Uint("max-conns", 5000, "Limit on the number of concurrent connections to the HTTP, HTTPS or proxy listeners") insecureCiphers = flag.Bool("insecure-ciphers", false, "Use default list of cipher suites, may contain insecure ones like 3DES and RC4") + tlsMinVersion = flag.String("tls-min-version", "tls1.2", tlsconfig.FlagUsage("min")) + tlsMaxVersion = flag.String("tls-max-version", "", tlsconfig.FlagUsage("max")) disableCrossOriginRequests = flag.Bool("disable-cross-origin-requests", false, "Disable cross-origin requests") @@ -84,6 +88,9 @@ func configFromFlags() appConfig { config.LogVerbose = *logVerbose config.MaxConns = int(*maxConns) config.InsecureCiphers = *insecureCiphers + // tlsMinVersion and tlsMaxVersion are validated in appMain + config.TLSMinVersion = tlsconfig.AllTLSVersions[*tlsMinVersion] + config.TLSMaxVersion = tlsconfig.AllTLSVersions[*tlsMaxVersion] for _, file := range []struct { contents *[]byte @@ -164,6 +171,9 @@ func appMain() { flag.String(flag.DefaultConfigFlagname, "", "path to config file") flag.Parse() + if err := tlsconfig.ValidateTLSVersions(*tlsMinVersion, *tlsMaxVersion); err != nil { + fatal(err) + } printVersion(*showVersion, VERSION) @@ -175,8 +185,7 @@ func appMain() { }).Print("GitLab Pages Daemon") log.Printf("URL: https://gitlab.com/gitlab-org/gitlab-pages") - err := os.Chdir(*pagesRoot) - if err != nil { + if err := os.Chdir(*pagesRoot); err != nil { fatal(err) } @@ -209,6 +218,8 @@ func appMain() { "root-cert": *pagesRootKey, "root-key": *pagesRootCert, "status_path": config.StatusPath, + "tls-min-version": *tlsMinVersion, + "tls-max-version": *tlsMaxVersion, "use-http-2": config.HTTP2, "auth-secret": config.StoreSecret, "auth-server": config.GitLabServer, @@ -12,10 +12,9 @@ import ( "golang.org/x/net/http2" "gitlab.com/gitlab-org/gitlab-pages/internal/netutil" + "gitlab.com/gitlab-org/gitlab-pages/internal/tlsconfig" ) -type tlsHandlerFunc func(*tls.ClientHelloInfo) (*tls.Certificate, error) - type keepAliveListener struct { net.Listener } @@ -25,15 +24,6 @@ type keepAliveSetter interface { SetKeepAlivePeriod(time.Duration) error } -var preferredCipherSuites = []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, -} - func (ln *keepAliveListener) Accept() (net.Conn, error) { conn, err := ln.Listener.Accept() if err != nil { @@ -74,20 +64,11 @@ func listenAndServe(fd uintptr, handler http.HandlerFunc, useHTTP2 bool, tlsConf return server.Serve(&keepAliveListener{l}) } -func listenAndServeTLS(fd uintptr, cert, key []byte, handler http.HandlerFunc, tlsHandler tlsHandlerFunc, useHTTP2 bool, insecureCiphers bool, limiter *netutil.Limiter) error { - certificate, err := tls.X509KeyPair(cert, key) +func listenAndServeTLS(fd uintptr, cert, key []byte, handler http.HandlerFunc, getCertificate tlsconfig.GetCertificateFunc, insecureCiphers bool, tlsMinVersion uint16, tlsMaxVersion uint16, useHTTP2 bool, limiter *netutil.Limiter) error { + tlsConfig, err := tlsconfig.Create(cert, key, getCertificate, insecureCiphers, tlsMinVersion, tlsMaxVersion) if err != nil { return err } - tlsConfig := &tls.Config{} - tlsConfig.GetCertificate = tlsHandler - tlsConfig.Certificates = []tls.Certificate{ - certificate, - } - if !insecureCiphers { - tlsConfig.PreferServerCipherSuites = true - tlsConfig.CipherSuites = preferredCipherSuites - } return listenAndServe(fd, handler, useHTTP2, tlsConfig, limiter) } |