diff --git a/api/v1beta2/helmrepository_types.go b/api/v1beta2/helmrepository_types.go index 4da992aba..e1df71568 100644 --- a/api/v1beta2/helmrepository_types.go +++ b/api/v1beta2/helmrepository_types.go @@ -56,10 +56,21 @@ type HelmRepositorySpec struct { // +optional SecretRef *meta.LocalObjectReference `json:"secretRef,omitempty"` - // CertSecretRef specifies the Secret containing the TLS authentication - // data. The secret must contain a 'certFile' and 'keyFile', and/or 'caFile' - // fields. It takes precedence over the values specified in the Secret - // referred to by `.spec.secretRef`. + // CertSecretRef can be given the name of a Secret containing + // either or both of + // + // - a PEM-encoded client certificate (`tls.crt`) and private + // key (`tls.key`); + // - a PEM-encoded CA certificate (`ca.crt`) + // + // and whichever are supplied, will be used for connecting to the + // registry. The client cert and key are useful if you are + // authenticating with a certificate; the CA cert is useful if + // you are using a self-signed server certificate. The Secret must + // be of type `Opaque` or `kubernetes.io/tls`. + // + // It takes precedence over the values specified in the Secret referred + // to by `.spec.secretRef`. // +optional CertSecretRef *meta.LocalObjectReference `json:"certSecretRef,omitempty"` diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml index c9a6b3fc7..6de6911d8 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml @@ -297,10 +297,15 @@ spec: - namespaceSelectors type: object certSecretRef: - description: CertSecretRef specifies the Secret containing the TLS - authentication data. The secret must contain a 'certFile' and 'keyFile', - and/or 'caFile' fields. It takes precedence over the values specified - in the Secret referred to by `.spec.secretRef`. + description: "CertSecretRef can be given the name of a Secret containing + either or both of \n - a PEM-encoded client certificate (`tls.crt`) + and private key (`tls.key`); - a PEM-encoded CA certificate (`ca.crt`) + \n and whichever are supplied, will be used for connecting to the + registry. The client cert and key are useful if you are authenticating + with a certificate; the CA cert is useful if you are using a self-signed + server certificate. The Secret must be of type `Opaque` or `kubernetes.io/tls`. + \n It takes precedence over the values specified in the Secret referred + to by `.spec.secretRef`." properties: name: description: Name of the referent. diff --git a/docs/api/v1beta2/source.md b/docs/api/v1beta2/source.md index be0c454ed..73899644f 100644 --- a/docs/api/v1beta2/source.md +++ b/docs/api/v1beta2/source.md @@ -811,10 +811,20 @@ github.com/fluxcd/pkg/apis/meta.LocalObjectReference (Optional) -

CertSecretRef specifies the Secret containing the TLS authentication -data. The secret must contain a ‘certFile’ and ‘keyFile’, and/or ‘caFile’ -fields. It takes precedence over the values specified in the Secret -referred to by .spec.secretRef.

+

CertSecretRef can be given the name of a Secret containing +either or both of

+ +

and whichever are supplied, will be used for connecting to the +registry. The client cert and key are useful if you are +authenticating with a certificate; the CA cert is useful if +you are using a self-signed server certificate. The Secret must +be of type Opaque or kubernetes.io/tls.

+

It takes precedence over the values specified in the Secret referred +to by .spec.secretRef.

@@ -2503,10 +2513,20 @@ github.com/fluxcd/pkg/apis/meta.LocalObjectReference (Optional) -

CertSecretRef specifies the Secret containing the TLS authentication -data. The secret must contain a ‘certFile’ and ‘keyFile’, and/or ‘caFile’ -fields. It takes precedence over the values specified in the Secret -referred to by .spec.secretRef.

+

CertSecretRef can be given the name of a Secret containing +either or both of

+ +

and whichever are supplied, will be used for connecting to the +registry. The client cert and key are useful if you are +authenticating with a certificate; the CA cert is useful if +you are using a self-signed server certificate. The Secret must +be of type Opaque or kubernetes.io/tls.

+

It takes precedence over the values specified in the Secret referred +to by .spec.secretRef.

diff --git a/docs/spec/v1beta2/helmrepositories.md b/docs/spec/v1beta2/helmrepositories.md index 8e46d2ca2..cd7c7d3a6 100644 --- a/docs/spec/v1beta2/helmrepositories.md +++ b/docs/spec/v1beta2/helmrepositories.md @@ -467,32 +467,33 @@ flux create secret oci ghcr-auth \ --password=${GITHUB_PAT} ``` -**Note:** Support for specifying TLS authentication data using this API has been +**Warning:** Support for specifying TLS authentication data using this API has been deprecated. Please use [`.spec.certSecretRef`](#cert-secret-reference) instead. If the controller uses the secret specfied by this field to configure TLS, then a deprecation warning will be logged. ### Cert secret reference -`.spec.certSecretRef.name` is an optional field to specify a secret containing TLS -certificate data. The secret can contain the following keys: +`.spec.certSecretRef.name` is an optional field to specify a secret containing +TLS certificate data. The secret can contain the following keys: -* `certFile` and `keyFile`, to specify the client certificate and private key used for -TLS client authentication. These must be used in conjunction, i.e. specifying one without -the other will lead to an error. -* `caFile`, to specify the CA certificate used to verify the server, which is required -if the server is using a self-signed certificate. +* `tls.crt` and `tls.key`, to specify the client certificate and private key used +for TLS client authentication. These must be used in conjunction, i.e. +specifying one without the other will lead to an error. +* `ca.crt`, to specify the CA certificate used to verify the server, which is +required if the server is using a self-signed certificate. -If the server is using a self-signed certificate and has TLS client authentication enabled, -all three values are required. +If the server is using a self-signed certificate and has TLS client +authentication enabled, all three values are required. -All the files in the secret are expected to be [PEM-encoded][pem-encoding]. Assuming you have -three files; `client.key`, `client.crt` and `ca.crt` for the client private key, client -certificate and the CA certificate respectively, you can generate the required secret using -the `flux creat secret helm` command: +The Secret should be of type `Opaque` or `kubernetes.io/tls`. All the files in +the Secret are expected to be [PEM-encoded][pem-encoding]. Assuming you have +three files; `client.key`, `client.crt` and `ca.crt` for the client private key, +client certificate and the CA certificate respectively, you can generate the +required Secret using the `flux create secret tls` command: ```sh -flux create secret helm tls --key-file=client.key --cert-file=client.crt --ca-file=ca.crt +flux create secret tls --tls-key-file=client.key --tls-crt-file=client.crt --ca-crt-file=ca.crt ``` Example usage: @@ -515,11 +516,12 @@ kind: Secret metadata: name: example-tls namespace: default +type: kubernetes.io/tls # or Opaque data: - certFile: - keyFile: + tls.crt: + tls.key: # NOTE: Can be supplied without the above values - caFile: + ca.crt: ``` ### Pass credentials diff --git a/internal/controller/helmchart_controller_test.go b/internal/controller/helmchart_controller_test.go index c0ad94380..9d45271dc 100644 --- a/internal/controller/helmchart_controller_test.go +++ b/internal/controller/helmchart_controller_test.go @@ -2248,7 +2248,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { registryOpts registryOptions secretOpts secretOptions secret *corev1.Secret - certsecret *corev1.Secret + certSecret *corev1.Secret insecure bool provider string providerImg string @@ -2363,16 +2363,16 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { Type: corev1.SecretTypeDockerConfigJson, Data: map[string][]byte{}, }, - certsecret: &corev1.Secret{ + certSecret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "certs-secretref", }, Data: map[string][]byte{ - "caFile": []byte("invalid caFile"), + "ca.crt": []byte("invalid caFile"), }, }, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to construct Helm client's TLS config: cannot append certificate into certificate pool: invalid caFile"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to construct Helm client's TLS config: cannot append certificate into certificate pool: invalid CA certificate"), }, }, { @@ -2393,14 +2393,14 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { Type: corev1.SecretTypeDockerConfigJson, Data: map[string][]byte{}, }, - certsecret: &corev1.Secret{ + certSecret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "certs-secretref", }, Data: map[string][]byte{ - "caFile": tlsCA, - "certFile": clientPublicKey, - "keyFile": clientPrivateKey, + "ca.crt": tlsCA, + "tls.crt": clientPublicKey, + "tls.key": clientPrivateKey, }, }, assertConditions: []metav1.Condition{ @@ -2472,11 +2472,11 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { clientBuilder.WithObjects(tt.secret) } - if tt.certsecret != nil { + if tt.certSecret != nil { repo.Spec.CertSecretRef = &meta.LocalObjectReference{ - Name: tt.certsecret.Name, + Name: tt.certSecret.Name, } - clientBuilder.WithObjects(tt.certsecret) + clientBuilder.WithObjects(tt.certSecret) } clientBuilder.WithObjects(repo) diff --git a/internal/controller/helmrepository_controller_oci_test.go b/internal/controller/helmrepository_controller_oci_test.go index d1252e709..2a33115c7 100644 --- a/internal/controller/helmrepository_controller_oci_test.go +++ b/internal/controller/helmrepository_controller_oci_test.go @@ -325,12 +325,12 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { Name: "certs-secretref", }, Data: map[string][]byte{ - "caFile": []byte("invalid caFile"), + "ca.crt": []byte("invalid caFile"), }, }, assertConditions: []metav1.Condition{ *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "processing object: new generation 0 -> 1"), - *conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid caFile"), + *conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid CA certificate"), }, }, { @@ -356,9 +356,9 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) { Name: "certs-secretref", }, Data: map[string][]byte{ - "caFile": tlsCA, - "certFile": clientPublicKey, - "keyFile": clientPrivateKey, + "ca.crt": tlsCA, + "tls.crt": clientPublicKey, + "tls.key": clientPrivateKey, }, }, assertConditions: []metav1.Condition{ diff --git a/internal/controller/helmrepository_controller_test.go b/internal/controller/helmrepository_controller_test.go index 370cac0ed..0513493eb 100644 --- a/internal/controller/helmrepository_controller_test.go +++ b/internal/controller/helmrepository_controller_test.go @@ -56,6 +56,7 @@ import ( "github.com/fluxcd/source-controller/internal/helm/repository" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" + stls "github.com/fluxcd/source-controller/internal/tls" ) func TestHelmRepositoryReconciler_deleteBeforeFinalizer(t *testing.T) { @@ -434,7 +435,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) { Name: "ca-file", }, Data: map[string][]byte{ - "caFile": tlsCA, + "ca.crt": tlsCA, }, }, beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) { @@ -445,6 +446,27 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) { *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"), }, }, + { + name: "HTTPS with secretRef pointing to CA cert but public repo URL succeeds", + protocol: "http", + url: "https://stefanprodan.github.io/podinfo", + want: sreconcile.ResultSuccess, + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ca-file", + }, + Data: map[string][]byte{ + "caFile": tlsCA, + }, + }, + beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) { + obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "ca-file"} + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"), + }, + }, { name: "HTTP without secretRef makes ArtifactOutdated=True", protocol: "http", @@ -502,7 +524,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) { Name: "invalid-ca", }, Data: map[string][]byte{ - "caFile": []byte("invalid"), + "ca.crt": []byte("invalid"), }, }, beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) { @@ -512,7 +534,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) { }, wantErr: true, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid caFile"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid CA certificate"), *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"), *conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"), }, @@ -769,7 +791,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) { if tt.url != "" { repoURL = tt.url } - tlsConf, _, serr = getter.TLSClientConfigFromSecret(*secret, repoURL) + tlsConf, _, serr = stls.KubeTLSClientConfigFromSecret(*secret, repoURL) if serr != nil { validSecret = false } diff --git a/internal/helm/getter/client_opts.go b/internal/helm/getter/client_opts.go index 58248d5b6..4e77f290a 100644 --- a/internal/helm/getter/client_opts.go +++ b/internal/helm/getter/client_opts.go @@ -19,10 +19,8 @@ package getter import ( "context" "crypto/tls" - "crypto/x509" "errors" "fmt" - "net/url" "os" "path" @@ -37,6 +35,7 @@ import ( helmv1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/source-controller/internal/helm/registry" soci "github.com/fluxcd/source-controller/internal/oci" + stls "github.com/fluxcd/source-controller/internal/tls" ) const ( @@ -47,16 +46,6 @@ const ( var ErrDeprecatedTLSConfig = errors.New("TLS configured in a deprecated manner") -// TLSBytes contains the bytes of the TLS files. -type TLSBytes struct { - // CertBytes is the bytes of the certificate file. - CertBytes []byte - // KeyBytes is the bytes of the key file. - KeyBytes []byte - // CABytes is the bytes of the CA file. - CABytes []byte -} - // ClientOpts contains the various options to use while constructing // a Helm repository client. type ClientOpts struct { @@ -91,7 +80,7 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit var ( certSecret *corev1.Secret - tlsBytes *TLSBytes + tlsBytes *stls.TLSBytes certFile string keyFile string caFile string @@ -105,7 +94,7 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit return nil, "", fmt.Errorf("failed to get TLS authentication secret '%s/%s': %w", obj.GetNamespace(), obj.Spec.CertSecretRef.Name, err) } - hrOpts.TlsConfig, tlsBytes, err = TLSClientConfigFromSecret(*certSecret, url) + hrOpts.TlsConfig, tlsBytes, err = stls.KubeTLSClientConfigFromSecret(*certSecret, url) if err != nil { return nil, "", fmt.Errorf("failed to construct Helm client's TLS config: %w", err) } @@ -128,8 +117,8 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit // If the TLS config is nil, i.e. one couldn't be constructed using `.spec.certSecretRef` // then try to use `.spec.secretRef`. - if hrOpts.TlsConfig == nil { - hrOpts.TlsConfig, tlsBytes, err = TLSClientConfigFromSecret(*authSecret, url) + if hrOpts.TlsConfig == nil && !ociRepo { + hrOpts.TlsConfig, tlsBytes, err = stls.TLSClientConfigFromSecret(*authSecret, url) if err != nil { return nil, "", fmt.Errorf("failed to construct Helm client's TLS config: %w", err) } @@ -162,7 +151,7 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit if err != nil { return nil, "", fmt.Errorf("cannot create temporary directory: %w", err) } - certFile, keyFile, caFile, err = StoreTLSCertificateFiles(tlsBytes, dir) + certFile, keyFile, caFile, err = storeTLSCertificateFiles(tlsBytes, dir) if err != nil { return nil, "", fmt.Errorf("cannot write certs files to path: %w", err) } @@ -198,60 +187,8 @@ func fetchSecret(ctx context.Context, c client.Client, name, namespace string) ( return &secret, nil } -// TLSClientConfigFromSecret attempts to construct a TLS client config -// for the given v1.Secret. It returns the TLS client config or an error. -// -// Secrets with no certFile, keyFile, AND caFile are ignored, if only a -// certBytes OR keyBytes is defined it returns an error. -func TLSClientConfigFromSecret(secret corev1.Secret, repositoryUrl string) (*tls.Config, *TLSBytes, error) { - certBytes, keyBytes, caBytes := secret.Data["certFile"], secret.Data["keyFile"], secret.Data["caFile"] - switch { - case len(certBytes)+len(keyBytes)+len(caBytes) == 0: - return nil, nil, nil - case (len(certBytes) > 0 && len(keyBytes) == 0) || (len(keyBytes) > 0 && len(certBytes) == 0): - return nil, nil, fmt.Errorf("invalid '%s' secret data: fields 'certFile' and 'keyFile' require each other's presence", - secret.Name) - } - - tlsConf := &tls.Config{} - if len(certBytes) > 0 && len(keyBytes) > 0 { - cert, err := tls.X509KeyPair(certBytes, keyBytes) - if err != nil { - return nil, nil, err - } - tlsConf.Certificates = append(tlsConf.Certificates, cert) - } - - if len(caBytes) > 0 { - cp, err := x509.SystemCertPool() - if err != nil { - return nil, nil, fmt.Errorf("cannot retrieve system certificate pool: %w", err) - } - if !cp.AppendCertsFromPEM(caBytes) { - return nil, nil, fmt.Errorf("cannot append certificate into certificate pool: invalid caFile") - } - - tlsConf.RootCAs = cp - } - - tlsConf.BuildNameToCertificate() - - u, err := url.Parse(repositoryUrl) - if err != nil { - return nil, nil, fmt.Errorf("cannot parse repository URL: %w", err) - } - - tlsConf.ServerName = u.Hostname() - - return tlsConf, &TLSBytes{ - CertBytes: certBytes, - KeyBytes: keyBytes, - CABytes: caBytes, - }, nil -} - -// StoreTLSCertificateFiles writes the certs files to the given path and returns the files paths. -func StoreTLSCertificateFiles(tlsBytes *TLSBytes, path string) (string, string, string, error) { +// storeTLSCertificateFiles writes the certs files to the given path and returns the files paths. +func storeTLSCertificateFiles(tlsBytes *stls.TLSBytes, path string) (string, string, string, error) { var ( certFile string keyFile string diff --git a/internal/helm/getter/client_opts_test.go b/internal/helm/getter/client_opts_test.go index 6b031851d..91bcd32f8 100644 --- a/internal/helm/getter/client_opts_test.go +++ b/internal/helm/getter/client_opts_test.go @@ -18,11 +18,6 @@ package getter import ( "context" - "crypto/rand" - "crypto/rsa" - "crypto/x509" - "encoding/pem" - "math/big" "os" "testing" "time" @@ -58,7 +53,7 @@ func TestGetClientOpts(t *testing.T) { Name: "ca-file", }, Data: map[string][]byte{ - "caFile": tlsCA, + "ca.crt": tlsCA, }, }, authSecret: &corev1.Secret{ @@ -160,42 +155,6 @@ func TestGetClientOpts(t *testing.T) { } } -func Test_tlsClientConfigFromSecret(t *testing.T) { - tlsSecretFixture := validTlsSecret(t) - - tests := []struct { - name string - secret corev1.Secret - modify func(secret *corev1.Secret) - wantErr bool - wantNil bool - }{ - {"certFile, keyFile and caFile", tlsSecretFixture, nil, false, false}, - {"without certFile", tlsSecretFixture, func(s *corev1.Secret) { delete(s.Data, "certFile") }, true, true}, - {"without keyFile", tlsSecretFixture, func(s *corev1.Secret) { delete(s.Data, "keyFile") }, true, true}, - {"without caFile", tlsSecretFixture, func(s *corev1.Secret) { delete(s.Data, "caFile") }, false, false}, - {"empty", corev1.Secret{}, nil, false, true}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - secret := tt.secret.DeepCopy() - if tt.modify != nil { - tt.modify(secret) - } - - got, _, err := TLSClientConfigFromSecret(*secret, "") - if (err != nil) != tt.wantErr { - t.Errorf("TLSClientConfigFromSecret() error = %v, wantErr %v", err, tt.wantErr) - return - } - if tt.wantNil && got != nil { - t.Error("TLSClientConfigFromSecret() != nil") - return - } - }) - } -} - func TestGetClientOpts_registryTLSLoginOption(t *testing.T) { tlsCA, err := os.ReadFile("../../controller/testdata/certs/ca.pem") if err != nil { @@ -215,7 +174,7 @@ func TestGetClientOpts_registryTLSLoginOption(t *testing.T) { Name: "ca-file", }, Data: map[string][]byte{ - "caFile": tlsCA, + "ca.crt": tlsCA, }, }, authSecret: &corev1.Secret{ @@ -307,60 +266,3 @@ func TestGetClientOpts_registryTLSLoginOption(t *testing.T) { }) } } - -// validTlsSecret creates a secret containing key pair and CA certificate that are -// valid from a syntax (minimum requirements) perspective. -func validTlsSecret(t *testing.T) corev1.Secret { - key, err := rsa.GenerateKey(rand.Reader, 2048) - if err != nil { - t.Fatal("Private key cannot be created.", err.Error()) - } - - certTemplate := x509.Certificate{ - SerialNumber: big.NewInt(1337), - } - cert, err := x509.CreateCertificate(rand.Reader, &certTemplate, &certTemplate, &key.PublicKey, key) - if err != nil { - t.Fatal("Certificate cannot be created.", err.Error()) - } - - ca := &x509.Certificate{ - SerialNumber: big.NewInt(7331), - IsCA: true, - ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, - KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - } - - caPrivKey, err := rsa.GenerateKey(rand.Reader, 4096) - if err != nil { - t.Fatal("CA private key cannot be created.", err.Error()) - } - - caBytes, err := x509.CreateCertificate(rand.Reader, ca, ca, &caPrivKey.PublicKey, caPrivKey) - if err != nil { - t.Fatal("CA certificate cannot be created.", err.Error()) - } - - keyPem := pem.EncodeToMemory(&pem.Block{ - Type: "RSA PRIVATE KEY", - Bytes: x509.MarshalPKCS1PrivateKey(key), - }) - - certPem := pem.EncodeToMemory(&pem.Block{ - Type: "CERTIFICATE", - Bytes: cert, - }) - - caPem := pem.EncodeToMemory(&pem.Block{ - Type: "CERTIFICATE", - Bytes: caBytes, - }) - - return corev1.Secret{ - Data: map[string][]byte{ - "certFile": []byte(certPem), - "keyFile": []byte(keyPem), - "caFile": []byte(caPem), - }, - } -} diff --git a/internal/tls/config.go b/internal/tls/config.go new file mode 100644 index 000000000..d6641d9d7 --- /dev/null +++ b/internal/tls/config.go @@ -0,0 +1,138 @@ +/* +Copyright 2023 The Flux 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 ( + "crypto/tls" + "crypto/x509" + "fmt" + neturl "net/url" + + corev1 "k8s.io/api/core/v1" +) + +const CACrtKey = "ca.crt" + +// TLSBytes contains the bytes of the TLS files. +type TLSBytes struct { + // CertBytes is the bytes of the certificate file. + CertBytes []byte + // KeyBytes is the bytes of the key file. + KeyBytes []byte + // CABytes is the bytes of the CA file. + CABytes []byte +} + +// KubeTLSClientConfigFromSecret returns a TLS client config as a `tls.Config` +// object and in its bytes representation. The secret is expected to have the +// following keys: +// - tls.key, for the private key +// - tls.crt, for the certificate +// - ca.crt, for the CA certificate +// +// Secrets with no certificate, private key, AND CA cert are ignored. If only a +// certificate OR private key is found, an error is returned. +func KubeTLSClientConfigFromSecret(secret corev1.Secret, url string) (*tls.Config, *TLSBytes, error) { + return tlsClientConfigFromSecret(secret, url, true) +} + +// TLSClientConfigFromSecret returns a TLS client config as a `tls.Config` +// object and in its bytes representation. The secret is expected to have the +// following keys: +// - keyFile, for the private key +// - certFile, for the certificate +// - caFile, for the CA certificate +// +// Secrets with no certificate, private key, AND CA cert are ignored. If only a +// certificate OR private key is found, an error is returned. +func TLSClientConfigFromSecret(secret corev1.Secret, url string) (*tls.Config, *TLSBytes, error) { + return tlsClientConfigFromSecret(secret, url, false) +} + +// tlsClientConfigFromSecret attempts to construct and return a TLS client +// config from the given Secret. If the Secret does not contain valid TLS +// data, it returns nil. +// +// kubernetesTLSKeys is a boolean indicating whether to check the Secret +// for keys expected to be present in a Kubernetes TLS Secret. Based on its +// value, the Secret is checked for the following keys: +// - tls.key/keyFile for the private key +// - tls.crt/certFile for the certificate +// - ca.crt/caFile for the CA certificate +// The keys should adhere to a single convention, i.e. a Secret with tls.key +// and certFile is invalid. +func tlsClientConfigFromSecret(secret corev1.Secret, url string, kubernetesTLSKeys bool) (*tls.Config, *TLSBytes, error) { + // Only Secrets of type Opaque and TLS are allowed. We also allow Secrets with a blank + // type, to avoid having to specify the type of the Secret for every test case. + // Since a real Kubernetes Secret is of type Opaque by default, its safe to allow this. + switch secret.Type { + case corev1.SecretTypeOpaque, corev1.SecretTypeTLS, "": + default: + return nil, nil, fmt.Errorf("cannot use secret '%s' to construct TLS config: invalid secret type: '%s'", secret.Name, secret.Type) + } + + var certBytes, keyBytes, caBytes []byte + if kubernetesTLSKeys { + certBytes, keyBytes, caBytes = secret.Data[corev1.TLSCertKey], secret.Data[corev1.TLSPrivateKeyKey], secret.Data[CACrtKey] + } else { + certBytes, keyBytes, caBytes = secret.Data["certFile"], secret.Data["keyFile"], secret.Data["caFile"] + } + + switch { + case len(certBytes)+len(keyBytes)+len(caBytes) == 0: + return nil, nil, nil + case (len(certBytes) > 0 && len(keyBytes) == 0) || (len(keyBytes) > 0 && len(certBytes) == 0): + return nil, nil, fmt.Errorf("invalid '%s' secret data: both certificate and private key need to be provided", + secret.Name) + } + + tlsConf := &tls.Config{} + if len(certBytes) > 0 && len(keyBytes) > 0 { + cert, err := tls.X509KeyPair(certBytes, keyBytes) + if err != nil { + return nil, nil, err + } + tlsConf.Certificates = append(tlsConf.Certificates, cert) + } + + if len(caBytes) > 0 { + cp, err := x509.SystemCertPool() + if err != nil { + return nil, nil, fmt.Errorf("cannot retrieve system certificate pool: %w", err) + } + if !cp.AppendCertsFromPEM(caBytes) { + return nil, nil, fmt.Errorf("cannot append certificate into certificate pool: invalid CA certificate") + } + + tlsConf.RootCAs = cp + } + + if url != "" { + u, err := neturl.Parse(url) + if err != nil { + return nil, nil, fmt.Errorf("cannot parse repository URL: %w", err) + } + + tlsConf.ServerName = u.Hostname() + } + + return tlsConf, &TLSBytes{ + CertBytes: certBytes, + KeyBytes: keyBytes, + CABytes: caBytes, + }, nil +} diff --git a/internal/tls/config_test.go b/internal/tls/config_test.go new file mode 100644 index 000000000..728b988b7 --- /dev/null +++ b/internal/tls/config_test.go @@ -0,0 +1,178 @@ +/* +Copyright 2023 The Flux 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 ( + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/pem" + "fmt" + "math/big" + "net/url" + "testing" + + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" +) + +func Test_tlsClientConfigFromSecret(t *testing.T) { + kubernetesTlsSecretFixture := validTlsSecret(t, true) + tlsSecretFixture := validTlsSecret(t, false) + + tests := []struct { + name string + secret corev1.Secret + modify func(secret *corev1.Secret) + tlsKeys bool + url string + wantErr bool + wantNil bool + }{ + { + name: "tls.crt, tls.key and ca.crt", + secret: kubernetesTlsSecretFixture, + modify: nil, + tlsKeys: true, + url: "https://example.com", + }, + { + name: "certFile, keyFile and caFile", + secret: tlsSecretFixture, + modify: nil, + tlsKeys: false, + url: "https://example.com", + }, + { + name: "without tls.crt", + secret: kubernetesTlsSecretFixture, + modify: func(s *corev1.Secret) { delete(s.Data, "tls.crt") }, + tlsKeys: true, + wantErr: true, + wantNil: true, + }, + { + name: "without tls.key", + secret: kubernetesTlsSecretFixture, + modify: func(s *corev1.Secret) { delete(s.Data, "tls.key") }, + tlsKeys: true, + wantErr: true, + wantNil: true, + }, + { + name: "without ca.crt", + secret: kubernetesTlsSecretFixture, + modify: func(s *corev1.Secret) { delete(s.Data, "ca.crt") }, + tlsKeys: true, + }, + { + name: "empty secret", + secret: corev1.Secret{}, + tlsKeys: true, + wantNil: true, + }, + { + name: "invalid secret type", + secret: corev1.Secret{Type: corev1.SecretTypeDockerConfigJson}, + wantErr: true, + wantNil: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + secret := tt.secret.DeepCopy() + if tt.modify != nil { + tt.modify(secret) + } + + tlsConfig, _, err := tlsClientConfigFromSecret(*secret, tt.url, tt.tlsKeys) + g.Expect(err != nil).To(Equal(tt.wantErr), fmt.Sprintf("expected error: %v, got: %v", tt.wantErr, err)) + g.Expect(tlsConfig == nil).To(Equal(tt.wantNil)) + if tt.url != "" { + u, _ := url.Parse(tt.url) + g.Expect(u.Hostname()).To(Equal(tlsConfig.ServerName)) + } + }) + } +} + +// validTlsSecret creates a secret containing key pair and CA certificate that are +// valid from a syntax (minimum requirements) perspective. +func validTlsSecret(t *testing.T, kubernetesTlsKeys bool) corev1.Secret { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal("Private key cannot be created.", err.Error()) + } + + certTemplate := x509.Certificate{ + SerialNumber: big.NewInt(1337), + } + cert, err := x509.CreateCertificate(rand.Reader, &certTemplate, &certTemplate, &key.PublicKey, key) + if err != nil { + t.Fatal("Certificate cannot be created.", err.Error()) + } + + ca := &x509.Certificate{ + SerialNumber: big.NewInt(7331), + IsCA: true, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + } + + caPrivKey, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + t.Fatal("CA private key cannot be created.", err.Error()) + } + + caBytes, err := x509.CreateCertificate(rand.Reader, ca, ca, &caPrivKey.PublicKey, caPrivKey) + if err != nil { + t.Fatal("CA certificate cannot be created.", err.Error()) + } + + keyPem := pem.EncodeToMemory(&pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509.MarshalPKCS1PrivateKey(key), + }) + + certPem := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: cert, + }) + + caPem := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: caBytes, + }) + + crtKey := corev1.TLSCertKey + pkKey := corev1.TLSPrivateKeyKey + caKey := CACrtKey + if !kubernetesTlsKeys { + crtKey = "certFile" + pkKey = "keyFile" + caKey = "caFile" + } + return corev1.Secret{ + Data: map[string][]byte{ + crtKey: []byte(certPem), + pkKey: []byte(keyPem), + caKey: []byte(caPem), + }, + } +}