Skip to content

Update default minimum TLS version to 1.3 #8916

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

Open
Fedosin wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update default minimum TLS version to 1.3 #8916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 0 additions & 5 deletions cmd/requestreply/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package main

import (
"crypto/tls"
"testing"

reconcilertesting "knative.dev/pkg/reconciler/testing"
Expand All @@ -41,10 +40,6 @@ func TestGetServerTLSConfig(t *testing.T) {
t.Fatal("expected non-nil TLS config")
}

if tlsConfig.MinVersion != tls.VersionTLS12 {
t.Fatalf("want MinVersion TLS 1.2 (%d), got %d", tls.VersionTLS12, tlsConfig.MinVersion)
}

if tlsConfig.GetCertificate == nil {
t.Fatal("expected GetCertificate to be set")
}
Expand Down
10 changes: 1 addition & 9 deletions pkg/eventingtls/eventingtls.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ const (
TLSKey = "tls.key"
// TLSCrt is the key in the TLS secret for the public key of TLS servers
TLSCrt = "tls.crt"
// DefaultMinTLSVersion is the default minimum TLS version for servers and clients.
DefaultMinTLSVersion = tls.VersionTLS12
// SecretCACrt is the name of the CA Cert in the secret
SecretCACert = "ca.crt"
// IMCDispatcherServerTLSSecretName is the name of the tls secret for the imc dispatcher server
Expand Down Expand Up @@ -198,19 +196,13 @@ func GetTLSServerConfig(config ServerConfig) (*tls.Config, error) {

// defaultTLSConfigFromEnv loads TLS configuration from environment variables
// using the shared knative/pkg/tls utility. DefaultConfigFromEnv defaults to
// TLS 1.3, but eventing historically defaults to TLS 1.2, so we fall back to
// 1.2 unless TLS_MIN_VERSION is explicitly set.
// TODO: switch to TLS 1.3 to align with the rest of the system.
// TLS 1.3.
func defaultTLSConfigFromEnv() (*tls.Config, error) {
cfg, err := pkgtls.DefaultConfigFromEnv("")
if err != nil {
return nil, fmt.Errorf("failed to load TLS config from env: %w", err)
}

if os.Getenv(pkgtls.MinVersionEnvKey) == "" {
cfg.MinVersion = DefaultMinTLSVersion
}

return cfg, nil
}

Expand Down
43 changes: 5 additions & 38 deletions pkg/eventingtls/eventingtls_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,14 @@ wSdZWoEx7ye2kUHEyRKdRGbHyJtY9YYvaROznzxqVpIqHxnRQnE/If7kcN4t/7vi
CACerts: pointer.String(""),
},
expected: tls.Config{
MinVersion: tls.VersionTLS12,
RootCAs: sysCertPool,
RootCAs: sysCertPool,
},
},
{
name: "nil CA certs",
cfg: ClientConfig{},
expected: tls.Config{
MinVersion: tls.VersionTLS12,
RootCAs: sysCertPool,
RootCAs: sysCertPool,
},
},
{
Expand All @@ -85,19 +83,16 @@ wSdZWoEx7ye2kUHEyRKdRGbHyJtY9YYvaROznzxqVpIqHxnRQnE/If7kcN4t/7vi
CACerts: pointer.String(pemCaCert),
},
expected: tls.Config{
MinVersion: tls.VersionTLS12,
RootCAs: WithCerts(sysCertPool, pemCaCert),
RootCAs: WithCerts(sysCertPool, pemCaCert),
},
},
{
name: "Additional broken CA certs",
cfg: ClientConfig{
CACerts: pointer.String(pemCaCert[:len(pemCaCert)-30]),
},
expected: tls.Config{
MinVersion: tls.VersionTLS12,
},
wantErr: true,
expected: tls.Config{},
wantErr: true,
},
}

Expand All @@ -115,10 +110,6 @@ wSdZWoEx7ye2kUHEyRKdRGbHyJtY9YYvaROznzxqVpIqHxnRQnE/If7kcN4t/7vi
if !got.RootCAs.Equal(tc.expected.RootCAs) {
t.Fatalf("Got RootCAs are not equal to expected RootCAs")
}

if got.MinVersion != tc.expected.MinVersion {
t.Fatalf("want MinVersion %v, got %v", tc.expected.MinVersion, got.MinVersion)
}
})
}
}
Expand All @@ -132,18 +123,6 @@ func WithCerts(pool *x509.CertPool, caCerts string) *x509.CertPool {
}

func TestGetTLSClientConfigEnv(t *testing.T) {
t.Run("defaults to TLS 1.2 when env not set", func(t *testing.T) {
t.Setenv(pkgtls.MinVersionEnvKey, "")

cfg, err := GetTLSClientConfig(NewDefaultClientConfig())
if err != nil {
t.Fatal("unexpected error:", err)
}
if cfg.MinVersion != tls.VersionTLS12 {
t.Fatalf("want MinVersion TLS 1.2 (%d), got %d", tls.VersionTLS12, cfg.MinVersion)
}
})

t.Run("uses TLS 1.3 when explicitly set via env", func(t *testing.T) {
t.Setenv(pkgtls.MinVersionEnvKey, "1.3")

Expand Down Expand Up @@ -206,18 +185,6 @@ func TestGetTLSClientConfigEnv(t *testing.T) {
}

func TestGetTLSServerConfig(t *testing.T) {
t.Run("defaults to TLS 1.2 when env not set", func(t *testing.T) {
t.Setenv(pkgtls.MinVersionEnvKey, "")

cfg, err := GetTLSServerConfig(NewDefaultServerConfig())
if err != nil {
t.Fatal("unexpected error:", err)
}
if cfg.MinVersion != tls.VersionTLS12 {
t.Fatalf("want MinVersion TLS 1.2 (%d), got %d", tls.VersionTLS12, cfg.MinVersion)
}
})

t.Run("uses TLS 1.3 when explicitly set via env", func(t *testing.T) {
t.Setenv(pkgtls.MinVersionEnvKey, "1.3")

Expand Down
Loading