diff --git a/api/v1/ocirepository_types.go b/api/v1/ocirepository_types.go index 5f0d64688..05d099fb9 100644 --- a/api/v1/ocirepository_types.go +++ b/api/v1/ocirepository_types.go @@ -210,6 +210,13 @@ type OCIRepositoryStatus struct { // +optional ObservedLayerSelector *OCILayerSelector `json:"observedLayerSelector,omitempty"` + // SourceVerificationFingerprint is the fingerprint of the verification + // material used to verify the signature of the current Artifact. It is + // used to detect changes to the verification policy, such as a key + // rotation, that require the current revision to be verified again. + // +optional + SourceVerificationFingerprint string `json:"sourceVerificationFingerprint,omitempty"` + meta.ReconcileRequestStatus `json:",inline"` } diff --git a/config/crd/bases/source.toolkit.fluxcd.io_ocirepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_ocirepositories.yaml index 26b04ca47..b152ec384 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_ocirepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_ocirepositories.yaml @@ -412,6 +412,13 @@ spec: - copy type: string type: object + sourceVerificationFingerprint: + description: |- + SourceVerificationFingerprint is the fingerprint of the verification + material used to verify the signature of the current Artifact. It is + used to detect changes to the verification policy, such as a key + rotation, that require the current revision to be verified again. + type: string url: description: URL is the download link for the artifact output of the last OCI Repository sync. diff --git a/docs/api/v1/source.md b/docs/api/v1/source.md index 9dadb7390..de7f9a566 100644 --- a/docs/api/v1/source.md +++ b/docs/api/v1/source.md @@ -3672,6 +3672,21 @@ the source artifact.

+sourceVerificationFingerprint
+ +string + + + +(Optional) +

SourceVerificationFingerprint is the fingerprint of the verification +material used to verify the signature of the current Artifact. It is +used to detect changes to the verification policy, such as a key +rotation, that require the current revision to be verified again.

+ + + + ReconcileRequestStatus
diff --git a/docs/spec/v1/ocirepositories.md b/docs/spec/v1/ocirepositories.md index 455d69ae5..bb742be39 100644 --- a/docs/spec/v1/ocirepositories.md +++ b/docs/spec/v1/ocirepositories.md @@ -644,6 +644,12 @@ spec: By default, 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 rotations of the public Sigstore trust anchors (Fulcio, Rekor, CT log +and TSA keys) are not detected. When no `.spec.verify.trustedRootSecretRef` is +set, the `SourceVerified` condition is not re-evaluated on a trust root change, +so verification is retried only when the artifact revision or the spec changes. +To track a specific trust root, pin it with `.spec.verify.trustedRootSecretRef`. + ##### Custom Sigstore infrastructure (self-hosted Rekor / Fulcio) To verify artifacts signed with a self-hosted Sigstore deployment, provide a @@ -1183,6 +1189,17 @@ status: ... ``` +### Source Verification Fingerprint + +The source-controller reports a fingerprint of the verification material it used +to verify the signature of the current Artifact in the OCIRepository's +`.status.sourceVerificationFingerprint`. The fingerprint is derived from the +referenced Secrets, such as the Cosign public keys or the Notation trust policy +and certificates, and does not depend on the Secret names. It is used by the +controller to detect a change in the verification policy, such as a key +rotation, that requires the current revision to be verified again even when its +revision did not change. + ### Observed Generation The source-controller reports an [observed generation][typical-status-properties] diff --git a/internal/controller/ocirepository_controller.go b/internal/controller/ocirepository_controller.go index 3ae2d40ed..cbd4f7777 100644 --- a/internal/controller/ocirepository_controller.go +++ b/internal/controller/ocirepository_controller.go @@ -38,6 +38,7 @@ import ( gcrv1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/notaryproject/notation-go/verifier/trustpolicy" + "github.com/opencontainers/go-digest" "github.com/sigstore/cosign/v3/pkg/cosign" "helm.sh/helm/v4/pkg/registry" corev1 "k8s.io/api/core/v1" @@ -473,12 +474,15 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch // - the upstream digest differs from the one in storage (revision drift) // - the OCIRepository spec has changed (generation drift) // - the previous reconciliation resulted in a failed artifact verification (retry with exponential backoff) + // - the verification policy (e.g. a key rotation) has changed (policy drift) if obj.Spec.Verify == nil { // Remove old observations if verification was disabled conditions.Delete(obj, sourcev1.SourceVerifiedCondition) + obj.Status.SourceVerificationFingerprint = "" } else if !obj.GetArtifact().HasRevision(revision) || conditions.GetObservedGeneration(obj, sourcev1.SourceVerifiedCondition) != obj.Generation || - conditions.IsFalse(obj, sourcev1.SourceVerifiedCondition) { + conditions.IsFalse(obj, sourcev1.SourceVerifiedCondition) || + r.verificationPolicyChanged(ctx, obj) { result, err := r.verifySignature(ctx, obj, digestRef, keychain, authenticator, transport, opts...) if err != nil { @@ -496,6 +500,9 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch if result == soci.VerificationResultSuccess { conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of revision %s", revision) + if fingerprint, err := r.verificationFingerprint(ctx, obj); err == nil { + obj.Status.SourceVerificationFingerprint = fingerprint + } } } @@ -661,6 +668,108 @@ func (r *OCIRepositoryReconciler) digestFromRevision(revision string) string { return parts[len(parts)-1] } +// verificationPolicyChanged returns true if the verification material referenced +// by the object differs from the one used for the last successful verification, +// or if the current policy can not be determined. A changed policy requires the +// current revision to be verified again, even if it did not change. +func (r *OCIRepositoryReconciler) verificationPolicyChanged(ctx context.Context, obj *sourcev1.OCIRepository) bool { + if obj.Spec.Verify == nil { + return false + } + fingerprint, err := r.verificationFingerprint(ctx, obj) + if err != nil { + // Return true so the full reconciliation surfaces the error. + return true + } + return fingerprint != obj.Status.SourceVerificationFingerprint +} + +// verificationFingerprint returns a stable fingerprint of the verification +// material referenced by the object. It is used to detect a change in the +// verification policy, e.g. a key rotation, that requires the current revision +// to be verified again even if it did not change. +func (r *OCIRepositoryReconciler) verificationFingerprint(ctx context.Context, obj *sourcev1.OCIRepository) (string, error) { + verify := obj.Spec.Verify + if verify == nil { + return "", nil + } + + var trustedRoot []byte + if ref := verify.TrustedRootSecretRef; ref != nil && verify.Provider == "cosign" { + data, err := readTrustedRootFromSecret(ctx, r.Client, obj.Namespace, ref) + if err != nil { + return "", err + } + trustedRoot = data + } + + var secret *corev1.Secret + if ref := verify.SecretRef; ref != nil { + s, err := r.retrieveSecret(ctx, types.NamespacedName{Namespace: obj.Namespace, Name: ref.Name}) + if err != nil { + return "", err + } + secret = &s + } + + return verificationMaterialFingerprint(verify.Provider, secret, trustedRoot), nil +} + +// verificationMaterialFingerprint returns a stable fingerprint of the given +// verification material. It is independent of the Secret name and of the data +// key names, as the policy is defined by the material itself, not its location. +func verificationMaterialFingerprint(provider string, secret *corev1.Secret, trustedRoot []byte) string { + var b strings.Builder + b.WriteString("provider:") + b.WriteString(provider) + b.WriteByte(0) + + if len(trustedRoot) > 0 { + b.WriteString("trustedroot:") + b.Write(trustedRoot) + b.WriteByte(0) + } + + if secret != nil { + switch provider { + case "notation": + if data, ok := secret.Data[notation.DefaultTrustPolicyKey]; ok { + b.WriteString("trustpolicy:") + b.Write(data) + b.WriteByte(0) + } + var certs []string + for k, v := range secret.Data { + if strings.HasSuffix(k, ".crt") || strings.HasSuffix(k, ".pem") { + certs = append(certs, string(v)) + } + } + sort.Strings(certs) + for _, cert := range certs { + b.WriteString("cert:") + b.WriteString(cert) + b.WriteByte(0) + } + default: + // cosign: the public keys used for verification. + var keys []string + for k, v := range secret.Data { + if strings.HasSuffix(k, ".pub") { + keys = append(keys, string(v)) + } + } + sort.Strings(keys) + for _, key := range keys { + b.WriteString("pub:") + b.WriteString(key) + b.WriteByte(0) + } + } + } + + return digest.Canonical.FromString(b.String()).String() +} + // verifySignature verifies the authenticity of the given image reference URL. // It supports two different verification providers: cosign and notation. // First, it tries to use a key if a Secret with a valid public key is provided. diff --git a/internal/controller/ocirepository_controller_test.go b/internal/controller/ocirepository_controller_test.go index 45037275d..4a5a56cc1 100644 --- a/internal/controller/ocirepository_controller_test.go +++ b/internal/controller/ocirepository_controller_test.go @@ -1397,18 +1397,19 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignatureNotation(t *testi g := NewWithT(t) tests := []struct { - name string - reference *sourcev1.OCIRepositoryRef - insecure bool - want sreconcile.Result - wantErr bool - wantErrMsg string - shouldSign bool - useDigest bool - addMultipleCerts bool - provideNoCert bool - beforeFunc func(obj *sourcev1.OCIRepository, tag, revision string) - assertConditions []metav1.Condition + name string + reference *sourcev1.OCIRepositoryRef + insecure bool + want sreconcile.Result + wantErr bool + wantErrMsg string + shouldSign bool + useDigest bool + addMultipleCerts bool + provideNoCert bool + verifiedWithCurrentKeys bool + beforeFunc func(obj *sourcev1.OCIRepository, tag, revision string) + assertConditions []metav1.Condition }{ { name: "signed image should pass verification", @@ -1468,6 +1469,9 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignatureNotation(t *testi name: "no verify for already verified, verified condition remains the same", reference: &sourcev1.OCIRepositoryRef{Tag: "6.1.4"}, shouldSign: true, + // The recorded fingerprint matches the referenced keys, so the + // verification can be skipped. + verifiedWithCurrentKeys: true, beforeFunc: func(obj *sourcev1.OCIRepository, tag, revision string) { // Artifact present and custom verified condition reason/message. obj.Status.Artifact = &meta.Artifact{Revision: fmt.Sprintf("%s@%s", tag, revision)} @@ -1478,6 +1482,21 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignatureNotation(t *testi *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, "Verified", "verified"), }, }, + { + name: "same artifact, verified before with rotated keys, verify again", + reference: &sourcev1.OCIRepositoryRef{Tag: "6.1.4"}, + shouldSign: true, + beforeFunc: func(obj *sourcev1.OCIRepository, tag, revision string) { + obj.Status.Artifact = &meta.Artifact{Revision: fmt.Sprintf("%s@%s", tag, revision)} + conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, "Verified", "verified") + // The recorded fingerprint no longer matches the referenced keys. + obj.Status.SourceVerificationFingerprint = "stale" + }, + want: sreconcile.ResultSuccess, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of revision "), + }, + }, { name: "signed image on an insecure registry passes verification", reference: &sourcev1.OCIRepositoryRef{ @@ -1723,6 +1742,9 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignatureNotation(t *testi if tt.beforeFunc != nil { tt.beforeFunc(obj, image.tag, image.digest.String()) } + if tt.verifiedWithCurrentKeys { + obj.Status.SourceVerificationFingerprint = verificationMaterialFingerprint(obj.Spec.Verify.Provider, secret, nil) + } g.Expect(r.Client.Create(ctx, obj)).ToNot(HaveOccurred()) defer func() { @@ -2095,16 +2117,17 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignatureCosign(t *testing g := NewWithT(t) tests := []struct { - name string - reference *sourcev1.OCIRepositoryRef - insecure bool - want sreconcile.Result - wantErr bool - wantErrMsg string - shouldSign bool - keyless bool - beforeFunc func(obj *sourcev1.OCIRepository, tag, revision string) - assertConditions []metav1.Condition + name string + reference *sourcev1.OCIRepositoryRef + insecure bool + want sreconcile.Result + wantErr bool + wantErrMsg string + shouldSign bool + keyless bool + verifiedWithCurrentKeys bool + beforeFunc func(obj *sourcev1.OCIRepository, tag, revision string) + assertConditions []metav1.Condition }{ { name: "signed image should pass verification", @@ -2177,6 +2200,9 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignatureCosign(t *testing name: "no verify for already verified, verified condition remains the same", reference: &sourcev1.OCIRepositoryRef{Tag: "6.1.4"}, shouldSign: true, + // The recorded fingerprint matches the referenced keys, so the + // verification can be skipped. + verifiedWithCurrentKeys: true, beforeFunc: func(obj *sourcev1.OCIRepository, tag, revision string) { // Artifact present and custom verified condition reason/message. obj.Status.Artifact = &meta.Artifact{Revision: fmt.Sprintf("%s@%s", tag, revision)} @@ -2187,6 +2213,21 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignatureCosign(t *testing *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, "Verified", "verified"), }, }, + { + name: "same artifact, verified before with rotated keys, verify again", + reference: &sourcev1.OCIRepositoryRef{Tag: "6.1.4"}, + shouldSign: true, + beforeFunc: func(obj *sourcev1.OCIRepository, tag, revision string) { + obj.Status.Artifact = &meta.Artifact{Revision: fmt.Sprintf("%s@%s", tag, revision)} + conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, "Verified", "verified") + // The recorded fingerprint no longer matches the referenced keys. + obj.Status.SourceVerificationFingerprint = "stale" + }, + want: sreconcile.ResultSuccess, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of revision "), + }, + }, { name: "signed image on an insecure registry passes verification", reference: &sourcev1.OCIRepositoryRef{ @@ -2339,6 +2380,9 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignatureCosign(t *testing if tt.beforeFunc != nil { tt.beforeFunc(obj, image.tag, image.digest.String()) } + if tt.verifiedWithCurrentKeys { + obj.Status.SourceVerificationFingerprint = verificationMaterialFingerprint(obj.Spec.Verify.Provider, secret, nil) + } g.Expect(r.Client.Create(ctx, obj)).ToNot(HaveOccurred()) defer func() { @@ -3876,3 +3920,117 @@ func TestOCIContentConfigChanged(t *testing.T) { }) } } + +func Test_verificationMaterialFingerprint(t *testing.T) { + g := NewWithT(t) + + pubKey := []byte("cosign public key") + cert := []byte("notation certificate") + policy := []byte(`{"version":"1.0"}`) + + // The same key material stored under a different data key must yield the + // same fingerprint, as the policy is defined by the keys, not their names. + cosignBase := &corev1.Secret{Data: map[string][]byte{"cosign.pub": pubKey}} + cosignSameKey := &corev1.Secret{Data: map[string][]byte{"other.pub": pubKey}} + g.Expect(verificationMaterialFingerprint("cosign", cosignBase, nil)). + To(Equal(verificationMaterialFingerprint("cosign", cosignSameKey, nil))) + + // Changing the key material changes the fingerprint. + cosignRotated := &corev1.Secret{Data: map[string][]byte{"cosign.pub": append(pubKey, '!')}} + g.Expect(verificationMaterialFingerprint("cosign", cosignRotated, nil)). + ToNot(Equal(verificationMaterialFingerprint("cosign", cosignBase, nil))) + + // Data entries that are not public keys are ignored for cosign. + cosignWithUnrelated := &corev1.Secret{Data: map[string][]byte{"cosign.pub": pubKey, "unrelated": []byte("x")}} + g.Expect(verificationMaterialFingerprint("cosign", cosignWithUnrelated, nil)). + To(Equal(verificationMaterialFingerprint("cosign", cosignBase, nil))) + + // The notation trust policy and certificates contribute to the fingerprint. + notationBase := &corev1.Secret{Data: map[string][]byte{"trustpolicy.json": policy, "ca.crt": cert}} + notationRotatedPolicy := &corev1.Secret{Data: map[string][]byte{"trustpolicy.json": append(policy, '!'), "ca.crt": cert}} + notationRotatedCert := &corev1.Secret{Data: map[string][]byte{"trustpolicy.json": policy, "ca.crt": append(cert, '!')}} + g.Expect(verificationMaterialFingerprint("notation", notationBase, nil)). + ToNot(Equal(verificationMaterialFingerprint("notation", notationRotatedPolicy, nil))) + g.Expect(verificationMaterialFingerprint("notation", notationBase, nil)). + ToNot(Equal(verificationMaterialFingerprint("notation", notationRotatedCert, nil))) + + // The trusted root contributes to the fingerprint. + trustedRoot := []byte(`{"mediaType":"application/vnd.dev.sigstore.trustedroot+json;version=0.1"}`) + g.Expect(verificationMaterialFingerprint("cosign", cosignBase, trustedRoot)). + ToNot(Equal(verificationMaterialFingerprint("cosign", cosignBase, nil))) +} + +func TestOCIRepositoryReconciler_verificationPolicyChanged(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "cosign-key", Namespace: "default"}, + Data: map[string][]byte{"cosign.pub": []byte("key")}, + } + rotatedSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "cosign-key", Namespace: "default"}, + Data: map[string][]byte{"cosign.pub": []byte("rotated-key")}, + } + + newObj := func(secretRef *meta.LocalObjectReference, fingerprint string) *sourcev1.OCIRepository { + obj := &sourcev1.OCIRepository{ + ObjectMeta: metav1.ObjectMeta{Name: "oci", Namespace: "default"}, + Spec: sourcev1.OCIRepositorySpec{ + Verify: &sourcev1.OCIRepositoryVerification{Provider: "cosign"}, + }, + } + if secretRef != nil { + obj.Spec.Verify.SecretRef = secretRef + } + obj.Status.SourceVerificationFingerprint = fingerprint + return obj + } + + keyRef := &meta.LocalObjectReference{Name: "cosign-key"} + + tests := []struct { + name string + obj *sourcev1.OCIRepository + secret *corev1.Secret + want bool + }{ + { + name: "no verification configured", + obj: &sourcev1.OCIRepository{}, + want: false, + }, + { + name: "missing secret requires verification", + obj: newObj(keyRef, ""), + want: true, + }, + { + name: "unchanged keys do not require verification", + obj: newObj(keyRef, verificationMaterialFingerprint("cosign", secret, nil)), + secret: secret, + want: false, + }, + { + name: "rotated keys require verification", + obj: newObj(keyRef, verificationMaterialFingerprint("cosign", secret, nil)), + secret: rotatedSecret, + want: true, + }, + { + name: "missing observed fingerprint requires verification", + obj: newObj(keyRef, ""), + secret: secret, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + clientBuilder := fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()) + if tt.secret != nil { + clientBuilder = clientBuilder.WithObjects(tt.secret) + } + r := &OCIRepositoryReconciler{Client: clientBuilder.Build()} + g.Expect(r.verificationPolicyChanged(ctx, tt.obj)).To(Equal(tt.want)) + }) + } +}