feat: add configurable cipher suites for tls listening (#10505)

* feat: add configurable cipher suites for tls listening
* tls.VersionName is go 1.21, copy the function

cli/server.go

@@ -354,7 +354,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 				logger.Debug(ctx, "tracing closed", slog.Error(traceCloseErr))
 			}()
 
-			httpServers, err := ConfigureHTTPServers(inv, vals)
+			httpServers, err := ConfigureHTTPServers(logger, inv, vals)
 			if err != nil {
 				return xerrors.Errorf("configure http(s): %w", err)
 			}
@@ -1021,7 +1021,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 					r.Verbosef(inv, "Shutting down provisioner daemon %d...", id)
 					err := shutdownWithTimeout(provisionerDaemon.Shutdown, 5*time.Second)
 					if err != nil {
-						cliui.Errorf(inv.Stderr, "Failed to shutdown provisioner daemon %d: %s\n", id, err)
+						cliui.Errorf(inv.Stderr, "Failed to shut down provisioner daemon %d: %s\n", id, err)
 						return
 					}
 					err = provisionerDaemon.Close()
@@ -1411,7 +1411,12 @@ func generateSelfSignedCertificate() (*tls.Certificate, error) {
 	return &cert, nil
 }
 
-func configureTLS(tlsMinVersion, tlsClientAuth string, tlsCertFiles, tlsKeyFiles []string, tlsClientCAFile string) (*tls.Config, error) {
+// configureServerTLS returns the TLS config used for the Coderd server
+// connections to clients. A logger is passed in to allow printing warning
+// messages that do not block startup.
+//
+//nolint:revive
+func configureServerTLS(ctx context.Context, logger slog.Logger, tlsMinVersion, tlsClientAuth string, tlsCertFiles, tlsKeyFiles []string, tlsClientCAFile string, ciphers []string, allowInsecureCiphers bool) (*tls.Config, error) {
 	tlsConfig := &tls.Config{
 		MinVersion: tls.VersionTLS12,
 		NextProtos: []string{"h2", "http/1.1"},
@@ -1429,6 +1434,15 @@ func configureTLS(tlsMinVersion, tlsClientAuth string, tlsCertFiles, tlsKeyFiles
 		return nil, xerrors.Errorf("unrecognized tls version: %q", tlsMinVersion)
 	}
 
+	// A custom set of supported ciphers.
+	if len(ciphers) > 0 {
+		cipherIDs, err := configureCipherSuites(ctx, logger, ciphers, allowInsecureCiphers, tlsConfig.MinVersion, tls.VersionTLS13)
+		if err != nil {
+			return nil, err
+		}
+		tlsConfig.CipherSuites = cipherIDs
+	}
+
 	switch tlsClientAuth {
 	case "none":
 		tlsConfig.ClientAuth = tls.NoClientCert
@@ -1487,6 +1501,160 @@ func configureTLS(tlsMinVersion, tlsClientAuth string, tlsCertFiles, tlsKeyFiles
 	return tlsConfig, nil
 }
 
+//nolint:revive
+func configureCipherSuites(ctx context.Context, logger slog.Logger, ciphers []string, allowInsecureCiphers bool, minTLS, maxTLS uint16) ([]uint16, error) {
+	if minTLS > maxTLS {
+		return nil, xerrors.Errorf("minimum tls version (%s) cannot be greater than maximum tls version (%s)", versionName(minTLS), versionName(maxTLS))
+	}
+	if minTLS >= tls.VersionTLS13 {
+		// The cipher suites config option is ignored for tls 1.3 and higher.
+		// So this user flag is a no-op if the min version is 1.3.
+		return nil, xerrors.Errorf("'--tls-ciphers' cannot be specified when using minimum tls version 1.3 or higher, %d ciphers found as input.", len(ciphers))
+	}
+	// Configure the cipher suites which parses the strings and converts them
+	// to golang cipher suites.
+	supported, err := parseTLSCipherSuites(ciphers)
+	if err != nil {
+		return nil, xerrors.Errorf("tls ciphers: %w", err)
+	}
+
+	// allVersions is all tls versions the server supports.
+	// We enumerate these to ensure if ciphers are configured, at least
+	// 1 cipher for each version exists.
+	allVersions := make(map[uint16]bool)
+	for v := minTLS; v <= maxTLS; v++ {
+		allVersions[v] = false
+	}
+
+	var insecure []string
+	cipherIDs := make([]uint16, 0, len(supported))
+	for _, cipher := range supported {
+		if cipher.Insecure {
+			// Always show this warning, even if they have allowInsecureCiphers
+			// specified.
+			logger.Warn(ctx, "insecure tls cipher specified for server use", slog.F("cipher", cipher.Name))
+			insecure = append(insecure, cipher.Name)
+		}
+
+		// This is a warning message to tell the user if they are specifying
+		// a cipher that does not support the tls versions they have specified.
+		// This makes the cipher essentially a "noop" cipher.
+		if !hasSupportedVersion(minTLS, maxTLS, cipher.SupportedVersions) {
+			versions := make([]string, 0, len(cipher.SupportedVersions))
+			for _, sv := range cipher.SupportedVersions {
+				versions = append(versions, versionName(sv))
+			}
+			logger.Warn(ctx, "cipher not supported for tls versions enabled, cipher will not be used",
+				slog.F("cipher", cipher.Name),
+				slog.F("cipher_supported_versions", strings.Join(versions, ",")),
+				slog.F("server_min_version", versionName(minTLS)),
+				slog.F("server_max_version", versionName(maxTLS)),
+			)
+		}
+
+		for _, v := range cipher.SupportedVersions {
+			allVersions[v] = true
+		}
+
+		cipherIDs = append(cipherIDs, cipher.ID)
+	}
+
+	if len(insecure) > 0 && !allowInsecureCiphers {
+		return nil, xerrors.Errorf("insecure tls ciphers specified, must use '--tls-allow-insecure-ciphers' to allow these: %s", strings.Join(insecure, ", "))
+	}
+
+	// This is an additional sanity check. The user can specify ciphers that
+	// do not cover the full range of tls versions they have specified.
+	// They can unintentionally break TLS for some tls configured versions.
+	var missedVersions []string
+	for version, covered := range allVersions {
+		if version == tls.VersionTLS13 {
+			continue // v1.3 ignores configured cipher suites.
+		}
+		if !covered {
+			missedVersions = append(missedVersions, versionName(version))
+		}
+	}
+	if len(missedVersions) > 0 {
+		return nil, xerrors.Errorf("no tls ciphers supported for tls versions %q."+
+			"Add additional ciphers, set the minimum version to 'tls13, or remove the ciphers configured and rely on the default",
+			strings.Join(missedVersions, ","))
+	}
+
+	return cipherIDs, nil
+}
+
+// parseTLSCipherSuites will parse cipher suite names like 'TLS_RSA_WITH_AES_128_CBC_SHA'
+// to their tls cipher suite structs. If a cipher suite that is unsupported is
+// passed in, this function will return an error.
+// This function can return insecure cipher suites.
+func parseTLSCipherSuites(ciphers []string) ([]tls.CipherSuite, error) {
+	if len(ciphers) == 0 {
+		return nil, nil
+	}
+
+	var unsupported []string
+	var supported []tls.CipherSuite
+	// A custom set of supported ciphers.
+	allCiphers := append(tls.CipherSuites(), tls.InsecureCipherSuites()...)
+	for _, cipher := range ciphers {
+		// For each cipher specified by the client, find the cipher in the
+		// list of golang supported ciphers.
+		var found *tls.CipherSuite
+		for _, supported := range allCiphers {
+			if strings.EqualFold(supported.Name, cipher) {
+				found = supported
+				break
+			}
+		}
+
+		if found == nil {
+			unsupported = append(unsupported, cipher)
+			continue
+		}
+
+		supported = append(supported, *found)
+	}
+
+	if len(unsupported) > 0 {
+		return nil, xerrors.Errorf("unsupported tls ciphers specified, see https://github.com/golang/go/blob/master/src/crypto/tls/cipher_suites.go#L53-L75: %s", strings.Join(unsupported, ", "))
+	}
+
+	return supported, nil
+}
+
+// hasSupportedVersion is a helper function that returns true if the list
+// of supported versions contains a version between min and max.
+// If the versions list is outside the min/max, then it returns false.
+func hasSupportedVersion(min, max uint16, versions []uint16) bool {
+	for _, v := range versions {
+		if v >= min && v <= max {
+			// If one version is in between min/max, return true.
+			return true
+		}
+	}
+	return false
+}
+
+// versionName is tls.VersionName in go 1.21.
+// Until the switch, the function is copied locally.
+func versionName(version uint16) string {
+	switch version {
+	case tls.VersionSSL30:
+		return "SSLv3"
+	case tls.VersionTLS10:
+		return "TLS 1.0"
+	case tls.VersionTLS11:
+		return "TLS 1.1"
+	case tls.VersionTLS12:
+		return "TLS 1.2"
+	case tls.VersionTLS13:
+		return "TLS 1.3"
+	default:
+		return fmt.Sprintf("0x%04X", version)
+	}
+}
+
 func configureOIDCPKI(orig *oauth2.Config, keyFile string, certFile string) (*oauthpki.Config, error) {
 	// Read the files
 	keyData, err := os.ReadFile(keyFile)
@@ -2078,7 +2246,8 @@ func ConfigureTraceProvider(
 	return tracerProvider, sqlDriver, closeTracing
 }
 
-func ConfigureHTTPServers(inv *clibase.Invocation, cfg *codersdk.DeploymentValues) (_ *HTTPServers, err error) {
+func ConfigureHTTPServers(logger slog.Logger, inv *clibase.Invocation, cfg *codersdk.DeploymentValues) (_ *HTTPServers, err error) {
+	ctx := inv.Context()
 	httpServers := &HTTPServers{}
 	defer func() {
 		if err != nil {
@@ -2154,16 +2323,20 @@ func ConfigureHTTPServers(inv *clibase.Invocation, cfg *codersdk.DeploymentValue
 		// DEPRECATED: This redirect used to default to true.
 		// It made more sense to have the redirect be opt-in.
 		if inv.Environ.Get("CODER_TLS_REDIRECT_HTTP") == "true" || inv.ParsedFlags().Changed("tls-redirect-http-to-https") {
-			cliui.Warn(inv.Stderr, "--tls-redirect-http-to-https is deprecated, please use --redirect-to-access-url instead")
+			logger.Warn(ctx, "--tls-redirect-http-to-https is deprecated, please use --redirect-to-access-url instead")
 			cfg.RedirectToAccessURL = cfg.TLS.RedirectHTTP
 		}
 
-		tlsConfig, err := configureTLS(
+		tlsConfig, err := configureServerTLS(
+			ctx,
+			logger,
 			cfg.TLS.MinVersion.String(),
 			cfg.TLS.ClientAuth.String(),
 			cfg.TLS.CertFiles,
 			cfg.TLS.KeyFiles,
 			cfg.TLS.ClientCAFile.String(),
+			cfg.TLS.SupportedCiphers.Value(),
+			cfg.TLS.AllowInsecureCiphers.Value(),
 		)
 		if err != nil {
 			return nil, xerrors.Errorf("configure tls: %w", err)

cli/server_internal_test.go

@@ -0,0 +1,171 @@
+package cli
+
+import (
+	"bytes"
+	"context"
+	"crypto/tls"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"cdr.dev/slog"
+	"cdr.dev/slog/sloggers/sloghuman"
+)
+
+func Test_configureCipherSuites(t *testing.T) {
+	t.Parallel()
+
+	cipherNames := func(ciphers []*tls.CipherSuite) []string {
+		var names []string
+		for _, c := range ciphers {
+			names = append(names, c.Name)
+		}
+		return names
+	}
+
+	cipherIDs := func(ciphers []*tls.CipherSuite) []uint16 {
+		var ids []uint16
+		for _, c := range ciphers {
+			ids = append(ids, c.ID)
+		}
+		return ids
+	}
+
+	cipherByName := func(cipher string) *tls.CipherSuite {
+		for _, c := range append(tls.CipherSuites(), tls.InsecureCipherSuites()...) {
+			if cipher == c.Name {
+				c := c
+				return c
+			}
+		}
+		return nil
+	}
+
+	tests := []struct {
+		name          string
+		wantErr       string
+		wantWarnings  []string
+		inputCiphers  []string
+		minTLS        uint16
+		maxTLS        uint16
+		allowInsecure bool
+		expectCiphers []uint16
+	}{
+		{
+			name:          "AllSecure",
+			minTLS:        tls.VersionTLS10,
+			maxTLS:        tls.VersionTLS13,
+			inputCiphers:  cipherNames(tls.CipherSuites()),
+			wantWarnings:  []string{},
+			expectCiphers: cipherIDs(tls.CipherSuites()),
+		},
+		{
+			name:          "AllowInsecure",
+			minTLS:        tls.VersionTLS10,
+			maxTLS:        tls.VersionTLS13,
+			inputCiphers:  append(cipherNames(tls.CipherSuites()), tls.InsecureCipherSuites()[0].Name),
+			allowInsecure: true,
+			wantWarnings: []string{
+				"insecure tls cipher specified",
+			},
+			expectCiphers: append(cipherIDs(tls.CipherSuites()), tls.InsecureCipherSuites()[0].ID),
+		},
+		{
+			name:          "AllInsecure",
+			minTLS:        tls.VersionTLS10,
+			maxTLS:        tls.VersionTLS13,
+			inputCiphers:  append(cipherNames(tls.CipherSuites()), cipherNames(tls.InsecureCipherSuites())...),
+			allowInsecure: true,
+			wantWarnings: []string{
+				"insecure tls cipher specified",
+			},
+			expectCiphers: append(cipherIDs(tls.CipherSuites()), cipherIDs(tls.InsecureCipherSuites())...),
+		},
+		{
+			// Providing ciphers that are not compatible with any tls version
+			// enabled should generate a warning.
+			name:   "ExcessiveCiphers",
+			minTLS: tls.VersionTLS10,
+			maxTLS: tls.VersionTLS11,
+			inputCiphers: []string{
+				"TLS_RSA_WITH_AES_128_CBC_SHA",
+				// Only for TLS 1.3
+				"TLS_AES_128_GCM_SHA256",
+			},
+			allowInsecure: true,
+			wantWarnings: []string{
+				"cipher not supported for tls versions",
+			},
+			expectCiphers: cipherIDs([]*tls.CipherSuite{
+				cipherByName("TLS_RSA_WITH_AES_128_CBC_SHA"),
+				cipherByName("TLS_AES_128_GCM_SHA256"),
+			}),
+		},
+		// Errors
+		{
+			name:         "NotRealCiphers",
+			minTLS:       tls.VersionTLS10,
+			maxTLS:       tls.VersionTLS13,
+			inputCiphers: []string{"RSA-Fake"},
+			wantErr:      "unsupported tls ciphers",
+		},
+		{
+			name:    "NoCiphers",
+			minTLS:  tls.VersionTLS10,
+			maxTLS:  tls.VersionTLS13,
+			wantErr: "no tls ciphers supported",
+		},
+		{
+			name:         "InsecureNotAllowed",
+			minTLS:       tls.VersionTLS10,
+			maxTLS:       tls.VersionTLS13,
+			inputCiphers: append(cipherNames(tls.CipherSuites()), tls.InsecureCipherSuites()[0].Name),
+			wantErr:      "insecure tls ciphers specified",
+		},
+		{
+			name:         "TLS1.3",
+			minTLS:       tls.VersionTLS13,
+			maxTLS:       tls.VersionTLS13,
+			inputCiphers: cipherNames(tls.CipherSuites()),
+			wantErr:      "'--tls-ciphers' cannot be specified when using minimum tls version 1.3",
+		},
+		{
+			name:   "TLSUnsupported",
+			minTLS: tls.VersionTLS10,
+			maxTLS: tls.VersionTLS13,
+			// TLS_RSA_WITH_AES_128_GCM_SHA256 only supports tls 1.2
+			inputCiphers: []string{"TLS_RSA_WITH_AES_128_GCM_SHA256"},
+			wantErr:      "no tls ciphers supported for tls versions",
+		},
+		{
+			name:    "Min>Max",
+			minTLS:  tls.VersionTLS13,
+			maxTLS:  tls.VersionTLS12,
+			wantErr: "minimum tls version (TLS 1.3) cannot be greater than maximum tls version (TLS 1.2)",
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			ctx := context.Background()
+			var out bytes.Buffer
+			logger := slog.Make(sloghuman.Sink(&out))
+
+			found, err := configureCipherSuites(ctx, logger, tt.inputCiphers, tt.allowInsecure, tt.minTLS, tt.maxTLS)
+			if tt.wantErr != "" {
+				require.ErrorContains(t, err, tt.wantErr)
+			} else {
+				require.NoError(t, err, "no error")
+				require.ElementsMatch(t, tt.expectCiphers, found, "expected ciphers")
+				if len(tt.wantWarnings) > 0 {
+					logger.Sync()
+					for _, w := range tt.wantWarnings {
+						assert.Contains(t, out.String(), w, "expected warning")
+					}
+				}
+			}
+		})
+	}
+}

cli/testdata/coder_server_--help.golden

@@ -249,12 +249,21 @@ can safely ignore these settings.
       --tls-address host:port, $CODER_TLS_ADDRESS (default: 127.0.0.1:3443)
           HTTPS bind address of the server.
 
+      --tls-allow-insecure-ciphers bool, $CODER_TLS_ALLOW_INSECURE_CIPHERS (default: false)
+          By default, only ciphers marked as 'secure' are allowed to be used.
+          See
+          https://github.com/golang/go/blob/master/src/crypto/tls/cipher_suites.go#L82-L95.
+
       --tls-cert-file string-array, $CODER_TLS_CERT_FILE
           Path to each certificate for TLS. It requires a PEM-encoded file. To
           configure the listener to use a CA certificate, concatenate the
           primary certificate and the CA certificate together. The primary
           certificate should appear first in the combined file.
 
+      --tls-ciphers string-array, $CODER_TLS_CIPHERS
+          Specify specific TLS ciphers that allowed to be used. See
+          https://github.com/golang/go/blob/master/src/crypto/tls/cipher_suites.go#L53-L75.
+
       --tls-client-auth string, $CODER_TLS_CLIENT_AUTH (default: none)
           Policy the server will follow for TLS Client Authentication. Accepted
           values are "none", "request", "require-any", "verify-if-given", or

cli/testdata/server-config.yaml.golden

@@ -83,6 +83,14 @@ networking:
     # Path to key for client TLS authentication. It requires a PEM-encoded file.
     # (default: <unset>, type: string)
     clientKeyFile: ""
+    # Specify specific TLS ciphers that allowed to be used. See
+    # https://github.com/golang/go/blob/master/src/crypto/tls/cipher_suites.go#L53-L75.
+    # (default: <unset>, type: string-array)
+    tlsCiphers: []
+    # By default, only ciphers marked as 'secure' are allowed to be used. See
+    # https://github.com/golang/go/blob/master/src/crypto/tls/cipher_suites.go#L82-L95.
+    # (default: false, type: bool)
+    tlsAllowInsecureCiphers: false
     # Controls if the 'Strict-Transport-Security' header is set on all static file
     # responses. This header should only be set if the server is accessed via HTTPS.
     # This value is the MaxAge in seconds of the header.