diff --git a/tls/config.go b/tls/config.go index 4d07cbb5bb..66f920e70b 100644 --- a/tls/config.go +++ b/tls/config.go @@ -23,6 +23,10 @@ import ( "strings" ) +// DefaultMinTLSVersion is the minimum TLS version used when no override +// is provided via environment variable or caller-supplied defaults. +const DefaultMinTLSVersion = cryptotls.VersionTLS13 + // Environment variable name suffixes for TLS configuration. // Use with a prefix to namespace them, e.g. "WEBHOOK_" + MinVersionEnvKey // reads the WEBHOOK_TLS_MIN_VERSION variable. @@ -46,9 +50,19 @@ type Config struct { // returns a Config. The prefix is prepended to each standard env-var suffix; // for example with prefix "WEBHOOK_" the function reads // WEBHOOK_TLS_MIN_VERSION, WEBHOOK_TLS_MAX_VERSION, etc. -// Fields whose corresponding env var is unset are left at their zero value. -func NewConfigFromEnv(prefix string) (*Config, error) { +// +// An optional defaults Config may be supplied; its values are used for any +// field whose corresponding environment variable is unset. When no defaults +// are provided, unset fields remain at their zero value. +func NewConfigFromEnv(prefix string, defaults ...Config) (*Config, error) { + if len(defaults) > 1 { + return nil, fmt.Errorf("at most one defaults Config may be provided, got %d", len(defaults)) + } + var cfg Config + if len(defaults) == 1 { + cfg = defaults[0] + } if v := os.Getenv(prefix + MinVersionEnvKey); v != "" { ver, err := parseVersion(v) diff --git a/tls/config_test.go b/tls/config_test.go index b97e4044d5..c5f09469f7 100644 --- a/tls/config_test.go +++ b/tls/config_test.go @@ -349,6 +349,68 @@ func TestNewConfigFromEnv(t *testing.T) { t.Fatal("expected error for invalid curve") } }) + + t.Run("defaults used when env vars are unset", func(t *testing.T) { + defaults := Config{ + MinVersion: DefaultMinTLSVersion, + MaxVersion: cryptotls.VersionTLS13, + CipherSuites: []uint16{cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + CurvePreferences: []cryptotls.CurveID{ + cryptotls.X25519, + }, + } + cfg, err := NewConfigFromEnv("", defaults) + if err != nil { + t.Fatal("unexpected error:", err) + } + if cfg.MinVersion != DefaultMinTLSVersion { + t.Errorf("MinVersion = %d, want %d", cfg.MinVersion, DefaultMinTLSVersion) + } + if cfg.MaxVersion != cryptotls.VersionTLS13 { + t.Errorf("MaxVersion = %d, want %d", cfg.MaxVersion, cryptotls.VersionTLS13) + } + if len(cfg.CipherSuites) != 1 || cfg.CipherSuites[0] != cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 { + t.Errorf("CipherSuites = %v, want [%d]", cfg.CipherSuites, cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) + } + if len(cfg.CurvePreferences) != 1 || cfg.CurvePreferences[0] != cryptotls.X25519 { + t.Errorf("CurvePreferences = %v, want [%d]", cfg.CurvePreferences, cryptotls.X25519) + } + }) + + t.Run("env vars override defaults", func(t *testing.T) { + t.Setenv(MinVersionEnvKey, "1.2") + t.Setenv(CurvePreferencesEnvKey, "CurveP256") + + defaults := Config{ + MinVersion: DefaultMinTLSVersion, + CurvePreferences: []cryptotls.CurveID{ + cryptotls.X25519, + }, + } + cfg, err := NewConfigFromEnv("", defaults) + if err != nil { + t.Fatal("unexpected error:", err) + } + if cfg.MinVersion != cryptotls.VersionTLS12 { + t.Errorf("MinVersion = %d, want %d (env should override default)", cfg.MinVersion, cryptotls.VersionTLS12) + } + if len(cfg.CurvePreferences) != 1 || cfg.CurvePreferences[0] != cryptotls.CurveP256 { + t.Errorf("CurvePreferences = %v, want [%d] (env should override default)", cfg.CurvePreferences, cryptotls.CurveP256) + } + }) + + t.Run("multiple defaults returns error", func(t *testing.T) { + _, err := NewConfigFromEnv("", Config{}, Config{}) + if err == nil { + t.Fatal("expected error when passing multiple defaults") + } + }) + + t.Run("DefaultMinTLSVersion equals TLS 1.3", func(t *testing.T) { + if DefaultMinTLSVersion != cryptotls.VersionTLS13 { + t.Errorf("DefaultMinTLSVersion = %d, want %d (TLS 1.3)", DefaultMinTLSVersion, cryptotls.VersionTLS13) + } + }) } func TestConfig_TLSConfig(t *testing.T) { diff --git a/webhook/webhook.go b/webhook/webhook.go index 8f3a3701cc..28525b4c41 100644 --- a/webhook/webhook.go +++ b/webhook/webhook.go @@ -191,20 +191,17 @@ func New( logger := logging.FromContext(ctx) - tlsCfg, err := knativetls.NewConfigFromEnv("WEBHOOK_") + tlsCfg, err := knativetls.NewConfigFromEnv("WEBHOOK_", knativetls.Config{ + MinVersion: knativetls.DefaultMinTLSVersion, + }) if err != nil { return nil, fmt.Errorf("reading TLS configuration from environment: %w", err) } - // Replace the TLS configuration with the one from the environment if not set. - // Default to TLS 1.3 as the minimum version when neither the caller nor the - // environment specifies one. - if opts.TLSMinVersion == 0 { - if tlsCfg.MinVersion != 0 { - opts.TLSMinVersion = tlsCfg.MinVersion - } else { - opts.TLSMinVersion = tls.VersionTLS13 - } + // Apply environment / default TLS settings for any fields the caller + // has not already configured. + if opts.TLSMinVersion == 0 && tlsCfg.MinVersion != 0 { + opts.TLSMinVersion = tlsCfg.MinVersion } if opts.TLSMaxVersion == 0 && tlsCfg.MaxVersion != 0 { opts.TLSMaxVersion = tlsCfg.MaxVersion