Merge pull request #2531 from mtrmac/5.32-backport

Release 5.32.2

docs/containers-policy.json.5.md

@@ -320,36 +320,43 @@ This requirement requires an image to be signed using a sigstore signature with
 {
     "type":    "sigstoreSigned",
     "keyPath": "/path/to/local/public/key/file",
+    "keyPaths": ["/path/to/first/public/key/one", "/path/to/first/public/key/two"],
     "keyData": "base64-encoded-public-key-data",
+    "keyDatas": ["base64-encoded-public-key-one-data", "base64-encoded-public-key-two-data"]
     "fulcio": {
         "caPath": "/path/to/local/CA/file",
         "caData": "base64-encoded-CA-data",
         "oidcIssuer": "https://expected.OIDC.issuer/",
         "subjectEmail", "[email protected]",
     },
     "rekorPublicKeyPath": "/path/to/local/public/key/file",
+    "rekorPublicKeyPaths": ["/path/to/local/public/key/one","/path/to/local/public/key/two"],
     "rekorPublicKeyData": "base64-encoded-public-key-data",
+    "rekorPublicKeyDatas": ["base64-encoded-public-key-one-data","base64-encoded-public-key-two-data"],
     "signedIdentity": identity_requirement
 }
 ```
-Exactly one of `keyPath`, `keyData` and `fulcio` must be present.
+Exactly one of `keyPath`, `keyPaths`, `keyData`, `keyDatas` and `fulcio` must be present.
 
 If `keyPath` or `keyData` is present, it contains a sigstore public key.
 Only signatures made by this key are accepted.
 
+If `keyPaths` or `keyDatas` is present, it contains sigstore public keys.
+Only signatures made by any key in the list are accepted.
+
 If `fulcio` is present, the signature must be based on a Fulcio-issued certificate.
 One of `caPath` and `caData` must be specified, containing the public key of the Fulcio instance.
 Both `oidcIssuer` and `subjectEmail` are mandatory,
 exactly specifying the expected identity provider,
 and the identity of the user obtaining the Fulcio certificate.
 
-At most one of `rekorPublicKeyPath` and `rekorPublicKeyData` can be present;
+At most one of `rekorPublicKeyPath`, `rekorPublicKeyPaths`, `rekorPublicKeyData` and `rekorPublicKeyDatas` can be present;
 it is mandatory if `fulcio` is specified.
 If a Rekor public key is specified,
 the signature must have been uploaded to a Rekor server
 and the signature must contain an (offline-verifiable) “signed entry timestamp”
 proving the existence of the Rekor log record,
-signed by the provided public key.
+signed by one of the provided public keys.
 
 The `signedIdentity` field has the same semantics as in the `signedBy` requirement described above.
 Note that `cosign`-created signatures only contain a repository, so only `matchRepository` and `exactRepository` can be used to accept them (and that does not protect against substitution of a signed image with an unexpected tag).

signature/fulcio_cert.go

@@ -195,10 +195,10 @@ func (f *fulcioTrustRoot) verifyFulcioCertificateAtTime(relevantTime time.Time,
 	return untrustedCertificate.PublicKey, nil
 }
 
-func verifyRekorFulcio(rekorPublicKey *ecdsa.PublicKey, fulcioTrustRoot *fulcioTrustRoot, untrustedRekorSET []byte,
+func verifyRekorFulcio(rekorPublicKeys []*ecdsa.PublicKey, fulcioTrustRoot *fulcioTrustRoot, untrustedRekorSET []byte,
 	untrustedCertificateBytes []byte, untrustedIntermediateChainBytes []byte, untrustedBase64Signature string,
 	untrustedPayloadBytes []byte) (crypto.PublicKey, error) {
-	rekorSETTime, err := internal.VerifyRekorSET(rekorPublicKey, untrustedRekorSET, untrustedCertificateBytes,
+	rekorSETTime, err := internal.VerifyRekorSET(rekorPublicKeys, untrustedRekorSET, untrustedCertificateBytes,
 		untrustedBase64Signature, untrustedPayloadBytes)
 	if err != nil {
 		return nil, err

signature/fulcio_cert_test.go

@@ -442,6 +442,7 @@ func TestVerifyRekorFulcio(t *testing.T) {
 	require.NoError(t, err)
 	rekorKeyECDSA, ok := rekorKey.(*ecdsa.PublicKey)
 	require.True(t, ok)
+	rekorKeysECDSA := []*ecdsa.PublicKey{rekorKeyECDSA}
 	setBytes, err := os.ReadFile("fixtures/rekor-set")
 	require.NoError(t, err)
 	sigBase64, err := os.ReadFile("fixtures/rekor-sig")
@@ -450,7 +451,7 @@ func TestVerifyRekorFulcio(t *testing.T) {
 	require.NoError(t, err)
 
 	// Success
-	pk, err := verifyRekorFulcio(rekorKeyECDSA, &fulcioTrustRoot{
+	pk, err := verifyRekorFulcio(rekorKeysECDSA, &fulcioTrustRoot{
 		caCertificates: caCertificates,
 		oidcIssuer:     "https://github.com/login/oauth",
 		subjectEmail:   "[email protected]",
@@ -459,7 +460,7 @@ func TestVerifyRekorFulcio(t *testing.T) {
 	assertPublicKeyMatchesCert(t, certBytes, pk)
 
 	// Rekor failure
-	pk, err = verifyRekorFulcio(rekorKeyECDSA, &fulcioTrustRoot{
+	pk, err = verifyRekorFulcio(rekorKeysECDSA, &fulcioTrustRoot{
 		caCertificates: caCertificates,
 		oidcIssuer:     "https://github.com/login/oauth",
 		subjectEmail:   "[email protected]",
@@ -468,7 +469,7 @@ func TestVerifyRekorFulcio(t *testing.T) {
 	assert.Nil(t, pk)
 
 	// Fulcio failure
-	pk, err = verifyRekorFulcio(rekorKeyECDSA, &fulcioTrustRoot{
+	pk, err = verifyRekorFulcio(rekorKeysECDSA, &fulcioTrustRoot{
 		caCertificates: caCertificates,
 		oidcIssuer:     "https://github.com/login/oauth",
 		subjectEmail:   "[email protected]",

signature/internal/rekor_set.go

@@ -113,7 +113,7 @@ func (p UntrustedRekorPayload) MarshalJSON() ([]byte, error) {
 
 // VerifyRekorSET verifies that unverifiedRekorSET is correctly signed by publicKey and matches the rest of the data.
 // Returns bundle upload time on success.
-func VerifyRekorSET(publicKey *ecdsa.PublicKey, unverifiedRekorSET []byte, unverifiedKeyOrCertBytes []byte, unverifiedBase64Signature string, unverifiedPayloadBytes []byte) (time.Time, error) {
+func VerifyRekorSET(publicKeys []*ecdsa.PublicKey, unverifiedRekorSET []byte, unverifiedKeyOrCertBytes []byte, unverifiedBase64Signature string, unverifiedPayloadBytes []byte) (time.Time, error) {
 	// FIXME: Should the publicKey parameter hard-code ecdsa?
 
 	// == Parse SET bytes
@@ -130,7 +130,14 @@ func VerifyRekorSET(publicKey *ecdsa.PublicKey, unverifiedRekorSET []byte, unver
 		return time.Time{}, NewInvalidSignatureError(fmt.Sprintf("canonicalizing Rekor SET JSON: %v", err))
 	}
 	untrustedSETPayloadHash := sha256.Sum256(untrustedSETPayloadCanonicalBytes)
-	if !ecdsa.VerifyASN1(publicKey, untrustedSETPayloadHash[:], untrustedSET.UntrustedSignedEntryTimestamp) {
+	publicKeymatched := false
+	for _, pk := range publicKeys {
+		if ecdsa.VerifyASN1(pk, untrustedSETPayloadHash[:], untrustedSET.UntrustedSignedEntryTimestamp) {
+			publicKeymatched = true
+			break
+		}
+	}
+	if !publicKeymatched {
 		return time.Time{}, NewInvalidSignatureError("cryptographic signature verification of Rekor SET failed")
 	}
 

signature/internal/rekor_set_test.go

@@ -185,6 +185,7 @@ func TestVerifyRekorSET(t *testing.T) {
 	require.NoError(t, err)
 	cosignRekorKeyECDSA, ok := cosignRekorKey.(*ecdsa.PublicKey)
 	require.True(t, ok)
+	cosignRekorKeysECDSA := []*ecdsa.PublicKey{cosignRekorKeyECDSA}
 	cosignSETBytes, err := os.ReadFile("testdata/rekor-set")
 	require.NoError(t, err)
 	cosignCertBytes, err := os.ReadFile("testdata/rekor-cert")
@@ -193,25 +194,34 @@ func TestVerifyRekorSET(t *testing.T) {
 	require.NoError(t, err)
 	cosignPayloadBytes, err := os.ReadFile("testdata/rekor-payload")
 	require.NoError(t, err)
+	mismatchingKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) // A key which did not sign anything
+	require.NoError(t, err)
 
 	// Successful verification
-	tm, err := VerifyRekorSET(cosignRekorKeyECDSA, cosignSETBytes, cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
-	require.NoError(t, err)
-	assert.Equal(t, time.Unix(1670870899, 0), tm)
+	for _, acceptableKeys := range [][]*ecdsa.PublicKey{
+		{cosignRekorKeyECDSA},
+		{cosignRekorKeyECDSA, &mismatchingKey.PublicKey},
+		{&mismatchingKey.PublicKey, cosignRekorKeyECDSA},
+	} {
+		tm, err := VerifyRekorSET(acceptableKeys, cosignSETBytes, cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
+		require.NoError(t, err)
+		assert.Equal(t, time.Unix(1670870899, 0), tm)
+	}
 
 	// For extra paranoia, test that we return a zero time on error.
 
 	// A completely invalid SET.
-	tm, err = VerifyRekorSET(cosignRekorKeyECDSA, []byte{}, cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
+	tm, err := VerifyRekorSET(cosignRekorKeysECDSA, []byte{}, cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
 	assert.Error(t, err)
 	assert.Zero(t, tm)
 
-	tm, err = VerifyRekorSET(cosignRekorKeyECDSA, []byte("invalid signature"), cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
+	tm, err = VerifyRekorSET(cosignRekorKeysECDSA, []byte("invalid signature"), cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
 	assert.Error(t, err)
 	assert.Zero(t, tm)
 
 	testKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
 	require.NoError(t, err)
+	testPublicKeys := []*ecdsa.PublicKey{&testKey.PublicKey}
 	testSigner, err := sigstoreSignature.LoadECDSASigner(testKey, crypto.SHA256)
 	require.NoError(t, err)
 
@@ -227,14 +237,19 @@ func TestVerifyRekorSET(t *testing.T) {
 		UntrustedPayload:              json.RawMessage(invalidPayload),
 	})
 	require.NoError(t, err)
-	tm, err = VerifyRekorSET(&testKey.PublicKey, invalidSET, cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
+	tm, err = VerifyRekorSET(testPublicKeys, invalidSET, cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
 	assert.Error(t, err)
 	assert.Zero(t, tm)
 
 	// Cryptographic verification fails (a mismatched public key)
-	tm, err = VerifyRekorSET(&testKey.PublicKey, cosignSETBytes, cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
-	assert.Error(t, err)
-	assert.Zero(t, tm)
+	for _, mismatchingKeys := range [][]*ecdsa.PublicKey{
+		{&testKey.PublicKey},
+		{&testKey.PublicKey, &mismatchingKey.PublicKey},
+	} {
+		tm, err := VerifyRekorSET(mismatchingKeys, cosignSETBytes, cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
+		assert.Error(t, err)
+		assert.Zero(t, tm)
+	}
 
 	// Parsing UntrustedRekorPayload fails
 	invalidPayload = []byte(`{}`)
@@ -245,7 +260,7 @@ func TestVerifyRekorSET(t *testing.T) {
 		UntrustedPayload:              json.RawMessage(invalidPayload),
 	})
 	require.NoError(t, err)
-	tm, err = VerifyRekorSET(&testKey.PublicKey, invalidSET, cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
+	tm, err = VerifyRekorSET(testPublicKeys, invalidSET, cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
 	assert.Error(t, err)
 	assert.Zero(t, tm)
 
@@ -379,15 +394,15 @@ func TestVerifyRekorSET(t *testing.T) {
 			UntrustedPayload:              json.RawMessage(testPayload),
 		})
 		require.NoError(t, err)
-		tm, err = VerifyRekorSET(&testKey.PublicKey, testSET, cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
+		tm, err = VerifyRekorSET(testPublicKeys, testSET, cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
 		assert.Error(t, err)
 		assert.Zero(t, tm)
 	}
 
 	// Invalid unverifiedBase64Signature parameter
 	truncatedBase64 := cosignSigBase64
 	truncatedBase64 = truncatedBase64[:len(truncatedBase64)-1]
-	tm, err = VerifyRekorSET(cosignRekorKeyECDSA, cosignSETBytes, cosignCertBytes,
+	tm, err = VerifyRekorSET(cosignRekorKeysECDSA, cosignSETBytes, cosignCertBytes,
 		string(truncatedBase64), cosignPayloadBytes)
 	assert.Error(t, err)
 	assert.Zero(t, tm)
@@ -399,7 +414,7 @@ func TestVerifyRekorSET(t *testing.T) {
 		[]byte("this is not PEM"),
 		bytes.Repeat(cosignCertBytes, 2),
 	} {
-		tm, err = VerifyRekorSET(cosignRekorKeyECDSA, cosignSETBytes, c,
+		tm, err = VerifyRekorSET(cosignRekorKeysECDSA, cosignSETBytes, c,
 			string(cosignSigBase64), cosignPayloadBytes)
 		assert.Error(t, err)
 		assert.Zero(t, tm)

signature/internal/sigstore_payload.go

@@ -7,6 +7,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/containers/image/v5/version"
@@ -171,24 +172,62 @@ type SigstorePayloadAcceptanceRules struct {
 	ValidateSignedDockerManifestDigest func(digest.Digest) error
 }
 
-// VerifySigstorePayload verifies unverifiedBase64Signature of unverifiedPayload was correctly created by publicKey, and that its principal components
+// verifySigstorePayloadBlobSignature verifies unverifiedSignature of unverifiedPayload was correctly created
+// by any of the public keys in publicKeys.
+//
+// This is an internal implementation detail of VerifySigstorePayload and should have no other callers.
+// It is INSUFFICIENT alone to consider the signature acceptable.
+func verifySigstorePayloadBlobSignature(publicKeys []crypto.PublicKey, unverifiedPayload, unverifiedSignature []byte) error {
+	if len(publicKeys) == 0 {
+		return errors.New("Need at least one public key to verify the sigstore payload, but got 0")
+	}
+
+	verifiers := make([]sigstoreSignature.Verifier, 0, len(publicKeys))
+	for _, key := range publicKeys {
+		// Failing to load a verifier indicates that something is really, really
+		// invalid about the public key; prefer to fail even if the signature might be
+		// valid with other keys, so that users fix their fallback keys before they need them.
+		// For that reason, we even initialize all verifiers before trying to validate the signature
+		// with any key.
+		verifier, err := sigstoreSignature.LoadVerifier(key, sigstoreHarcodedHashAlgorithm)
+		if err != nil {
+			return err
+		}
+		verifiers = append(verifiers, verifier)
+	}
+
+	var failures []string
+	for _, verifier := range verifiers {
+		// github.com/sigstore/cosign/pkg/cosign.verifyOCISignature uses signatureoptions.WithContext(),
+		// which seems to be not used by anything. So we don’t bother.
+		err := verifier.VerifySignature(bytes.NewReader(unverifiedSignature), bytes.NewReader(unverifiedPayload))
+		if err == nil {
+			return nil
+		}
+
+		failures = append(failures, err.Error())
+	}
+
+	if len(failures) == 0 {
+		// Coverage: We have checked there is at least one public key, any success causes an early return,
+		// and any failure adds an entry to failures => there must be at least one error.
+		return fmt.Errorf("Internal error: signature verification failed but no errors have been recorded")
+	}
+	return NewInvalidSignatureError("cryptographic signature verification failed: " + strings.Join(failures, ", "))
+}
+
+// VerifySigstorePayload verifies unverifiedBase64Signature of unverifiedPayload was correctly created by any of the public keys in publicKeys, and that its principal components
 // match expected values, both as specified by rules, and returns it.
 // We return an *UntrustedSigstorePayload, although nothing actually uses it,
 // just to double-check against stupid typos.
-func VerifySigstorePayload(publicKey crypto.PublicKey, unverifiedPayload []byte, unverifiedBase64Signature string, rules SigstorePayloadAcceptanceRules) (*UntrustedSigstorePayload, error) {
-	verifier, err := sigstoreSignature.LoadVerifier(publicKey, sigstoreHarcodedHashAlgorithm)
-	if err != nil {
-		return nil, fmt.Errorf("creating verifier: %w", err)
-	}
-
+func VerifySigstorePayload(publicKeys []crypto.PublicKey, unverifiedPayload []byte, unverifiedBase64Signature string, rules SigstorePayloadAcceptanceRules) (*UntrustedSigstorePayload, error) {
 	unverifiedSignature, err := base64.StdEncoding.DecodeString(unverifiedBase64Signature)
 	if err != nil {
 		return nil, NewInvalidSignatureError(fmt.Sprintf("base64 decoding: %v", err))
 	}
-	// github.com/sigstore/cosign/pkg/cosign.verifyOCISignature uses signatureoptions.WithContext(),
-	// which seems to be not used by anything. So we don’t bother.
-	if err := verifier.VerifySignature(bytes.NewReader(unverifiedSignature), bytes.NewReader(unverifiedPayload)); err != nil {
-		return nil, NewInvalidSignatureError(fmt.Sprintf("cryptographic signature verification failed: %v", err))
+
+	if err := verifySigstorePayloadBlobSignature(publicKeys, unverifiedPayload, unverifiedSignature); err != nil {
+		return nil, err
 	}
 
 	var unmatchedPayload UntrustedSigstorePayload