Skip to content

Commit a254eac

Browse files
cappyzawacappyzawa
committed
runtime/secrets: remove ServerName pinning from TLS config
Remove ServerName pinning functionality that can cause TLS verification failures in production environments with redirects, proxies, and multi-host scenarios. The Go standard library automatically handles SNI and hostname verification based on the actual connection target, providing better compatibility and security than fixed ServerName values. Signed-off-by: cappyzawa <cappyzawa@gmail.com>
1 parent 6bf77f0 commit a254eac

File tree

5 files changed

+24
-92
lines changed

5 files changed

+24
-92
lines changed

‎runtime/secrets/converter.go

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,7 @@ func WithSystemCertPool() TLSConfigOption {
5757
// Multiple authentication methods can be present in a single secret and will be extracted
5858
// simultaneously, enabling use cases like Basic Auth + TLS or Bearer Token + TLS.
5959
//
60-
// Options can be provided to configure TLS extraction behavior. Use WithTargetURL() to specify
61-
// target URL for complete TLS configuration.
60+
// Options can be provided to configure TLS extraction behavior.
6261
func AuthMethodsFromSecret(ctx context.Context, secret *corev1.Secret, opts ...AuthMethodsOption) (*AuthMethods, error) {
6362
config := &authMethodsConfig{}
6463
for _, opt := range opts {
@@ -88,9 +87,7 @@ func AuthMethodsFromSecret(ctx context.Context, secret *corev1.Secret, opts ...A
8887
}
8988

9089
if err := trySetAuth(ctx, secret, &methods.TLS, func(ctx context.Context, secret *corev1.Secret) (*tls.Config, error) {
91-
// targetURL is empty here but will be set by TLSConfigOption if WithTargetURL was specified
92-
const targetURL = ""
93-
return TLSConfigFromSecret(ctx, secret, targetURL, config.tlsConfigOpts...)
90+
return TLSConfigFromSecret(ctx, secret, config.tlsConfigOpts...)
9491
}); err != nil {
9592
return nil, err
9693
}
@@ -106,15 +103,10 @@ func AuthMethodsFromSecret(ctx context.Context, secret *corev1.Secret, opts ...A
106103
//
107104
// Standard field names always take precedence over legacy ones.
108105
//
109-
// The targetURL parameter is used to set the ServerName for proper SNI support
110-
// in virtual hosting environments.
111-
//
112106
// Optional TLSConfigOption parameters can be used to configure CA certificate handling:
113107
// - WithSystemCertPool(): Include system certificates in addition to user-provided CA
114-
func TLSConfigFromSecret(ctx context.Context, secret *corev1.Secret, targetURL string, opts ...TLSConfigOption) (*tls.Config, error) {
115-
config := &tlsConfig{
116-
targetURL: targetURL,
117-
}
108+
func TLSConfigFromSecret(ctx context.Context, secret *corev1.Secret, opts ...TLSConfigOption) (*tls.Config, error) {
109+
config := &tlsConfig{}
118110
for _, opt := range opts {
119111
opt(config)
120112
}
@@ -317,17 +309,6 @@ func buildTLSConfig(certData *tlsCertificateData, config *tlsConfig) (*tls.Confi
317309
InsecureSkipVerify: false,
318310
}
319311

320-
// Set ServerName for proper SNI support if targetURL is provided
321-
if config.targetURL != "" {
322-
u, err := url.Parse(config.targetURL)
323-
if err != nil {
324-
return nil, fmt.Errorf("failed to parse target URL '%s': %w", config.targetURL, err)
325-
}
326-
if hostname := u.Hostname(); hostname != "" {
327-
tlsConfig.ServerName = hostname
328-
}
329-
}
330-
331312
if certData.hasCertPair() {
332313
cert, err := tls.X509KeyPair(certData.cert, certData.key)
333314
if err != nil {

‎runtime/secrets/converter_test.go

Lines changed: 10 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ func TestAuthMethodsFromSecret(t *testing.T) {
227227
secretData: map[string][]byte{
228228
secrets.KeyCACert: validCACert,
229229
},
230-
opt: []secrets.AuthMethodsOption{secrets.WithTargetURL("https://example.com"), secrets.WithTLSSystemCertPool()},
230+
opt: []secrets.AuthMethodsOption{secrets.WithTLSSystemCertPool()},
231231
wantTLS: true,
232232
},
233233
{
@@ -236,7 +236,7 @@ func TestAuthMethodsFromSecret(t *testing.T) {
236236
secrets.KeyUsername: []byte("testuser"),
237237
secrets.KeyPassword: []byte("testpass"),
238238
},
239-
opt: []secrets.AuthMethodsOption{secrets.WithTargetURL("https://example.com"), secrets.WithTLSSystemCertPool()},
239+
opt: []secrets.AuthMethodsOption{secrets.WithTLSSystemCertPool()},
240240
wantBasic: true,
241241
wantTLS: false,
242242
},
@@ -443,13 +443,11 @@ func TestTLSConfigFromSecret(t *testing.T) {
443443
caCert, tlsCert, tlsKey := generateTestCertificates(t)
444444

445445
tests := []struct {
446-
name string
447-
secret *corev1.Secret
448-
targetURL string
449-
opts []secrets.TLSConfigOption
450-
expectedServerName string
451-
errMsg string
452-
expectedFields map[string]string // legacy key -> preferred key mapping
446+
name string
447+
secret *corev1.Secret
448+
opts []secrets.TLSConfigOption
449+
errMsg string
450+
expectedFields map[string]string // legacy key -> preferred key mapping
453451
}{
454452
{
455453
name: "valid TLS secret with standard fields",
@@ -576,28 +574,6 @@ func TestTLSConfigFromSecret(t *testing.T) {
576574
),
577575
errMsg: "secret 'default/tls-secret' must contain either 'ca.crt' or both 'tls.crt' and 'tls.key'",
578576
},
579-
{
580-
name: "targetURL parameter",
581-
secret: testSecret(
582-
withName("tls-secret"),
583-
withData(map[string][]byte{
584-
secrets.KeyCACert: caCert,
585-
}),
586-
),
587-
targetURL: "https://example.com:8443",
588-
expectedServerName: "example.com",
589-
},
590-
{
591-
name: "invalid target URL",
592-
secret: testSecret(
593-
withName("tls-secret"),
594-
withData(map[string][]byte{
595-
secrets.KeyCACert: caCert,
596-
}),
597-
),
598-
targetURL: "://invalid-url",
599-
errMsg: "failed to parse target URL '://invalid-url'",
600-
},
601577
{
602578
name: "WithSystemCertPool option",
603579
secret: testSecret(
@@ -630,15 +606,16 @@ func TestTLSConfigFromSecret(t *testing.T) {
630606
ctx = log.IntoContext(ctx, logr.Discard())
631607
}
632608

633-
tlsConfig, err := secrets.TLSConfigFromSecret(ctx, tt.secret, tt.targetURL, tt.opts...)
609+
tlsConfig, err := secrets.TLSConfigFromSecret(ctx, tt.secret, tt.opts...)
634610

635611
if tt.errMsg != "" {
636612
g.Expect(err).To(MatchError(ContainSubstring(tt.errMsg)))
637613
} else {
638614
g.Expect(err).ToNot(HaveOccurred())
639615
g.Expect(tlsConfig).ToNot(BeNil())
640616

641-
g.Expect(tlsConfig.ServerName).To(Equal(tt.expectedServerName))
617+
// ServerName should be empty to allow automatic hostname verification
618+
g.Expect(tlsConfig.ServerName).To(BeEmpty())
642619
// InsecureSkipVerify must always be false per Flux security policy.
643620
// The insecure parameter was removed to prevent bypassing certificate validation.
644621
g.Expect(tlsConfig.InsecureSkipVerify).To(BeFalse())

‎runtime/secrets/reader.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,17 +33,14 @@ import (
3333
// The function fetches the secret from the API server and then processes it using
3434
// TLSConfigFromSecret. It supports the same field names and legacy field handling.
3535
//
36-
// The targetURL parameter is used to set the ServerName for proper SNI support
37-
// in virtual hosting environments.
38-
//
3936
// Optional TLSConfigOption parameters can be used to configure CA certificate handling:
4037
// - WithSystemCertPool(): Include system certificates in addition to user-provided CA
41-
func TLSConfigFromSecretRef(ctx context.Context, c client.Client, secretRef types.NamespacedName, targetURL string, opts ...TLSConfigOption) (*tls.Config, error) {
38+
func TLSConfigFromSecretRef(ctx context.Context, c client.Client, secretRef types.NamespacedName, opts ...TLSConfigOption) (*tls.Config, error) {
4239
secret, err := getSecret(ctx, c, secretRef)
4340
if err != nil {
4441
return nil, err
4542
}
46-
return TLSConfigFromSecret(ctx, secret, targetURL, opts...)
43+
return TLSConfigFromSecret(ctx, secret, opts...)
4744
}
4845

4946
// ProxyURLFromSecretRef creates a proxy URL from a Kubernetes secret reference.

‎runtime/secrets/reader_test.go

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,11 @@ func TestTLSConfigFromSecretRef(t *testing.T) {
3838
caCert, tlsCert, tlsKey := generateTestCertificates(t)
3939

4040
tests := []struct {
41-
name string
42-
secretRef types.NamespacedName
43-
secret *corev1.Secret // Secret to add to fake client (nil = not added)
44-
targetURL string
45-
opts []secrets.TLSConfigOption
46-
expectedServerName string
47-
errMsg string
41+
name string
42+
secretRef types.NamespacedName
43+
secret *corev1.Secret // Secret to add to fake client (nil = not added)
44+
opts []secrets.TLSConfigOption
45+
errMsg string
4846
}{
4947
{
5048
name: "integration test - basic TLS secret functionality",
@@ -63,18 +61,6 @@ func TestTLSConfigFromSecretRef(t *testing.T) {
6361
secretRef: types.NamespacedName{Name: "missing-secret", Namespace: testNS},
6462
errMsg: "secret 'default/missing-secret' not found",
6563
},
66-
{
67-
name: "TLS secret with parameters",
68-
secretRef: types.NamespacedName{Name: "tls-secret", Namespace: testNS},
69-
secret: testSecret(
70-
withName("tls-secret"),
71-
withData(map[string][]byte{
72-
secrets.KeyCACert: caCert,
73-
}),
74-
),
75-
targetURL: "https://example.com",
76-
expectedServerName: "example.com",
77-
},
7864
{
7965
name: "TLS secret with WithSystemCertPool option",
8066
secretRef: types.NamespacedName{Name: "tls-secret", Namespace: testNS},
@@ -103,15 +89,16 @@ func TestTLSConfigFromSecretRef(t *testing.T) {
10389
}
10490
c := fakeClient(objects...)
10591

106-
tlsConfig, err := secrets.TLSConfigFromSecretRef(ctx, c, tt.secretRef, tt.targetURL, tt.opts...)
92+
tlsConfig, err := secrets.TLSConfigFromSecretRef(ctx, c, tt.secretRef, tt.opts...)
10793

10894
if tt.errMsg != "" {
10995
g.Expect(err).To(MatchError(ContainSubstring(tt.errMsg)))
11096
} else {
11197
g.Expect(err).ToNot(HaveOccurred())
11298
g.Expect(tlsConfig).ToNot(BeNil())
11399

114-
g.Expect(tlsConfig.ServerName).To(Equal(tt.expectedServerName))
100+
// ServerName should be empty to allow automatic hostname verification
101+
g.Expect(tlsConfig.ServerName).To(BeEmpty())
115102
// InsecureSkipVerify must always be false per Flux security policy.
116103
// The insecure parameter was removed to prevent bypassing certificate validation.
117104
g.Expect(tlsConfig.InsecureSkipVerify).To(BeFalse())

‎runtime/secrets/secrets.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -121,19 +121,9 @@ type authMethodsConfig struct {
121121

122122
// tlsConfig holds TLS-specific configuration options.
123123
type tlsConfig struct {
124-
targetURL string
125124
useSystemCertPool bool
126125
}
127126

128-
// WithTargetURL configures TLS extraction with the specified target URL for ServerName configuration.
129-
func WithTargetURL(targetURL string) AuthMethodsOption {
130-
return func(cfg *authMethodsConfig) {
131-
cfg.tlsConfigOpts = append(cfg.tlsConfigOpts, func(c *tlsConfig) {
132-
c.targetURL = targetURL
133-
})
134-
}
135-
}
136-
137127
// WithTLSSystemCertPool enables the use of system certificate pool in addition to user-provided CA certificates.
138128
func WithTLSSystemCertPool() AuthMethodsOption {
139129
return func(cfg *authMethodsConfig) {

0 commit comments

Comments
 (0)