diff --git a/api/v1/gitrepository_types.go b/api/v1/gitrepository_types.go
index eb8be2d32..569c6c91d 100644
--- a/api/v1/gitrepository_types.go
+++ b/api/v1/gitrepository_types.go
@@ -288,6 +288,13 @@ type GitRepositoryStatus struct {
// +optional
ObservedSparseCheckout []string `json:"observedSparseCheckout,omitempty"`
+ // SourceVerificationFingerprint is the fingerprint of the public keys used
+ // to verify the signature of the Git object(s) for 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"`
+
// SourceVerificationMode is the last used verification mode indicating
// which Git object(s) have been verified.
// +optional
diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go
index 10be7301e..1bc9d6296 100644
--- a/api/v1beta1/zz_generated.deepcopy.go
+++ b/api/v1beta1/zz_generated.deepcopy.go
@@ -24,7 +24,7 @@ import (
"github.com/fluxcd/pkg/apis/acl"
"github.com/fluxcd/pkg/apis/meta"
"k8s.io/apimachinery/pkg/apis/meta/v1"
- runtime "k8s.io/apimachinery/pkg/runtime"
+ "k8s.io/apimachinery/pkg/runtime"
)
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
diff --git a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml
index 5fa5cf33c..dfb8628a8 100644
--- a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml
+++ b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml
@@ -477,6 +477,13 @@ spec:
items:
type: string
type: array
+ sourceVerificationFingerprint:
+ description: |-
+ SourceVerificationFingerprint is the fingerprint of the public keys used
+ to verify the signature of the Git object(s) for 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
sourceVerificationMode:
description: |-
SourceVerificationMode is the last used verification mode indicating
diff --git a/docs/api/v1/source.md b/docs/api/v1/source.md
index a3ea61377..871f7a196 100644
--- a/docs/api/v1/source.md
+++ b/docs/api/v1/source.md
@@ -2413,6 +2413,21 @@ produce the current Artifact.
+sourceVerificationFingerprint
+
+string
+
+ |
+
+(Optional)
+ SourceVerificationFingerprint is the fingerprint of the public keys used
+to verify the signature of the Git object(s) for 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.
+ |
+
+
+
sourceVerificationMode
diff --git a/docs/spec/v1/gitrepositories.md b/docs/spec/v1/gitrepositories.md
index 4c9d244af..a2148f380 100644
--- a/docs/spec/v1/gitrepositories.md
+++ b/docs/spec/v1/gitrepositories.md
@@ -1342,6 +1342,16 @@ mode in spec](#verification). The verification status is applicable only to the
latest Git repository revision used to successfully build and store an
artifact.
+### Source Verification Fingerprint
+
+The source-controller reports a fingerprint of the public keys it used to verify
+the Git object(s) in the GitRepository's
+`.status.sourceVerificationFingerprint`. The fingerprint is derived from the key
+material in the referenced Secret and does not depend on the Secret name or the
+Secret data key 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/gitrepository_controller.go b/internal/controller/gitrepository_controller.go
index a3c27c9b6..df3051fff 100644
--- a/internal/controller/gitrepository_controller.go
+++ b/internal/controller/gitrepository_controller.go
@@ -23,6 +23,7 @@ import (
"net/url"
"os"
"path/filepath"
+ "sort"
"strings"
"time"
@@ -33,6 +34,7 @@ import (
"github.com/fluxcd/pkg/runtime/logger"
"github.com/fluxcd/pkg/runtime/secrets"
"github.com/go-git/go-git/v5/plumbing/transport"
+ "github.com/opencontainers/go-digest"
ssh "golang.org/x/crypto/ssh"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
@@ -592,7 +594,9 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
// reconciliation can be skipped if other configurations have not changed.
if !git.IsConcreteCommit(*commit) {
// Check if the content config contributing to the artifact has changed.
- if !gitContentConfigChanged(obj, includes) {
+ // A change to the verification policy (e.g. a key rotation) also
+ // requires a new verification of the unchanged revision.
+ if !gitContentConfigChanged(obj, includes) && !r.verificationPolicyChanged(ctx, obj) {
ge := serror.NewGeneric(
fmt.Errorf("no changes since last reconciliation: observed revision '%s'",
commitReference(obj, commit)), sourcev1.GitOperationSucceedReason,
@@ -1112,6 +1116,64 @@ func (r *GitRepositoryReconciler) fetchIncludes(ctx context.Context, obj *source
return &artifacts, nil
}
+// verificationKeys returns the PGP key rings and SSH authorized keys contained
+// in the given Secret. Data entries with an SSH public key suffix are treated as
+// authorized keys, entries with a PGP public key suffix (or no known suffix) as
+// PGP key rings.
+func verificationKeys(secret *corev1.Secret) (keyRings, authorizedKeys []string) {
+ for k, v := range secret.Data {
+ switch {
+ case strings.HasSuffix(k, publicKeySSHSuffix):
+ authorizedKeys = append(authorizedKeys, string(v))
+ case strings.HasSuffix(k, publicKeyPGPSuffix):
+ keyRings = append(keyRings, string(v))
+ default:
+ // Provide fallback to support previous undocumented behavior
+ keyRings = append(keyRings, string(v))
+ }
+ }
+ return keyRings, authorizedKeys
+}
+
+// verificationFingerprint returns a stable fingerprint of the public keys in
+// the given Secret, independent of the Secret name or the data key names. 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.
+func verificationFingerprint(secret *corev1.Secret) string {
+ keyRings, authorizedKeys := verificationKeys(secret)
+ sort.Strings(keyRings)
+ sort.Strings(authorizedKeys)
+
+ var b strings.Builder
+ for _, k := range keyRings {
+ b.WriteString("pgp:")
+ b.WriteString(k)
+ b.WriteByte(0)
+ }
+ for _, k := range authorizedKeys {
+ b.WriteString("ssh:")
+ b.WriteString(k)
+ b.WriteByte(0)
+ }
+ return digest.Canonical.FromString(b.String()).String()
+}
+
+// verificationPolicyChanged returns true if the public keys trusted for
+// verification differ from the ones 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 *GitRepositoryReconciler) verificationPolicyChanged(ctx context.Context, obj *sourcev1.GitRepository) bool {
+ if obj.Spec.Verification == nil || obj.Spec.Verification.Mode == "" {
+ return false
+ }
+ secret, err := r.getSecret(ctx, obj.Spec.Verification.SecretRef.Name, obj.GetNamespace())
+ if err != nil {
+ // Return true so the full reconciliation surfaces the error.
+ return true
+ }
+ return verificationFingerprint(secret) != obj.Status.SourceVerificationFingerprint
+}
+
// verifySignature verifies the signature of the given Git commit and/or its referencing tag
// depending on the verification mode specified on the object.
// If the signature can not be verified or the verification fails, it records
@@ -1124,6 +1186,7 @@ func (r *GitRepositoryReconciler) verifySignature(ctx context.Context, obj *sour
// observations if there is none
if obj.Spec.Verification == nil || obj.Spec.Verification.Mode == "" {
obj.Status.SourceVerificationMode = nil
+ obj.Status.SourceVerificationFingerprint = ""
conditions.Delete(obj, sourcev1.SourceVerifiedCondition)
return sreconcile.ResultSuccess, nil
}
@@ -1143,18 +1206,7 @@ func (r *GitRepositoryReconciler) verifySignature(ctx context.Context, obj *sour
return sreconcile.ResultEmpty, e
}
- var keyRings []string
- var authorizedKeys []string
- for k, v := range secret.Data {
- if strings.HasSuffix(k, publicKeySSHSuffix) {
- authorizedKeys = append(authorizedKeys, string(v))
- } else if strings.HasSuffix(k, publicKeyPGPSuffix) {
- keyRings = append(keyRings, string(v))
- } else {
- // Provide fallback to support previous undocumented behavior
- keyRings = append(keyRings, string(v))
- }
- }
+ keyRings, authorizedKeys := verificationKeys(secret)
var message strings.Builder
if obj.Spec.Verification.VerifyTag() {
@@ -1217,6 +1269,7 @@ func (r *GitRepositoryReconciler) verifySignature(ctx context.Context, obj *sour
reason := meta.SucceededReason
mode := obj.Spec.Verification.GetMode()
obj.Status.SourceVerificationMode = &mode
+ obj.Status.SourceVerificationFingerprint = verificationFingerprint(secret)
conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, reason, "%s", message.String())
r.eventLogf(ctx, obj, eventv1.EventTypeTrace, reason, "%s", message.String())
return sreconcile.ResultSuccess, nil
diff --git a/internal/controller/gitrepository_controller_test.go b/internal/controller/gitrepository_controller_test.go
index 84b2074a8..d5b3b7c2f 100644
--- a/internal/controller/gitrepository_controller_test.go
+++ b/internal/controller/gitrepository_controller_test.go
@@ -2149,6 +2149,7 @@ func TestGitRepositoryReconciler_verifySignature(t *testing.T) {
name: "Source verification mode in status is unset if there's no verification in spec",
beforeFunc: func(obj *sourcev1.GitRepository) {
obj.Status.SourceVerificationMode = ptrToVerificationMode(sourcev1.ModeGitHEAD)
+ obj.Status.SourceVerificationFingerprint = "stale-fingerprint"
obj.Spec.Verification = nil
},
want: sreconcile.ResultSuccess,
@@ -2789,8 +2790,12 @@ func TestGitRepositoryReconciler_verifySignature(t *testing.T) {
g.Expect(got).To(Equal(tt.want))
if tt.wantSourceVerificationMode != nil {
g.Expect(*obj.Status.SourceVerificationMode).To(Equal(*tt.wantSourceVerificationMode))
+ if tt.secret != nil {
+ g.Expect(obj.Status.SourceVerificationFingerprint).To(Equal(verificationFingerprint(tt.secret)))
+ }
} else {
g.Expect(obj.Status.SourceVerificationMode).To(BeNil())
+ g.Expect(obj.Status.SourceVerificationFingerprint).To(BeEmpty())
}
})
}
@@ -4084,6 +4089,219 @@ func Test_requiresVerification(t *testing.T) {
}
}
+func Test_verificationFingerprint(t *testing.T) {
+ g := NewWithT(t)
+
+ pgpKey := armoredKeyRingFixture
+ sshKey := "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIExampleKeyForFingerprintTests"
+
+ // 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.
+ base := &corev1.Secret{Data: map[string][]byte{"foo": []byte(pgpKey)}}
+ sameKey := &corev1.Secret{Data: map[string][]byte{"bar.asc": []byte(pgpKey)}}
+ g.Expect(verificationFingerprint(base)).To(Equal(verificationFingerprint(sameKey)))
+
+ // The fingerprint must be stable regardless of the map iteration order.
+ mixed := &corev1.Secret{Data: map[string][]byte{
+ "a.asc": []byte(pgpKey),
+ "b.sshpub": []byte(sshKey),
+ "c.sshpub": []byte(sshKey + "\n"),
+ }}
+ g.Expect(verificationFingerprint(mixed)).To(Equal(verificationFingerprint(mixed)))
+
+ // Adding an SSH key changes the fingerprint.
+ g.Expect(verificationFingerprint(mixed)).ToNot(Equal(verificationFingerprint(base)))
+
+ // Changing the key material changes the fingerprint.
+ changed := &corev1.Secret{Data: map[string][]byte{"foo": []byte(pgpKey + "\n")}}
+ g.Expect(verificationFingerprint(changed)).ToNot(Equal(verificationFingerprint(base)))
+}
+
+func TestGitRepositoryReconciler_verificationPolicyChanged(t *testing.T) {
+ secret := &corev1.Secret{
+ ObjectMeta: metav1.ObjectMeta{Name: "keys", Namespace: "default"},
+ Data: map[string][]byte{"foo": []byte(armoredKeyRingFixture)},
+ }
+ rotatedSecret := &corev1.Secret{
+ ObjectMeta: metav1.ObjectMeta{Name: "keys", Namespace: "default"},
+ Data: map[string][]byte{"foo": []byte(armoredKeyRingFixture + "\n")},
+ }
+
+ newObj := func(mode sourcev1.GitVerificationMode, fingerprint string) *sourcev1.GitRepository {
+ obj := &sourcev1.GitRepository{
+ ObjectMeta: metav1.ObjectMeta{Name: "repo", Namespace: "default"},
+ Spec: sourcev1.GitRepositorySpec{
+ Verification: &sourcev1.GitRepositoryVerification{
+ Mode: mode,
+ SecretRef: meta.LocalObjectReference{Name: "keys"},
+ },
+ },
+ }
+ obj.Status.SourceVerificationFingerprint = fingerprint
+ return obj
+ }
+
+ tests := []struct {
+ name string
+ obj *sourcev1.GitRepository
+ secret *corev1.Secret
+ want bool
+ }{
+ {
+ name: "no verification configured",
+ obj: &sourcev1.GitRepository{},
+ want: false,
+ },
+ {
+ name: "verification without mode",
+ obj: &sourcev1.GitRepository{
+ Spec: sourcev1.GitRepositorySpec{
+ Verification: &sourcev1.GitRepositoryVerification{
+ SecretRef: meta.LocalObjectReference{Name: "keys"},
+ },
+ },
+ },
+ want: false,
+ },
+ {
+ name: "missing secret requires verification",
+ obj: newObj(sourcev1.ModeGitHEAD, ""),
+ want: true,
+ },
+ {
+ name: "unchanged keys do not require verification",
+ obj: newObj(sourcev1.ModeGitHEAD, verificationFingerprint(secret)),
+ secret: secret,
+ want: false,
+ },
+ {
+ name: "rotated keys require verification",
+ obj: newObj(sourcev1.ModeGitHEAD, verificationFingerprint(secret)),
+ secret: rotatedSecret,
+ want: true,
+ },
+ {
+ name: "missing observed fingerprint requires verification",
+ obj: newObj(sourcev1.ModeGitHEAD, ""),
+ 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 := &GitRepositoryReconciler{Client: clientBuilder.Build()}
+ g.Expect(r.verificationPolicyChanged(ctx, tt.obj)).To(Equal(tt.want))
+ })
+ }
+}
+
+func TestGitRepositoryReconciler_reconcileSource_verificationPolicyChange(t *testing.T) {
+ g := NewWithT(t)
+
+ server, err := gittestserver.NewTempGitServer()
+ g.Expect(err).NotTo(HaveOccurred())
+ defer os.RemoveAll(server.Root())
+ server.AutoCreate()
+ g.Expect(server.StartHTTP()).To(Succeed())
+ defer server.StopHTTP()
+
+ repoPath := "/test.git"
+ localRepo, err := initGitRepo(server, "testdata/git/repository", git.DefaultBranch, repoPath)
+ g.Expect(err).NotTo(HaveOccurred())
+
+ headRef, err := localRepo.Head()
+ g.Expect(err).NotTo(HaveOccurred())
+ g.Expect(remoteBranchForHead(localRepo, headRef, "staging")).To(Succeed())
+
+ secret := &corev1.Secret{
+ ObjectMeta: metav1.ObjectMeta{Name: "keys", Namespace: "default"},
+ Data: map[string][]byte{"foo": []byte(armoredKeyRingFixture)},
+ }
+
+ tests := []struct {
+ name string
+ fingerprint string
+ wantVerification bool
+ }{
+ {
+ name: "unchanged policy skips verification of the unchanged revision",
+ fingerprint: verificationFingerprint(secret),
+ },
+ {
+ name: "changed policy reverifies the unchanged revision",
+ fingerprint: "stale",
+ wantVerification: true,
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ g := NewWithT(t)
+
+ obj := &sourcev1.GitRepository{
+ ObjectMeta: metav1.ObjectMeta{
+ GenerateName: "verification-policy-",
+ Namespace: "default",
+ Generation: 1,
+ },
+ Spec: sourcev1.GitRepositorySpec{
+ Interval: metav1.Duration{Duration: interval},
+ Timeout: &metav1.Duration{Duration: timeout},
+ URL: server.HTTPAddress() + repoPath,
+ Reference: &sourcev1.GitRepositoryRef{
+ Branch: "staging",
+ },
+ Verification: &sourcev1.GitRepositoryVerification{
+ Mode: sourcev1.ModeGitHEAD,
+ SecretRef: meta.LocalObjectReference{Name: "keys"},
+ },
+ },
+ Status: sourcev1.GitRepositoryStatus{
+ Artifact: &meta.Artifact{
+ Revision: "staging@sha1:" + headRef.Hash().String(),
+ Path: randStringRunes(10),
+ },
+ SourceVerificationMode: ptrToVerificationMode(sourcev1.ModeGitHEAD),
+ SourceVerificationFingerprint: tt.fingerprint,
+ },
+ }
+ conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "foo")
+
+ c := fakeclient.NewClientBuilder().
+ WithScheme(testEnv.GetScheme()).
+ WithObjects(obj, secret).
+ WithStatusSubresource(&sourcev1.GitRepository{}).
+ Build()
+ r := &GitRepositoryReconciler{
+ Client: c,
+ EventRecorder: record.NewFakeRecorder(32),
+ Storage: testStorage,
+ patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
+ }
+
+ var commit git.Commit
+ var includes artifactSet
+ sp := patch.NewSerialPatcher(obj, c)
+ _, err := r.reconcileSource(ctx, sp, obj, &commit, &includes, t.TempDir())
+ g.Expect(err).To(HaveOccurred())
+
+ if tt.wantVerification {
+ // The full checkout is performed, but the unsigned test commit
+ // fails verification.
+ g.Expect(err.Error()).To(ContainSubstring("signature verification of commit"))
+ } else {
+ g.Expect(err.Error()).To(ContainSubstring("no changes since last reconciliation"))
+ }
+ })
+ }
+}
+
func ptrToVerificationMode(mode sourcev1.GitVerificationMode) *sourcev1.GitVerificationMode {
return &mode
}
|