Skip to content

fix!: remove TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA cipher by default #13837

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 9, 2024
Merged

fix!: remove TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA cipher by default #13837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
81d87dd
fix!(cli): remove `TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA` cipher by def…
coadler Jul 8, 2024
964d3bd
fixup! fix!(cli): remove `TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA` cipher…
coadler Jul 9, 2024
fed2813
fixup! fix!(cli): remove `TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA` cipher…
coadler Jul 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .github/workflows/typos.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@ darcula = "darcula"
Hashi = "Hashi"
trialer = "trialer"
encrypter = "encrypter"
hel = "hel" # as in helsinki
pn = "pn" # this is used as proto node
# as in helsinki
hel = "hel"
# this is used as proto node
pn = "pn"
# typos doesn't like the EDE in TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA
EDE = "EDE"

[files]
extend-exclude = [
Expand Down
15 changes: 15 additions & 0 deletions cli/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1569,6 +1569,19 @@ func generateSelfSignedCertificate() (*tls.Certificate, error) {
return &cert, nil
}

// defaultCipherSuites is a list of safe cipher suites that we default to. This
// is different from Golang's list of defaults, which unfortunately includes
// `TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA`.
var defaultCipherSuites = func() []uint16 {
ret := []uint16{}

for _, suite := range tls.CipherSuites() {
ret = append(ret, suite.ID)
}

return ret
}()

// 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.
Expand Down Expand Up @@ -1599,6 +1612,8 @@ func configureServerTLS(ctx context.Context, logger slog.Logger, tlsMinVersion,
return nil, err
}
tlsConfig.CipherSuites = cipherIDs
} else {
tlsConfig.CipherSuites = defaultCipherSuites
}

switch tlsClientAuth {
Expand Down
22 changes: 22 additions & 0 deletions cli/server_internal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,28 @@ import (
"github.com/coder/serpent"
)

func Test_configureServerTLS(t *testing.T) {
t.Parallel()
t.Run("DefaultNoInsecureCiphers", func(t *testing.T) {
t.Parallel()
logger := slogtest.Make(t, nil)
cfg, err := configureServerTLS(context.Background(), logger, "tls12", "none", nil, nil, "", nil, false)
require.NoError(t, err)

require.NotEmpty(t, cfg)

insecureCiphers := tls.InsecureCipherSuites()
for _, cipher := range cfg.CipherSuites {
for _, insecure := range insecureCiphers {
if cipher == insecure.ID {
t.Logf("Insecure cipher found by default: %s", insecure.Name)
t.Fail()
}
}
}
})
}

func Test_configureCipherSuites(t *testing.T) {
t.Parallel()

Expand Down
Loading