Skip to content

Commit de67dbc

Browse files
EmyrkEmyrk
committed
feat: add configurable cipher suites for tls listening
1 parent e756a95 commit de67dbc

File tree

4 files changed

+266
-16
lines changed

4 files changed

+266
-16
lines changed

‎cli/server.go

+151-5
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
354354
logger.Debug(ctx, "tracing closed", slog.Error(traceCloseErr))
355355
}()
356356

357-
httpServers, err := ConfigureHTTPServers(inv, vals)
357+
httpServers, err := ConfigureHTTPServers(logger, inv, vals)
358358
if err != nil {
359359
return xerrors.Errorf("configure http(s): %w", err)
360360
}
@@ -1411,7 +1411,10 @@ func generateSelfSignedCertificate() (*tls.Certificate, error) {
14111411
return &cert, nil
14121412
}
14131413

1414-
func configureTLS(tlsMinVersion, tlsClientAuth string, tlsCertFiles, tlsKeyFiles []string, tlsClientCAFile string) (*tls.Config, error) {
1414+
// configureServerTLS returns the TLS config used for the Coderd server
1415+
// connections to clients. A logger is passed in to allow printing warning
1416+
// messages that do not block startup.
1417+
func configureServerTLS(ctx context.Context, logger slog.Logger, tlsMinVersion, tlsClientAuth string, tlsCertFiles, tlsKeyFiles []string, tlsClientCAFile string, ciphers []string, allowInsecureCiphers bool) (*tls.Config, error) {
14151418
tlsConfig := &tls.Config{
14161419
MinVersion: tls.VersionTLS12,
14171420
NextProtos: []string{"h2", "http/1.1"},
@@ -1429,6 +1432,15 @@ func configureTLS(tlsMinVersion, tlsClientAuth string, tlsCertFiles, tlsKeyFiles
14291432
return nil, xerrors.Errorf("unrecognized tls version: %q", tlsMinVersion)
14301433
}
14311434

1435+
// A custom set of supported ciphers.
1436+
if len(ciphers) > 0 {
1437+
cipherIDs, err := configureCipherSuites(ctx, logger, ciphers, allowInsecureCiphers, tlsConfig.MinVersion, tls.VersionTLS13)
1438+
if err != nil {
1439+
return nil, err
1440+
}
1441+
tlsConfig.CipherSuites = cipherIDs
1442+
}
1443+
14321444
switch tlsClientAuth {
14331445
case "none":
14341446
tlsConfig.ClientAuth = tls.NoClientCert
@@ -1487,6 +1499,135 @@ func configureTLS(tlsMinVersion, tlsClientAuth string, tlsCertFiles, tlsKeyFiles
14871499
return tlsConfig, nil
14881500
}
14891501

1502+
func configureCipherSuites(ctx context.Context, logger slog.Logger, ciphers []string, allowInsecureCiphers bool, minTLS, maxTLS uint16) ([]uint16, error) {
1503+
if minTLS >= tls.VersionTLS13 {
1504+
// The cipher suites config option is ignored for tls 1.3 and higher.
1505+
// So this user flag is a no-op if the min version is 1.3.
1506+
return nil, xerrors.Errorf("tls ciphers cannot be specified when using minimum tls version 1.3 or higher")
1507+
}
1508+
// Configure the cipher suites which parses the strings and converts them
1509+
// to golang cipher suites.
1510+
supported, err := parseTLSCipherSuites(ciphers)
1511+
if err != nil {
1512+
return nil, xerrors.Errorf("tls ciphers: %w", err)
1513+
}
1514+
1515+
// allVersions is all tls versions the server supports.
1516+
allVersions := make(map[uint16]bool)
1517+
for v := minTLS; v <= maxTLS; v++ {
1518+
allVersions[v] = false
1519+
}
1520+
1521+
var insecure []string
1522+
cipherIDs := make([]uint16, 0, len(supported))
1523+
for _, cipher := range supported {
1524+
if cipher.Insecure {
1525+
// Always show this warning, even if they have allowInsecureCiphers
1526+
// specified.
1527+
logger.Warn(ctx, "insecure tls cipher specified for server use", slog.F("cipher", cipher.Name))
1528+
insecure = append(insecure, cipher.Name)
1529+
}
1530+
1531+
// This is a warning message to tell the user if they are specifying
1532+
// a cipher that does not support the tls versions they have specified.
1533+
// This makes the cipher essentially a "noop" cipher.
1534+
if !hasSupportedVersion(minTLS, maxTLS, cipher.SupportedVersions) {
1535+
versions := make([]string, 0, len(cipher.SupportedVersions))
1536+
for _, sv := range cipher.SupportedVersions {
1537+
versions = append(versions, tls.VersionName(sv))
1538+
}
1539+
logger.Warn(ctx, "cipher not supported for tls versions allowed, cipher will not be used",
1540+
slog.F("cipher", cipher.Name),
1541+
slog.F("cipher_supported_versions", strings.Join(versions, ",")),
1542+
slog.F("server_min_version", tls.VersionName(minTLS)),
1543+
slog.F("server_max_version", tls.VersionName(maxTLS)),
1544+
)
1545+
}
1546+
1547+
for _, v := range cipher.SupportedVersions {
1548+
allVersions[v] = true
1549+
}
1550+
1551+
cipherIDs = append(cipherIDs, cipher.ID)
1552+
}
1553+
1554+
if len(insecure) > 0 && !allowInsecureCiphers {
1555+
return nil, xerrors.Errorf("insecure tls ciphers specified, must use '--tls-allow-insecure-ciphers' to allow these: %s", strings.Join(insecure, ", "))
1556+
}
1557+
1558+
// This is an additional sanity check. The user can specify ciphers that
1559+
// do not cover the full range of tls versions they have specified.
1560+
// They can unintentionally break TLS for some tls configured versions.
1561+
var missedVersions []string
1562+
for version, covered := range allVersions {
1563+
if version == tls.VersionTLS13 {
1564+
continue // v1.3 ignores configured cipher suites.
1565+
}
1566+
if !covered {
1567+
missedVersions = append(missedVersions, tls.VersionName(version))
1568+
}
1569+
}
1570+
if len(missedVersions) > 0 {
1571+
return nil, xerrors.Errorf("no tls ciphers supported for tls versions %q."+
1572+
"Add additional ciphers, specify the minimum version to 'tls13, or remove the ciphers configured and rely on the default",
1573+
strings.Join(missedVersions, ","))
1574+
}
1575+
1576+
return cipherIDs, nil
1577+
}
1578+
1579+
// parseTLSCipherSuites will parse cipher suite names like 'TLS_RSA_WITH_AES_128_CBC_SHA'
1580+
// to their tls cipher suite structs. If a cipher suite that is unsupported is
1581+
// passed in, this function will return an error.
1582+
// This function can return insecure cipher suites.
1583+
func parseTLSCipherSuites(ciphers []string) ([]tls.CipherSuite, error) {
1584+
if len(ciphers) == 0 {
1585+
return nil, nil
1586+
}
1587+
1588+
var unsupported []string
1589+
var supported []tls.CipherSuite
1590+
// A custom set of supported ciphers.
1591+
allCiphers := append(tls.CipherSuites(), tls.InsecureCipherSuites()...)
1592+
for _, cipher := range ciphers {
1593+
// For each cipher specified by the client, find the cipher in the
1594+
// list of golang supported ciphers.
1595+
var found *tls.CipherSuite
1596+
for _, supported := range allCiphers {
1597+
if strings.EqualFold(supported.Name, cipher) {
1598+
found = supported
1599+
break
1600+
}
1601+
}
1602+
1603+
if found == nil {
1604+
unsupported = append(unsupported, cipher)
1605+
continue
1606+
}
1607+
1608+
supported = append(supported, *found)
1609+
}
1610+
1611+
if len(unsupported) > 0 {
1612+
return nil, xerrors.Errorf("unsupported tls ciphers specified: %s", strings.Join(unsupported, ", "))
1613+
}
1614+
1615+
return supported, nil
1616+
}
1617+
1618+
// hasSupportedVersion is a helper function that returns true if the list
1619+
// of supported versions contains a version between min and max.
1620+
// If the versions list is outside the min/max, then it returns false.
1621+
func hasSupportedVersion(min, max uint16, versions []uint16) bool {
1622+
for _, v := range versions {
1623+
if v >= min && v <= max {
1624+
// If one version is in between min/max, return true.
1625+
return true
1626+
}
1627+
}
1628+
return false
1629+
}
1630+
14901631
func configureOIDCPKI(orig *oauth2.Config, keyFile string, certFile string) (*oauthpki.Config, error) {
14911632
// Read the files
14921633
keyData, err := os.ReadFile(keyFile)
@@ -2078,7 +2219,8 @@ func ConfigureTraceProvider(
20782219
return tracerProvider, sqlDriver, closeTracing
20792220
}
20802221

2081-
func ConfigureHTTPServers(inv *clibase.Invocation, cfg *codersdk.DeploymentValues) (_ *HTTPServers, err error) {
2222+
func ConfigureHTTPServers(logger slog.Logger, inv *clibase.Invocation, cfg *codersdk.DeploymentValues) (_ *HTTPServers, err error) {
2223+
ctx := inv.Context()
20822224
httpServers := &HTTPServers{}
20832225
defer func() {
20842226
if err != nil {
@@ -2154,16 +2296,20 @@ func ConfigureHTTPServers(inv *clibase.Invocation, cfg *codersdk.DeploymentValue
21542296
// DEPRECATED: This redirect used to default to true.
21552297
// It made more sense to have the redirect be opt-in.
21562298
if inv.Environ.Get("CODER_TLS_REDIRECT_HTTP") == "true" || inv.ParsedFlags().Changed("tls-redirect-http-to-https") {
2157-
cliui.Warn(inv.Stderr, "--tls-redirect-http-to-https is deprecated, please use --redirect-to-access-url instead")
2299+
logger.Warn(ctx, "--tls-redirect-http-to-https is deprecated, please use --redirect-to-access-url instead")
21582300
cfg.RedirectToAccessURL = cfg.TLS.RedirectHTTP
21592301
}
21602302

2161-
tlsConfig, err := configureTLS(
2303+
tlsConfig, err := configureServerTLS(
2304+
ctx,
2305+
logger,
21622306
cfg.TLS.MinVersion.String(),
21632307
cfg.TLS.ClientAuth.String(),
21642308
cfg.TLS.CertFiles,
21652309
cfg.TLS.KeyFiles,
21662310
cfg.TLS.ClientCAFile.String(),
2311+
cfg.TLS.SupportedCiphers.Value(),
2312+
cfg.TLS.AllowInsecureCiphers.Value(),
21672313
)
21682314
if err != nil {
21692315
return nil, xerrors.Errorf("configure tls: %w", err)

‎cli/server_internal_test.go

+102
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
package cli
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"crypto/tls"
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
11+
"cdr.dev/slog/sloggers/sloghuman"
12+
"github.com/stretchr/testify/require"
13+
14+
"cdr.dev/slog"
15+
)
16+
17+
func Test_configureCipherSuites(t *testing.T) {
18+
t.Parallel()
19+
20+
cipherNames := func(ciphers []*tls.CipherSuite) []string {
21+
var names []string
22+
for _, c := range ciphers {
23+
names = append(names, c.Name)
24+
}
25+
return names
26+
}
27+
28+
cipherIDs := func(ciphers []*tls.CipherSuite) []uint16 {
29+
var ids []uint16
30+
for _, c := range ciphers {
31+
ids = append(ids, c.ID)
32+
}
33+
return ids
34+
}
35+
36+
tests := []struct {
37+
name string
38+
wantErr string
39+
wantWarnings []string
40+
inputCiphers []string
41+
minTLS uint16
42+
maxTLS uint16
43+
allowInsecure bool
44+
expectCiphers []uint16
45+
}{
46+
{
47+
name: "AllowInsecure",
48+
minTLS: tls.VersionTLS10,
49+
maxTLS: tls.VersionTLS13,
50+
inputCiphers: append(cipherNames(tls.CipherSuites()), tls.InsecureCipherSuites()[0].Name),
51+
allowInsecure: true,
52+
wantWarnings: []string{
53+
"insecure tls cipher specified",
54+
},
55+
expectCiphers: append(cipherIDs(tls.CipherSuites()), tls.InsecureCipherSuites()[0].ID),
56+
},
57+
// Errors
58+
{
59+
name: "NoCiphers",
60+
minTLS: tls.VersionTLS10,
61+
maxTLS: tls.VersionTLS13,
62+
wantErr: "no tls ciphers supported",
63+
},
64+
{
65+
name: "InsecureNotAllowed",
66+
minTLS: tls.VersionTLS10,
67+
maxTLS: tls.VersionTLS13,
68+
inputCiphers: append(cipherNames(tls.CipherSuites()), tls.InsecureCipherSuites()[0].Name),
69+
wantErr: "insecure tls ciphers specified",
70+
},
71+
{
72+
name: "TLS1.3",
73+
minTLS: tls.VersionTLS13,
74+
maxTLS: tls.VersionTLS13,
75+
inputCiphers: cipherNames(tls.CipherSuites()),
76+
wantErr: "tls ciphers cannot be specified when using minimum tls version 1.3",
77+
},
78+
}
79+
for _, tt := range tests {
80+
tt := tt
81+
t.Run(tt.name, func(t *testing.T) {
82+
t.Parallel()
83+
ctx := context.Background()
84+
var out bytes.Buffer
85+
logger := slog.Make(sloghuman.Sink(&out))
86+
87+
found, err := configureCipherSuites(ctx, logger, tt.inputCiphers, tt.allowInsecure, tt.minTLS, tt.maxTLS)
88+
if tt.wantErr != "" {
89+
require.ErrorContains(t, err, tt.wantErr)
90+
} else {
91+
require.NoError(t, err, "no error")
92+
require.ElementsMatch(t, tt.expectCiphers, found, "expected ciphers")
93+
if len(tt.wantWarnings) > 0 {
94+
logger.Sync()
95+
for _, w := range tt.wantWarnings {
96+
assert.Contains(t, out.String(), w, "expected warning")
97+
}
98+
}
99+
}
100+
})
101+
}
102+
}

‎codersdk/deployment.go

+12-10
Original file line numberDiff line numberDiff line change
@@ -305,16 +305,18 @@ type TelemetryConfig struct {
305305
}
306306

307307
type TLSConfig struct {
308-
Enable clibase.Bool `json:"enable" typescript:",notnull"`
309-
Address clibase.HostPort `json:"address" typescript:",notnull"`
310-
RedirectHTTP clibase.Bool `json:"redirect_http" typescript:",notnull"`
311-
CertFiles clibase.StringArray `json:"cert_file" typescript:",notnull"`
312-
ClientAuth clibase.String `json:"client_auth" typescript:",notnull"`
313-
ClientCAFile clibase.String `json:"client_ca_file" typescript:",notnull"`
314-
KeyFiles clibase.StringArray `json:"key_file" typescript:",notnull"`
315-
MinVersion clibase.String `json:"min_version" typescript:",notnull"`
316-
ClientCertFile clibase.String `json:"client_cert_file" typescript:",notnull"`
317-
ClientKeyFile clibase.String `json:"client_key_file" typescript:",notnull"`
308+
Enable clibase.Bool `json:"enable" typescript:",notnull"`
309+
Address clibase.HostPort `json:"address" typescript:",notnull"`
310+
RedirectHTTP clibase.Bool `json:"redirect_http" typescript:",notnull"`
311+
CertFiles clibase.StringArray `json:"cert_file" typescript:",notnull"`
312+
ClientAuth clibase.String `json:"client_auth" typescript:",notnull"`
313+
ClientCAFile clibase.String `json:"client_ca_file" typescript:",notnull"`
314+
KeyFiles clibase.StringArray `json:"key_file" typescript:",notnull"`
315+
MinVersion clibase.String `json:"min_version" typescript:",notnull"`
316+
ClientCertFile clibase.String `json:"client_cert_file" typescript:",notnull"`
317+
ClientKeyFile clibase.String `json:"client_key_file" typescript:",notnull"`
318+
SupportedCiphers clibase.StringArray `json:"supported_ciphers" typescript:",notnull"`
319+
AllowInsecureCiphers clibase.Bool `json:"allow_insecure_ciphers" typescript:",notnull"`
318320
}
319321

320322
type TraceConfig struct {

‎enterprise/cli/proxyserver.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func (*RootCmd) proxyServer() *clibase.Cmd {
158158
logger.Debug(ctx, "tracing closed", slog.Error(traceCloseErr))
159159
}()
160160

161-
httpServers, err := cli.ConfigureHTTPServers(inv, cfg)
161+
httpServers, err := cli.ConfigureHTTPServers(logger, inv, cfg)
162162
if err != nil {
163163
return xerrors.Errorf("configure http(s): %w", err)
164164
}

0 commit comments

Comments
 (0)