Skip to content

Commit 4b1ace6

Browse files
committed
Enforce TLS certificate verification in Helm/OCI Repository controllers
Remove the insecure parameter from TLS configuration function calls to prevent InsecureSkipVerify from being set when using certificate-based authentication. This ensures TLS certificate verification is always performed when certificates are provided, aligning with our security policy. Updated pkg/runtime to v0.76.0 which no longer accepts the insecure parameter in TLS configuration functions. For OCIRepository, maintain backward compatibility by handling the specific case where no certificate is provided and insecure is explicitly set to true. This is the only allowed exception in Flux controllers. Signed-off-by: cappyzawa <[email protected]>
1 parent a0b4969 commit 4b1ace6

File tree

6 files changed

+60
-9
lines changed

6 files changed

+60
-9
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.51.0
41-
github.com/fluxcd/pkg/runtime v0.75.0
41+
github.com/fluxcd/pkg/runtime v0.76.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.51.0 h1:9oYnm+T4SCVSBif9gn80ALJkMGSERabVMDJiaMIdr7Y=
400400
github.com/fluxcd/pkg/oci v0.51.0/go.mod h1:5J6IhHoDVYCVeBEC+4E3nPeKh7d0kjJ8IEL6NVCiTx4=
401-
github.com/fluxcd/pkg/runtime v0.75.0 h1:wIaODmU5D54nyrehTqA9oQDFoi6BbBj/24adLStXc0I=
402-
github.com/fluxcd/pkg/runtime v0.75.0/go.mod h1:iGhdaEq+lMJQTJNAFEPOU4gUJ7kt3yeDcJPZy7O9IUw=
401+
github.com/fluxcd/pkg/runtime v0.76.0 h1:VoN508i65E/zK0iNXk1Ubvb2VcA8uADqckF+7nuof20=
402+
github.com/fluxcd/pkg/runtime v0.76.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/helmrepository_controller_test.go

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

485-
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL, false)
485+
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
486486
t.Expect(err).ToNot(HaveOccurred())
487487

488488
getterOpts := []helmgetter.Option{
@@ -534,7 +534,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
534534
repoURL, err := repository.NormalizeURL(serverURL)
535535
t.Expect(err).ToNot(HaveOccurred())
536536

537-
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL, false)
537+
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
538538
t.Expect(err).ToNot(HaveOccurred())
539539

540540
getterOpts := []helmgetter.Option{
@@ -588,7 +588,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
588588
repoURL, err := repository.NormalizeURL(serverURL)
589589
t.Expect(err).ToNot(HaveOccurred())
590590

591-
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL, false)
591+
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
592592
t.Expect(err).ToNot(HaveOccurred())
593593

594594
getterOpts := []helmgetter.Option{

internal/controller/ocirepository_controller.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -986,6 +986,11 @@ func (r *OCIRepositoryReconciler) transport(ctx context.Context, obj *sourcev1.O
986986
func (r *OCIRepositoryReconciler) getTLSConfig(ctx context.Context, obj *sourcev1.OCIRepository) (*cryptotls.Config, error) {
987987
if obj.Spec.CertSecretRef == nil || obj.Spec.CertSecretRef.Name == "" {
988988
if obj.Spec.Insecure {
989+
// NOTE: This is the only place in Flux where InsecureSkipVerify is allowed.
990+
// This exception is made for OCIRepository to maintain backward compatibility
991+
// with tools like crane that require insecure connections without certificates.
992+
// This only applies when no CertSecretRef is provided AND insecure is explicitly set.
993+
// All other controllers must NOT allow InsecureSkipVerify per our security policy.
989994
return &cryptotls.Config{
990995
InsecureSkipVerify: true,
991996
}, nil
@@ -997,7 +1002,7 @@ func (r *OCIRepositoryReconciler) getTLSConfig(ctx context.Context, obj *sourcev
9971002
Namespace: obj.Namespace,
9981003
Name: obj.Spec.CertSecretRef.Name,
9991004
}
1000-
return secrets.TLSConfigFromSecretRef(ctx, r.Client, secretName, obj.Spec.URL, obj.Spec.Insecure)
1005+
return secrets.TLSConfigFromSecretRef(ctx, r.Client, secretName, obj.Spec.URL)
10011006
}
10021007

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

internal/helm/getter/client_opts.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1
122122
}
123123
certSecret = secret
124124

125-
tlsConfig, err := secrets.TLSConfigFromSecret(ctx, secret, obj.Spec.URL, obj.Spec.Insecure)
125+
tlsConfig, err := secrets.TLSConfigFromSecret(ctx, secret, obj.Spec.URL)
126126
if err != nil {
127127
return false, nil, nil, fmt.Errorf("failed to construct Helm client's TLS config: %w", err)
128128
}
@@ -138,7 +138,7 @@ func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1
138138
}
139139
authSecret = secret
140140

141-
methods, err := secrets.AuthMethodsFromSecret(ctx, secret, secrets.WithTLS(obj.Spec.URL, obj.Spec.Insecure))
141+
methods, err := secrets.AuthMethodsFromSecret(ctx, secret, secrets.WithTargetURL(obj.Spec.URL))
142142
if err != nil {
143143
return false, nil, nil, fmt.Errorf("failed to detect authentication methods: %w", err)
144144
}

internal/helm/getter/client_opts_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,3 +298,49 @@ 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)