From 0e97547eebc1fa7b90bcfdb5a97d41f0766e5c39 Mon Sep 17 00:00:00 2001 From: Soule BA Date: Mon, 3 Oct 2022 17:07:00 +0200 Subject: [PATCH] implement Cosign verification for HelmCharts If implemented, users will be able to enable chart verification for OCI based helm charts. Signed-off-by: Soule BA --- api/v1beta2/helmchart_types.go | 8 + api/v1beta2/zz_generated.deepcopy.go | 5 + .../source.toolkit.fluxcd.io_helmcharts.yaml | 27 +++ .../testdata/helmchart-from-oci/source.yaml | 14 ++ controllers/helmchart_controller.go | 106 +++++++- controllers/helmchart_controller_test.go | 227 ++++++++++++++++++ controllers/ocirepository_controller.go | 4 +- docs/api/source.md | 37 +++ hack/ci/e2e.sh | 1 + internal/helm/chart/builder.go | 2 + internal/helm/chart/builder_remote.go | 15 +- internal/helm/chart/errors.go | 1 + internal/helm/repository/chart_repository.go | 10 + .../helm/repository/oci_chart_repository.go | 45 +++- internal/helm/repository/repository.go | 3 + internal/oci/verifier.go | 35 ++- 16 files changed, 522 insertions(+), 18 deletions(-) diff --git a/api/v1beta2/helmchart_types.go b/api/v1beta2/helmchart_types.go index 5b12f1f5..aca993fd 100644 --- a/api/v1beta2/helmchart_types.go +++ b/api/v1beta2/helmchart_types.go @@ -86,6 +86,14 @@ type HelmChartSpec struct { // NOTE: Not implemented, provisional as of https://github.com/fluxcd/flux2/pull/2092 // +optional AccessFrom *acl.AccessFrom `json:"accessFrom,omitempty"` + + // Verify contains the secret name containing the trusted public keys + // used to verify the signature and specifies which provider to use to check + // whether OCI image is authentic. + // This field is only supported for OCI sources. + // Chart dependencies, which are not bundled in the umbrella chart artifact, are not verified. + // +optional + Verify *OCIRepositoryVerification `json:"verify,omitempty"` } const ( diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 106a042c..c196f4e5 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -464,6 +464,11 @@ func (in *HelmChartSpec) DeepCopyInto(out *HelmChartSpec) { *out = new(acl.AccessFrom) (*in).DeepCopyInto(*out) } + if in.Verify != nil { + in, out := &in.Verify, &out.Verify + *out = new(OCIRepositoryVerification) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HelmChartSpec. diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml index 7ef36829..c6cdeefe 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml @@ -403,6 +403,33 @@ spec: items: type: string type: array + verify: + description: Verify contains the secret name containing the trusted + public keys used to verify the signature and specifies which provider + to use to check whether OCI image is authentic. This field is only + supported for OCI sources. Chart dependencies, which are not bundled + in the umbrella chart artifact, are not verified. + properties: + provider: + default: cosign + description: Provider specifies the technology used to sign the + OCI Artifact. + enum: + - cosign + type: string + secretRef: + description: SecretRef specifies the Kubernetes Secret containing + the trusted public keys. + properties: + name: + description: Name of the referent. + type: string + required: + - name + type: object + required: + - provider + type: object version: default: '*' description: Version is the chart version semver expression, ignored diff --git a/config/testdata/helmchart-from-oci/source.yaml b/config/testdata/helmchart-from-oci/source.yaml index 9d9945ff..354325ef 100644 --- a/config/testdata/helmchart-from-oci/source.yaml +++ b/config/testdata/helmchart-from-oci/source.yaml @@ -19,3 +19,17 @@ spec: name: podinfo version: '6.1.*' interval: 1m +--- +apiVersion: source.toolkit.fluxcd.io/v1beta2 +kind: HelmChart +metadata: + name: podinfo-keyless +spec: + chart: podinfo + sourceRef: + kind: HelmRepository + name: podinfo + version: '6.2.1' + interval: 1m + verify: + provider: cosign diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 73662da0..773bce46 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -28,6 +28,7 @@ import ( "strings" "time" + soci "github.com/fluxcd/source-controller/internal/oci" helmgetter "helm.sh/helm/v3/pkg/getter" helmreg "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" @@ -57,6 +58,7 @@ import ( "github.com/fluxcd/pkg/runtime/predicates" "github.com/fluxcd/pkg/untar" "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/v1/remote" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/source-controller/internal/cache" @@ -80,6 +82,7 @@ var helmChartReadyCondition = summarize.Conditions{ sourcev1.BuildFailedCondition, sourcev1.ArtifactOutdatedCondition, sourcev1.ArtifactInStorageCondition, + sourcev1.SourceVerifiedCondition, meta.ReadyCondition, meta.ReconcilingCondition, meta.StalledCondition, @@ -90,6 +93,7 @@ var helmChartReadyCondition = summarize.Conditions{ sourcev1.BuildFailedCondition, sourcev1.ArtifactOutdatedCondition, sourcev1.ArtifactInStorageCondition, + sourcev1.SourceVerifiedCondition, meta.StalledCondition, meta.ReconcilingCondition, }, @@ -564,9 +568,30 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * }() } + var verifiers []soci.Verifier + if obj.Spec.Verify != nil { + provider := obj.Spec.Verify.Provider + verifiers, err = r.makeVerifiers(ctx, obj, authenticator, keychain) + if err != nil { + if obj.Spec.Verify.SecretRef == nil { + provider = fmt.Sprintf("%s keyless", provider) + } + e := serror.NewGeneric( + fmt.Errorf("failed to verify the signature using provider '%s': %w", provider, err), + sourcev1.VerificationError, + ) + conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e + } + } + // Tell the chart repository to use the OCI client with the configured getter clientOpts = append(clientOpts, helmgetter.WithRegistryClient(registryClient)) - ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL, repository.WithOCIGetter(r.Getters), repository.WithOCIGetterOptions(clientOpts), repository.WithOCIRegistryClient(registryClient)) + ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL, + repository.WithOCIGetter(r.Getters), + repository.WithOCIGetterOptions(clientOpts), + repository.WithOCIRegistryClient(registryClient), + repository.WithVerifiers(verifiers)) if err != nil { return chartRepoConfigErrorReturn(err, obj) } @@ -574,7 +599,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * // If login options are configured, use them to login to the registry // The OCIGetter will later retrieve the stored credentials to pull the chart - if keychain != nil { + if loginOpt != nil { err = ociChartRepo.Login(loginOpt) if err != nil { e := &serror.Event{ @@ -622,6 +647,17 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * opts := chart.BuildOptions{ ValuesFiles: obj.GetValuesFiles(), Force: obj.Generation != obj.Status.ObservedGeneration, + // The remote builder will not attempt to download the chart if + // an artifact exist with the same name and version and the force is false. + // It will try to verify the chart if: + // - we are on the first reconciliation + // - the HelmChart spec has changed (generation drift) + // - the previous reconciliation resulted in a failed artifact verification + // - there is no artifact in storage + Verify: obj.Spec.Verify != nil && (obj.Generation <= 0 || + conditions.GetObservedGeneration(obj, sourcev1.SourceVerifiedCondition) != obj.Generation || + conditions.IsFalse(obj, sourcev1.SourceVerifiedCondition) || + obj.GetArtifact() == nil), } if artifact := obj.GetArtifact(); artifact != nil { opts.CachedChart = r.Storage.LocalPath(*artifact) @@ -1030,7 +1066,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont // If login options are configured, use them to login to the registry // The OCIGetter will later retrieve the stored credentials to pull the chart - if keychain != nil { + if loginOpt != nil { err = ociChartRepo.Login(loginOpt) if err != nil { errs = append(errs, fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", repo.Name, err)) @@ -1239,6 +1275,11 @@ func observeChartBuild(obj *sourcev1.HelmChart, build *chart.Build, err error) { if build.Complete() { conditions.Delete(obj, sourcev1.FetchFailedCondition) conditions.Delete(obj, sourcev1.BuildFailedCondition) + conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, meta.SucceededReason, fmt.Sprintf("verified signature of version %s", build.Version)) + } + + if obj.Spec.Verify == nil { + conditions.Delete(obj, sourcev1.SourceVerifiedCondition) } if err != nil { @@ -1251,7 +1292,7 @@ func observeChartBuild(obj *sourcev1.HelmChart, build *chart.Build, err error) { } switch buildErr.Reason { - case chart.ErrChartMetadataPatch, chart.ErrValuesFilesMerge, chart.ErrDependencyBuild, chart.ErrChartPackage: + case chart.ErrChartMetadataPatch, chart.ErrValuesFilesMerge, chart.ErrDependencyBuild, chart.ErrChartPackage, chart.ErrChartVerification: conditions.Delete(obj, sourcev1.FetchFailedCondition) conditions.MarkTrue(obj, sourcev1.BuildFailedCondition, buildErr.Reason.Reason, buildErr.Error()) default: @@ -1290,3 +1331,60 @@ func chartRepoConfigErrorReturn(err error, obj *sourcev1.HelmChart) (sreconcile. return sreconcile.ResultEmpty, e } } + +// makeVerifiers returns a list of verifiers for the given chart. +func (r *HelmChartReconciler) makeVerifiers(ctx context.Context, obj *sourcev1.HelmChart, auth authn.Authenticator, keychain authn.Keychain) ([]soci.Verifier, error) { + var verifiers []soci.Verifier + verifyOpts := []remote.Option{} + if auth != nil { + verifyOpts = append(verifyOpts, remote.WithAuth(auth)) + } else { + verifyOpts = append(verifyOpts, remote.WithAuthFromKeychain(keychain)) + } + + switch obj.Spec.Verify.Provider { + case "cosign": + defaultCosignOciOpts := []soci.Options{ + soci.WithRemoteOptions(verifyOpts...), + } + + // get the public keys from the given secret + if secretRef := obj.Spec.Verify.SecretRef; secretRef != nil { + certSecretName := types.NamespacedName{ + Namespace: obj.Namespace, + Name: secretRef.Name, + } + + var pubSecret corev1.Secret + if err := r.Get(ctx, certSecretName, &pubSecret); err != nil { + return nil, err + } + + for k, data := range pubSecret.Data { + // search for public keys in the secret + if strings.HasSuffix(k, ".pub") { + verifier, err := soci.NewCosignVerifier(ctx, append(defaultCosignOciOpts, soci.WithPublicKey(data))...) + if err != nil { + return nil, err + } + verifiers = append(verifiers, verifier) + } + } + + if len(verifiers) == 0 { + return nil, fmt.Errorf("no public keys found in secret '%s'", certSecretName) + } + return verifiers, nil + } + + // if no secret is provided, add a keyless verifier + verifier, err := soci.NewCosignVerifier(ctx, defaultCosignOciOpts...) + if err != nil { + return nil, err + } + verifiers = append(verifiers, verifier) + return verifiers, nil + default: + return nil, fmt.Errorf("unsupported verification provider: %s", obj.Spec.Verify.Provider) + } +} diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go index 43ddd883..f9aaf2c8 100644 --- a/controllers/helmchart_controller_test.go +++ b/controllers/helmchart_controller_test.go @@ -26,6 +26,7 @@ import ( "io/ioutil" "net/http" "os" + "path" "path/filepath" "reflect" "strings" @@ -33,6 +34,9 @@ import ( "time" . "github.com/onsi/gomega" + coptions "github.com/sigstore/cosign/cmd/cosign/cli/options" + "github.com/sigstore/cosign/cmd/cosign/cli/sign" + "github.com/sigstore/cosign/pkg/cosign" hchart "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" helmreg "helm.sh/helm/v3/pkg/registry" @@ -57,6 +61,7 @@ import ( serror "github.com/fluxcd/source-controller/internal/error" "github.com/fluxcd/source-controller/internal/helm/chart" "github.com/fluxcd/source-controller/internal/helm/registry" + "github.com/fluxcd/source-controller/internal/oci" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" ) @@ -2213,6 +2218,228 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { } } +func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignature(t *testing.T) { + g := NewWithT(t) + + tmpDir := t.TempDir() + server, err := setupRegistryServer(ctx, tmpDir, registryOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + + const ( + chartPath = "testdata/charts/helmchart-0.1.0.tgz" + ) + + // Load a test chart + chartData, err := ioutil.ReadFile(chartPath) + + // Upload the test chart + metadata, err := loadTestChartToOCI(chartData, chartPath, server) + g.Expect(err).NotTo(HaveOccurred()) + + storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords) + g.Expect(err).ToNot(HaveOccurred()) + + cachedArtifact := &sourcev1.Artifact{ + Revision: "0.1.0", + Path: metadata.Name + "-" + metadata.Version + ".tgz", + } + g.Expect(storage.CopyFromPath(cachedArtifact, "testdata/charts/helmchart-0.1.0.tgz")).To(Succeed()) + + pf := func(b bool) ([]byte, error) { + return []byte("cosign-password"), nil + } + + keys, err := cosign.GenerateKeyPair(pf) + g.Expect(err).ToNot(HaveOccurred()) + + err = os.WriteFile(path.Join(tmpDir, "cosign.key"), keys.PrivateBytes, 0600) + g.Expect(err).ToNot(HaveOccurred()) + + defer func() { + err := os.Remove(path.Join(tmpDir, "cosign.key")) + g.Expect(err).ToNot(HaveOccurred()) + }() + + tests := []struct { + name string + want sreconcile.Result + wantErr bool + wantErrMsg string + shouldSign bool + beforeFunc func(obj *sourcev1.HelmChart) + assertConditions []metav1.Condition + cleanFunc func(g *WithT, build *chart.Build) + }{ + { + name: "unsigned charts should not pass verification", + beforeFunc: func(obj *sourcev1.HelmChart) { + obj.Spec.Chart = metadata.Name + obj.Spec.Version = metadata.Version + obj.Spec.Verify = &sourcev1.OCIRepositoryVerification{ + Provider: "cosign", + SecretRef: &meta.LocalObjectReference{Name: "cosign-key"}, + } + }, + wantErr: true, + wantErrMsg: "chart verification error: failed to verify : no matching signatures:", + want: sreconcile.ResultEmpty, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.BuildFailedCondition, "ChartVerificationError", "chart verification error: failed to verify : no matching signatures:"), + }, + }, + { + name: "unsigned charts should not pass keyless verification", + beforeFunc: func(obj *sourcev1.HelmChart) { + obj.Spec.Chart = metadata.Name + obj.Spec.Version = metadata.Version + obj.Spec.Verify = &sourcev1.OCIRepositoryVerification{ + Provider: "cosign", + } + }, + wantErr: true, + want: sreconcile.ResultEmpty, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.BuildFailedCondition, "ChartVerificationError", "chart verification error: failed to verify : no matching signatures:"), + }, + }, + { + name: "signed charts should pass verification", + beforeFunc: func(obj *sourcev1.HelmChart) { + obj.Spec.Chart = metadata.Name + obj.Spec.Version = metadata.Version + obj.Spec.Verify = &sourcev1.OCIRepositoryVerification{ + Provider: "cosign", + SecretRef: &meta.LocalObjectReference{Name: "cosign-key"}, + } + }, + shouldSign: true, + want: sreconcile.ResultSuccess, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled '' chart with version ''"), + *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of version "), + }, + cleanFunc: func(g *WithT, build *chart.Build) { + g.Expect(os.Remove(build.Path)).To(Succeed()) + }, + }, + { + name: "verify failed before, removed from spec, remove condition", + beforeFunc: func(obj *sourcev1.HelmChart) { + obj.Spec.Chart = metadata.Name + obj.Spec.Version = metadata.Version + obj.Spec.Verify = nil + conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, "VerifyFailed", "fail msg") + obj.Status.Artifact = &sourcev1.Artifact{Path: metadata.Name + "-" + metadata.Version + ".tgz"} + }, + want: sreconcile.ResultSuccess, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled '' chart with version ''"), + }, + cleanFunc: func(g *WithT, build *chart.Build) { + g.Expect(os.Remove(build.Path)).To(Succeed()) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + clientBuilder := fake.NewClientBuilder() + + repository := &sourcev1.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmrepository-", + }, + Spec: sourcev1.HelmRepositorySpec{ + URL: fmt.Sprintf("oci://%s/testrepo", server.registryHost), + Timeout: &metav1.Duration{Duration: timeout}, + Provider: sourcev1.GenericOCIProvider, + Type: sourcev1.HelmRepositoryTypeOCI, + }, + } + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cosign-key", + }, + Data: map[string][]byte{ + "cosign.pub": keys.PublicBytes, + }} + + clientBuilder.WithObjects(repository, secret) + + r := &HelmChartReconciler{ + Client: clientBuilder.Build(), + EventRecorder: record.NewFakeRecorder(32), + Getters: testGetters, + Storage: storage, + RegistryClientGenerator: registry.ClientGenerator, + } + + obj := &sourcev1.HelmChart{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmchart-", + }, + Spec: sourcev1.HelmChartSpec{ + SourceRef: sourcev1.LocalHelmChartSourceReference{ + Kind: sourcev1.HelmRepositoryKind, + Name: repository.Name, + }, + }, + } + + chartUrl := fmt.Sprintf("oci://%s/testrepo/%s:%s", server.registryHost, metadata.Name, metadata.Version) + + if tt.beforeFunc != nil { + tt.beforeFunc(obj) + } + + if tt.shouldSign { + ko := coptions.KeyOpts{ + KeyRef: path.Join(tmpDir, "cosign.key"), + PassFunc: pf, + } + + ro := &coptions.RootOptions{ + Timeout: timeout, + } + + err = sign.SignCmd(ro, ko, coptions.RegistryOptions{Keychain: oci.Anonymous{}}, + nil, []string{fmt.Sprintf("%s/testrepo/%s:%s", server.registryHost, metadata.Name, metadata.Version)}, "", + "", true, "", + "", "", false, + false, "", false) + g.Expect(err).ToNot(HaveOccurred()) + } + + assertConditions := tt.assertConditions + for k := range assertConditions { + assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "", metadata.Name) + assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "", metadata.Version) + assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "", chartUrl) + assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "", "cosign") + } + + var b chart.Build + if tt.cleanFunc != nil { + defer tt.cleanFunc(g, &b) + } + + got, err := r.reconcileSource(ctx, obj, &b) + if tt.wantErr { + tt.wantErrMsg = strings.ReplaceAll(tt.wantErrMsg, "", chartUrl) + g.Expect(err).ToNot(BeNil()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErrMsg)) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + g.Expect(got).To(Equal(tt.want)) + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions)) + }) + } +} + // extractChartMeta is used to extract a chart metadata from a byte array func extractChartMeta(chartData []byte) (*hchart.Metadata, error) { ch, err := loader.LoadArchive(bytes.NewReader(chartData)) diff --git a/controllers/ocirepository_controller.go b/controllers/ocirepository_controller.go index 35aec494..1f3dcffb 100644 --- a/controllers/ocirepository_controller.go +++ b/controllers/ocirepository_controller.go @@ -628,7 +628,7 @@ func (r *OCIRepositoryReconciler) verifySignature(ctx context.Context, obj *sour for k, data := range pubSecret.Data { // search for public keys in the secret if strings.HasSuffix(k, ".pub") { - verifier, err := soci.NewVerifier(ctxTimeout, append(defaultCosignOciOpts, soci.WithPublicKey(data))...) + verifier, err := soci.NewCosignVerifier(ctxTimeout, append(defaultCosignOciOpts, soci.WithPublicKey(data))...) if err != nil { return err } @@ -654,7 +654,7 @@ func (r *OCIRepositoryReconciler) verifySignature(ctx context.Context, obj *sour // if no secret is provided, try keyless verification ctrl.LoggerFrom(ctx).Info("no secret reference is provided, trying to verify the image using keyless method") - verifier, err := soci.NewVerifier(ctxTimeout, defaultCosignOciOpts...) + verifier, err := soci.NewCosignVerifier(ctxTimeout, defaultCosignOciOpts...) if err != nil { return err } diff --git a/docs/api/source.md b/docs/api/source.md index d5762fc3..060197be 100644 --- a/docs/api/source.md +++ b/docs/api/source.md @@ -670,6 +670,24 @@ references to this object. NOTE: Not implemented, provisional as of https://github.com/fluxcd/flux2/pull/2092

+ + +verify
+ + +OCIRepositoryVerification + + + + +(Optional) +

Verify contains the secret name containing the trusted public keys +used to verify the signature and specifies which provider to use to check +whether OCI image is authentic. +This field is only supported for OCI sources. +Chart dependencies, which are not bundled in the umbrella chart artifact, are not verified.

+ + @@ -2237,6 +2255,24 @@ references to this object. NOTE: Not implemented, provisional as of https://github.com/fluxcd/flux2/pull/2092

+ + +verify
+ + +OCIRepositoryVerification + + + + +(Optional) +

Verify contains the secret name containing the trusted public keys +used to verify the signature and specifies which provider to use to check +whether OCI image is authentic. +This field is only supported for OCI sources. +Chart dependencies, which are not bundled in the umbrella chart artifact, are not verified.

+ + @@ -3123,6 +3159,7 @@ github.com/fluxcd/pkg/apis/meta.ReconcileRequestStatus

(Appears on: +HelmChartSpec, OCIRepositorySpec)

OCIRepositoryVerification verifies the authenticity of an OCI Artifact

diff --git a/hack/ci/e2e.sh b/hack/ci/e2e.sh index 314eb5b1..3e578de2 100755 --- a/hack/ci/e2e.sh +++ b/hack/ci/e2e.sh @@ -165,6 +165,7 @@ echo "Run HelmChart from OCI registry tests" kubectl -n source-system apply -f "${ROOT_DIR}/config/testdata/helmchart-from-oci/source.yaml" kubectl -n source-system wait helmrepository/podinfo --for=condition=ready --timeout=1m kubectl -n source-system wait helmchart/podinfo --for=condition=ready --timeout=1m +kubectl -n source-system wait helmchart/podinfo-keyless --for=condition=ready --timeout=1m echo "Run OCIRepository verify tests" kubectl -n source-system apply -f "${ROOT_DIR}/config/testdata/ocirepository/signed-with-key.yaml" diff --git a/internal/helm/chart/builder.go b/internal/helm/chart/builder.go index 76dc517c..5be208d8 100644 --- a/internal/helm/chart/builder.go +++ b/internal/helm/chart/builder.go @@ -113,6 +113,8 @@ type BuildOptions struct { // Force can be set to force the build of the chart, for example // because the list of ValuesFiles has changed. Force bool + // Verifier can be set to the verification of the chart. + Verify bool } // GetValuesFiles returns BuildOptions.ValuesFiles, except if it equals diff --git a/internal/helm/chart/builder_remote.go b/internal/helm/chart/builder_remote.go index d15e2429..20589472 100644 --- a/internal/helm/chart/builder_remote.go +++ b/internal/helm/chart/builder_remote.go @@ -63,7 +63,7 @@ func NewRemoteBuilder(repository repository.Downloader) Builder { // After downloading the chart, it is only packaged if required due to BuildOptions // modifying the chart, otherwise the exact data as retrieved from the repository // is written to p, after validating it to be a chart. -func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, opts BuildOptions) (*Build, error) { +func (b *remoteChartBuilder) Build(ctx context.Context, ref Reference, p string, opts BuildOptions) (*Build, error) { remoteRef, ok := ref.(RemoteReference) if !ok { err := fmt.Errorf("expected remote chart reference") @@ -74,9 +74,9 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o return nil, &BuildError{Reason: ErrChartReference, Err: err} } - res, result, err := b.downloadFromRepository(b.remote, remoteRef, opts) + res, result, err := b.downloadFromRepository(ctx, b.remote, remoteRef, opts) if err != nil { - return nil, &BuildError{Reason: ErrChartPull, Err: err} + return nil, err } if res == nil { return result, nil @@ -124,7 +124,7 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o return result, nil } -func (b *remoteChartBuilder) downloadFromRepository(remote repository.Downloader, remoteRef RemoteReference, opts BuildOptions) (*bytes.Buffer, *Build, error) { +func (b *remoteChartBuilder) downloadFromRepository(ctx context.Context, remote repository.Downloader, remoteRef RemoteReference, opts BuildOptions) (*bytes.Buffer, *Build, error) { // Get the current version for the RemoteReference cv, err := remote.GetChartVersion(remoteRef.Name, remoteRef.Version) if err != nil { @@ -132,6 +132,13 @@ func (b *remoteChartBuilder) downloadFromRepository(remote repository.Downloader return nil, nil, &BuildError{Reason: ErrChartReference, Err: err} } + // Verify the chart if necessary + if opts.Verify { + if err := remote.VerifyChart(ctx, cv); err != nil { + return nil, nil, &BuildError{Reason: ErrChartVerification, Err: err} + } + } + result, shouldReturn, err := generateBuildResult(cv, opts) if err != nil { return nil, nil, err diff --git a/internal/helm/chart/errors.go b/internal/helm/chart/errors.go index dedff9e3..7b1b7f3b 100644 --- a/internal/helm/chart/errors.go +++ b/internal/helm/chart/errors.go @@ -84,5 +84,6 @@ var ( ErrValuesFilesMerge = BuildErrorReason{Reason: "ValuesFilesError", Summary: "values files merge error"} ErrDependencyBuild = BuildErrorReason{Reason: "DependencyBuildError", Summary: "dependency build error"} ErrChartPackage = BuildErrorReason{Reason: "ChartPackageError", Summary: "chart package error"} + ErrChartVerification = BuildErrorReason{Reason: "ChartVerificationError", Summary: "chart verification error"} ErrUnknown = BuildErrorReason{Reason: "Unknown", Summary: "unknown build error"} ) diff --git a/internal/helm/repository/chart_repository.go b/internal/helm/repository/chart_repository.go index 282d49a5..3997d5f3 100644 --- a/internal/helm/repository/chart_repository.go +++ b/internal/helm/repository/chart_repository.go @@ -18,6 +18,7 @@ package repository import ( "bytes" + "context" "crypto/sha256" "crypto/tls" "encoding/hex" @@ -520,3 +521,12 @@ func (r *ChartRepository) RemoveCache() error { } return nil } + +// VerifyChart verifies the chart against a signature. +// If no signature is provided, a keyless verification is performed. +// It returns an error on failure. +func (r *ChartRepository) VerifyChart(_ context.Context, _ *repo.ChartVersion) error { + // no-op + // this is a no-op because this is not implemented yet. + return nil +} diff --git a/internal/helm/repository/oci_chart_repository.go b/internal/helm/repository/oci_chart_repository.go index a037e6b4..fe03a0e6 100644 --- a/internal/helm/repository/oci_chart_repository.go +++ b/internal/helm/repository/oci_chart_repository.go @@ -18,6 +18,7 @@ package repository import ( "bytes" + "context" "crypto/tls" "fmt" "net/url" @@ -32,7 +33,10 @@ import ( "helm.sh/helm/v3/pkg/repo" "github.com/Masterminds/semver/v3" + "github.com/google/go-containerregistry/pkg/name" + "github.com/fluxcd/pkg/version" + "github.com/fluxcd/source-controller/internal/oci" "github.com/fluxcd/source-controller/internal/transport" ) @@ -63,12 +67,23 @@ type OCIChartRepository struct { RegistryClient RegistryClient // credentialsFile is a temporary credentials file to use while downloading tags or charts from a registry. credentialsFile string + + // verifiers is a list of verifiers to use when verifying a chart. + verifiers []oci.Verifier } // OCIChartRepositoryOption is a function that can be passed to NewOCIChartRepository // to configure an OCIChartRepository. type OCIChartRepositoryOption func(*OCIChartRepository) error +// WithVerifiers returns a ChartRepositoryOption that will set the chart verifiers +func WithVerifiers(verifiers []oci.Verifier) OCIChartRepositoryOption { + return func(r *OCIChartRepository) error { + r.verifiers = verifiers + return nil + } +} + // WithOCIRegistryClient returns a ChartRepositoryOption that will set the registry client func WithOCIRegistryClient(client RegistryClient) OCIChartRepositoryOption { return func(r *OCIChartRepository) error { @@ -215,7 +230,6 @@ func (r *OCIChartRepository) DownloadChart(chart *repo.ChartVersion) (*bytes.Buf // Login attempts to login to the OCI registry. // It returns an error on failure. func (r *OCIChartRepository) Login(opts ...registry.LoginOption) error { - // Get login credentials from keychain err := r.RegistryClient.Login(r.URL.Host, opts...) if err != nil { return err @@ -297,3 +311,32 @@ func getLastMatchingVersionOrConstraint(cvs []string, ver string) (string, error return matchingVersions[0].Original(), nil } + +// VerifyChart verifies the chart against a signature. +// If no signature is provided, a keyless verification is performed. +// It returns an error on failure. +func (r *OCIChartRepository) VerifyChart(ctx context.Context, chart *repo.ChartVersion) error { + if len(r.verifiers) == 0 { + return fmt.Errorf("no verifiers available") + } + + if len(chart.URLs) == 0 { + return fmt.Errorf("chart '%s' has no downloadable URLs", chart.Name) + } + + ref, err := name.ParseReference(strings.TrimPrefix(chart.URLs[0], fmt.Sprintf("%s://", registry.OCIScheme))) + if err != nil { + return fmt.Errorf("invalid chart reference: %s", err) + } + + // verify the chart + for _, verifier := range r.verifiers { + if verified, err := verifier.Verify(ctx, ref); err != nil { + return fmt.Errorf("failed to verify %s: %w", chart.URLs[0], err) + } else if verified { + return nil + } + } + + return fmt.Errorf("no matching signatures were found for '%s'", ref.Name()) +} diff --git a/internal/helm/repository/repository.go b/internal/helm/repository/repository.go index 4c8cb7ff..5fdf62bf 100644 --- a/internal/helm/repository/repository.go +++ b/internal/helm/repository/repository.go @@ -18,6 +18,7 @@ package repository import ( "bytes" + "context" "helm.sh/helm/v3/pkg/repo" ) @@ -29,6 +30,8 @@ type Downloader interface { GetChartVersion(name, version string) (*repo.ChartVersion, error) // DownloadChart downloads a chart from the remote Helm repository or OCI Helm repository. DownloadChart(chart *repo.ChartVersion) (*bytes.Buffer, error) + // VerifyChart verifies the chart against a signature. + VerifyChart(ctx context.Context, chart *repo.ChartVersion) error // Clear removes all temporary files created by the downloader, caching the files if the cache is configured, // and calling garbage collector to remove unused files. Clear() error diff --git a/internal/oci/verifier.go b/internal/oci/verifier.go index b8d9c5d4..490b3ef4 100644 --- a/internal/oci/verifier.go +++ b/internal/oci/verifier.go @@ -34,13 +34,18 @@ import ( "github.com/sigstore/sigstore/pkg/signature" ) +// Verifier is an interface for verifying the authenticity of an OCI image. +type Verifier interface { + Verify(ctx context.Context, ref name.Reference) (bool, error) +} + // options is a struct that holds options for verifier. type options struct { PublicKey []byte ROpt []remote.Option } -// Options is a function that configures the options applied to a Verifier. +// Options is a function that configures the options applied to a CosignVerifier. type Options func(opts *options) // WithPublicKey sets the public key. @@ -58,13 +63,13 @@ func WithRemoteOptions(opts ...remote.Option) Options { } } -// Verifier is a struct which is responsible for executing verification logic. -type Verifier struct { +// CosignVerifier is a struct which is responsible for executing verification logic. +type CosignVerifier struct { opts *cosign.CheckOpts } -// NewVerifier initializes a new Verifier. -func NewVerifier(ctx context.Context, opts ...Options) (*Verifier, error) { +// NewCosignVerifier initializes a new CosignVerifier. +func NewCosignVerifier(ctx context.Context, opts ...Options) (*CosignVerifier, error) { o := options{} for _, opt := range opts { opt(&o) @@ -117,12 +122,28 @@ func NewVerifier(ctx context.Context, opts ...Options) (*Verifier, error) { checkOpts.RekorClient = rc } - return &Verifier{ + return &CosignVerifier{ opts: checkOpts, }, nil } // VerifyImageSignatures verify the authenticity of the given ref OCI image. -func (v *Verifier) VerifyImageSignatures(ctx context.Context, ref name.Reference) ([]oci.Signature, bool, error) { +func (v *CosignVerifier) VerifyImageSignatures(ctx context.Context, ref name.Reference) ([]oci.Signature, bool, error) { return cosign.VerifyImageSignatures(ctx, ref, v.opts) } + +// Verify verifies the authenticity of the given ref OCI image. +// It returns a boolean indicating if the verification was successful. +// It returns an error if the verification fails, nil otherwise. +func (v *CosignVerifier) Verify(ctx context.Context, ref name.Reference) (bool, error) { + signatures, _, err := v.VerifyImageSignatures(ctx, ref) + if err != nil { + return false, err + } + + if len(signatures) == 0 { + return false, nil + } + + return true, nil +}