From 19097acfecd4889f9aead281b063cc59b151bd26 Mon Sep 17 00:00:00 2001 From: Mike Fedosin Date: Mon, 2 Mar 2026 15:07:26 +0100 Subject: [PATCH 1/4] feat: add shared tls package for reading TLS config from environment (#3324) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add shared tls package for reading TLS config from environment Extract TLS configuration parsing into a reusable knative.dev/pkg/tls package so that any Knative component (not just webhooks) can read TLS_MIN_VERSION, TLS_MAX_VERSION, TLS_CIPHER_SUITES, and TLS_CURVE_PREFERENCES from environment variables with an optional prefix. The webhook package is updated to use the new tls package, extending env var support from just WEBHOOK_TLS_MIN_VERSION to all four WEBHOOK_TLS_* variables. Programmatic Options values continue to take precedence over environment variables. Signed-off-by: Mikhail Fedosin * fix: address review feedback on tls package Reduce the public API surface of the tls package by unexporting ParseVersion, ParseCipherSuites, and ParseCurvePreferences since they are implementation details of NewConfigFromEnv. Also validate that TLS max version is not smaller than min version in webhook.New(), document the Options TLS field precedence (programmatic > env vars > defaults), and broaden TestConfig_TLSConfig to exercise the full NewConfigFromEnv → TLSConfig path. Signed-off-by: Mikhail Fedosin --------- Signed-off-by: Mikhail Fedosin --- tls/config.go | 176 ++++++++++++++++++ tls/config_test.go | 385 ++++++++++++++++++++++++++++++++++++++++ webhook/env.go | 2 + webhook/webhook.go | 44 ++++- webhook/webhook_test.go | 190 +++++++++++++++++++- 5 files changed, 790 insertions(+), 7 deletions(-) create mode 100644 tls/config.go create mode 100644 tls/config_test.go diff --git a/tls/config.go b/tls/config.go new file mode 100644 index 0000000000..4d07cbb5bb --- /dev/null +++ b/tls/config.go @@ -0,0 +1,176 @@ +/* +Copyright 2026 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tls + +import ( + cryptotls "crypto/tls" + "fmt" + "os" + "strings" +) + +// 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. +const ( + MinVersionEnvKey = "TLS_MIN_VERSION" + MaxVersionEnvKey = "TLS_MAX_VERSION" + CipherSuitesEnvKey = "TLS_CIPHER_SUITES" + CurvePreferencesEnvKey = "TLS_CURVE_PREFERENCES" +) + +// Config holds parsed TLS configuration values that can be used +// to build a *crypto/tls.Config. +type Config struct { + MinVersion uint16 + MaxVersion uint16 + CipherSuites []uint16 + CurvePreferences []cryptotls.CurveID +} + +// NewConfigFromEnv reads TLS configuration from environment variables and +// 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) { + var cfg Config + + if v := os.Getenv(prefix + MinVersionEnvKey); v != "" { + ver, err := parseVersion(v) + if err != nil { + return nil, fmt.Errorf("invalid %s%s %q: %w", prefix, MinVersionEnvKey, v, err) + } + cfg.MinVersion = ver + } + + if v := os.Getenv(prefix + MaxVersionEnvKey); v != "" { + ver, err := parseVersion(v) + if err != nil { + return nil, fmt.Errorf("invalid %s%s %q: %w", prefix, MaxVersionEnvKey, v, err) + } + cfg.MaxVersion = ver + } + + if v := os.Getenv(prefix + CipherSuitesEnvKey); v != "" { + suites, err := parseCipherSuites(v) + if err != nil { + return nil, fmt.Errorf("invalid %s%s: %w", prefix, CipherSuitesEnvKey, err) + } + cfg.CipherSuites = suites + } + + if v := os.Getenv(prefix + CurvePreferencesEnvKey); v != "" { + curves, err := parseCurvePreferences(v) + if err != nil { + return nil, fmt.Errorf("invalid %s%s: %w", prefix, CurvePreferencesEnvKey, err) + } + cfg.CurvePreferences = curves + } + + return &cfg, nil +} + +// TLSConfig constructs a *crypto/tls.Config from the parsed configuration. +// The caller typically adds additional fields such as GetCertificate. +func (c *Config) TLSConfig() *cryptotls.Config { + //nolint:gosec // Min version is caller-configurable; default is TLS 1.3. + return &cryptotls.Config{ + MinVersion: c.MinVersion, + MaxVersion: c.MaxVersion, + CipherSuites: c.CipherSuites, + CurvePreferences: c.CurvePreferences, + } +} + +// parseVersion converts a TLS version string to the corresponding +// crypto/tls constant. Accepted values are "1.2" and "1.3". +func parseVersion(v string) (uint16, error) { + switch v { + case "1.2": + return cryptotls.VersionTLS12, nil + case "1.3": + return cryptotls.VersionTLS13, nil + default: + return 0, fmt.Errorf("unsupported TLS version %q: must be %q or %q", v, "1.2", "1.3") + } +} + +// parseCipherSuites parses a comma-separated list of TLS cipher-suite names +// (e.g. "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384") +// into a slice of cipher-suite IDs. Names must match those returned by +// crypto/tls.CipherSuiteName. +func parseCipherSuites(s string) ([]uint16, error) { + lookup := cipherSuiteLookup() + parts := strings.Split(s, ",") + suites := make([]uint16, 0, len(parts)) + + for _, name := range parts { + name = strings.TrimSpace(name) + if name == "" { + continue + } + id, ok := lookup[name] + if !ok { + return nil, fmt.Errorf("unknown cipher suite %q", name) + } + suites = append(suites, id) + } + + return suites, nil +} + +// parseCurvePreferences parses a comma-separated list of elliptic-curve names +// (e.g. "X25519,CurveP256") into a slice of crypto/tls.CurveID values. +// Both Go constant names (CurveP256) and standard names (P-256) are accepted. +func parseCurvePreferences(s string) ([]cryptotls.CurveID, error) { + parts := strings.Split(s, ",") + curves := make([]cryptotls.CurveID, 0, len(parts)) + + for _, name := range parts { + name = strings.TrimSpace(name) + if name == "" { + continue + } + id, ok := curvesByName[name] + if !ok { + return nil, fmt.Errorf("unknown curve %q", name) + } + curves = append(curves, id) + } + + return curves, nil +} + +func cipherSuiteLookup() map[string]uint16 { + m := make(map[string]uint16) + for _, cs := range cryptotls.CipherSuites() { + m[cs.Name] = cs.ID + } + return m +} + +var curvesByName = map[string]cryptotls.CurveID{ + "CurveP256": cryptotls.CurveP256, + "CurveP384": cryptotls.CurveP384, + "CurveP521": cryptotls.CurveP521, + "X25519": cryptotls.X25519, + "X25519MLKEM768": cryptotls.X25519MLKEM768, + "P-256": cryptotls.CurveP256, + "P-384": cryptotls.CurveP384, + "P-521": cryptotls.CurveP521, +} diff --git a/tls/config_test.go b/tls/config_test.go new file mode 100644 index 0000000000..b97e4044d5 --- /dev/null +++ b/tls/config_test.go @@ -0,0 +1,385 @@ +/* +Copyright 2026 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tls + +import ( + cryptotls "crypto/tls" + "testing" +) + +func Test_parseVersion(t *testing.T) { + tests := []struct { + name string + input string + want uint16 + wantErr bool + }{ + {name: "TLS 1.2", input: "1.2", want: cryptotls.VersionTLS12}, + {name: "TLS 1.3", input: "1.3", want: cryptotls.VersionTLS13}, + {name: "unsupported version", input: "1.0", wantErr: true}, + {name: "unsupported version 1.1", input: "1.1", wantErr: true}, + {name: "trailing space", input: "1.2 ", wantErr: true}, + {name: "empty string", input: "", wantErr: true}, + {name: "garbage", input: "abc", wantErr: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := parseVersion(tc.input) + if tc.wantErr { + if err == nil { + t.Fatalf("parseVersion(%q) = %d, want error", tc.input, got) + } + return + } + if err != nil { + t.Fatalf("parseVersion(%q) unexpected error: %v", tc.input, err) + } + if got != tc.want { + t.Errorf("parseVersion(%q) = %d, want %d", tc.input, got, tc.want) + } + }) + } +} + +func Test_parseCipherSuites(t *testing.T) { + tests := []struct { + name string + input string + want []uint16 + wantErr bool + }{ + { + name: "single suite", + input: "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + want: []uint16{cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + }, + { + name: "multiple suites", + input: "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", + want: []uint16{ + cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + cryptotls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + }, + }, + { + name: "whitespace trimmed", + input: " TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 , TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 ", + want: []uint16{ + cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + cryptotls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + }, + }, + { + name: "empty parts skipped", + input: "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,,", + want: []uint16{cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + }, + { + name: "unknown suite", + input: "DOES_NOT_EXIST", + wantErr: true, + }, + { + name: "empty string", + want: []uint16{}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := parseCipherSuites(tc.input) + if tc.wantErr { + if err == nil { + t.Fatalf("parseCipherSuites(%q) = %v, want error", tc.input, got) + } + return + } + if err != nil { + t.Fatalf("parseCipherSuites(%q) unexpected error: %v", tc.input, err) + } + if len(got) != len(tc.want) { + t.Fatalf("parseCipherSuites(%q) returned %d suites, want %d", tc.input, len(got), len(tc.want)) + } + for i := range tc.want { + if got[i] != tc.want[i] { + t.Errorf("parseCipherSuites(%q)[%d] = %d, want %d", tc.input, i, got[i], tc.want[i]) + } + } + }) + } +} + +func Test_parseCurvePreferences(t *testing.T) { + tests := []struct { + name string + input string + want []cryptotls.CurveID + wantErr bool + }{ + { + name: "Go constant name X25519", + input: "X25519", + want: []cryptotls.CurveID{cryptotls.X25519}, + }, + { + name: "Go constant name CurveP256", + input: "CurveP256", + want: []cryptotls.CurveID{cryptotls.CurveP256}, + }, + { + name: "standard name P-256", + input: "P-256", + want: []cryptotls.CurveID{cryptotls.CurveP256}, + }, + { + name: "multiple curves with mixed naming", + input: "X25519,P-256,CurveP384", + want: []cryptotls.CurveID{ + cryptotls.X25519, + cryptotls.CurveP256, + cryptotls.CurveP384, + }, + }, + { + name: "whitespace trimmed", + input: " X25519 , CurveP256 ", + want: []cryptotls.CurveID{ + cryptotls.X25519, + cryptotls.CurveP256, + }, + }, + { + name: "all curves by standard name", + input: "P-256,P-384,P-521,X25519", + want: []cryptotls.CurveID{ + cryptotls.CurveP256, + cryptotls.CurveP384, + cryptotls.CurveP521, + cryptotls.X25519, + }, + }, + { + name: "post-quantum hybrid X25519MLKEM768", + input: "X25519MLKEM768", + want: []cryptotls.CurveID{cryptotls.X25519MLKEM768}, + }, + { + name: "unknown curve", + input: "CurveP128", + wantErr: true, + }, + { + name: "empty string", + want: []cryptotls.CurveID{}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := parseCurvePreferences(tc.input) + if tc.wantErr { + if err == nil { + t.Fatalf("parseCurvePreferences(%q) = %v, want error", tc.input, got) + } + return + } + if err != nil { + t.Fatalf("parseCurvePreferences(%q) unexpected error: %v", tc.input, err) + } + if len(got) != len(tc.want) { + t.Fatalf("parseCurvePreferences(%q) returned %d curves, want %d", tc.input, len(got), len(tc.want)) + } + for i := range tc.want { + if got[i] != tc.want[i] { + t.Errorf("parseCurvePreferences(%q)[%d] = %d, want %d", tc.input, i, got[i], tc.want[i]) + } + } + }) + } +} + +func TestNewConfigFromEnv(t *testing.T) { + t.Run("no env vars set returns zero value", func(t *testing.T) { + cfg, err := NewConfigFromEnv("") + if err != nil { + t.Fatal("unexpected error:", err) + } + if cfg.MinVersion != 0 { + t.Errorf("MinVersion = %d, want 0", cfg.MinVersion) + } + if cfg.MaxVersion != 0 { + t.Errorf("MaxVersion = %d, want 0", cfg.MaxVersion) + } + if cfg.CipherSuites != nil { + t.Errorf("CipherSuites = %v, want nil", cfg.CipherSuites) + } + if cfg.CurvePreferences != nil { + t.Errorf("CurvePreferences = %v, want nil", cfg.CurvePreferences) + } + }) + + t.Run("min version from env", func(t *testing.T) { + t.Setenv(MinVersionEnvKey, "1.2") + cfg, err := NewConfigFromEnv("") + if err != nil { + t.Fatal("unexpected error:", err) + } + if cfg.MinVersion != cryptotls.VersionTLS12 { + t.Errorf("MinVersion = %d, want %d", cfg.MinVersion, cryptotls.VersionTLS12) + } + }) + + t.Run("max version from env", func(t *testing.T) { + t.Setenv(MaxVersionEnvKey, "1.3") + cfg, err := NewConfigFromEnv("") + if err != nil { + t.Fatal("unexpected error:", err) + } + if cfg.MaxVersion != cryptotls.VersionTLS13 { + t.Errorf("MaxVersion = %d, want %d", cfg.MaxVersion, cryptotls.VersionTLS13) + } + }) + + t.Run("cipher suites from env", func(t *testing.T) { + t.Setenv(CipherSuitesEnvKey, "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256") + cfg, err := NewConfigFromEnv("") + if err != nil { + t.Fatal("unexpected error:", err) + } + 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) + } + }) + + t.Run("curve preferences from env", func(t *testing.T) { + t.Setenv(CurvePreferencesEnvKey, "X25519,CurveP256") + cfg, err := NewConfigFromEnv("") + if err != nil { + t.Fatal("unexpected error:", err) + } + if len(cfg.CurvePreferences) != 2 { + t.Fatalf("CurvePreferences has %d entries, want 2", len(cfg.CurvePreferences)) + } + if cfg.CurvePreferences[0] != cryptotls.X25519 { + t.Errorf("CurvePreferences[0] = %d, want %d", cfg.CurvePreferences[0], cryptotls.X25519) + } + if cfg.CurvePreferences[1] != cryptotls.CurveP256 { + t.Errorf("CurvePreferences[1] = %d, want %d", cfg.CurvePreferences[1], cryptotls.CurveP256) + } + }) + + t.Run("prefix is prepended to env key", func(t *testing.T) { + t.Setenv("WEBHOOK_TLS_MIN_VERSION", "1.2") + cfg, err := NewConfigFromEnv("WEBHOOK_") + if err != nil { + t.Fatal("unexpected error:", err) + } + if cfg.MinVersion != cryptotls.VersionTLS12 { + t.Errorf("MinVersion = %d, want %d", cfg.MinVersion, cryptotls.VersionTLS12) + } + }) + + t.Run("all env vars set", func(t *testing.T) { + t.Setenv(MinVersionEnvKey, "1.2") + t.Setenv(MaxVersionEnvKey, "1.3") + t.Setenv(CipherSuitesEnvKey, "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384") + t.Setenv(CurvePreferencesEnvKey, "X25519,P-256") + + cfg, err := NewConfigFromEnv("") + if err != nil { + t.Fatal("unexpected error:", err) + } + if cfg.MinVersion != cryptotls.VersionTLS12 { + t.Errorf("MinVersion = %d, want %d", cfg.MinVersion, cryptotls.VersionTLS12) + } + if cfg.MaxVersion != cryptotls.VersionTLS13 { + t.Errorf("MaxVersion = %d, want %d", cfg.MaxVersion, cryptotls.VersionTLS13) + } + if len(cfg.CipherSuites) != 2 { + t.Fatalf("CipherSuites has %d entries, want 2", len(cfg.CipherSuites)) + } + if len(cfg.CurvePreferences) != 2 { + t.Fatalf("CurvePreferences has %d entries, want 2", len(cfg.CurvePreferences)) + } + }) + + t.Run("invalid min version", func(t *testing.T) { + t.Setenv(MinVersionEnvKey, "1.0") + _, err := NewConfigFromEnv("") + if err == nil { + t.Fatal("expected error for invalid min version") + } + }) + + t.Run("invalid max version", func(t *testing.T) { + t.Setenv(MaxVersionEnvKey, "bad") + _, err := NewConfigFromEnv("") + if err == nil { + t.Fatal("expected error for invalid max version") + } + }) + + t.Run("invalid cipher suite", func(t *testing.T) { + t.Setenv(CipherSuitesEnvKey, "NOT_A_REAL_CIPHER") + _, err := NewConfigFromEnv("") + if err == nil { + t.Fatal("expected error for invalid cipher suite") + } + }) + + t.Run("invalid curve", func(t *testing.T) { + t.Setenv(CurvePreferencesEnvKey, "NotACurve") + _, err := NewConfigFromEnv("") + if err == nil { + t.Fatal("expected error for invalid curve") + } + }) +} + +func TestConfig_TLSConfig(t *testing.T) { + t.Setenv(MinVersionEnvKey, "1.2") + t.Setenv(MaxVersionEnvKey, "1.3") + t.Setenv(CipherSuitesEnvKey, "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256") + t.Setenv(CurvePreferencesEnvKey, "X25519,CurveP256") + + cfg, err := NewConfigFromEnv("") + if err != nil { + t.Fatal("unexpected error:", err) + } + + tc := cfg.TLSConfig() + + if tc.MinVersion != cryptotls.VersionTLS12 { + t.Errorf("MinVersion = %d, want %d", tc.MinVersion, cryptotls.VersionTLS12) + } + if tc.MaxVersion != cryptotls.VersionTLS13 { + t.Errorf("MaxVersion = %d, want %d", tc.MaxVersion, cryptotls.VersionTLS13) + } + if len(tc.CipherSuites) != 1 || tc.CipherSuites[0] != cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 { + t.Errorf("CipherSuites = %v, want [%d]", tc.CipherSuites, cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) + } + if len(tc.CurvePreferences) != 2 { + t.Fatalf("CurvePreferences has %d entries, want 2", len(tc.CurvePreferences)) + } + if tc.CurvePreferences[0] != cryptotls.X25519 { + t.Errorf("CurvePreferences[0] = %d, want %d", tc.CurvePreferences[0], cryptotls.X25519) + } + if tc.CurvePreferences[1] != cryptotls.CurveP256 { + t.Errorf("CurvePreferences[1] = %d, want %d", tc.CurvePreferences[1], cryptotls.CurveP256) + } +} diff --git a/webhook/env.go b/webhook/env.go index e622f5f97b..1dd38585b6 100644 --- a/webhook/env.go +++ b/webhook/env.go @@ -72,6 +72,8 @@ func SecretNameFromEnv(defaultSecretName string) string { return secret } +// Deprecated: Use knative.dev/pkg/tls.NewConfigFromEnv instead. +// TLS configuration is now read automatically inside webhook.New via the shared tls package. func TLSMinVersionFromEnv(defaultTLSMinVersion uint16) uint16 { switch tlsMinVersion := os.Getenv(tlsMinVersionEnvKey); tlsMinVersion { case "1.2": diff --git a/webhook/webhook.go b/webhook/webhook.go index badc7fef83..ed3c178482 100644 --- a/webhook/webhook.go +++ b/webhook/webhook.go @@ -33,6 +33,7 @@ import ( kubeinformerfactory "knative.dev/pkg/injection/clients/namespacedkube/informers/factory" "knative.dev/pkg/network" "knative.dev/pkg/network/handlers" + knativetls "knative.dev/pkg/tls" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "go.opentelemetry.io/otel/metric" @@ -46,7 +47,15 @@ import ( "knative.dev/pkg/system" ) -// Options contains the configuration for the webhook +// Options contains the configuration for the webhook. +// +// TLS fields (TLSMinVersion, TLSMaxVersion, TLSCipherSuites, TLSCurvePreferences) +// are resolved with the following precedence: +// 1. Values set explicitly in Options (programmatic). +// 2. WEBHOOK_TLS_* environment variables (WEBHOOK_TLS_MIN_VERSION, +// WEBHOOK_TLS_MAX_VERSION, WEBHOOK_TLS_CIPHER_SUITES, WEBHOOK_TLS_CURVE_PREFERENCES). +// 3. Defaults (TLS 1.3 minimum version; zero values for the rest, meaning the +// Go standard library picks its defaults). type Options struct { // TLSMinVersion contains the minimum TLS version that is acceptable to communicate with the API server. // TLS 1.3 is the minimum version if not specified otherwise. @@ -180,11 +189,36 @@ func New( logger := logging.FromContext(ctx) - defaultTLSMinVersion := uint16(tls.VersionTLS13) + tlsCfg, err := knativetls.NewConfigFromEnv("WEBHOOK_") + 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 { - opts.TLSMinVersion = TLSMinVersionFromEnv(defaultTLSMinVersion) - } else if opts.TLSMinVersion != tls.VersionTLS12 && opts.TLSMinVersion != tls.VersionTLS13 { - return nil, fmt.Errorf("unsupported TLS version: %d", opts.TLSMinVersion) + if tlsCfg.MinVersion != 0 { + opts.TLSMinVersion = tlsCfg.MinVersion + } else { + opts.TLSMinVersion = tls.VersionTLS13 + } + } + if opts.TLSMaxVersion == 0 && tlsCfg.MaxVersion != 0 { + opts.TLSMaxVersion = tlsCfg.MaxVersion + } + if opts.TLSCipherSuites == nil && len(tlsCfg.CipherSuites) > 0 { + opts.TLSCipherSuites = tlsCfg.CipherSuites + } + if opts.TLSCurvePreferences == nil && len(tlsCfg.CurvePreferences) > 0 { + opts.TLSCurvePreferences = tlsCfg.CurvePreferences + } + + if opts.TLSMinVersion != 0 && opts.TLSMinVersion != tls.VersionTLS12 && opts.TLSMinVersion != tls.VersionTLS13 { + return nil, fmt.Errorf("unsupported TLS minimum version %d: must be TLS 1.2 or TLS 1.3", opts.TLSMinVersion) + } + if opts.TLSMaxVersion != 0 && opts.TLSMinVersion > opts.TLSMaxVersion { + return nil, fmt.Errorf("TLS minimum version (%#x) is greater than maximum version (%#x)", opts.TLSMinVersion, opts.TLSMaxVersion) } syncCtx, cancel := context.WithCancel(context.Background()) diff --git a/webhook/webhook_test.go b/webhook/webhook_test.go index d4bb521dff..cb72a759b9 100644 --- a/webhook/webhook_test.go +++ b/webhook/webhook_test.go @@ -103,11 +103,17 @@ func newAdmissionControllerWebhook(t *testing.T, options Options, acs ...interfa func TestTLSMinVersionWebhookOption(t *testing.T) { opts := newDefaultOptions() - t.Run("when TLSMinVersion is not configured, and the default is used", func(t *testing.T) { - _, err := newAdmissionControllerWebhook(t, opts) + t.Run("when TLSMinVersion is not configured, default is TLS 1.3", func(t *testing.T) { + wh, err := newAdmissionControllerWebhook(t, opts) if err != nil { t.Fatal("Unexpected error", err) } + if wh.tlsConfig == nil { + t.Fatal("Expected tlsConfig to be set") + } + if wh.tlsConfig.MinVersion != tls.VersionTLS13 { + t.Errorf("Expected default MinVersion to be TLS 1.3 (%#x), got %#x", tls.VersionTLS13, wh.tlsConfig.MinVersion) + } }) t.Run("when the TLS minimum version configured is supported", func(t *testing.T) { opts.TLSMinVersion = tls.VersionTLS12 @@ -169,6 +175,36 @@ func TestTLSMaxVersionWebhookOption(t *testing.T) { }) } +func TestTLSMinMaxVersionValidation(t *testing.T) { + t.Run("max version less than min version returns error", func(t *testing.T) { + opts := newDefaultOptions() + opts.TLSMinVersion = tls.VersionTLS13 + opts.TLSMaxVersion = tls.VersionTLS12 + _, err := newAdmissionControllerWebhook(t, opts) + if err == nil { + t.Fatal("Expected error when TLS max version is less than min version") + } + }) + t.Run("max version equal to min version is ok", func(t *testing.T) { + opts := newDefaultOptions() + opts.TLSMinVersion = tls.VersionTLS13 + opts.TLSMaxVersion = tls.VersionTLS13 + _, err := newAdmissionControllerWebhook(t, opts) + if err != nil { + t.Fatal("Unexpected error:", err) + } + }) + t.Run("max version zero skips validation", func(t *testing.T) { + opts := newDefaultOptions() + opts.TLSMinVersion = tls.VersionTLS13 + opts.TLSMaxVersion = 0 + _, err := newAdmissionControllerWebhook(t, opts) + if err != nil { + t.Fatal("Unexpected error:", err) + } + }) +} + func TestTLSCipherSuitesWebhookOption(t *testing.T) { opts := newDefaultOptions() t.Run("when TLSCipherSuites is not configured", func(t *testing.T) { @@ -242,6 +278,156 @@ func TestTLSCurvePreferencesWebhookOption(t *testing.T) { }) } +func TestTLSConfigFromEnvironment(t *testing.T) { + t.Run("env min version used when opts min version is zero", func(t *testing.T) { + t.Setenv("WEBHOOK_TLS_MIN_VERSION", "1.2") + opts := newDefaultOptions() + wh, err := newAdmissionControllerWebhook(t, opts) + if err != nil { + t.Fatal("Unexpected error", err) + } + if wh.tlsConfig == nil { + t.Fatal("Expected tlsConfig to be set") + } + if wh.tlsConfig.MinVersion != tls.VersionTLS12 { + t.Errorf("Expected MinVersion from env to be TLS 1.2, got %d", wh.tlsConfig.MinVersion) + } + }) + + t.Run("opts min version takes precedence over env", func(t *testing.T) { + t.Setenv("WEBHOOK_TLS_MIN_VERSION", "1.2") + opts := newDefaultOptions() + opts.TLSMinVersion = tls.VersionTLS13 + wh, err := newAdmissionControllerWebhook(t, opts) + if err != nil { + t.Fatal("Unexpected error", err) + } + if wh.tlsConfig == nil { + t.Fatal("Expected tlsConfig to be set") + } + if wh.tlsConfig.MinVersion != tls.VersionTLS13 { + t.Errorf("Expected MinVersion from opts (TLS 1.3), got %d", wh.tlsConfig.MinVersion) + } + }) + + t.Run("env max version used when opts max version is zero", func(t *testing.T) { + t.Setenv("WEBHOOK_TLS_MAX_VERSION", "1.3") + opts := newDefaultOptions() + wh, err := newAdmissionControllerWebhook(t, opts) + if err != nil { + t.Fatal("Unexpected error", err) + } + if wh.tlsConfig == nil { + t.Fatal("Expected tlsConfig to be set") + } + if wh.tlsConfig.MaxVersion != tls.VersionTLS13 { + t.Errorf("Expected MaxVersion from env to be TLS 1.3, got %d", wh.tlsConfig.MaxVersion) + } + }) + + t.Run("opts max version takes precedence over env", func(t *testing.T) { + t.Setenv("WEBHOOK_TLS_MAX_VERSION", "1.2") + opts := newDefaultOptions() + opts.TLSMaxVersion = tls.VersionTLS13 + wh, err := newAdmissionControllerWebhook(t, opts) + if err != nil { + t.Fatal("Unexpected error", err) + } + if wh.tlsConfig == nil { + t.Fatal("Expected tlsConfig to be set") + } + if wh.tlsConfig.MaxVersion != tls.VersionTLS13 { + t.Errorf("Expected MaxVersion from opts (TLS 1.3), got %d", wh.tlsConfig.MaxVersion) + } + }) + + t.Run("env cipher suites used when opts cipher suites is nil", func(t *testing.T) { + t.Setenv("WEBHOOK_TLS_CIPHER_SUITES", "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256") + opts := newDefaultOptions() + wh, err := newAdmissionControllerWebhook(t, opts) + if err != nil { + t.Fatal("Unexpected error", err) + } + if wh.tlsConfig == nil { + t.Fatal("Expected tlsConfig to be set") + } + if len(wh.tlsConfig.CipherSuites) != 1 { + t.Fatalf("Expected 1 cipher suite from env, got %d", len(wh.tlsConfig.CipherSuites)) + } + if wh.tlsConfig.CipherSuites[0] != tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 { + t.Errorf("Expected CipherSuites from env, got %v", wh.tlsConfig.CipherSuites) + } + }) + + t.Run("opts cipher suites take precedence over env", func(t *testing.T) { + t.Setenv("WEBHOOK_TLS_CIPHER_SUITES", "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256") + opts := newDefaultOptions() + opts.TLSCipherSuites = []uint16{tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384} + wh, err := newAdmissionControllerWebhook(t, opts) + if err != nil { + t.Fatal("Unexpected error", err) + } + if wh.tlsConfig == nil { + t.Fatal("Expected tlsConfig to be set") + } + if len(wh.tlsConfig.CipherSuites) != 1 { + t.Fatalf("Expected 1 cipher suite from opts, got %d", len(wh.tlsConfig.CipherSuites)) + } + if wh.tlsConfig.CipherSuites[0] != tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 { + t.Errorf("Expected CipherSuites from opts, got %v", wh.tlsConfig.CipherSuites) + } + }) + + t.Run("env curve preferences used when opts curve preferences is nil", func(t *testing.T) { + t.Setenv("WEBHOOK_TLS_CURVE_PREFERENCES", "X25519,CurveP256") + opts := newDefaultOptions() + wh, err := newAdmissionControllerWebhook(t, opts) + if err != nil { + t.Fatal("Unexpected error", err) + } + if wh.tlsConfig == nil { + t.Fatal("Expected tlsConfig to be set") + } + if len(wh.tlsConfig.CurvePreferences) != 2 { + t.Fatalf("Expected 2 curve preferences from env, got %d", len(wh.tlsConfig.CurvePreferences)) + } + if wh.tlsConfig.CurvePreferences[0] != tls.X25519 { + t.Errorf("Expected CurvePreferences[0] = X25519, got %d", wh.tlsConfig.CurvePreferences[0]) + } + if wh.tlsConfig.CurvePreferences[1] != tls.CurveP256 { + t.Errorf("Expected CurvePreferences[1] = CurveP256, got %d", wh.tlsConfig.CurvePreferences[1]) + } + }) + + t.Run("opts curve preferences take precedence over env", func(t *testing.T) { + t.Setenv("WEBHOOK_TLS_CURVE_PREFERENCES", "X25519") + opts := newDefaultOptions() + opts.TLSCurvePreferences = []tls.CurveID{tls.CurveP384} + wh, err := newAdmissionControllerWebhook(t, opts) + if err != nil { + t.Fatal("Unexpected error", err) + } + if wh.tlsConfig == nil { + t.Fatal("Expected tlsConfig to be set") + } + if len(wh.tlsConfig.CurvePreferences) != 1 { + t.Fatalf("Expected 1 curve preference from opts, got %d", len(wh.tlsConfig.CurvePreferences)) + } + if wh.tlsConfig.CurvePreferences[0] != tls.CurveP384 { + t.Errorf("Expected CurvePreferences from opts, got %v", wh.tlsConfig.CurvePreferences) + } + }) + + t.Run("invalid env TLS config returns error", func(t *testing.T) { + t.Setenv("WEBHOOK_TLS_MIN_VERSION", "bad") + opts := newDefaultOptions() + _, err := newAdmissionControllerWebhook(t, opts) + if err == nil { + t.Fatal("Expected error for invalid env TLS min version") + } + }) +} + func TestTLSConfigCombinedOptions(t *testing.T) { opts := newDefaultOptions() t.Run("when all TLS options are configured together", func(t *testing.T) { From cd14421ec2ac9355ceb37a176280f741938fe4c9 Mon Sep 17 00:00:00 2001 From: Vincent Link Date: Wed, 4 Mar 2026 14:11:55 +0100 Subject: [PATCH 2/4] Replace NewConfigFromEnv with DefaultConfigFromEnv (#3328) DefaultConfigFromEnv replaces NewConfigFromEnv by returning a full default tls.Config with overrides from env vars. This avoids specifying e.g. the TLS MinVersion explicitely. --- tls/config.go | 34 ++++------------ tls/config_test.go | 65 ++++++++----------------------- webhook/env.go | 2 +- webhook/webhook.go | 96 ++++++++++++++++++++-------------------------- 4 files changed, 65 insertions(+), 132 deletions(-) diff --git a/tls/config.go b/tls/config.go index 4d07cbb5bb..6cd205baeb 100644 --- a/tls/config.go +++ b/tls/config.go @@ -33,22 +33,14 @@ const ( CurvePreferencesEnvKey = "TLS_CURVE_PREFERENCES" ) -// Config holds parsed TLS configuration values that can be used -// to build a *crypto/tls.Config. -type Config struct { - MinVersion uint16 - MaxVersion uint16 - CipherSuites []uint16 - CurvePreferences []cryptotls.CurveID -} - -// NewConfigFromEnv reads TLS configuration from environment variables and -// returns a Config. The prefix is prepended to each standard env-var suffix; +// DefaultConfigFromEnv returns a tls.Config with secure defaults. +// 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) { - var cfg Config +func DefaultConfigFromEnv(prefix string) (*cryptotls.Config, error) { + cfg := &cryptotls.Config{ + MinVersion: cryptotls.VersionTLS13, + } if v := os.Getenv(prefix + MinVersionEnvKey); v != "" { ver, err := parseVersion(v) @@ -82,19 +74,7 @@ func NewConfigFromEnv(prefix string) (*Config, error) { cfg.CurvePreferences = curves } - return &cfg, nil -} - -// TLSConfig constructs a *crypto/tls.Config from the parsed configuration. -// The caller typically adds additional fields such as GetCertificate. -func (c *Config) TLSConfig() *cryptotls.Config { - //nolint:gosec // Min version is caller-configurable; default is TLS 1.3. - return &cryptotls.Config{ - MinVersion: c.MinVersion, - MaxVersion: c.MaxVersion, - CipherSuites: c.CipherSuites, - CurvePreferences: c.CurvePreferences, - } + return cfg, nil } // parseVersion converts a TLS version string to the corresponding diff --git a/tls/config_test.go b/tls/config_test.go index b97e4044d5..3ee91da727 100644 --- a/tls/config_test.go +++ b/tls/config_test.go @@ -213,14 +213,14 @@ func Test_parseCurvePreferences(t *testing.T) { } } -func TestNewConfigFromEnv(t *testing.T) { - t.Run("no env vars set returns zero value", func(t *testing.T) { - cfg, err := NewConfigFromEnv("") +func TestDefaultConfigFromEnv(t *testing.T) { + t.Run("no env vars returns TLS 1.3 default", func(t *testing.T) { + cfg, err := DefaultConfigFromEnv("") if err != nil { t.Fatal("unexpected error:", err) } - if cfg.MinVersion != 0 { - t.Errorf("MinVersion = %d, want 0", cfg.MinVersion) + if cfg.MinVersion != cryptotls.VersionTLS13 { + t.Errorf("MinVersion = %d, want %d", cfg.MinVersion, cryptotls.VersionTLS13) } if cfg.MaxVersion != 0 { t.Errorf("MaxVersion = %d, want 0", cfg.MaxVersion) @@ -233,9 +233,9 @@ func TestNewConfigFromEnv(t *testing.T) { } }) - t.Run("min version from env", func(t *testing.T) { + t.Run("min version from env overrides default", func(t *testing.T) { t.Setenv(MinVersionEnvKey, "1.2") - cfg, err := NewConfigFromEnv("") + cfg, err := DefaultConfigFromEnv("") if err != nil { t.Fatal("unexpected error:", err) } @@ -246,7 +246,7 @@ func TestNewConfigFromEnv(t *testing.T) { t.Run("max version from env", func(t *testing.T) { t.Setenv(MaxVersionEnvKey, "1.3") - cfg, err := NewConfigFromEnv("") + cfg, err := DefaultConfigFromEnv("") if err != nil { t.Fatal("unexpected error:", err) } @@ -257,7 +257,7 @@ func TestNewConfigFromEnv(t *testing.T) { t.Run("cipher suites from env", func(t *testing.T) { t.Setenv(CipherSuitesEnvKey, "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256") - cfg, err := NewConfigFromEnv("") + cfg, err := DefaultConfigFromEnv("") if err != nil { t.Fatal("unexpected error:", err) } @@ -268,7 +268,7 @@ func TestNewConfigFromEnv(t *testing.T) { t.Run("curve preferences from env", func(t *testing.T) { t.Setenv(CurvePreferencesEnvKey, "X25519,CurveP256") - cfg, err := NewConfigFromEnv("") + cfg, err := DefaultConfigFromEnv("") if err != nil { t.Fatal("unexpected error:", err) } @@ -285,7 +285,7 @@ func TestNewConfigFromEnv(t *testing.T) { t.Run("prefix is prepended to env key", func(t *testing.T) { t.Setenv("WEBHOOK_TLS_MIN_VERSION", "1.2") - cfg, err := NewConfigFromEnv("WEBHOOK_") + cfg, err := DefaultConfigFromEnv("WEBHOOK_") if err != nil { t.Fatal("unexpected error:", err) } @@ -300,7 +300,7 @@ func TestNewConfigFromEnv(t *testing.T) { t.Setenv(CipherSuitesEnvKey, "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384") t.Setenv(CurvePreferencesEnvKey, "X25519,P-256") - cfg, err := NewConfigFromEnv("") + cfg, err := DefaultConfigFromEnv("") if err != nil { t.Fatal("unexpected error:", err) } @@ -320,7 +320,7 @@ func TestNewConfigFromEnv(t *testing.T) { t.Run("invalid min version", func(t *testing.T) { t.Setenv(MinVersionEnvKey, "1.0") - _, err := NewConfigFromEnv("") + _, err := DefaultConfigFromEnv("") if err == nil { t.Fatal("expected error for invalid min version") } @@ -328,7 +328,7 @@ func TestNewConfigFromEnv(t *testing.T) { t.Run("invalid max version", func(t *testing.T) { t.Setenv(MaxVersionEnvKey, "bad") - _, err := NewConfigFromEnv("") + _, err := DefaultConfigFromEnv("") if err == nil { t.Fatal("expected error for invalid max version") } @@ -336,7 +336,7 @@ func TestNewConfigFromEnv(t *testing.T) { t.Run("invalid cipher suite", func(t *testing.T) { t.Setenv(CipherSuitesEnvKey, "NOT_A_REAL_CIPHER") - _, err := NewConfigFromEnv("") + _, err := DefaultConfigFromEnv("") if err == nil { t.Fatal("expected error for invalid cipher suite") } @@ -344,42 +344,9 @@ func TestNewConfigFromEnv(t *testing.T) { t.Run("invalid curve", func(t *testing.T) { t.Setenv(CurvePreferencesEnvKey, "NotACurve") - _, err := NewConfigFromEnv("") + _, err := DefaultConfigFromEnv("") if err == nil { t.Fatal("expected error for invalid curve") } }) } - -func TestConfig_TLSConfig(t *testing.T) { - t.Setenv(MinVersionEnvKey, "1.2") - t.Setenv(MaxVersionEnvKey, "1.3") - t.Setenv(CipherSuitesEnvKey, "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256") - t.Setenv(CurvePreferencesEnvKey, "X25519,CurveP256") - - cfg, err := NewConfigFromEnv("") - if err != nil { - t.Fatal("unexpected error:", err) - } - - tc := cfg.TLSConfig() - - if tc.MinVersion != cryptotls.VersionTLS12 { - t.Errorf("MinVersion = %d, want %d", tc.MinVersion, cryptotls.VersionTLS12) - } - if tc.MaxVersion != cryptotls.VersionTLS13 { - t.Errorf("MaxVersion = %d, want %d", tc.MaxVersion, cryptotls.VersionTLS13) - } - if len(tc.CipherSuites) != 1 || tc.CipherSuites[0] != cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 { - t.Errorf("CipherSuites = %v, want [%d]", tc.CipherSuites, cryptotls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) - } - if len(tc.CurvePreferences) != 2 { - t.Fatalf("CurvePreferences has %d entries, want 2", len(tc.CurvePreferences)) - } - if tc.CurvePreferences[0] != cryptotls.X25519 { - t.Errorf("CurvePreferences[0] = %d, want %d", tc.CurvePreferences[0], cryptotls.X25519) - } - if tc.CurvePreferences[1] != cryptotls.CurveP256 { - t.Errorf("CurvePreferences[1] = %d, want %d", tc.CurvePreferences[1], cryptotls.CurveP256) - } -} diff --git a/webhook/env.go b/webhook/env.go index 1dd38585b6..0a9f0b8ddb 100644 --- a/webhook/env.go +++ b/webhook/env.go @@ -72,7 +72,7 @@ func SecretNameFromEnv(defaultSecretName string) string { return secret } -// Deprecated: Use knative.dev/pkg/tls.NewConfigFromEnv instead. +// Deprecated: Use knative.dev/pkg/tls.DefaultConfigFromEnv instead. // TLS configuration is now read automatically inside webhook.New via the shared tls package. func TLSMinVersionFromEnv(defaultTLSMinVersion uint16) uint16 { switch tlsMinVersion := os.Getenv(tlsMinVersionEnvKey); tlsMinVersion { diff --git a/webhook/webhook.go b/webhook/webhook.go index ed3c178482..b5166bb4bd 100644 --- a/webhook/webhook.go +++ b/webhook/webhook.go @@ -189,36 +189,29 @@ func New( logger := logging.FromContext(ctx) - tlsCfg, err := knativetls.NewConfigFromEnv("WEBHOOK_") + tlsCfg, err := knativetls.DefaultConfigFromEnv("WEBHOOK_") 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 - } + if opts.TLSMinVersion != 0 { + tlsCfg.MinVersion = opts.TLSMinVersion } - if opts.TLSMaxVersion == 0 && tlsCfg.MaxVersion != 0 { - opts.TLSMaxVersion = tlsCfg.MaxVersion + if opts.TLSMaxVersion != 0 { + tlsCfg.MaxVersion = opts.TLSMaxVersion } - if opts.TLSCipherSuites == nil && len(tlsCfg.CipherSuites) > 0 { - opts.TLSCipherSuites = tlsCfg.CipherSuites + if opts.TLSCipherSuites != nil { + tlsCfg.CipherSuites = opts.TLSCipherSuites } - if opts.TLSCurvePreferences == nil && len(tlsCfg.CurvePreferences) > 0 { - opts.TLSCurvePreferences = tlsCfg.CurvePreferences + if opts.TLSCurvePreferences != nil { + tlsCfg.CurvePreferences = opts.TLSCurvePreferences } - if opts.TLSMinVersion != 0 && opts.TLSMinVersion != tls.VersionTLS12 && opts.TLSMinVersion != tls.VersionTLS13 { - return nil, fmt.Errorf("unsupported TLS minimum version %d: must be TLS 1.2 or TLS 1.3", opts.TLSMinVersion) + if tlsCfg.MinVersion != tls.VersionTLS12 && tlsCfg.MinVersion != tls.VersionTLS13 { + return nil, fmt.Errorf("unsupported TLS minimum version %d: must be TLS 1.2 or TLS 1.3", tlsCfg.MinVersion) } - if opts.TLSMaxVersion != 0 && opts.TLSMinVersion > opts.TLSMaxVersion { - return nil, fmt.Errorf("TLS minimum version (%#x) is greater than maximum version (%#x)", opts.TLSMinVersion, opts.TLSMaxVersion) + if tlsCfg.MaxVersion != 0 && tlsCfg.MinVersion > tlsCfg.MaxVersion { + return nil, fmt.Errorf("TLS minimum version (%#x) is greater than maximum version (%#x)", tlsCfg.MinVersion, tlsCfg.MaxVersion) } syncCtx, cancel := context.WithCancel(context.Background()) @@ -238,42 +231,35 @@ func New( // a new secret informer from it. secretInformer := kubeinformerfactory.Get(ctx).Core().V1().Secrets() - //nolint:gosec // operator configures TLS min version (default is 1.3) - webhook.tlsConfig = &tls.Config{ - MinVersion: opts.TLSMinVersion, - MaxVersion: opts.TLSMaxVersion, - CipherSuites: opts.TLSCipherSuites, - CurvePreferences: opts.TLSCurvePreferences, - - // If we return (nil, error) the client sees - 'tls: internal error" - // If we return (nil, nil) the client sees - 'tls: no certificates configured' - // - // We'll return (nil, nil) when we don't find a certificate - GetCertificate: func(*tls.ClientHelloInfo) (*tls.Certificate, error) { - secret, err := secretInformer.Lister().Secrets(system.Namespace()).Get(opts.SecretName) - if err != nil { - logger.Errorw("failed to fetch secret", zap.Error(err)) - return nil, nil - } - webOpts := GetOptions(ctx) - sKey, sCert := getSecretDataKeyNamesOrDefault(webOpts.ServerPrivateKeyName, webOpts.ServerCertificateName) - serverKey, ok := secret.Data[sKey] - if !ok { - logger.Warn("server key missing") - return nil, nil - } - serverCert, ok := secret.Data[sCert] - if !ok { - logger.Warn("server cert missing") - return nil, nil - } - cert, err := tls.X509KeyPair(serverCert, serverKey) - if err != nil { - return nil, err - } - return &cert, nil - }, + // If we return (nil, error) the client sees - 'tls: internal error' + // If we return (nil, nil) the client sees - 'tls: no certificates configured' + // + // We'll return (nil, nil) when we don't find a certificate + tlsCfg.GetCertificate = func(*tls.ClientHelloInfo) (*tls.Certificate, error) { + secret, err := secretInformer.Lister().Secrets(system.Namespace()).Get(opts.SecretName) + if err != nil { + logger.Errorw("failed to fetch secret", zap.Error(err)) + return nil, nil + } + webOpts := GetOptions(ctx) + sKey, sCert := getSecretDataKeyNamesOrDefault(webOpts.ServerPrivateKeyName, webOpts.ServerCertificateName) + serverKey, ok := secret.Data[sKey] + if !ok { + logger.Warn("server key missing") + return nil, nil + } + serverCert, ok := secret.Data[sCert] + if !ok { + logger.Warn("server cert missing") + return nil, nil + } + cert, err := tls.X509KeyPair(serverCert, serverKey) + if err != nil { + return nil, err + } + return &cert, nil } + webhook.tlsConfig = tlsCfg } webhook.mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { From af701907f62f487340a8860e957c0038b85fe3ae Mon Sep 17 00:00:00 2001 From: Mike Fedosin Date: Mon, 9 Mar 2026 15:04:46 +0100 Subject: [PATCH 3/4] Move tls package to network/tls, keep aliases for backward compatibility (#3331) The TLS configuration package is moved from tls/ to network/tls/ to co-locate it with the rest of the networking code. The old tls/ package now re-exports all public symbols as deprecated aliases so that existing consumers continue to compile without changes. The webhook package is updated to import from the new location directly. Signed-off-by: Mikhail Fedosin --- network/depcheck_test.go | 1 + network/tls/config.go | 156 ++++++++++++++++++++++++++++ {tls => network/tls}/config_test.go | 0 tls/config.go | 145 +++----------------------- webhook/env.go | 2 +- webhook/webhook.go | 2 +- 6 files changed, 173 insertions(+), 133 deletions(-) create mode 100644 network/tls/config.go rename {tls => network/tls}/config_test.go (100%) diff --git a/network/depcheck_test.go b/network/depcheck_test.go index aeb905c02d..19865e460d 100644 --- a/network/depcheck_test.go +++ b/network/depcheck_test.go @@ -26,5 +26,6 @@ func TestNoDeps(t *testing.T) { depcheck.AssertNoDependency(t, map[string][]string{ "knative.dev/pkg/network": depcheck.KnownHeavyDependencies, "knative.dev/pkg/network/handlers": depcheck.KnownHeavyDependencies, + "knative.dev/pkg/network/tls": depcheck.KnownHeavyDependencies, }) } diff --git a/network/tls/config.go b/network/tls/config.go new file mode 100644 index 0000000000..6cd205baeb --- /dev/null +++ b/network/tls/config.go @@ -0,0 +1,156 @@ +/* +Copyright 2026 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tls + +import ( + cryptotls "crypto/tls" + "fmt" + "os" + "strings" +) + +// 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. +const ( + MinVersionEnvKey = "TLS_MIN_VERSION" + MaxVersionEnvKey = "TLS_MAX_VERSION" + CipherSuitesEnvKey = "TLS_CIPHER_SUITES" + CurvePreferencesEnvKey = "TLS_CURVE_PREFERENCES" +) + +// DefaultConfigFromEnv returns a tls.Config with secure defaults. +// 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. +func DefaultConfigFromEnv(prefix string) (*cryptotls.Config, error) { + cfg := &cryptotls.Config{ + MinVersion: cryptotls.VersionTLS13, + } + + if v := os.Getenv(prefix + MinVersionEnvKey); v != "" { + ver, err := parseVersion(v) + if err != nil { + return nil, fmt.Errorf("invalid %s%s %q: %w", prefix, MinVersionEnvKey, v, err) + } + cfg.MinVersion = ver + } + + if v := os.Getenv(prefix + MaxVersionEnvKey); v != "" { + ver, err := parseVersion(v) + if err != nil { + return nil, fmt.Errorf("invalid %s%s %q: %w", prefix, MaxVersionEnvKey, v, err) + } + cfg.MaxVersion = ver + } + + if v := os.Getenv(prefix + CipherSuitesEnvKey); v != "" { + suites, err := parseCipherSuites(v) + if err != nil { + return nil, fmt.Errorf("invalid %s%s: %w", prefix, CipherSuitesEnvKey, err) + } + cfg.CipherSuites = suites + } + + if v := os.Getenv(prefix + CurvePreferencesEnvKey); v != "" { + curves, err := parseCurvePreferences(v) + if err != nil { + return nil, fmt.Errorf("invalid %s%s: %w", prefix, CurvePreferencesEnvKey, err) + } + cfg.CurvePreferences = curves + } + + return cfg, nil +} + +// parseVersion converts a TLS version string to the corresponding +// crypto/tls constant. Accepted values are "1.2" and "1.3". +func parseVersion(v string) (uint16, error) { + switch v { + case "1.2": + return cryptotls.VersionTLS12, nil + case "1.3": + return cryptotls.VersionTLS13, nil + default: + return 0, fmt.Errorf("unsupported TLS version %q: must be %q or %q", v, "1.2", "1.3") + } +} + +// parseCipherSuites parses a comma-separated list of TLS cipher-suite names +// (e.g. "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384") +// into a slice of cipher-suite IDs. Names must match those returned by +// crypto/tls.CipherSuiteName. +func parseCipherSuites(s string) ([]uint16, error) { + lookup := cipherSuiteLookup() + parts := strings.Split(s, ",") + suites := make([]uint16, 0, len(parts)) + + for _, name := range parts { + name = strings.TrimSpace(name) + if name == "" { + continue + } + id, ok := lookup[name] + if !ok { + return nil, fmt.Errorf("unknown cipher suite %q", name) + } + suites = append(suites, id) + } + + return suites, nil +} + +// parseCurvePreferences parses a comma-separated list of elliptic-curve names +// (e.g. "X25519,CurveP256") into a slice of crypto/tls.CurveID values. +// Both Go constant names (CurveP256) and standard names (P-256) are accepted. +func parseCurvePreferences(s string) ([]cryptotls.CurveID, error) { + parts := strings.Split(s, ",") + curves := make([]cryptotls.CurveID, 0, len(parts)) + + for _, name := range parts { + name = strings.TrimSpace(name) + if name == "" { + continue + } + id, ok := curvesByName[name] + if !ok { + return nil, fmt.Errorf("unknown curve %q", name) + } + curves = append(curves, id) + } + + return curves, nil +} + +func cipherSuiteLookup() map[string]uint16 { + m := make(map[string]uint16) + for _, cs := range cryptotls.CipherSuites() { + m[cs.Name] = cs.ID + } + return m +} + +var curvesByName = map[string]cryptotls.CurveID{ + "CurveP256": cryptotls.CurveP256, + "CurveP384": cryptotls.CurveP384, + "CurveP521": cryptotls.CurveP521, + "X25519": cryptotls.X25519, + "X25519MLKEM768": cryptotls.X25519MLKEM768, + "P-256": cryptotls.CurveP256, + "P-384": cryptotls.CurveP384, + "P-521": cryptotls.CurveP521, +} diff --git a/tls/config_test.go b/network/tls/config_test.go similarity index 100% rename from tls/config_test.go rename to network/tls/config_test.go diff --git a/tls/config.go b/tls/config.go index 6cd205baeb..e4d975e018 100644 --- a/tls/config.go +++ b/tls/config.go @@ -14,143 +14,26 @@ See the License for the specific language governing permissions and limitations under the License. */ +// Deprecated: Use knative.dev/pkg/network/tls instead. +// This package is kept for backward compatibility and re-exports all +// public symbols from knative.dev/pkg/network/tls. package tls import ( - cryptotls "crypto/tls" - "fmt" - "os" - "strings" + networktls "knative.dev/pkg/network/tls" ) -// 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. -const ( - MinVersionEnvKey = "TLS_MIN_VERSION" - MaxVersionEnvKey = "TLS_MAX_VERSION" - CipherSuitesEnvKey = "TLS_CIPHER_SUITES" - CurvePreferencesEnvKey = "TLS_CURVE_PREFERENCES" -) - -// DefaultConfigFromEnv returns a tls.Config with secure defaults. -// 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. -func DefaultConfigFromEnv(prefix string) (*cryptotls.Config, error) { - cfg := &cryptotls.Config{ - MinVersion: cryptotls.VersionTLS13, - } - - if v := os.Getenv(prefix + MinVersionEnvKey); v != "" { - ver, err := parseVersion(v) - if err != nil { - return nil, fmt.Errorf("invalid %s%s %q: %w", prefix, MinVersionEnvKey, v, err) - } - cfg.MinVersion = ver - } - - if v := os.Getenv(prefix + MaxVersionEnvKey); v != "" { - ver, err := parseVersion(v) - if err != nil { - return nil, fmt.Errorf("invalid %s%s %q: %w", prefix, MaxVersionEnvKey, v, err) - } - cfg.MaxVersion = ver - } - - if v := os.Getenv(prefix + CipherSuitesEnvKey); v != "" { - suites, err := parseCipherSuites(v) - if err != nil { - return nil, fmt.Errorf("invalid %s%s: %w", prefix, CipherSuitesEnvKey, err) - } - cfg.CipherSuites = suites - } - - if v := os.Getenv(prefix + CurvePreferencesEnvKey); v != "" { - curves, err := parseCurvePreferences(v) - if err != nil { - return nil, fmt.Errorf("invalid %s%s: %w", prefix, CurvePreferencesEnvKey, err) - } - cfg.CurvePreferences = curves - } - - return cfg, nil -} - -// parseVersion converts a TLS version string to the corresponding -// crypto/tls constant. Accepted values are "1.2" and "1.3". -func parseVersion(v string) (uint16, error) { - switch v { - case "1.2": - return cryptotls.VersionTLS12, nil - case "1.3": - return cryptotls.VersionTLS13, nil - default: - return 0, fmt.Errorf("unsupported TLS version %q: must be %q or %q", v, "1.2", "1.3") - } -} - -// parseCipherSuites parses a comma-separated list of TLS cipher-suite names -// (e.g. "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384") -// into a slice of cipher-suite IDs. Names must match those returned by -// crypto/tls.CipherSuiteName. -func parseCipherSuites(s string) ([]uint16, error) { - lookup := cipherSuiteLookup() - parts := strings.Split(s, ",") - suites := make([]uint16, 0, len(parts)) - - for _, name := range parts { - name = strings.TrimSpace(name) - if name == "" { - continue - } - id, ok := lookup[name] - if !ok { - return nil, fmt.Errorf("unknown cipher suite %q", name) - } - suites = append(suites, id) - } - - return suites, nil -} - -// parseCurvePreferences parses a comma-separated list of elliptic-curve names -// (e.g. "X25519,CurveP256") into a slice of crypto/tls.CurveID values. -// Both Go constant names (CurveP256) and standard names (P-256) are accepted. -func parseCurvePreferences(s string) ([]cryptotls.CurveID, error) { - parts := strings.Split(s, ",") - curves := make([]cryptotls.CurveID, 0, len(parts)) +// Deprecated: Use knative.dev/pkg/network/tls.MinVersionEnvKey instead. +const MinVersionEnvKey = networktls.MinVersionEnvKey - for _, name := range parts { - name = strings.TrimSpace(name) - if name == "" { - continue - } - id, ok := curvesByName[name] - if !ok { - return nil, fmt.Errorf("unknown curve %q", name) - } - curves = append(curves, id) - } +// Deprecated: Use knative.dev/pkg/network/tls.MaxVersionEnvKey instead. +const MaxVersionEnvKey = networktls.MaxVersionEnvKey - return curves, nil -} +// Deprecated: Use knative.dev/pkg/network/tls.CipherSuitesEnvKey instead. +const CipherSuitesEnvKey = networktls.CipherSuitesEnvKey -func cipherSuiteLookup() map[string]uint16 { - m := make(map[string]uint16) - for _, cs := range cryptotls.CipherSuites() { - m[cs.Name] = cs.ID - } - return m -} +// Deprecated: Use knative.dev/pkg/network/tls.CurvePreferencesEnvKey instead. +const CurvePreferencesEnvKey = networktls.CurvePreferencesEnvKey -var curvesByName = map[string]cryptotls.CurveID{ - "CurveP256": cryptotls.CurveP256, - "CurveP384": cryptotls.CurveP384, - "CurveP521": cryptotls.CurveP521, - "X25519": cryptotls.X25519, - "X25519MLKEM768": cryptotls.X25519MLKEM768, - "P-256": cryptotls.CurveP256, - "P-384": cryptotls.CurveP384, - "P-521": cryptotls.CurveP521, -} +// Deprecated: Use knative.dev/pkg/network/tls.DefaultConfigFromEnv instead. +var DefaultConfigFromEnv = networktls.DefaultConfigFromEnv diff --git a/webhook/env.go b/webhook/env.go index 0a9f0b8ddb..6d3d32203f 100644 --- a/webhook/env.go +++ b/webhook/env.go @@ -72,7 +72,7 @@ func SecretNameFromEnv(defaultSecretName string) string { return secret } -// Deprecated: Use knative.dev/pkg/tls.DefaultConfigFromEnv instead. +// Deprecated: Use knative.dev/pkg/network/tls.DefaultConfigFromEnv instead. // TLS configuration is now read automatically inside webhook.New via the shared tls package. func TLSMinVersionFromEnv(defaultTLSMinVersion uint16) uint16 { switch tlsMinVersion := os.Getenv(tlsMinVersionEnvKey); tlsMinVersion { diff --git a/webhook/webhook.go b/webhook/webhook.go index b5166bb4bd..e8895db75e 100644 --- a/webhook/webhook.go +++ b/webhook/webhook.go @@ -33,7 +33,7 @@ import ( kubeinformerfactory "knative.dev/pkg/injection/clients/namespacedkube/informers/factory" "knative.dev/pkg/network" "knative.dev/pkg/network/handlers" - knativetls "knative.dev/pkg/tls" + knativetls "knative.dev/pkg/network/tls" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "go.opentelemetry.io/otel/metric" From 1415e5550fa5ee1feafedd243189d363bb2a31ff Mon Sep 17 00:00:00 2001 From: Dave Protasowski Date: Sat, 14 Mar 2026 08:45:21 -0400 Subject: [PATCH 4/4] remove deprecated TLS package (#3333) --- tls/config.go | 39 --------------------------------------- 1 file changed, 39 deletions(-) delete mode 100644 tls/config.go diff --git a/tls/config.go b/tls/config.go deleted file mode 100644 index e4d975e018..0000000000 --- a/tls/config.go +++ /dev/null @@ -1,39 +0,0 @@ -/* -Copyright 2026 The Knative Authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// Deprecated: Use knative.dev/pkg/network/tls instead. -// This package is kept for backward compatibility and re-exports all -// public symbols from knative.dev/pkg/network/tls. -package tls - -import ( - networktls "knative.dev/pkg/network/tls" -) - -// Deprecated: Use knative.dev/pkg/network/tls.MinVersionEnvKey instead. -const MinVersionEnvKey = networktls.MinVersionEnvKey - -// Deprecated: Use knative.dev/pkg/network/tls.MaxVersionEnvKey instead. -const MaxVersionEnvKey = networktls.MaxVersionEnvKey - -// Deprecated: Use knative.dev/pkg/network/tls.CipherSuitesEnvKey instead. -const CipherSuitesEnvKey = networktls.CipherSuitesEnvKey - -// Deprecated: Use knative.dev/pkg/network/tls.CurvePreferencesEnvKey instead. -const CurvePreferencesEnvKey = networktls.CurvePreferencesEnvKey - -// Deprecated: Use knative.dev/pkg/network/tls.DefaultConfigFromEnv instead. -var DefaultConfigFromEnv = networktls.DefaultConfigFromEnv