From 082028e115bb39fa1f036fc99453293c19432488 Mon Sep 17 00:00:00 2001 From: Stefan Prodan Date: Tue, 20 Sep 2022 12:40:05 +0300 Subject: [PATCH] Refactor internal OCI package Signed-off-by: Stefan Prodan --- Makefile | 8 ++-- controllers/ocirepository_controller.go | 47 +++++++++++--------- controllers/ocirepository_controller_test.go | 8 ++-- docs/spec/v1beta2/ocirepositories.md | 12 ++++- internal/{util => oci}/auth.go | 2 +- internal/oci/{oci.go => verifier.go} | 23 +++------- 6 files changed, 52 insertions(+), 48 deletions(-) rename internal/{util => oci}/auth.go (98%) rename internal/oci/{oci.go => verifier.go} (86%) diff --git a/Makefile b/Makefile index c9786666..fd731a92 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ # Image URL to use all building/pushing image targets -IMG ?= fluxcd/source-controller -TAG ?= latest +IMG ?= localhost:5050/source-controller +TAG ?= test1 # Base image used to build the Go binary LIBGIT2_IMG ?= ghcr.io/fluxcd/golang-with-libgit2-only @@ -14,9 +14,9 @@ GO_TEST_PREFIX ?= # Allows for defining additional Docker buildx arguments, # e.g. '--push'. -BUILD_ARGS ?= +BUILD_ARGS ?= --load # Architectures to build images for -BUILD_PLATFORMS ?= linux/amd64,linux/arm64,linux/arm/v7 +BUILD_PLATFORMS ?= linux/arm64 # Go additional tag arguments, e.g. 'integration', # this is append to the tag arguments required for static builds diff --git a/controllers/ocirepository_controller.go b/controllers/ocirepository_controller.go index 32c93ba9..bed13181 100644 --- a/controllers/ocirepository_controller.go +++ b/controllers/ocirepository_controller.go @@ -311,7 +311,7 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, obj *sour } options = append(options, crane.WithAuthFromKeychain(keychain)) - if _, ok := keychain.(util.Anonymous); obj.Spec.Provider != sourcev1.GenericOCIProvider && ok { + if _, ok := keychain.(soci.Anonymous); obj.Spec.Provider != sourcev1.GenericOCIProvider && ok { auth, authErr := oidcAuth(ctxTimeout, obj.Spec.URL, obj.Spec.Provider) if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) { e := serror.NewGeneric( @@ -409,22 +409,28 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, obj *sour } }() + // Verify artifact + if obj.Spec.Verify == nil { + // Remove old observations if verification was disabled + conditions.Delete(obj, sourcev1.SourceVerifiedCondition) + } else if !obj.GetArtifact().HasRevision(revision) || conditions.GetObservedGeneration(obj, sourcev1.SourceVerifiedCondition) != obj.Generation { + provider := obj.Spec.Verify.Provider + err := r.verifyOCISourceSignature(ctx, obj, url, keychain) + if err != nil { + 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()) + conditions.MarkFalse(obj, meta.ReconcilingCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e + } + + conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of digest %s", revision) + } + // Extract the content of the first artifact layer if !obj.GetArtifact().HasRevision(revision) { - if obj.Spec.Verify != nil { - provider := obj.Spec.Verify.Provider - err := r.verifyOCISourceSignature(ctx, obj, url, keychain) - if err != nil { - e := serror.NewGeneric( - fmt.Errorf("failed to verify OCI image signature '%s' using provider '%s': %w", url, provider, err), - sourcev1.VerificationError, - ) - conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, e.Err.Error()) - return sreconcile.ResultEmpty, e - } - - conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, meta.SucceededReason, "OCI image %s with digest %s verified.", url, revision) - } layers, err := img.Layers() if err != nil { e := serror.NewGeneric( @@ -512,7 +518,6 @@ func (r *OCIRepositoryReconciler) verifyOCISourceSignature(ctx context.Context, case "cosign": defaultCosignOciOpts := []soci.Options{ soci.WithAuthnKeychain(keychain), - soci.WithContext(ctxTimeout), } ref, err := name.ParseReference(url) @@ -536,12 +541,12 @@ func (r *OCIRepositoryReconciler) verifyOCISourceSignature(ctx context.Context, for k, data := range pubSecret.Data { // search for public keys in the secret if strings.HasSuffix(k, ".pub") { - verifier, err := soci.New(append(defaultCosignOciOpts, soci.WithPublicKey(data))...) + verifier, err := soci.NewVerifier(ctxTimeout, append(defaultCosignOciOpts, soci.WithPublicKey(data))...) if err != nil { return err } - signatures, _, err := verifier.VerifyImageSignatures(ref) + signatures, _, err := verifier.VerifyImageSignatures(ctxTimeout, ref) if err != nil { continue } @@ -562,12 +567,12 @@ func (r *OCIRepositoryReconciler) verifyOCISourceSignature(ctx context.Context, // if no secret is provided, try keyless verification ctrl.LoggerFrom(ctx).Info("no secret reference is provided, trying to verify the image using keyless approach") - verifier, err := soci.New(defaultCosignOciOpts...) + verifier, err := soci.NewVerifier(ctxTimeout, defaultCosignOciOpts...) if err != nil { return err } - signatures, _, err := verifier.VerifyImageSignatures(ref) + signatures, _, err := verifier.VerifyImageSignatures(ctxTimeout, ref) if err != nil { return err } @@ -689,7 +694,7 @@ func (r *OCIRepositoryReconciler) keychain(ctx context.Context, obj *sourcev1.OC // if no pullsecrets available return an AnonymousKeychain if len(pullSecretNames) == 0 { - return util.Anonymous{}, nil + return soci.Anonymous{}, nil } // lookup image pull secrets diff --git a/controllers/ocirepository_controller_test.go b/controllers/ocirepository_controller_test.go index fdd538a5..476066fe 100644 --- a/controllers/ocirepository_controller_test.go +++ b/controllers/ocirepository_controller_test.go @@ -1042,22 +1042,22 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) { assertConditions: []metav1.Condition{ *conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new digest '' for ''"), *conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new digest '' for ''"), - *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, meta.SucceededReason, "OCI image with digest verified."), + *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of digest "), }, }, { - name: "not signed image should not pass verification", + name: "unsigned image should not pass verification", reference: &sourcev1.OCIRepositoryRef{ Tag: "6.1.5", }, digest: img5.digest.Hex, wantErr: true, - wantErrMsg: "failed to verify OCI image signature '' using provider 'cosign': no matching signatures were found for '", + wantErrMsg: "failed to verify the signature using provider 'cosign': no matching signatures were found for ''", want: sreconcile.ResultEmpty, assertConditions: []metav1.Condition{ *conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new digest '' for ''"), *conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new digest '' for ''"), - *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "failed to verify OCI image signature '' using provider '': no matching signatures were found for ''"), + *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "failed to verify the signature using provider '': no matching signatures were found for ''"), }, }, } diff --git a/docs/spec/v1beta2/ocirepositories.md b/docs/spec/v1beta2/ocirepositories.md index 39545fbe..9e2e5069 100644 --- a/docs/spec/v1beta2/ocirepositories.md +++ b/docs/spec/v1beta2/ocirepositories.md @@ -455,7 +455,7 @@ data: key2.pub: ``` -Note that the keys must have the `.pub` extension for Flux to make user of them. +Note that the keys must have the `.pub` extension for Flux to make use of them. #### Keyless verification @@ -482,7 +482,7 @@ The controller verifies the signatures using the Fulcio root CA and the Rekor instance hosted at [rekor.sigstore.dev](https://rekor.sigstore.dev/). Note that keyless verification is an **experimental feature**, using -custom root CAs or self-hosted Rekor instances are not currency supported. +custom root CAs or self-hosted Rekor instances are not currently supported. ### Suspend @@ -839,6 +839,14 @@ and is only present on the OCIRepository while the status value is `"True"`. There may be more arbitrary values for the `reason` field to provide accurate reason for a condition. +In addition to the above Condition types, when the signature +[verification](#verification) fails. A condition with +the following attributes is added to the GitRepository's `.status.conditions`: + +- `type: SourceVerified` +- `status: "False"` +- `reason: VerificationError` + While the OCIRepository has one or more of these Conditions, the controller will continue to attempt to produce an Artifact for the resource with an exponential backoff, until it succeeds and the OCIRepository is marked as diff --git a/internal/util/auth.go b/internal/oci/auth.go similarity index 98% rename from internal/util/auth.go rename to internal/oci/auth.go index 8b944cc3..88b0e944 100644 --- a/internal/util/auth.go +++ b/internal/oci/auth.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package util +package oci import "github.com/google/go-containerregistry/pkg/authn" diff --git a/internal/oci/oci.go b/internal/oci/verifier.go similarity index 86% rename from internal/oci/oci.go rename to internal/oci/verifier.go index 850e3890..17a5345d 100644 --- a/internal/oci/oci.go +++ b/internal/oci/verifier.go @@ -38,7 +38,6 @@ import ( type options struct { PublicKey []byte Keychain authn.Keychain - Context context.Context } // Options is a function that configures the options applied to a Verifier. @@ -57,20 +56,13 @@ func WithAuthnKeychain(keychain authn.Keychain) Options { } } -func WithContext(ctx context.Context) Options { - return func(opts *options) { - opts.Context = ctx - } -} - // Verifier is a struct which is responsible for executing verification logic. type Verifier struct { - opts *cosign.CheckOpts - context context.Context + opts *cosign.CheckOpts } -// New initializes a new Verifier. -func New(opts ...Options) (*Verifier, error) { +// NewVerifier initializes a new Verifier. +func NewVerifier(ctx context.Context, opts ...Options) (*Verifier, error) { o := options{} for _, opt := range opts { opt(&o) @@ -79,7 +71,7 @@ func New(opts ...Options) (*Verifier, error) { checkOpts := &cosign.CheckOpts{} ro := coptions.RegistryOptions{} - co, err := ro.ClientOpts(o.Context) + co, err := ro.ClientOpts(ctx) if err != nil { return nil, err } @@ -124,12 +116,11 @@ func New(opts ...Options) (*Verifier, error) { } return &Verifier{ - opts: checkOpts, - context: o.Context, + opts: checkOpts, }, nil } // VerifyImageSignatures verify the authenticity of the given ref OCI image. -func (v *Verifier) VerifyImageSignatures(ref name.Reference) ([]oci.Signature, bool, error) { - return cosign.VerifyImageSignatures(v.context, ref, v.opts) +func (v *Verifier) VerifyImageSignatures(ctx context.Context, ref name.Reference) ([]oci.Signature, bool, error) { + return cosign.VerifyImageSignatures(ctx, ref, v.opts) }