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 <cappyzawa@gmail.com>
This commit is contained in:
parent
a0b4969dc9
commit
4b1ace618a
2
go.mod
2
go.mod
|
@ -38,7 +38,7 @@ require (
|
|||
github.com/fluxcd/pkg/lockedfile v0.6.0
|
||||
github.com/fluxcd/pkg/masktoken v0.7.0
|
||||
github.com/fluxcd/pkg/oci v0.51.0
|
||||
github.com/fluxcd/pkg/runtime v0.75.0
|
||||
github.com/fluxcd/pkg/runtime v0.76.0
|
||||
github.com/fluxcd/pkg/sourceignore v0.13.0
|
||||
github.com/fluxcd/pkg/ssh v0.20.0
|
||||
github.com/fluxcd/pkg/tar v0.13.0
|
||||
|
|
4
go.sum
4
go.sum
|
@ -398,8 +398,8 @@ github.com/fluxcd/pkg/masktoken v0.7.0 h1:pitmyOg2pUVdW+nn2Lk/xqm2TaA08uxvOC0ns3
|
|||
github.com/fluxcd/pkg/masktoken v0.7.0/go.mod h1:Lc1uoDjO1GY6+YdkK+ZqqBIBWquyV58nlSJ5S1N1IYU=
|
||||
github.com/fluxcd/pkg/oci v0.51.0 h1:9oYnm+T4SCVSBif9gn80ALJkMGSERabVMDJiaMIdr7Y=
|
||||
github.com/fluxcd/pkg/oci v0.51.0/go.mod h1:5J6IhHoDVYCVeBEC+4E3nPeKh7d0kjJ8IEL6NVCiTx4=
|
||||
github.com/fluxcd/pkg/runtime v0.75.0 h1:wIaODmU5D54nyrehTqA9oQDFoi6BbBj/24adLStXc0I=
|
||||
github.com/fluxcd/pkg/runtime v0.75.0/go.mod h1:iGhdaEq+lMJQTJNAFEPOU4gUJ7kt3yeDcJPZy7O9IUw=
|
||||
github.com/fluxcd/pkg/runtime v0.76.0 h1:VoN508i65E/zK0iNXk1Ubvb2VcA8uADqckF+7nuof20=
|
||||
github.com/fluxcd/pkg/runtime v0.76.0/go.mod h1:iGhdaEq+lMJQTJNAFEPOU4gUJ7kt3yeDcJPZy7O9IUw=
|
||||
github.com/fluxcd/pkg/sourceignore v0.13.0 h1:ZvkzX2WsmyZK9cjlqOFFW1onHVzhPZIqDbCh96rPqbU=
|
||||
github.com/fluxcd/pkg/sourceignore v0.13.0/go.mod h1:Z9H1GoBx0ljOhptnzoV0PL6Nd/UzwKcSphP27lqb4xI=
|
||||
github.com/fluxcd/pkg/ssh v0.20.0 h1:Ak0laIYIc/L8lEfqls/LDWRW8wYPESGaravQsCRGLb8=
|
||||
|
|
|
@ -482,7 +482,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
|
|||
repoURL, err := repository.NormalizeURL(serverURL)
|
||||
t.Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL, false)
|
||||
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
|
||||
t.Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
getterOpts := []helmgetter.Option{
|
||||
|
@ -534,7 +534,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
|
|||
repoURL, err := repository.NormalizeURL(serverURL)
|
||||
t.Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL, false)
|
||||
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
|
||||
t.Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
getterOpts := []helmgetter.Option{
|
||||
|
@ -588,7 +588,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
|
|||
repoURL, err := repository.NormalizeURL(serverURL)
|
||||
t.Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL, false)
|
||||
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
|
||||
t.Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
getterOpts := []helmgetter.Option{
|
||||
|
|
|
@ -986,6 +986,11 @@ func (r *OCIRepositoryReconciler) transport(ctx context.Context, obj *sourcev1.O
|
|||
func (r *OCIRepositoryReconciler) getTLSConfig(ctx context.Context, obj *sourcev1.OCIRepository) (*cryptotls.Config, error) {
|
||||
if obj.Spec.CertSecretRef == nil || obj.Spec.CertSecretRef.Name == "" {
|
||||
if obj.Spec.Insecure {
|
||||
// NOTE: This is the only place in Flux where InsecureSkipVerify is allowed.
|
||||
// This exception is made for OCIRepository to maintain backward compatibility
|
||||
// with tools like crane that require insecure connections without certificates.
|
||||
// This only applies when no CertSecretRef is provided AND insecure is explicitly set.
|
||||
// All other controllers must NOT allow InsecureSkipVerify per our security policy.
|
||||
return &cryptotls.Config{
|
||||
InsecureSkipVerify: true,
|
||||
}, nil
|
||||
|
@ -997,7 +1002,7 @@ func (r *OCIRepositoryReconciler) getTLSConfig(ctx context.Context, obj *sourcev
|
|||
Namespace: obj.Namespace,
|
||||
Name: obj.Spec.CertSecretRef.Name,
|
||||
}
|
||||
return secrets.TLSConfigFromSecretRef(ctx, r.Client, secretName, obj.Spec.URL, obj.Spec.Insecure)
|
||||
return secrets.TLSConfigFromSecretRef(ctx, r.Client, secretName, obj.Spec.URL)
|
||||
}
|
||||
|
||||
// reconcileStorage ensures the current state of the storage matches the
|
||||
|
|
|
@ -122,7 +122,7 @@ func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1
|
|||
}
|
||||
certSecret = secret
|
||||
|
||||
tlsConfig, err := secrets.TLSConfigFromSecret(ctx, secret, obj.Spec.URL, obj.Spec.Insecure)
|
||||
tlsConfig, err := secrets.TLSConfigFromSecret(ctx, secret, obj.Spec.URL)
|
||||
if err != nil {
|
||||
return false, nil, nil, fmt.Errorf("failed to construct Helm client's TLS config: %w", err)
|
||||
}
|
||||
|
@ -138,7 +138,7 @@ func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1
|
|||
}
|
||||
authSecret = secret
|
||||
|
||||
methods, err := secrets.AuthMethodsFromSecret(ctx, secret, secrets.WithTLS(obj.Spec.URL, obj.Spec.Insecure))
|
||||
methods, err := secrets.AuthMethodsFromSecret(ctx, secret, secrets.WithTargetURL(obj.Spec.URL))
|
||||
if err != nil {
|
||||
return false, nil, nil, fmt.Errorf("failed to detect authentication methods: %w", err)
|
||||
}
|
||||
|
|
|
@ -298,3 +298,49 @@ func TestGetClientOpts_registryTLSLoginOption(t *testing.T) {
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestConfigureAuthentication_WithTargetURL(t *testing.T) {
|
||||
g := NewWithT(t)
|
||||
|
||||
tlsCA, err := os.ReadFile("../../controller/testdata/certs/ca.pem")
|
||||
if err != nil {
|
||||
t.Errorf("could not read CA file: %s", err)
|
||||
return
|
||||
}
|
||||
|
||||
helmRepo := &helmv1.HelmRepository{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: "test-repo",
|
||||
Namespace: "default",
|
||||
},
|
||||
Spec: helmv1.HelmRepositorySpec{
|
||||
URL: "https://example.com/charts",
|
||||
},
|
||||
}
|
||||
|
||||
secret := &corev1.Secret{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: "auth-secret",
|
||||
Namespace: "default",
|
||||
},
|
||||
Data: map[string][]byte{
|
||||
"username": []byte("testuser"),
|
||||
"password": []byte("testpass"),
|
||||
"ca.crt": tlsCA,
|
||||
},
|
||||
}
|
||||
|
||||
client := fakeclient.NewClientBuilder().WithObjects(secret).Build()
|
||||
helmRepo.Spec.SecretRef = &meta.LocalObjectReference{Name: secret.Name}
|
||||
|
||||
opts := &ClientOpts{}
|
||||
deprecatedTLS, certSecret, authSecret, err := configureAuthentication(context.TODO(), client, helmRepo, opts, helmRepo.Spec.URL)
|
||||
g.Expect(err).ToNot(HaveOccurred())
|
||||
g.Expect(deprecatedTLS).To(BeTrue()) // TLS from SecretRef is deprecated
|
||||
g.Expect(certSecret).To(BeNil())
|
||||
g.Expect(authSecret).To(Equal(secret))
|
||||
|
||||
// Regression test: verify ServerName is set from target URL when WithTargetURL is used
|
||||
g.Expect(opts.TlsConfig).ToNot(BeNil())
|
||||
g.Expect(opts.TlsConfig.ServerName).To(Equal("example.com"))
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue