Skip to content

Commit 683719d

Browse files
committed
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 <[email protected]>
1 parent cd5eebf commit 683719d

File tree

8 files changed

+11
-59
lines changed

8 files changed

+11
-59
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ require (
3838
github.com/fluxcd/pkg/lockedfile v0.6.0
3939
github.com/fluxcd/pkg/masktoken v0.7.0
4040
github.com/fluxcd/pkg/oci v0.52.0
41-
github.com/fluxcd/pkg/runtime v0.79.0
41+
github.com/fluxcd/pkg/runtime v0.80.0
4242
github.com/fluxcd/pkg/sourceignore v0.13.0
4343
github.com/fluxcd/pkg/ssh v0.20.0
4444
github.com/fluxcd/pkg/tar v0.13.0

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,8 +398,8 @@ github.com/fluxcd/pkg/masktoken v0.7.0 h1:pitmyOg2pUVdW+nn2Lk/xqm2TaA08uxvOC0ns3
398398
github.com/fluxcd/pkg/masktoken v0.7.0/go.mod h1:Lc1uoDjO1GY6+YdkK+ZqqBIBWquyV58nlSJ5S1N1IYU=
399399
github.com/fluxcd/pkg/oci v0.52.0 h1:rkHMtXYm21MtDrjNcR5KScqOe6C1JHPExoShuVdNm8M=
400400
github.com/fluxcd/pkg/oci v0.52.0/go.mod h1:5J6IhHoDVYCVeBEC+4E3nPeKh7d0kjJ8IEL6NVCiTx4=
401-
github.com/fluxcd/pkg/runtime v0.79.0 h1:9tv79EiQDx/QJH9mYDd9kZ9WybCVWBUGoiBHij+eKkc=
402-
github.com/fluxcd/pkg/runtime v0.79.0/go.mod h1:iGhdaEq+lMJQTJNAFEPOU4gUJ7kt3yeDcJPZy7O9IUw=
401+
github.com/fluxcd/pkg/runtime v0.80.0 h1:vknT2vdQSGTFnAhz4xGk2ZXUWCrXh3whsISStgA57Go=
402+
github.com/fluxcd/pkg/runtime v0.80.0/go.mod h1:iGhdaEq+lMJQTJNAFEPOU4gUJ7kt3yeDcJPZy7O9IUw=
403403
github.com/fluxcd/pkg/sourceignore v0.13.0 h1:ZvkzX2WsmyZK9cjlqOFFW1onHVzhPZIqDbCh96rPqbU=
404404
github.com/fluxcd/pkg/sourceignore v0.13.0/go.mod h1:Z9H1GoBx0ljOhptnzoV0PL6Nd/UzwKcSphP27lqb4xI=
405405
github.com/fluxcd/pkg/ssh v0.20.0 h1:Ak0laIYIc/L8lEfqls/LDWRW8wYPESGaravQsCRGLb8=

internal/controller/bucket_controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,7 @@ func (r *BucketReconciler) setupCredentials(ctx context.Context, obj *sourcev1.B
831831
Namespace: obj.GetNamespace(),
832832
Name: obj.Spec.CertSecretRef.Name,
833833
}
834-
tlsConfig, err = secrets.TLSConfigFromSecretRef(ctx, r.Client, secretRef, obj.Spec.Endpoint, secrets.WithSystemCertPool())
834+
tlsConfig, err = secrets.TLSConfigFromSecretRef(ctx, r.Client, secretRef, secrets.WithSystemCertPool())
835835
if err != nil {
836836
return nil, fmt.Errorf("failed to get TLS config: %w", err)
837837
}
@@ -842,7 +842,7 @@ func (r *BucketReconciler) setupCredentials(ctx context.Context, obj *sourcev1.B
842842
Namespace: obj.GetNamespace(),
843843
Name: obj.Spec.STS.CertSecretRef.Name,
844844
}
845-
stsTLSConfig, err = secrets.TLSConfigFromSecretRef(ctx, r.Client, secretRef, obj.Spec.STS.Endpoint, secrets.WithSystemCertPool())
845+
stsTLSConfig, err = secrets.TLSConfigFromSecretRef(ctx, r.Client, secretRef, secrets.WithSystemCertPool())
846846
if err != nil {
847847
return nil, fmt.Errorf("failed to get STS TLS config: %w", err)
848848
}

internal/controller/gitrepository_controller.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -689,8 +689,7 @@ func (r *GitRepositoryReconciler) getAuthOpts(ctx context.Context, obj *sourcev1
689689
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e)
690690
return nil, e
691691
}
692-
targetURL := fmt.Sprintf("%s://%s", u.Scheme, u.Host)
693-
authMethods, err := secrets.AuthMethodsFromSecret(ctx, secret, secrets.WithTargetURL(targetURL), secrets.WithTLSSystemCertPool())
692+
authMethods, err := secrets.AuthMethodsFromSecret(ctx, secret, secrets.WithTLSSystemCertPool())
694693
if err != nil {
695694
return nil, err
696695
}

internal/controller/helmrepository_controller_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
474474
repoURL, err := repository.NormalizeURL(serverURL)
475475
t.Expect(err).ToNot(HaveOccurred())
476476

477-
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
477+
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret)
478478
t.Expect(err).ToNot(HaveOccurred())
479479

480480
getterOpts := []helmgetter.Option{
@@ -526,7 +526,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
526526
repoURL, err := repository.NormalizeURL(serverURL)
527527
t.Expect(err).ToNot(HaveOccurred())
528528

529-
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
529+
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret)
530530
t.Expect(err).ToNot(HaveOccurred())
531531

532532
getterOpts := []helmgetter.Option{
@@ -580,7 +580,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
580580
repoURL, err := repository.NormalizeURL(serverURL)
581581
t.Expect(err).ToNot(HaveOccurred())
582582

583-
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
583+
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret)
584584
t.Expect(err).ToNot(HaveOccurred())
585585

586586
getterOpts := []helmgetter.Option{

internal/controller/ocirepository_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1007,7 +1007,7 @@ func (r *OCIRepositoryReconciler) getTLSConfig(ctx context.Context, obj *sourcev
10071007
// extend approach (system CAs + user CA) rather than the default replace approach (user CA only).
10081008
// This ensures source-controller continues to work with both system and user-provided CA certificates.
10091009
var tlsOpts = []secrets.TLSConfigOption{secrets.WithSystemCertPool()}
1010-
return secrets.TLSConfigFromSecretRef(ctx, r.Client, secretName, obj.Spec.URL, tlsOpts...)
1010+
return secrets.TLSConfigFromSecretRef(ctx, r.Client, secretName, tlsOpts...)
10111011
}
10121012

10131013
// reconcileStorage ensures the current state of the storage matches the

internal/helm/getter/client_opts.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1
127127
// extend approach (system CAs + user CA) rather than the default replace approach (user CA only).
128128
// This ensures HelmRepository continues to work with both system and user-provided CA certificates.
129129
var tlsOpts = []secrets.TLSConfigOption{secrets.WithSystemCertPool()}
130-
tlsConfig, err := secrets.TLSConfigFromSecret(ctx, secret, obj.Spec.URL, tlsOpts...)
130+
tlsConfig, err := secrets.TLSConfigFromSecret(ctx, secret, tlsOpts...)
131131
if err != nil {
132132
return false, nil, nil, fmt.Errorf("failed to construct Helm client's TLS config: %w", err)
133133
}
@@ -148,7 +148,6 @@ func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1
148148
// extend approach (system CAs + user CA) rather than the default replace approach (user CA only).
149149
// This ensures HelmRepository auth methods work with both system and user-provided CA certificates.
150150
var authOpts = []secrets.AuthMethodsOption{
151-
secrets.WithTargetURL(obj.Spec.URL),
152151
secrets.WithTLSSystemCertPool(),
153152
}
154153
methods, err := secrets.AuthMethodsFromSecret(ctx, secret, authOpts...)

internal/helm/getter/client_opts_test.go

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -298,49 +298,3 @@ func TestGetClientOpts_registryTLSLoginOption(t *testing.T) {
298298
})
299299
}
300300
}
301-
302-
func TestConfigureAuthentication_WithTargetURL(t *testing.T) {
303-
g := NewWithT(t)
304-
305-
tlsCA, err := os.ReadFile("../../controller/testdata/certs/ca.pem")
306-
if err != nil {
307-
t.Errorf("could not read CA file: %s", err)
308-
return
309-
}
310-
311-
helmRepo := &helmv1.HelmRepository{
312-
ObjectMeta: metav1.ObjectMeta{
313-
Name: "test-repo",
314-
Namespace: "default",
315-
},
316-
Spec: helmv1.HelmRepositorySpec{
317-
URL: "https://example.com/charts",
318-
},
319-
}
320-
321-
secret := &corev1.Secret{
322-
ObjectMeta: metav1.ObjectMeta{
323-
Name: "auth-secret",
324-
Namespace: "default",
325-
},
326-
Data: map[string][]byte{
327-
"username": []byte("testuser"),
328-
"password": []byte("testpass"),
329-
"ca.crt": tlsCA,
330-
},
331-
}
332-
333-
client := fakeclient.NewClientBuilder().WithObjects(secret).Build()
334-
helmRepo.Spec.SecretRef = &meta.LocalObjectReference{Name: secret.Name}
335-
336-
opts := &ClientOpts{}
337-
deprecatedTLS, certSecret, authSecret, err := configureAuthentication(context.TODO(), client, helmRepo, opts, helmRepo.Spec.URL)
338-
g.Expect(err).ToNot(HaveOccurred())
339-
g.Expect(deprecatedTLS).To(BeTrue()) // TLS from SecretRef is deprecated
340-
g.Expect(certSecret).To(BeNil())
341-
g.Expect(authSecret).To(Equal(secret))
342-
343-
// Regression test: verify ServerName is set from target URL when WithTargetURL is used
344-
g.Expect(opts.TlsConfig).ToNot(BeNil())
345-
g.Expect(opts.TlsConfig.ServerName).To(Equal("example.com"))
346-
}

0 commit comments

Comments
 (0)