diff --git a/docs/containers-policy.json.5.md b/docs/containers-policy.json.5.md index 5b42e21aed..ad3a1f5db2 100644 --- a/docs/containers-policy.json.5.md +++ b/docs/containers-policy.json.5.md @@ -330,7 +330,9 @@ This requirement requires an image to be signed using a sigstore signature with "subjectEmail", "expected-signing-user@example.com", }, "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 } ``` @@ -348,13 +350,13 @@ 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). diff --git a/signature/fulcio_cert.go b/signature/fulcio_cert.go index 4e99864226..31dfdd3422 100644 --- a/signature/fulcio_cert.go +++ b/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 diff --git a/signature/fulcio_cert_test.go b/signature/fulcio_cert_test.go index 3fefbab850..4c9c6465c0 100644 --- a/signature/fulcio_cert_test.go +++ b/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: "mitr@redhat.com", @@ -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: "mitr@redhat.com", @@ -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: "this-does-not-match@example.com", diff --git a/signature/internal/rekor_set.go b/signature/internal/rekor_set.go index e065b405c0..bddaca690b 100644 --- a/signature/internal/rekor_set.go +++ b/signature/internal/rekor_set.go @@ -110,7 +110,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 @@ -127,7 +127,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") } diff --git a/signature/internal/rekor_set_test.go b/signature/internal/rekor_set_test.go index 9aa306caa9..08ba69cb6b 100644 --- a/signature/internal/rekor_set_test.go +++ b/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,7 +394,7 @@ 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) } @@ -387,7 +402,7 @@ func TestVerifyRekorSET(t *testing.T) { // 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) diff --git a/signature/policy_config_sigstore.go b/signature/policy_config_sigstore.go index 26e7d355bb..965901e187 100644 --- a/signature/policy_config_sigstore.go +++ b/signature/policy_config_sigstore.go @@ -82,6 +82,20 @@ func PRSigstoreSignedWithRekorPublicKeyPath(rekorPublicKeyPath string) PRSigstor } } +// PRSigstoreSignedWithRekorPublicKeyPaths specifies a value for the rRekorPublickeyPaths" field when calling NewPRSigstoreSigned. +func PRSigstoreSignedWithRekorPublicKeyPaths(rekorPublickeyPaths []string) PRSigstoreSignedOption { + return func(pr *prSigstoreSigned) error { + if pr.RekorPublicKeyPaths != nil { + return InvalidPolicyFormatError(`"rekorPublickeyPaths" already specified`) + } + if len(rekorPublickeyPaths) == 0 { + return InvalidPolicyFormatError(`"rekorPublickeyPaths" contains no entries`) + } + pr.RekorPublicKeyPaths = rekorPublickeyPaths + return nil + } +} + // PRSigstoreSignedWithRekorPublicKeyData specifies a value for the "rekorPublicKeyData" field when calling NewPRSigstoreSigned. func PRSigstoreSignedWithRekorPublicKeyData(rekorPublicKeyData []byte) PRSigstoreSignedOption { return func(pr *prSigstoreSigned) error { @@ -93,6 +107,20 @@ func PRSigstoreSignedWithRekorPublicKeyData(rekorPublicKeyData []byte) PRSigstor } } +// PRSigstoreSignedWithRekorPublicKeyDatas specifies a value for the "rekorPublickeyDatas" field when calling NewPRSigstoreSigned. +func PRSigstoreSignedWithRekorPublicKeyDatas(rekorPublickeyDatas [][]byte) PRSigstoreSignedOption { + return func(pr *prSigstoreSigned) error { + if pr.RekorPublicKeyDatas != nil { + return InvalidPolicyFormatError(`"rekorPublickeyDatas" already specified`) + } + if len(rekorPublickeyDatas) == 0 { + return InvalidPolicyFormatError(`"rekorPublickeyDatas" contains no entries`) + } + pr.RekorPublicKeyDatas = rekorPublickeyDatas + return nil + } +} + // PRSigstoreSignedWithSignedIdentity specifies a value for the "signedIdentity" field when calling NewPRSigstoreSigned. func PRSigstoreSignedWithSignedIdentity(signedIdentity PolicyReferenceMatch) PRSigstoreSignedOption { return func(pr *prSigstoreSigned) error { @@ -135,11 +163,24 @@ func newPRSigstoreSigned(options ...PRSigstoreSignedOption) (*prSigstoreSigned, return nil, InvalidPolicyFormatError("exactly one of keyPath, keyPaths, keyData, keyDatas and fulcio must be specified") } - if res.RekorPublicKeyPath != "" && res.RekorPublicKeyData != nil { - return nil, InvalidPolicyFormatError("rekorPublickeyType and rekorPublickeyData cannot be used simultaneously") + rekorSources := 0 + if res.RekorPublicKeyPath != "" { + rekorSources++ + } + if res.RekorPublicKeyPaths != nil { + rekorSources++ } - if res.Fulcio != nil && res.RekorPublicKeyPath == "" && res.RekorPublicKeyData == nil { - return nil, InvalidPolicyFormatError("At least one of RekorPublickeyPath and RekorPublickeyData must be specified if fulcio is used") + if res.RekorPublicKeyData != nil { + rekorSources++ + } + if res.RekorPublicKeyDatas != nil { + rekorSources++ + } + if rekorSources > 1 { + return nil, InvalidPolicyFormatError("at most one of rekorPublickeyPath, rekorPublicKeyPaths, rekorPublickeyData and rekorPublicKeyDatas can be used simultaneously") + } + if res.Fulcio != nil && rekorSources == 0 { + return nil, InvalidPolicyFormatError("At least one of rekorPublickeyPath, rekorPublicKeyPaths, rekorPublickeyData and rekorPublicKeyDatas must be specified if fulcio is used") } if res.SignedIdentity == nil { @@ -177,7 +218,8 @@ var _ json.Unmarshaler = (*prSigstoreSigned)(nil) func (pr *prSigstoreSigned) UnmarshalJSON(data []byte) error { *pr = prSigstoreSigned{} var tmp prSigstoreSigned - var gotKeyPath, gotKeyPaths, gotKeyData, gotKeyDatas, gotFulcio, gotRekorPublicKeyPath, gotRekorPublicKeyData bool + var gotKeyPath, gotKeyPaths, gotKeyData, gotKeyDatas, gotFulcio bool + var gotRekorPublicKeyPath, gotRekorPublicKeyPaths, gotRekorPublicKeyData, gotRekorPublicKeyDatas bool var fulcio prSigstoreSignedFulcio var signedIdentity json.RawMessage if err := internal.ParanoidUnmarshalJSONObject(data, func(key string) any { @@ -202,9 +244,15 @@ func (pr *prSigstoreSigned) UnmarshalJSON(data []byte) error { case "rekorPublicKeyPath": gotRekorPublicKeyPath = true return &tmp.RekorPublicKeyPath + case "rekorPublicKeyPaths": + gotRekorPublicKeyPaths = true + return &tmp.RekorPublicKeyPaths case "rekorPublicKeyData": gotRekorPublicKeyData = true return &tmp.RekorPublicKeyData + case "rekorPublicKeyDatas": + gotRekorPublicKeyDatas = true + return &tmp.RekorPublicKeyDatas case "signedIdentity": return &signedIdentity default: @@ -246,9 +294,15 @@ func (pr *prSigstoreSigned) UnmarshalJSON(data []byte) error { if gotRekorPublicKeyPath { opts = append(opts, PRSigstoreSignedWithRekorPublicKeyPath(tmp.RekorPublicKeyPath)) } + if gotRekorPublicKeyPaths { + opts = append(opts, PRSigstoreSignedWithRekorPublicKeyPaths(tmp.RekorPublicKeyPaths)) + } if gotRekorPublicKeyData { opts = append(opts, PRSigstoreSignedWithRekorPublicKeyData(tmp.RekorPublicKeyData)) } + if gotRekorPublicKeyDatas { + opts = append(opts, PRSigstoreSignedWithRekorPublicKeyDatas(tmp.RekorPublicKeyDatas)) + } opts = append(opts, PRSigstoreSignedWithSignedIdentity(tmp.SignedIdentity)) res, err := newPRSigstoreSigned(opts...) diff --git a/signature/policy_config_sigstore_test.go b/signature/policy_config_sigstore_test.go index 1eec5ac3c5..c6ec3d8018 100644 --- a/signature/policy_config_sigstore_test.go +++ b/signature/policy_config_sigstore_test.go @@ -132,6 +132,14 @@ func TestNewPRSigstoreSigned(t *testing.T) { RekorPublicKeyPath: testRekorKeyPath, }, }, + { + rekorOptions: []PRSigstoreSignedOption{ + PRSigstoreSignedWithRekorPublicKeyPaths([]string{testRekorKeyPath, testKeyPath}), + }, + rekorExpected: prSigstoreSigned{ + RekorPublicKeyPaths: []string{testRekorKeyPath, testKeyPath}, + }, + }, { rekorOptions: []PRSigstoreSignedOption{ PRSigstoreSignedWithRekorPublicKeyData(testRekorKeyData), @@ -140,6 +148,14 @@ func TestNewPRSigstoreSigned(t *testing.T) { RekorPublicKeyData: testRekorKeyData, }, }, + { + rekorOptions: []PRSigstoreSignedOption{ + PRSigstoreSignedWithRekorPublicKeyDatas([][]byte{testRekorKeyData, testKeyData}), + }, + rekorExpected: prSigstoreSigned{ + RekorPublicKeyDatas: [][]byte{testRekorKeyData, testKeyData}, + }, + }, } { // Special-case this rejected combination: if c.requiresRekor && len(c2.rekorOptions) == 0 { @@ -149,7 +165,9 @@ func TestNewPRSigstoreSigned(t *testing.T) { require.NoError(t, err) expected := c.expected // A shallow copy expected.RekorPublicKeyPath = c2.rekorExpected.RekorPublicKeyPath + expected.RekorPublicKeyPaths = c2.rekorExpected.RekorPublicKeyPaths expected.RekorPublicKeyData = c2.rekorExpected.RekorPublicKeyData + expected.RekorPublicKeyDatas = c2.rekorExpected.RekorPublicKeyDatas assert.Equal(t, &expected, pr) } } @@ -161,19 +179,19 @@ func TestNewPRSigstoreSigned(t *testing.T) { ) require.NoError(t, err) for _, c := range [][]PRSigstoreSignedOption{ - {}, // None of keyPath nor keyData, fulcio specified + {}, // None of keyPath, keyPaths, keyData, keyDatas, fulcio specified { // Both keyPath and keyData specified PRSigstoreSignedWithKeyPath(testKeyPath), PRSigstoreSignedWithKeyData(testKeyData), PRSigstoreSignedWithSignedIdentity(testIdentity), }, - { // both keyPath and fulcio specified + { // Both keyPath and fulcio specified PRSigstoreSignedWithKeyPath(testKeyPath), PRSigstoreSignedWithFulcio(testFulcio), PRSigstoreSignedWithRekorPublicKeyPath(testRekorKeyPath), PRSigstoreSignedWithSignedIdentity(testIdentity), }, - { // both keyData and fulcio specified + { // Both keyData and fulcio specified PRSigstoreSignedWithKeyData(testKeyData), PRSigstoreSignedWithFulcio(testFulcio), PRSigstoreSignedWithRekorPublicKeyPath(testRekorKeyPath), @@ -239,12 +257,46 @@ func TestNewPRSigstoreSigned(t *testing.T) { PRSigstoreSignedWithRekorPublicKeyPath(testRekorKeyPath + "1"), PRSigstoreSignedWithSignedIdentity(testIdentity), }, + { // Both rekorKeyPath and rekorKeyPaths specified + PRSigstoreSignedWithKeyPath(testKeyPath), + PRSigstoreSignedWithRekorPublicKeyPath(testRekorKeyPath), + PRSigstoreSignedWithRekorPublicKeyPaths([]string{testRekorKeyPath, testKeyPath}), + PRSigstoreSignedWithSignedIdentity(testIdentity), + }, + { // Empty rekorKeyPaths + PRSigstoreSignedWithKeyPath(testKeyPath), + PRSigstoreSignedWithRekorPublicKeyPaths([]string{}), + PRSigstoreSignedWithSignedIdentity(testIdentity), + }, + { // Duplicate rekorKeyPaths + PRSigstoreSignedWithKeyPath(testKeyPath), + PRSigstoreSignedWithRekorPublicKeyPaths([]string{testRekorKeyPath, testKeyPath}), + PRSigstoreSignedWithRekorPublicKeyPaths([]string{testRekorKeyPath + "1", testKeyPath + "1"}), + PRSigstoreSignedWithSignedIdentity(testIdentity), + }, { // Duplicate rekorKeyData PRSigstoreSignedWithKeyPath(testKeyPath), PRSigstoreSignedWithRekorPublicKeyData(testRekorKeyData), PRSigstoreSignedWithRekorPublicKeyData([]byte("def")), PRSigstoreSignedWithSignedIdentity(testIdentity), }, + { // Both rekorKeyData and rekorKeyDatas specified + PRSigstoreSignedWithKeyPath(testKeyPath), + PRSigstoreSignedWithRekorPublicKeyData(testRekorKeyData), + PRSigstoreSignedWithRekorPublicKeyDatas([][]byte{testRekorKeyData, []byte("def")}), + PRSigstoreSignedWithSignedIdentity(testIdentity), + }, + { // Empty rekorKeyDatas + PRSigstoreSignedWithKeyPath(testKeyPath), + PRSigstoreSignedWithRekorPublicKeyDatas([][]byte{}), + PRSigstoreSignedWithSignedIdentity(testIdentity), + }, + { // Duplicate rekorKeyData + PRSigstoreSignedWithKeyPath(testKeyPath), + PRSigstoreSignedWithRekorPublicKeyDatas([][]byte{testRekorKeyData, testKeyData}), + PRSigstoreSignedWithRekorPublicKeyDatas([][]byte{[]byte("abc"), []byte("def")}), + PRSigstoreSignedWithSignedIdentity(testIdentity), + }, { // Missing signedIdentity PRSigstoreSignedWithKeyPath(testKeyPath), }, @@ -355,9 +407,27 @@ func TestPRSigstoreSignedUnmarshalJSON(t *testing.T) { }, // Invalid "rekorPublicKeyPath" field func(v mSA) { v["rekorPublicKeyPath"] = 1 }, + // Both "rekorKeyPath" and "rekorKeyPaths" is present + func(v mSA) { + v["rekorPublicKeyPath"] = "/foo/baz" + v["rekorPublicKeyPaths"] = []string{"/baz/a", "/baz/b"} + }, + // Invalid "rekorPublicKeyPaths" field + func(v mSA) { v["rekorPublicKeyPaths"] = 1 }, + func(v mSA) { v["rekorPublicKeyPaths"] = mSA{} }, + func(v mSA) { v["rekorPublicKeyPaths"] = []string{} }, // Invalid "rekorPublicKeyData" field func(v mSA) { v["rekorPublicKeyData"] = 1 }, func(v mSA) { v["rekorPublicKeyData"] = "this is invalid base64" }, + // Both "rekorPublicKeyData" and "rekorPublicKeyDatas" is present + func(v mSA) { + v["rekorPublicKeyData"] = []byte("a") + v["rekorPublicKeyDatas"] = [][]byte{[]byte("a"), []byte("b")} + }, + // Invalid "rekorPublicKeyDatas" field + func(v mSA) { v["rekorPublicKeyDatas"] = 1 }, + func(v mSA) { v["rekorPublicKeyDatas"] = mSA{} }, + func(v mSA) { v["rekorPublicKeyDatas"] = [][]byte{} }, // Invalid "signedIdentity" field func(v mSA) { v["signedIdentity"] = "this is invalid" }, // "signedIdentity" an explicit nil @@ -418,6 +488,19 @@ func TestPRSigstoreSignedUnmarshalJSON(t *testing.T) { otherJSONParser: newPolicyRequirementFromJSON, duplicateFields: []string{"type", "fulcio", "rekorPublicKeyPath", "signedIdentity"}, }.run(t) + // Test rekorPublicKeyPaths duplicate fields + policyJSONUmarshallerTests[PolicyRequirement]{ + newDest: func() json.Unmarshaler { return &prSigstoreSigned{} }, + newValidObject: func() (PolicyRequirement, error) { + return NewPRSigstoreSigned( + PRSigstoreSignedWithKeyPath("/foo/bar"), + PRSigstoreSignedWithRekorPublicKeyPaths([]string{"/baz/a", "/baz/b"}), + PRSigstoreSignedWithSignedIdentity(NewPRMMatchRepoDigestOrExact()), + ) + }, + otherJSONParser: newPolicyRequirementFromJSON, + duplicateFields: []string{"type", "keyPath", "rekorPublicKeyPaths", "signedIdentity"}, + }.run(t) // Test rekorPublicKeyData duplicate fields policyJSONUmarshallerTests[PolicyRequirement]{ newDest: func() json.Unmarshaler { return &prSigstoreSigned{} }, @@ -431,6 +514,19 @@ func TestPRSigstoreSignedUnmarshalJSON(t *testing.T) { otherJSONParser: newPolicyRequirementFromJSON, duplicateFields: []string{"type", "keyPath", "rekorPublicKeyData", "signedIdentity"}, }.run(t) + // Test rekorPublicKeyDatas duplicate fields + policyJSONUmarshallerTests[PolicyRequirement]{ + newDest: func() json.Unmarshaler { return &prSigstoreSigned{} }, + newValidObject: func() (PolicyRequirement, error) { + return NewPRSigstoreSigned( + PRSigstoreSignedWithKeyPath("/foo/bar"), + PRSigstoreSignedWithRekorPublicKeyDatas([][]byte{[]byte("foo"), []byte("bar")}), + PRSigstoreSignedWithSignedIdentity(NewPRMMatchRepoDigestOrExact()), + ) + }, + otherJSONParser: newPolicyRequirementFromJSON, + duplicateFields: []string{"type", "keyPath", "rekorPublicKeyDatas", "signedIdentity"}, + }.run(t) var pr prSigstoreSigned diff --git a/signature/policy_eval_sigstore.go b/signature/policy_eval_sigstore.go index 60bcabeb56..9c553771cb 100644 --- a/signature/policy_eval_sigstore.go +++ b/signature/policy_eval_sigstore.go @@ -99,9 +99,9 @@ func (f *prSigstoreSignedFulcio) prepareTrustRoot() (*fulcioTrustRoot, error) { // sigstoreSignedTrustRoot contains an already parsed version of the prSigstoreSigned policy type sigstoreSignedTrustRoot struct { - publicKeys []crypto.PublicKey - fulcio *fulcioTrustRoot - rekorPublicKey *ecdsa.PublicKey + publicKeys []crypto.PublicKey + fulcio *fulcioTrustRoot + rekorPublicKeys []*ecdsa.PublicKey } func (pr *prSigstoreSigned) prepareTrustRoot() (*sigstoreSignedTrustRoot, error) { @@ -141,27 +141,29 @@ func (pr *prSigstoreSigned) prepareTrustRoot() (*sigstoreSignedTrustRoot, error) rekorPublicKeyPEMs, err := loadBytesFromConfigSources(configBytesSources{ inconsistencyErrorMessage: `Internal inconsistency: both "rekorPublicKeyPath" and "rekorPublicKeyData" specified`, path: pr.RekorPublicKeyPath, + paths: pr.RekorPublicKeyPaths, data: pr.RekorPublicKeyData, + datas: pr.RekorPublicKeyDatas, // codespell:ignore datas }) if err != nil { return nil, err } if rekorPublicKeyPEMs != nil { - if len(rekorPublicKeyPEMs) != 1 { - // Coverage: This should never happen, we only provide single-element sources - // to loadBytesFromConfigSources, and at most one is allowed. - return nil, errors.New(`Internal inconsistency: got more than one element in "rekorPublicKeyPath" and "rekorPublicKeyData"`) - } - pk, err := cryptoutils.UnmarshalPEMToPublicKey(rekorPublicKeyPEMs[0]) - if err != nil { - return nil, fmt.Errorf("parsing Rekor public key: %w", err) - } - pkECDSA, ok := pk.(*ecdsa.PublicKey) - if !ok { - return nil, fmt.Errorf("Rekor public key is not using ECDSA") + for index, pem := range rekorPublicKeyPEMs { + pk, err := cryptoutils.UnmarshalPEMToPublicKey(pem) + if err != nil { + return nil, fmt.Errorf("parsing Rekor public key %d: %w", index+1, err) + } + pkECDSA, ok := pk.(*ecdsa.PublicKey) + if !ok { + return nil, fmt.Errorf("Rekor public key %d is not using ECDSA", index+1) + } + res.rekorPublicKeys = append(res.rekorPublicKeys, pkECDSA) + } + if len(res.rekorPublicKeys) == 0 { + return nil, errors.New(`Internal inconsistency: "rekorPublicKeyPath", "rekorPublicKeyPaths", "rekorPublicKeyData" and "rekorPublicKeyDatas" produced no public keys`) } - res.rekorPublicKey = pkECDSA } return &res, nil @@ -195,7 +197,7 @@ func (pr *prSigstoreSigned) isSignatureAccepted(ctx context.Context, image priva return sarRejected, errors.New("Internal inconsistency: Neither a public key nor a Fulcio CA specified") case trustRoot.publicKeys != nil: - if trustRoot.rekorPublicKey != nil { + if trustRoot.rekorPublicKeys != nil { untrustedSET, ok := untrustedAnnotations[signature.SigstoreSETAnnotationKey] if !ok { // For user convenience; passing an empty []byte to VerifyRekorSet should work. return sarRejected, fmt.Errorf("missing %s annotation", signature.SigstoreSETAnnotationKey) @@ -212,7 +214,7 @@ func (pr *prSigstoreSigned) isSignatureAccepted(ctx context.Context, image priva return sarRejected, fmt.Errorf("re-marshaling public key to PEM: %w", err) } // We don’t care about the Rekor timestamp, just about log presence. - _, err = internal.VerifyRekorSET(trustRoot.rekorPublicKey, []byte(untrustedSET), recreatedPublicKeyPEM, untrustedBase64Signature, untrustedPayload) + _, err = internal.VerifyRekorSET(trustRoot.rekorPublicKeys, []byte(untrustedSET), recreatedPublicKeyPEM, untrustedBase64Signature, untrustedPayload) if err == nil { publicKeys = append(publicKeys, candidatePublicKey) break // The SET can only accept one public key entry, so if we found one, the rest either doesn’t match or is a duplicate @@ -231,7 +233,7 @@ func (pr *prSigstoreSigned) isSignatureAccepted(ctx context.Context, image priva } case trustRoot.fulcio != nil: - if trustRoot.rekorPublicKey == nil { // newPRSigstoreSigned rejects such combinations. + if trustRoot.rekorPublicKeys == nil { // newPRSigstoreSigned rejects such combinations. return sarRejected, errors.New("Internal inconsistency: Fulcio CA specified without a Rekor public key") } untrustedSET, ok := untrustedAnnotations[signature.SigstoreSETAnnotationKey] @@ -246,7 +248,7 @@ func (pr *prSigstoreSigned) isSignatureAccepted(ctx context.Context, image priva if untrustedIntermediateChain, ok := untrustedAnnotations[signature.SigstoreIntermediateCertificateChainAnnotationKey]; ok { untrustedIntermediateChainBytes = []byte(untrustedIntermediateChain) } - pk, err := verifyRekorFulcio(trustRoot.rekorPublicKey, trustRoot.fulcio, + pk, err := verifyRekorFulcio(trustRoot.rekorPublicKeys, trustRoot.fulcio, []byte(untrustedSET), []byte(untrustedCert), untrustedIntermediateChainBytes, untrustedBase64Signature, untrustedPayload) if err != nil { return sarRejected, err diff --git a/signature/policy_eval_sigstore_test.go b/signature/policy_eval_sigstore_test.go index d8b7d01299..b5e74c4d15 100644 --- a/signature/policy_eval_sigstore_test.go +++ b/signature/policy_eval_sigstore_test.go @@ -123,7 +123,7 @@ func TestPRSigstoreSignedPrepareTrustRoot(t *testing.T) { assert.NotNil(t, res.publicKeys) assert.Len(t, res.publicKeys, c.numKeys) assert.Nil(t, res.fulcio) - assert.Nil(t, res.rekorPublicKey) + assert.Nil(t, res.rekorPublicKeys) } // Success with Fulcio pr, err := newPRSigstoreSigned( @@ -136,37 +136,31 @@ func TestPRSigstoreSignedPrepareTrustRoot(t *testing.T) { require.NoError(t, err) assert.Nil(t, res.publicKeys) assert.NotNil(t, res.fulcio) - assert.NotNil(t, res.rekorPublicKey) + assert.Len(t, res.rekorPublicKeys, 1) // Success with Rekor public key - for _, c := range [][]PRSigstoreSignedOption{ - { - PRSigstoreSignedWithKeyData(testKeyData), - PRSigstoreSignedWithRekorPublicKeyPath(testRekorPublicKeyPath), - testIdentityOption, - }, - { - PRSigstoreSignedWithKeyPaths([]string{testKeyPath, testKeyPath2}), - PRSigstoreSignedWithRekorPublicKeyPath(testRekorPublicKeyPath), - testIdentityOption, - }, - { - PRSigstoreSignedWithKeyData(testKeyData), - PRSigstoreSignedWithRekorPublicKeyData(testRekorPublicKeyData), - testIdentityOption, - }, - { - PRSigstoreSignedWithKeyDatas([][]byte{testKeyData, testKeyData2}), - PRSigstoreSignedWithRekorPublicKeyData(testRekorPublicKeyData), - testIdentityOption, - }, + for _, keyOption := range []PRSigstoreSignedOption{ + PRSigstoreSignedWithKeyData(testKeyData), + PRSigstoreSignedWithKeyPaths([]string{testKeyPath, testKeyPath2}), + PRSigstoreSignedWithKeyData(testKeyData), + PRSigstoreSignedWithKeyDatas([][]byte{testKeyData, testKeyData2}), } { - pr, err := newPRSigstoreSigned(c...) - require.NoError(t, err) - res, err := pr.prepareTrustRoot() - require.NoError(t, err) - assert.NotNil(t, res.publicKeys) - assert.Nil(t, res.fulcio) - assert.NotNil(t, res.rekorPublicKey) + for _, rekor := range []struct { + option PRSigstoreSignedOption + numKeys int + }{ + {PRSigstoreSignedWithRekorPublicKeyPath(testRekorPublicKeyPath), 1}, + {PRSigstoreSignedWithRekorPublicKeyPaths([]string{testRekorPublicKeyPath, testKeyPath}), 2}, + {PRSigstoreSignedWithRekorPublicKeyData(testRekorPublicKeyData), 1}, + {PRSigstoreSignedWithRekorPublicKeyDatas([][]byte{testRekorPublicKeyData, testKeyData}), 2}, + } { + pr, err := newPRSigstoreSigned(keyOption, rekor.option, testIdentityOption) + require.NoError(t, err) + res, err := pr.prepareTrustRoot() + require.NoError(t, err) + assert.NotNil(t, res.publicKeys) + assert.Nil(t, res.fulcio) + assert.Len(t, res.rekorPublicKeys, rekor.numKeys) + } } // Failure @@ -246,11 +240,53 @@ func TestPRSigstoreSignedPrepareTrustRoot(t *testing.T) { RekorPublicKeyPath: "fixtures/image.signature", SignedIdentity: testIdentity, }, + { // Both RekorPublicKeyPath and RekorPublicKeyPaths specified + KeyData: testKeyData, + RekorPublicKeyPath: testRekorPublicKeyPath, + RekorPublicKeyPaths: []string{testRekorPublicKeyPath, testKeyPath}, + SignedIdentity: testIdentity, + }, + { // Empty RekorPublicKeyPaths + KeyData: testKeyData, + RekorPublicKeyPaths: []string{}, + SignedIdentity: testIdentity, + }, + { // Invalid RekorPublicKeyPaths + KeyData: testKeyData, + RekorPublicKeyPaths: []string{"fixtures/image.signature", testRekorPublicKeyPath}, + SignedIdentity: testIdentity, + }, + { // Invalid RekorPublicKeyPaths + KeyData: testKeyData, + RekorPublicKeyPaths: []string{testRekorPublicKeyPath, "fixtures/image.signature"}, + SignedIdentity: testIdentity, + }, { // Invalid Rekor public key data KeyData: testKeyData, RekorPublicKeyData: []byte("this is invalid"), SignedIdentity: testIdentity, }, + { // Both RekorPublicKeyData and RekorPublicKeyDatas specified + KeyData: testKeyData, + RekorPublicKeyData: testRekorPublicKeyData, + RekorPublicKeyDatas: [][]byte{testRekorPublicKeyData, testKeyData}, + SignedIdentity: testIdentity, + }, + { // Empty RekorPublicKeyDatas + KeyData: testKeyData, + RekorPublicKeyDatas: [][]byte{}, + SignedIdentity: testIdentity, + }, + { // Invalid RekorPublicKeyDatas + KeyData: testKeyData, + RekorPublicKeyDatas: [][]byte{[]byte("this is invalid"), testRekorPublicKeyData}, + SignedIdentity: testIdentity, + }, + { + KeyData: testKeyData, + RekorPublicKeyDatas: [][]byte{testRekorPublicKeyData, []byte("this is invalid")}, + SignedIdentity: testIdentity, + }, { // Rekor public key is not ECDSA KeyData: testKeyData, RekorPublicKeyPath: "fixtures/some-rsa-key.pub", @@ -381,15 +417,21 @@ func TestPRrSigstoreSignedIsSignatureAccepted(t *testing.T) { {"fixtures/cosign2.pub", "fixtures/cosign.pub"}, {"fixtures/cosign.pub", "fixtures/cosign2.pub"}, } { - pr, err := newPRSigstoreSigned( - PRSigstoreSignedWithKeyPaths(keyPaths), - PRSigstoreSignedWithRekorPublicKeyPath("fixtures/rekor.pub"), - PRSigstoreSignedWithSignedIdentity(prm), - ) - require.NoError(t, err) - sar, err := pr.isSignatureAccepted(context.Background(), testKeyRekorImage, testKeyRekorImageSig) - require.NoError(t, err) - assertAccepted(sar, err) + for _, rekorKeyPaths := range [][]string{ + {"fixtures/rekor.pub"}, + {"fixtures/rekor.pub", "fixtures/cosign.pub"}, + {"fixtures/cosign.pub", "fixtures/rekor.pub"}, + } { + pr, err := newPRSigstoreSigned( + PRSigstoreSignedWithKeyPaths(keyPaths), + PRSigstoreSignedWithRekorPublicKeyPaths(rekorKeyPaths), + PRSigstoreSignedWithSignedIdentity(prm), + ) + require.NoError(t, err) + sar, err := pr.isSignatureAccepted(context.Background(), testKeyRekorImage, testKeyRekorImageSig) + require.NoError(t, err) + assertAccepted(sar, err) + } } // key+Rekor, missing Rekor SET annotation @@ -448,15 +490,21 @@ func TestPRrSigstoreSignedIsSignatureAccepted(t *testing.T) { PRSigstoreSignedFulcioWithSubjectEmail("mitr@redhat.com"), ) require.NoError(t, err) - pr, err = newPRSigstoreSigned( - PRSigstoreSignedWithFulcio(fulcio), - PRSigstoreSignedWithRekorPublicKeyPath("fixtures/rekor.pub"), - PRSigstoreSignedWithSignedIdentity(prm), - ) - require.NoError(t, err) - sar, err = pr.isSignatureAccepted(context.Background(), testFulcioRekorImage, - testFulcioRekorImageSig) - assertAccepted(sar, err) + for _, rekorKeyPaths := range [][]string{ + {"fixtures/rekor.pub"}, + {"fixtures/rekor.pub", "fixtures/cosign.pub"}, + {"fixtures/cosign.pub", "fixtures/rekor.pub"}, + } { + pr, err = newPRSigstoreSigned( + PRSigstoreSignedWithFulcio(fulcio), + PRSigstoreSignedWithRekorPublicKeyPaths(rekorKeyPaths), + PRSigstoreSignedWithSignedIdentity(prm), + ) + require.NoError(t, err) + sar, err = pr.isSignatureAccepted(context.Background(), testFulcioRekorImage, + testFulcioRekorImageSig) + assertAccepted(sar, err) + } // Fulcio, no Rekor requirement pr2 := &prSigstoreSigned{ diff --git a/signature/policy_types.go b/signature/policy_types.go index 9134cac652..32aa1c0ad4 100644 --- a/signature/policy_types.go +++ b/signature/policy_types.go @@ -125,13 +125,21 @@ type prSigstoreSigned struct { Fulcio PRSigstoreSignedFulcio `json:"fulcio,omitempty"` // RekorPublicKeyPath is a pathname to local file containing a public key of a Rekor server which must record acceptable signatures. - // If Fulcio is used, one of RekorPublicKeyPath or RekorPublicKeyData must be specified as well; otherwise it is optional - // (and Rekor inclusion is not required if a Rekor public key is not specified). + // If Fulcio is used, one of RekorPublicKeyPath, RekorPublicKeyPaths, RekorPublicKeyData and RekorPublicKeyDatas must be specified as well; + // otherwise it is optional (and Rekor inclusion is not required if a Rekor public key is not specified). RekorPublicKeyPath string `json:"rekorPublicKeyPath,omitempty"` + // RekorPublicKeyPaths is a set of pathnames to local files, each containing a public key of a Rekor server. One of the keys must record acceptable signatures. + // If Fulcio is used, one of RekorPublicKeyPath, RekorPublicKeyPaths, RekorPublicKeyData and RekorPublicKeyDatas must be specified as well; + // otherwise it is optional (and Rekor inclusion is not required if a Rekor public key is not specified). + RekorPublicKeyPaths []string `json:"rekorPublicKeyPaths,omitempty"` // RekorPublicKeyPath contain a base64-encoded public key of a Rekor server which must record acceptable signatures. - // If Fulcio is used, one of RekorPublicKeyPath or RekorPublicKeyData must be specified as well; otherwise it is optional - // (and Rekor inclusion is not required if a Rekor public key is not specified). + // If Fulcio is used, one of RekorPublicKeyPath, RekorPublicKeyPaths, RekorPublicKeyData and RekorPublicKeyDatas must be specified as well; + // otherwise it is optional (and Rekor inclusion is not required if a Rekor public key is not specified). RekorPublicKeyData []byte `json:"rekorPublicKeyData,omitempty"` + // RekorPublicKeyDatas each contain a base64-encoded public key of a Rekor server. One of the keys must record acceptable signatures. + // If Fulcio is used, one of RekorPublicKeyPath, RekorPublicKeyPaths, RekorPublicKeyData and RekorPublicKeyDatas must be specified as well; + // otherwise it is optional (and Rekor inclusion is not required if a Rekor public key is not specified). + RekorPublicKeyDatas [][]byte `json:"rekorPublicKeyDatas,omitempty"` // SignedIdentity specifies what image identity the signature must be claiming about the image. // Defaults to "matchRepoDigestOrExact" if not specified.