From c94eadbdd26b6e794e49fd8793fd82c6c7687d73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 9 Aug 2024 22:24:08 +0200 Subject: [PATCH 1/9] Fix the version number MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We are on the 5.32 branch and the previous release was 5.32.1. Signed-off-by: Miloslav Trmač --- version/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version/version.go b/version/version.go index 6c25bc3ea..60cfbb79b 100644 --- a/version/version.go +++ b/version/version.go @@ -6,7 +6,7 @@ const ( // VersionMajor is for an API incompatible changes VersionMajor = 5 // VersionMinor is for functionality in a backwards-compatible manner - VersionMinor = 33 + VersionMinor = 32 // VersionPatch is for backwards-compatible bug fixes VersionPatch = 2 From c0c8d340d9e342de07041ff0f53c8a10ba7fc395 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 16 Aug 2024 01:37:07 +0200 Subject: [PATCH 2/9] Use InvalidPolicyFormatError for invalid sigstore options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- signature/policy_config_sigstore.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/signature/policy_config_sigstore.go b/signature/policy_config_sigstore.go index beb5d0673..e77b3c419 100644 --- a/signature/policy_config_sigstore.go +++ b/signature/policy_config_sigstore.go @@ -2,7 +2,6 @@ package signature import ( "encoding/json" - "errors" "fmt" "github.com/containers/image/v5/signature/internal" @@ -15,7 +14,7 @@ type PRSigstoreSignedOption func(*prSigstoreSigned) error func PRSigstoreSignedWithKeyPath(keyPath string) PRSigstoreSignedOption { return func(pr *prSigstoreSigned) error { if pr.KeyPath != "" { - return errors.New(`"keyPath" already specified`) + return InvalidPolicyFormatError(`"keyPath" already specified`) } pr.KeyPath = keyPath return nil @@ -26,7 +25,7 @@ func PRSigstoreSignedWithKeyPath(keyPath string) PRSigstoreSignedOption { func PRSigstoreSignedWithKeyData(keyData []byte) PRSigstoreSignedOption { return func(pr *prSigstoreSigned) error { if pr.KeyData != nil { - return errors.New(`"keyData" already specified`) + return InvalidPolicyFormatError(`"keyData" already specified`) } pr.KeyData = keyData return nil @@ -37,7 +36,7 @@ func PRSigstoreSignedWithKeyData(keyData []byte) PRSigstoreSignedOption { func PRSigstoreSignedWithFulcio(fulcio PRSigstoreSignedFulcio) PRSigstoreSignedOption { return func(pr *prSigstoreSigned) error { if pr.Fulcio != nil { - return errors.New(`"fulcio" already specified`) + return InvalidPolicyFormatError(`"fulcio" already specified`) } pr.Fulcio = fulcio return nil @@ -48,7 +47,7 @@ func PRSigstoreSignedWithFulcio(fulcio PRSigstoreSignedFulcio) PRSigstoreSignedO func PRSigstoreSignedWithRekorPublicKeyPath(rekorPublicKeyPath string) PRSigstoreSignedOption { return func(pr *prSigstoreSigned) error { if pr.RekorPublicKeyPath != "" { - return errors.New(`"rekorPublicKeyPath" already specified`) + return InvalidPolicyFormatError(`"rekorPublicKeyPath" already specified`) } pr.RekorPublicKeyPath = rekorPublicKeyPath return nil @@ -59,7 +58,7 @@ func PRSigstoreSignedWithRekorPublicKeyPath(rekorPublicKeyPath string) PRSigstor func PRSigstoreSignedWithRekorPublicKeyData(rekorPublicKeyData []byte) PRSigstoreSignedOption { return func(pr *prSigstoreSigned) error { if pr.RekorPublicKeyData != nil { - return errors.New(`"rekorPublicKeyData" already specified`) + return InvalidPolicyFormatError(`"rekorPublicKeyData" already specified`) } pr.RekorPublicKeyData = rekorPublicKeyData return nil @@ -70,7 +69,7 @@ func PRSigstoreSignedWithRekorPublicKeyData(rekorPublicKeyData []byte) PRSigstor func PRSigstoreSignedWithSignedIdentity(signedIdentity PolicyReferenceMatch) PRSigstoreSignedOption { return func(pr *prSigstoreSigned) error { if pr.SignedIdentity != nil { - return errors.New(`"signedIdentity" already specified`) + return InvalidPolicyFormatError(`"signedIdentity" already specified`) } pr.SignedIdentity = signedIdentity return nil @@ -221,7 +220,7 @@ type PRSigstoreSignedFulcioOption func(*prSigstoreSignedFulcio) error func PRSigstoreSignedFulcioWithCAPath(caPath string) PRSigstoreSignedFulcioOption { return func(f *prSigstoreSignedFulcio) error { if f.CAPath != "" { - return errors.New(`"caPath" already specified`) + return InvalidPolicyFormatError(`"caPath" already specified`) } f.CAPath = caPath return nil @@ -232,7 +231,7 @@ func PRSigstoreSignedFulcioWithCAPath(caPath string) PRSigstoreSignedFulcioOptio func PRSigstoreSignedFulcioWithCAData(caData []byte) PRSigstoreSignedFulcioOption { return func(f *prSigstoreSignedFulcio) error { if f.CAData != nil { - return errors.New(`"caData" already specified`) + return InvalidPolicyFormatError(`"caData" already specified`) } f.CAData = caData return nil @@ -243,7 +242,7 @@ func PRSigstoreSignedFulcioWithCAData(caData []byte) PRSigstoreSignedFulcioOptio func PRSigstoreSignedFulcioWithOIDCIssuer(oidcIssuer string) PRSigstoreSignedFulcioOption { return func(f *prSigstoreSignedFulcio) error { if f.OIDCIssuer != "" { - return errors.New(`"oidcIssuer" already specified`) + return InvalidPolicyFormatError(`"oidcIssuer" already specified`) } f.OIDCIssuer = oidcIssuer return nil @@ -254,7 +253,7 @@ func PRSigstoreSignedFulcioWithOIDCIssuer(oidcIssuer string) PRSigstoreSignedFul func PRSigstoreSignedFulcioWithSubjectEmail(subjectEmail string) PRSigstoreSignedFulcioOption { return func(f *prSigstoreSignedFulcio) error { if f.SubjectEmail != "" { - return errors.New(`"subjectEmail" already specified`) + return InvalidPolicyFormatError(`"subjectEmail" already specified`) } f.SubjectEmail = subjectEmail return nil From 03550b4d03eb697df6c26f4113818b6629ff5694 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 16 Aug 2024 02:27:52 +0200 Subject: [PATCH 3/9] Turn loadBytesFromDataOrPath into loadBytesFromConfigSources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use a struct as an input, so that the parameters are named and we minimize risk of inconsistencies, and make it easier to add more sources. Should not change behavior. Signed-off-by: Miloslav Trmač --- signature/policy_eval_sigstore.go | 53 ++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/signature/policy_eval_sigstore.go b/signature/policy_eval_sigstore.go index 485165077..99d3a1e01 100644 --- a/signature/policy_eval_sigstore.go +++ b/signature/policy_eval_sigstore.go @@ -20,29 +20,44 @@ import ( "github.com/sigstore/sigstore/pkg/cryptoutils" ) -// loadBytesFromDataOrPath ensures there is at most one of ${prefix}Data and ${prefix}Path set, +// configBytesSources contains configuration fields which may result in one or more []byte values +type configBytesSources struct { + inconsistencyErrorMessage string // Error to return if more than one source is set + path string // …Path: a path to a file containing the data, or "" + data []byte // …Data: The raw data, or nil +} + +// loadBytesFromConfigSources ensures at most one of the sources in src is set, // and returns the referenced data, or nil if neither is set. -func loadBytesFromDataOrPath(prefix string, data []byte, path string) ([]byte, error) { - switch { - case data != nil && path != "": - return nil, fmt.Errorf(`Internal inconsistency: both "%sPath" and "%sData" specified`, prefix, prefix) - case path != "": - d, err := os.ReadFile(path) +func loadBytesFromConfigSources(src configBytesSources) ([]byte, error) { + sources := 0 + var data []byte // = nil + if src.path != "" { + sources++ + d, err := os.ReadFile(src.path) if err != nil { return nil, err } - return d, nil - case data != nil: - return data, nil - default: // Nothing - return nil, nil + data = d + } + if src.data != nil { + sources++ + data = src.data } + if sources > 1 { + return nil, errors.New(src.inconsistencyErrorMessage) + } + return data, nil } // prepareTrustRoot creates a fulcioTrustRoot from the input data. // (This also prevents external implementations of this interface, ensuring that prSigstoreSignedFulcio is the only one.) func (f *prSigstoreSignedFulcio) prepareTrustRoot() (*fulcioTrustRoot, error) { - caCertBytes, err := loadBytesFromDataOrPath("fulcioCA", f.CAData, f.CAPath) + caCertBytes, err := loadBytesFromConfigSources(configBytesSources{ + inconsistencyErrorMessage: `Internal inconsistency: both "caPath" and "caData" specified`, + path: f.CAPath, + data: f.CAData, + }) if err != nil { return nil, err } @@ -74,7 +89,11 @@ type sigstoreSignedTrustRoot struct { func (pr *prSigstoreSigned) prepareTrustRoot() (*sigstoreSignedTrustRoot, error) { res := sigstoreSignedTrustRoot{} - publicKeyPEM, err := loadBytesFromDataOrPath("key", pr.KeyData, pr.KeyPath) + publicKeyPEM, err := loadBytesFromConfigSources(configBytesSources{ + inconsistencyErrorMessage: `Internal inconsistency: both "keyPath" and "keyData" specified`, + path: pr.KeyPath, + data: pr.KeyData, + }) if err != nil { return nil, err } @@ -94,7 +113,11 @@ func (pr *prSigstoreSigned) prepareTrustRoot() (*sigstoreSignedTrustRoot, error) res.fulcio = f } - rekorPublicKeyPEM, err := loadBytesFromDataOrPath("rekorPublicKey", pr.RekorPublicKeyData, pr.RekorPublicKeyPath) + rekorPublicKeyPEM, err := loadBytesFromConfigSources(configBytesSources{ + inconsistencyErrorMessage: `Internal inconsistency: both "rekorPublicKeyPath" and "rekorPublicKeyData" specified`, + path: pr.RekorPublicKeyPath, + data: pr.RekorPublicKeyData, + }) if err != nil { return nil, err } From a5ea0164e384f81fd2766e964bf4746f492d14e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 16 Aug 2024 02:33:19 +0200 Subject: [PATCH 4/9] Use loadBytesFromConfigSources in prSignedBy.isSignatureAuthorAccepted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend loadBytesFromConfigSources to return multiple values, and to support reading the from files; then share the code. Signed-off-by: Miloslav Trmač --- signature/policy_eval_signedby.go | 38 ++++++--------------- signature/policy_eval_sigstore.go | 56 +++++++++++++++++++++---------- 2 files changed, 50 insertions(+), 44 deletions(-) diff --git a/signature/policy_eval_signedby.go b/signature/policy_eval_signedby.go index 896ca5a60..e5c932918 100644 --- a/signature/policy_eval_signedby.go +++ b/signature/policy_eval_signedby.go @@ -6,7 +6,6 @@ import ( "context" "errors" "fmt" - "os" "slices" "github.com/containers/image/v5/internal/multierr" @@ -27,33 +26,18 @@ func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image priva } // FIXME: move this to per-context initialization - var data [][]byte - keySources := 0 - if pr.KeyPath != "" { - keySources++ - d, err := os.ReadFile(pr.KeyPath) - if err != nil { - return sarRejected, nil, err - } - data = [][]byte{d} - } - if pr.KeyPaths != nil { - keySources++ - data = [][]byte{} - for _, path := range pr.KeyPaths { - d, err := os.ReadFile(path) - if err != nil { - return sarRejected, nil, err - } - data = append(data, d) - } - } - if pr.KeyData != nil { - keySources++ - data = [][]byte{pr.KeyData} + const notOneSourceErrorText = `Internal inconsistency: not exactly one of "keyPath", "keyPaths" and "keyData" specified` + data, err := loadBytesFromConfigSources(configBytesSources{ + inconsistencyErrorMessage: notOneSourceErrorText, + path: pr.KeyPath, + paths: pr.KeyPaths, + data: pr.KeyData, + }) + if err != nil { + return sarRejected, nil, err } - if keySources != 1 { - return sarRejected, nil, errors.New(`Internal inconsistency: not exactly one of "keyPath", "keyPaths" and "keyData" specified`) + if data == nil { + return sarRejected, nil, errors.New(notOneSourceErrorText) } // FIXME: move this to per-context initialization diff --git a/signature/policy_eval_sigstore.go b/signature/policy_eval_sigstore.go index 99d3a1e01..610a82080 100644 --- a/signature/policy_eval_sigstore.go +++ b/signature/policy_eval_sigstore.go @@ -22,27 +22,39 @@ import ( // configBytesSources contains configuration fields which may result in one or more []byte values type configBytesSources struct { - inconsistencyErrorMessage string // Error to return if more than one source is set - path string // …Path: a path to a file containing the data, or "" - data []byte // …Data: The raw data, or nil + inconsistencyErrorMessage string // Error to return if more than one source is set + path string // …Path: a path to a file containing the data, or "" + paths []string // …Paths: paths to files containing the data, or nil + data []byte // …Data: a single instance ofhe raw data, or nil } // loadBytesFromConfigSources ensures at most one of the sources in src is set, // and returns the referenced data, or nil if neither is set. -func loadBytesFromConfigSources(src configBytesSources) ([]byte, error) { +func loadBytesFromConfigSources(src configBytesSources) ([][]byte, error) { sources := 0 - var data []byte // = nil + var data [][]byte // = nil if src.path != "" { sources++ d, err := os.ReadFile(src.path) if err != nil { return nil, err } - data = d + data = [][]byte{d} + } + if src.paths != nil { + sources++ + data = [][]byte{} + for _, path := range src.paths { + d, err := os.ReadFile(path) + if err != nil { + return nil, err + } + data = append(data, d) + } } if src.data != nil { sources++ - data = src.data + data = [][]byte{src.data} } if sources > 1 { return nil, errors.New(src.inconsistencyErrorMessage) @@ -53,7 +65,7 @@ func loadBytesFromConfigSources(src configBytesSources) ([]byte, error) { // prepareTrustRoot creates a fulcioTrustRoot from the input data. // (This also prevents external implementations of this interface, ensuring that prSigstoreSignedFulcio is the only one.) func (f *prSigstoreSignedFulcio) prepareTrustRoot() (*fulcioTrustRoot, error) { - caCertBytes, err := loadBytesFromConfigSources(configBytesSources{ + caCertPEMs, err := loadBytesFromConfigSources(configBytesSources{ inconsistencyErrorMessage: `Internal inconsistency: both "caPath" and "caData" specified`, path: f.CAPath, data: f.CAData, @@ -61,11 +73,11 @@ func (f *prSigstoreSignedFulcio) prepareTrustRoot() (*fulcioTrustRoot, error) { if err != nil { return nil, err } - if caCertBytes == nil { - return nil, errors.New(`Internal inconsistency: Fulcio specified with neither "caPath" nor "caData"`) + if len(caCertPEMs) != 1 { + return nil, errors.New(`Internal inconsistency: Fulcio specified with not exactly one of "caPath" nor "caData"`) } certs := x509.NewCertPool() - if ok := certs.AppendCertsFromPEM(caCertBytes); !ok { + if ok := certs.AppendCertsFromPEM(caCertPEMs[0]); !ok { return nil, errors.New("error loading Fulcio CA certificates") } fulcio := fulcioTrustRoot{ @@ -89,7 +101,7 @@ type sigstoreSignedTrustRoot struct { func (pr *prSigstoreSigned) prepareTrustRoot() (*sigstoreSignedTrustRoot, error) { res := sigstoreSignedTrustRoot{} - publicKeyPEM, err := loadBytesFromConfigSources(configBytesSources{ + publicKeyPEMs, err := loadBytesFromConfigSources(configBytesSources{ inconsistencyErrorMessage: `Internal inconsistency: both "keyPath" and "keyData" specified`, path: pr.KeyPath, data: pr.KeyData, @@ -97,8 +109,13 @@ func (pr *prSigstoreSigned) prepareTrustRoot() (*sigstoreSignedTrustRoot, error) if err != nil { return nil, err } - if publicKeyPEM != nil { - pk, err := cryptoutils.UnmarshalPEMToPublicKey(publicKeyPEM) + if publicKeyPEMs != nil { + if len(publicKeyPEMs) != 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 "keyPath" and "keyData"`) + } + pk, err := cryptoutils.UnmarshalPEMToPublicKey(publicKeyPEMs[0]) if err != nil { return nil, fmt.Errorf("parsing public key: %w", err) } @@ -113,7 +130,7 @@ func (pr *prSigstoreSigned) prepareTrustRoot() (*sigstoreSignedTrustRoot, error) res.fulcio = f } - rekorPublicKeyPEM, err := loadBytesFromConfigSources(configBytesSources{ + rekorPublicKeyPEMs, err := loadBytesFromConfigSources(configBytesSources{ inconsistencyErrorMessage: `Internal inconsistency: both "rekorPublicKeyPath" and "rekorPublicKeyData" specified`, path: pr.RekorPublicKeyPath, data: pr.RekorPublicKeyData, @@ -121,8 +138,13 @@ func (pr *prSigstoreSigned) prepareTrustRoot() (*sigstoreSignedTrustRoot, error) if err != nil { return nil, err } - if rekorPublicKeyPEM != nil { - pk, err := cryptoutils.UnmarshalPEMToPublicKey(rekorPublicKeyPEM) + 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) } From d42de59cf66ca53257e8a6ee8e912c1a41b7689f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 16 Aug 2024 00:43:42 +0200 Subject: [PATCH 5/9] Split verifySigstorePayloadBlobSignature from VerifySigstorePayload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit because we will want to support multiple public keys, and that's easier to do in a separate function. Should not change behavior except for order of error checks. Signed-off-by: Miloslav Trmač --- signature/internal/sigstore_payload.go | 31 ++++++++++++++++++-------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/signature/internal/sigstore_payload.go b/signature/internal/sigstore_payload.go index a2609c954..a031695ba 100644 --- a/signature/internal/sigstore_payload.go +++ b/signature/internal/sigstore_payload.go @@ -171,24 +171,37 @@ type SigstorePayloadAcceptanceRules struct { ValidateSignedDockerManifestDigest func(digest.Digest) error } +// verifySigstorePayloadBlobSignature verifies unverifiedSignature of unverifiedPayload was correctly created +// by publicKey. +// +// 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(publicKey crypto.PublicKey, unverifiedPayload, unverifiedSignature []byte) error { + verifier, err := sigstoreSignature.LoadVerifier(publicKey, sigstoreHarcodedHashAlgorithm) + if err != nil { + return 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 NewInvalidSignatureError(fmt.Sprintf("cryptographic signature verification failed: %v", err)) + } + return nil +} + // VerifySigstorePayload verifies unverifiedBase64Signature of unverifiedPayload was correctly created by publicKey, 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) - } - 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(publicKey, unverifiedPayload, unverifiedSignature); err != nil { + return nil, err } var unmatchedPayload UntrustedSigstorePayload From 95c0635e4c273c963c966173c482896cd4c37d60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= Date: Thu, 20 Jun 2024 11:50:57 +0200 Subject: [PATCH 6/9] Add field `KeyPaths` and `KeyDatas` to `prSigstoreSigned` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new fields `KeyPaths` and `KeyDatas` is taken directly from `/etc/containers/policy.json` and allows users to provide multiple signature keys to be used to verify images. Only one of the keys has to verify, thereby this mechanism allows us to have support seamless key rotation on a registry. This fixes https://github.com/containers/image/issues/2319 Signed-off-by: Dan Čermák Co-authored-by: Danish Prakash Signed-off-by: Miloslav Trmač --- docs/containers-policy.json.5.md | 7 +- signature/internal/sigstore_payload.go | 52 ++++-- signature/internal/sigstore_payload_test.go | 89 ++++++++--- signature/internal/testdata/cosign2.pub | 1 + signature/policy_config_sigstore.go | 50 +++++- signature/policy_config_sigstore_test.go | 116 +++++++++++++- signature/policy_eval_sigstore.go | 81 ++++++---- signature/policy_eval_sigstore_test.go | 166 +++++++++++++++----- signature/policy_types.go | 13 +- signature/sigstore/generate_test.go | 3 +- 10 files changed, 458 insertions(+), 120 deletions(-) create mode 120000 signature/internal/testdata/cosign2.pub diff --git a/docs/containers-policy.json.5.md b/docs/containers-policy.json.5.md index 909d04afd..5b42e21ae 100644 --- a/docs/containers-policy.json.5.md +++ b/docs/containers-policy.json.5.md @@ -320,7 +320,9 @@ 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", @@ -332,11 +334,14 @@ This requirement requires an image to be signed using a sigstore signature with "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, diff --git a/signature/internal/sigstore_payload.go b/signature/internal/sigstore_payload.go index a031695ba..8630b08e3 100644 --- a/signature/internal/sigstore_payload.go +++ b/signature/internal/sigstore_payload.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "strings" "time" "github.com/containers/image/v5/version" @@ -172,35 +173,60 @@ type SigstorePayloadAcceptanceRules struct { } // verifySigstorePayloadBlobSignature verifies unverifiedSignature of unverifiedPayload was correctly created -// by publicKey. +// 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(publicKey crypto.PublicKey, unverifiedPayload, unverifiedSignature []byte) error { - verifier, err := sigstoreSignature.LoadVerifier(publicKey, sigstoreHarcodedHashAlgorithm) - if err != nil { - return err +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()) } - // 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 NewInvalidSignatureError(fmt.Sprintf("cryptographic signature verification failed: %v", err)) + 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 nil + return NewInvalidSignatureError("cryptographic signature verification failed: " + strings.Join(failures, ", ")) } -// VerifySigstorePayload verifies unverifiedBase64Signature of unverifiedPayload was correctly created by publicKey, and that its principal components +// 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) { +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)) } - if err := verifySigstorePayloadBlobSignature(publicKey, unverifiedPayload, unverifiedSignature); err != nil { + if err := verifySigstorePayloadBlobSignature(publicKeys, unverifiedPayload, unverifiedSignature); err != nil { return nil, err } diff --git a/signature/internal/sigstore_payload_test.go b/signature/internal/sigstore_payload_test.go index 61b011ffe..d6be57d78 100644 --- a/signature/internal/sigstore_payload_test.go +++ b/signature/internal/sigstore_payload_test.go @@ -2,6 +2,7 @@ package internal import ( "bytes" + "crypto" "encoding/base64" "encoding/json" "errors" @@ -204,11 +205,18 @@ func TestUntrustedSigstorePayloadUnmarshalJSON(t *testing.T) { assert.Equal(t, validSig, s) } +// verifySigstorePayloadBlobSignature is tested by TestVerifySigstorePayload + func TestVerifySigstorePayload(t *testing.T) { publicKeyPEM, err := os.ReadFile("./testdata/cosign.pub") require.NoError(t, err) publicKey, err := cryptoutils.UnmarshalPEMToPublicKey(publicKeyPEM) require.NoError(t, err) + publicKeyPEM2, err := os.ReadFile("./testdata/cosign2.pub") + require.NoError(t, err) + publicKey2, err := cryptoutils.UnmarshalPEMToPublicKey(publicKeyPEM2) + require.NoError(t, err) + singlePublicKey := []crypto.PublicKey{publicKey} type acceptanceData struct { signedDockerReference string @@ -248,28 +256,26 @@ func TestVerifySigstorePayload(t *testing.T) { } // Successful verification - wanted = signatureData - recorded = acceptanceData{} - res, err := VerifySigstorePayload(publicKey, sigstoreSig.UntrustedPayload(), cryptoBase64Sig, recordingRules) - require.NoError(t, err) - assert.Equal(t, res, &UntrustedSigstorePayload{ - untrustedDockerManifestDigest: TestSigstoreManifestDigest, - untrustedDockerReference: TestSigstoreSignatureReference, - untrustedCreatorID: nil, - untrustedTimestamp: nil, - }) - assert.Equal(t, signatureData, recorded) + for _, publicKeys := range [][]crypto.PublicKey{ + singlePublicKey, + {publicKey, publicKey2}, // The matching key is first + {publicKey2, publicKey}, // The matching key is second + } { + wanted = signatureData + recorded = acceptanceData{} + res, err := VerifySigstorePayload(publicKeys, sigstoreSig.UntrustedPayload(), cryptoBase64Sig, recordingRules) + require.NoError(t, err) + assert.Equal(t, res, &UntrustedSigstorePayload{ + untrustedDockerManifestDigest: TestSigstoreManifestDigest, + untrustedDockerReference: TestSigstoreSignatureReference, + untrustedCreatorID: nil, + untrustedTimestamp: nil, + }) + assert.Equal(t, signatureData, recorded) + } // For extra paranoia, test that we return a nil signature object on error. - // Invalid verifier - recorded = acceptanceData{} - invalidPublicKey := struct{}{} // crypto.PublicKey is, for some reason, just an any, so this is acceptable. - res, err = VerifySigstorePayload(invalidPublicKey, sigstoreSig.UntrustedPayload(), cryptoBase64Sig, recordingRules) - assert.Error(t, err) - assert.Nil(t, res) - assert.Equal(t, acceptanceData{}, recorded) - // Invalid base64 encoding for _, invalidBase64Sig := range []string{ "&", // Invalid base64 characters @@ -277,7 +283,28 @@ func TestVerifySigstorePayload(t *testing.T) { cryptoBase64Sig[:len(cryptoBase64Sig)-1], // Truncated base64 data } { recorded = acceptanceData{} - res, err = VerifySigstorePayload(publicKey, sigstoreSig.UntrustedPayload(), invalidBase64Sig, recordingRules) + res, err := VerifySigstorePayload(singlePublicKey, sigstoreSig.UntrustedPayload(), invalidBase64Sig, recordingRules) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, acceptanceData{}, recorded) + } + + // No public keys + recorded = acceptanceData{} + res, err := VerifySigstorePayload([]crypto.PublicKey{}, sigstoreSig.UntrustedPayload(), cryptoBase64Sig, recordingRules) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, acceptanceData{}, recorded) + + // Invalid verifier: + // crypto.PublicKey is, for some reason, just an any, so using a struct{}{} to trigger this code path works. + for _, invalidPublicKeys := range [][]crypto.PublicKey{ + {struct{}{}}, // A single invalid key + {struct{}{}, publicKey}, // An invalid key, followed by a matching key + {publicKey, struct{}{}}, // A matching key, but the configuration also includes an invalid key + } { + recorded = acceptanceData{} + res, err = VerifySigstorePayload(invalidPublicKeys, sigstoreSig.UntrustedPayload(), cryptoBase64Sig, recordingRules) assert.Error(t, err) assert.Nil(t, res) assert.Equal(t, acceptanceData{}, recorded) @@ -292,7 +319,19 @@ func TestVerifySigstorePayload(t *testing.T) { append(bytes.Clone(validSignatureBytes), validSignatureBytes...), } { recorded = acceptanceData{} - res, err = VerifySigstorePayload(publicKey, sigstoreSig.UntrustedPayload(), base64.StdEncoding.EncodeToString(invalidSig), recordingRules) + res, err = VerifySigstorePayload(singlePublicKey, sigstoreSig.UntrustedPayload(), base64.StdEncoding.EncodeToString(invalidSig), recordingRules) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, acceptanceData{}, recorded) + } + + // No matching public keys + for _, nonmatchingPublicKeys := range [][]crypto.PublicKey{ + {publicKey2}, + {publicKey2, publicKey2}, + } { + recorded = acceptanceData{} + res, err = VerifySigstorePayload(nonmatchingPublicKeys, sigstoreSig.UntrustedPayload(), cryptoBase64Sig, recordingRules) assert.Error(t, err) assert.Nil(t, res) assert.Equal(t, acceptanceData{}, recorded) @@ -300,14 +339,14 @@ func TestVerifySigstorePayload(t *testing.T) { // Valid signature of non-JSON recorded = acceptanceData{} - res, err = VerifySigstorePayload(publicKey, []byte("&"), "MEUCIARnnxZQPALBfqkB4aNAYXad79Qs6VehcrgIeZ8p7I2FAiEAzq2HXwXlz1iJeh+ucUR3L0zpjynQk6Rk0+/gXYp49RU=", recordingRules) + res, err = VerifySigstorePayload(singlePublicKey, []byte("&"), "MEUCIARnnxZQPALBfqkB4aNAYXad79Qs6VehcrgIeZ8p7I2FAiEAzq2HXwXlz1iJeh+ucUR3L0zpjynQk6Rk0+/gXYp49RU=", recordingRules) assert.Error(t, err) assert.Nil(t, res) assert.Equal(t, acceptanceData{}, recorded) // Valid signature of an unacceptable JSON recorded = acceptanceData{} - res, err = VerifySigstorePayload(publicKey, []byte("{}"), "MEUCIQDkySOBGxastVP0+koTA33NH5hXjwosFau4rxTPN6g48QIgb7eWKkGqfEpHMM3aT4xiqyP/170jEkdFuciuwN4mux4=", recordingRules) + res, err = VerifySigstorePayload(singlePublicKey, []byte("{}"), "MEUCIQDkySOBGxastVP0+koTA33NH5hXjwosFau4rxTPN6g48QIgb7eWKkGqfEpHMM3aT4xiqyP/170jEkdFuciuwN4mux4=", recordingRules) assert.Error(t, err) assert.Nil(t, res) assert.Equal(t, acceptanceData{}, recorded) @@ -316,7 +355,7 @@ func TestVerifySigstorePayload(t *testing.T) { wanted = signatureData wanted.signedDockerManifestDigest = "invalid digest" recorded = acceptanceData{} - res, err = VerifySigstorePayload(publicKey, sigstoreSig.UntrustedPayload(), cryptoBase64Sig, recordingRules) + res, err = VerifySigstorePayload(singlePublicKey, sigstoreSig.UntrustedPayload(), cryptoBase64Sig, recordingRules) assert.Error(t, err) assert.Nil(t, res) assert.Equal(t, acceptanceData{ @@ -327,7 +366,7 @@ func TestVerifySigstorePayload(t *testing.T) { wanted = signatureData wanted.signedDockerReference = "unexpected docker reference" recorded = acceptanceData{} - res, err = VerifySigstorePayload(publicKey, sigstoreSig.UntrustedPayload(), cryptoBase64Sig, recordingRules) + res, err = VerifySigstorePayload(singlePublicKey, sigstoreSig.UntrustedPayload(), cryptoBase64Sig, recordingRules) assert.Error(t, err) assert.Nil(t, res) assert.Equal(t, signatureData, recorded) diff --git a/signature/internal/testdata/cosign2.pub b/signature/internal/testdata/cosign2.pub new file mode 120000 index 000000000..ea2aa9077 --- /dev/null +++ b/signature/internal/testdata/cosign2.pub @@ -0,0 +1 @@ +../../fixtures/cosign2.pub \ No newline at end of file diff --git a/signature/policy_config_sigstore.go b/signature/policy_config_sigstore.go index e77b3c419..26e7d355b 100644 --- a/signature/policy_config_sigstore.go +++ b/signature/policy_config_sigstore.go @@ -21,6 +21,20 @@ func PRSigstoreSignedWithKeyPath(keyPath string) PRSigstoreSignedOption { } } +// PRSigstoreSignedWithKeyPaths specifies a value for the "keyPaths" field when calling NewPRSigstoreSigned. +func PRSigstoreSignedWithKeyPaths(keyPaths []string) PRSigstoreSignedOption { + return func(pr *prSigstoreSigned) error { + if pr.KeyPaths != nil { + return InvalidPolicyFormatError(`"keyPaths" already specified`) + } + if len(keyPaths) == 0 { + return InvalidPolicyFormatError(`"keyPaths" contains no entries`) + } + pr.KeyPaths = keyPaths + return nil + } +} + // PRSigstoreSignedWithKeyData specifies a value for the "keyData" field when calling NewPRSigstoreSigned. func PRSigstoreSignedWithKeyData(keyData []byte) PRSigstoreSignedOption { return func(pr *prSigstoreSigned) error { @@ -32,6 +46,20 @@ func PRSigstoreSignedWithKeyData(keyData []byte) PRSigstoreSignedOption { } } +// PRSigstoreSignedWithKeyDatas specifies a value for the "keyDatas" field when calling NewPRSigstoreSigned. +func PRSigstoreSignedWithKeyDatas(keyDatas [][]byte) PRSigstoreSignedOption { + return func(pr *prSigstoreSigned) error { + if pr.KeyDatas != nil { + return InvalidPolicyFormatError(`"keyDatas" already specified`) + } + if len(keyDatas) == 0 { + return InvalidPolicyFormatError(`"keyDatas" contains no entries`) + } + pr.KeyDatas = keyDatas + return nil + } +} + // PRSigstoreSignedWithFulcio specifies a value for the "fulcio" field when calling NewPRSigstoreSigned. func PRSigstoreSignedWithFulcio(fulcio PRSigstoreSignedFulcio) PRSigstoreSignedOption { return func(pr *prSigstoreSigned) error { @@ -91,14 +119,20 @@ func newPRSigstoreSigned(options ...PRSigstoreSignedOption) (*prSigstoreSigned, if res.KeyPath != "" { keySources++ } + if res.KeyPaths != nil { + keySources++ + } if res.KeyData != nil { keySources++ } + if res.KeyDatas != nil { + keySources++ + } if res.Fulcio != nil { keySources++ } if keySources != 1 { - return nil, InvalidPolicyFormatError("exactly one of keyPath, keyData and fulcio must be specified") + return nil, InvalidPolicyFormatError("exactly one of keyPath, keyPaths, keyData, keyDatas and fulcio must be specified") } if res.RekorPublicKeyPath != "" && res.RekorPublicKeyData != nil { @@ -143,7 +177,7 @@ var _ json.Unmarshaler = (*prSigstoreSigned)(nil) func (pr *prSigstoreSigned) UnmarshalJSON(data []byte) error { *pr = prSigstoreSigned{} var tmp prSigstoreSigned - var gotKeyPath, gotKeyData, gotFulcio, gotRekorPublicKeyPath, gotRekorPublicKeyData bool + var gotKeyPath, gotKeyPaths, gotKeyData, gotKeyDatas, gotFulcio, gotRekorPublicKeyPath, gotRekorPublicKeyData bool var fulcio prSigstoreSignedFulcio var signedIdentity json.RawMessage if err := internal.ParanoidUnmarshalJSONObject(data, func(key string) any { @@ -153,9 +187,15 @@ func (pr *prSigstoreSigned) UnmarshalJSON(data []byte) error { case "keyPath": gotKeyPath = true return &tmp.KeyPath + case "keyPaths": + gotKeyPaths = true + return &tmp.KeyPaths case "keyData": gotKeyData = true return &tmp.KeyData + case "keyDatas": + gotKeyDatas = true + return &tmp.KeyDatas case "fulcio": gotFulcio = true return &fulcio @@ -191,9 +231,15 @@ func (pr *prSigstoreSigned) UnmarshalJSON(data []byte) error { if gotKeyPath { opts = append(opts, PRSigstoreSignedWithKeyPath(tmp.KeyPath)) } + if gotKeyPaths { + opts = append(opts, PRSigstoreSignedWithKeyPaths(tmp.KeyPaths)) + } if gotKeyData { opts = append(opts, PRSigstoreSignedWithKeyData(tmp.KeyData)) } + if gotKeyDatas { + opts = append(opts, PRSigstoreSignedWithKeyDatas(tmp.KeyDatas)) + } if gotFulcio { opts = append(opts, PRSigstoreSignedWithFulcio(&fulcio)) } diff --git a/signature/policy_config_sigstore_test.go b/signature/policy_config_sigstore_test.go index d71ae5f89..1eec5ac3c 100644 --- a/signature/policy_config_sigstore_test.go +++ b/signature/policy_config_sigstore_test.go @@ -20,7 +20,9 @@ func xNewPRSigstoreSigned(options ...PRSigstoreSignedOption) PolicyRequirement { func TestNewPRSigstoreSigned(t *testing.T) { const testKeyPath = "/foo/bar" + const testKeyPath2 = "/baz/bar" testKeyData := []byte("abc") + testKeyData2 := []byte("def") testFulcio, err := NewPRSigstoreSignedFulcio( PRSigstoreSignedFulcioWithCAPath("fixtures/fulcio_v1.crt.pem"), PRSigstoreSignedFulcioWithOIDCIssuer("https://github.com/login/oauth"), @@ -45,7 +47,24 @@ func TestNewPRSigstoreSigned(t *testing.T) { expected: prSigstoreSigned{ prCommon: prCommon{prTypeSigstoreSigned}, KeyPath: testKeyPath, + KeyPaths: nil, KeyData: nil, + KeyDatas: nil, + Fulcio: nil, + SignedIdentity: testIdentity, + }, + }, + { + options: []PRSigstoreSignedOption{ + PRSigstoreSignedWithKeyPaths([]string{testKeyPath, testKeyPath2}), + PRSigstoreSignedWithSignedIdentity(testIdentity), + }, + expected: prSigstoreSigned{ + prCommon: prCommon{prTypeSigstoreSigned}, + KeyPath: "", + KeyPaths: []string{testKeyPath, testKeyPath2}, + KeyData: nil, + KeyDatas: nil, Fulcio: nil, SignedIdentity: testIdentity, }, @@ -58,7 +77,24 @@ func TestNewPRSigstoreSigned(t *testing.T) { expected: prSigstoreSigned{ prCommon: prCommon{prTypeSigstoreSigned}, KeyPath: "", + KeyPaths: nil, KeyData: testKeyData, + KeyDatas: nil, + Fulcio: nil, + SignedIdentity: testIdentity, + }, + }, + { + options: []PRSigstoreSignedOption{ + PRSigstoreSignedWithKeyDatas([][]byte{testKeyData, testKeyData2}), + PRSigstoreSignedWithSignedIdentity(testIdentity), + }, + expected: prSigstoreSigned{ + prCommon: prCommon{prTypeSigstoreSigned}, + KeyPath: "", + KeyPaths: nil, + KeyData: nil, + KeyDatas: [][]byte{testKeyData, testKeyData2}, Fulcio: nil, SignedIdentity: testIdentity, }, @@ -72,7 +108,9 @@ func TestNewPRSigstoreSigned(t *testing.T) { expected: prSigstoreSigned{ prCommon: prCommon{prTypeSigstoreSigned}, KeyPath: "", + KeyPaths: nil, KeyData: nil, + KeyDatas: nil, Fulcio: testFulcio, SignedIdentity: testIdentity, }, @@ -146,11 +184,39 @@ func TestNewPRSigstoreSigned(t *testing.T) { PRSigstoreSignedWithKeyPath(testKeyPath + "1"), PRSigstoreSignedWithSignedIdentity(testIdentity), }, + { // Empty keypaths + PRSigstoreSignedWithKeyPaths([]string{}), + PRSigstoreSignedWithSignedIdentity(testIdentity), + }, + { // Duplicate keyPaths + PRSigstoreSignedWithKeyPaths([]string{testKeyPath, testKeyPath2}), + PRSigstoreSignedWithKeyPaths([]string{testKeyPath + "1", testKeyPath2 + "1"}), + PRSigstoreSignedWithSignedIdentity(testIdentity), + }, + { // keyPath & keyPaths both set + PRSigstoreSignedWithKeyPath("foobar"), + PRSigstoreSignedWithKeyPaths([]string{"foobar"}), + PRSigstoreSignedWithSignedIdentity(testIdentity), + }, { // Duplicate keyData PRSigstoreSignedWithKeyData(testKeyData), PRSigstoreSignedWithKeyData([]byte("def")), PRSigstoreSignedWithSignedIdentity(testIdentity), }, + { // Empty keyDatas + PRSigstoreSignedWithKeyDatas([][]byte{}), + PRSigstoreSignedWithSignedIdentity(testIdentity), + }, + { // Duplicate keyDatas + PRSigstoreSignedWithKeyDatas([][]byte{testKeyData, testKeyData2}), + PRSigstoreSignedWithKeyDatas([][]byte{append(testKeyData, 'a'), append(testKeyData2, 'a')}), + PRSigstoreSignedWithSignedIdentity(testIdentity), + }, + { // keyData & keyDatas both set + PRSigstoreSignedWithKeyData([]byte("bar")), + PRSigstoreSignedWithKeyDatas([][]byte{[]byte("foo")}), + PRSigstoreSignedWithSignedIdentity(testIdentity), + }, { // Duplicate fulcio PRSigstoreSignedWithFulcio(testFulcio), PRSigstoreSignedWithFulcio(testFulcio2), @@ -173,7 +239,7 @@ func TestNewPRSigstoreSigned(t *testing.T) { PRSigstoreSignedWithRekorPublicKeyPath(testRekorKeyPath + "1"), PRSigstoreSignedWithSignedIdentity(testIdentity), }, - { // Duplicate keyData + { // Duplicate rekorKeyData PRSigstoreSignedWithKeyPath(testKeyPath), PRSigstoreSignedWithRekorPublicKeyData(testRekorKeyData), PRSigstoreSignedWithRekorPublicKeyData([]byte("def")), @@ -182,7 +248,7 @@ func TestNewPRSigstoreSigned(t *testing.T) { { // Missing signedIdentity PRSigstoreSignedWithKeyPath(testKeyPath), }, - { // Duplicate signedIdentity} + { // Duplicate signedIdentity PRSigstoreSignedWithKeyPath(testKeyPath), PRSigstoreSignedWithSignedIdentity(testIdentity), PRSigstoreSignedWithSignedIdentity(newPRMMatchRepository()), @@ -248,10 +314,14 @@ func TestPRSigstoreSignedUnmarshalJSON(t *testing.T) { func(v mSA) { v["type"] = "this is invalid" }, // Extra top-level sub-object func(v mSA) { v["unexpected"] = 1 }, - // All of "keyPath" and "keyData", and "fulcio" is missing + // All of "keyPath", "keyPaths", "keyData", "keyDatas", and "fulcio" is missing func(v mSA) { delete(v, "keyData") }, // Both "keyPath" and "keyData" is present func(v mSA) { v["keyPath"] = "/foo/bar" }, + // Both "keyPaths" and "keyData" is present + func(v mSA) { v["keyPaths"] = []string{"/foo/bar", "/foo/baz"} }, + // Both "keyData" and "keyDatas" is present + func(v mSA) { v["keyDatas"] = [][]byte{[]byte("abc"), []byte("def")} }, // Both "keyData" and "fulcio" is present func(v mSA) { v["fulcio"] = mSA{ @@ -262,14 +332,22 @@ func TestPRSigstoreSignedUnmarshalJSON(t *testing.T) { }, // Invalid "keyPath" field func(v mSA) { delete(v, "keyData"); v["keyPath"] = 1 }, + // Invalid "keyPaths" field + func(v mSA) { delete(v, "keyData"); v["keyPaths"] = 1 }, + func(v mSA) { delete(v, "keyData"); v["keyPaths"] = mSA{} }, + func(v mSA) { delete(v, "keyData"); v["keyPaths"] = []string{} }, // Invalid "keyData" field func(v mSA) { v["keyData"] = 1 }, func(v mSA) { v["keyData"] = "this is invalid base64" }, + // Invalid "keyDatas" field + func(v mSA) { delete(v, "keyData"); v["keyDatas"] = 1 }, + func(v mSA) { delete(v, "keyData"); v["keyDatas"] = mSA{} }, + func(v mSA) { delete(v, "keyData"); v["keyDatas"] = [][]byte{} }, // Invalid "fulcio" field - func(v mSA) { v["fulcio"] = 1 }, - func(v mSA) { v["fulcio"] = mSA{} }, + func(v mSA) { delete(v, "keyData"); v["fulcio"] = 1 }, + func(v mSA) { delete(v, "keyData"); v["fulcio"] = mSA{} }, // "fulcio" is explicit nil - func(v mSA) { v["fulcio"] = nil }, + func(v mSA) { delete(v, "keyData"); v["fulcio"] = nil }, // Both "rekorKeyPath" and "rekorKeyData" is present func(v mSA) { v["rekorPublicKeyPath"] = "/foo/baz" @@ -288,7 +366,7 @@ func TestPRSigstoreSignedUnmarshalJSON(t *testing.T) { duplicateFields: []string{"type", "keyData", "signedIdentity"}, } keyDataTests.run(t) - // Test keyPath-specific duplicate fields + // Test keyPath and keyPath-specific duplicate fields policyJSONUmarshallerTests[PolicyRequirement]{ newDest: func() json.Unmarshaler { return &prSigstoreSigned{} }, newValidObject: func() (PolicyRequirement, error) { @@ -297,6 +375,30 @@ func TestPRSigstoreSignedUnmarshalJSON(t *testing.T) { otherJSONParser: newPolicyRequirementFromJSON, duplicateFields: []string{"type", "keyPath", "signedIdentity"}, }.run(t) + // Test keyPaths and keyPaths-specific duplicate fields + policyJSONUmarshallerTests[PolicyRequirement]{ + newDest: func() json.Unmarshaler { return &prSigstoreSigned{} }, + newValidObject: func() (PolicyRequirement, error) { + return NewPRSigstoreSigned( + PRSigstoreSignedWithKeyPaths([]string{"/foo/bar", "/foo/baz"}), + PRSigstoreSignedWithSignedIdentity(NewPRMMatchRepoDigestOrExact()), + ) + }, + otherJSONParser: newPolicyRequirementFromJSON, + duplicateFields: []string{"type", "keyPaths", "signedIdentity"}, + }.run(t) + // Test keyDatas and keyDatas-specific duplicate fields + policyJSONUmarshallerTests[PolicyRequirement]{ + newDest: func() json.Unmarshaler { return &prSigstoreSigned{} }, + newValidObject: func() (PolicyRequirement, error) { + return NewPRSigstoreSigned( + PRSigstoreSignedWithKeyDatas([][]byte{[]byte("abc"), []byte("def")}), + PRSigstoreSignedWithSignedIdentity(NewPRMMatchRepoDigestOrExact()), + ) + }, + otherJSONParser: newPolicyRequirementFromJSON, + duplicateFields: []string{"type", "keyDatas", "signedIdentity"}, + }.run(t) // Test Fulcio and rekorPublicKeyPath duplicate fields testFulcio, err := NewPRSigstoreSignedFulcio( PRSigstoreSignedFulcioWithCAPath("fixtures/fulcio_v1.crt.pem"), diff --git a/signature/policy_eval_sigstore.go b/signature/policy_eval_sigstore.go index 610a82080..60bcabeb5 100644 --- a/signature/policy_eval_sigstore.go +++ b/signature/policy_eval_sigstore.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "os" + "strings" "github.com/containers/image/v5/internal/multierr" "github.com/containers/image/v5/internal/private" @@ -26,6 +27,7 @@ type configBytesSources struct { path string // …Path: a path to a file containing the data, or "" paths []string // …Paths: paths to files containing the data, or nil data []byte // …Data: a single instance ofhe raw data, or nil + datas [][]byte // …Datas: the raw data, or nil // codespell:ignore datas } // loadBytesFromConfigSources ensures at most one of the sources in src is set, @@ -56,6 +58,10 @@ func loadBytesFromConfigSources(src configBytesSources) ([][]byte, error) { sources++ data = [][]byte{src.data} } + if src.datas != nil { // codespell:ignore datas + sources++ + data = src.datas // codespell:ignore datas + } if sources > 1 { return nil, errors.New(src.inconsistencyErrorMessage) } @@ -93,7 +99,7 @@ func (f *prSigstoreSignedFulcio) prepareTrustRoot() (*fulcioTrustRoot, error) { // sigstoreSignedTrustRoot contains an already parsed version of the prSigstoreSigned policy type sigstoreSignedTrustRoot struct { - publicKey crypto.PublicKey + publicKeys []crypto.PublicKey fulcio *fulcioTrustRoot rekorPublicKey *ecdsa.PublicKey } @@ -102,24 +108,26 @@ func (pr *prSigstoreSigned) prepareTrustRoot() (*sigstoreSignedTrustRoot, error) res := sigstoreSignedTrustRoot{} publicKeyPEMs, err := loadBytesFromConfigSources(configBytesSources{ - inconsistencyErrorMessage: `Internal inconsistency: both "keyPath" and "keyData" specified`, + inconsistencyErrorMessage: `Internal inconsistency: more than one of "keyPath", "keyPaths", "keyData", "keyDatas" specified`, path: pr.KeyPath, + paths: pr.KeyPaths, data: pr.KeyData, + datas: pr.KeyDatas, // codespell:ignore datas }) if err != nil { return nil, err } if publicKeyPEMs != nil { - if len(publicKeyPEMs) != 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 "keyPath" and "keyData"`) + for index, keyData := range publicKeyPEMs { + pk, err := cryptoutils.UnmarshalPEMToPublicKey(keyData) + if err != nil { + return nil, fmt.Errorf("parsing public key %d: %w", index+1, err) + } + res.publicKeys = append(res.publicKeys, pk) } - pk, err := cryptoutils.UnmarshalPEMToPublicKey(publicKeyPEMs[0]) - if err != nil { - return nil, fmt.Errorf("parsing public key: %w", err) + if len(res.publicKeys) == 0 { + return nil, errors.New(`Internal inconsistency: "keyPath", "keyPaths", "keyData" and "keyDatas" produced no public keys`) } - res.publicKey = pk } if pr.Fulcio != nil { @@ -179,34 +187,48 @@ func (pr *prSigstoreSigned) isSignatureAccepted(ctx context.Context, image priva } untrustedPayload := sig.UntrustedPayload() - var publicKey crypto.PublicKey + var publicKeys []crypto.PublicKey switch { - case trustRoot.publicKey != nil && trustRoot.fulcio != nil: // newPRSigstoreSigned rejects such combinations. + case trustRoot.publicKeys != nil && trustRoot.fulcio != nil: // newPRSigstoreSigned rejects such combinations. return sarRejected, errors.New("Internal inconsistency: Both a public key and Fulcio CA specified") - case trustRoot.publicKey == nil && trustRoot.fulcio == nil: // newPRSigstoreSigned rejects such combinations. + case trustRoot.publicKeys == nil && trustRoot.fulcio == nil: // newPRSigstoreSigned rejects such combinations. return sarRejected, errors.New("Internal inconsistency: Neither a public key nor a Fulcio CA specified") - case trustRoot.publicKey != nil: + case trustRoot.publicKeys != nil: if trustRoot.rekorPublicKey != 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) } - // We could use publicKeyPEM directly, but let’s re-marshal to avoid inconsistencies. - // FIXME: We could just generate DER instead of the full PEM text - recreatedPublicKeyPEM, err := cryptoutils.MarshalPublicKeyToPEM(trustRoot.publicKey) - if err != nil { - // Coverage: The key was loaded from a PEM format, so it’s unclear how this could fail. - // (PEM is not essential, MarshalPublicKeyToPEM can only fail if marshaling to ASN1.DER fails.) - return sarRejected, fmt.Errorf("re-marshaling public key to PEM: %w", err) + var rekorFailures []string + for _, candidatePublicKey := range trustRoot.publicKeys { + // We could use publicKeyPEM directly, but let’s re-marshal to avoid inconsistencies. + // FIXME: We could just generate DER instead of the full PEM text + recreatedPublicKeyPEM, err := cryptoutils.MarshalPublicKeyToPEM(candidatePublicKey) + if err != nil { + // Coverage: The key was loaded from a PEM format, so it’s unclear how this could fail. + // (PEM is not essential, MarshalPublicKeyToPEM can only fail if marshaling to ASN1.DER fails.) + 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) + 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 + } + rekorFailures = append(rekorFailures, err.Error()) } - // We don’t care about the Rekor timestamp, just about log presence. - if _, err := internal.VerifyRekorSET(trustRoot.rekorPublicKey, []byte(untrustedSET), recreatedPublicKeyPEM, untrustedBase64Signature, untrustedPayload); err != nil { - return sarRejected, err + if len(publicKeys) == 0 { + if len(rekorFailures) == 0 { + // Coverage: We have ensured that len(trustRoot.publicKeys) != 0, when nothing succeeds, there must be at least one failure. + return sarRejected, errors.New(`Internal inconsistency: Rekor SET did not match any key but we have no failures.`) + } + return sarRejected, internal.NewInvalidSignatureError(fmt.Sprintf("No public key verified against the RekorSET: %s", strings.Join(rekorFailures, ", "))) } + } else { + publicKeys = trustRoot.publicKeys } - publicKey = trustRoot.publicKey case trustRoot.fulcio != nil: if trustRoot.rekorPublicKey == nil { // newPRSigstoreSigned rejects such combinations. @@ -229,14 +251,15 @@ func (pr *prSigstoreSigned) isSignatureAccepted(ctx context.Context, image priva if err != nil { return sarRejected, err } - publicKey = pk + publicKeys = []crypto.PublicKey{pk} } - if publicKey == nil { - // Coverage: This should never happen, we have already excluded the possibility in the switch above. + if len(publicKeys) == 0 { + // Coverage: This should never happen, we ensured that trustRoot.publicKeys is non-empty if set, + // and we have already excluded the possibility in the switch above. return sarRejected, fmt.Errorf("Internal inconsistency: publicKey not set before verifying sigstore payload") } - signature, err := internal.VerifySigstorePayload(publicKey, untrustedPayload, untrustedBase64Signature, internal.SigstorePayloadAcceptanceRules{ + signature, err := internal.VerifySigstorePayload(publicKeys, untrustedPayload, untrustedBase64Signature, internal.SigstorePayloadAcceptanceRules{ ValidateSignedDockerReference: func(ref string) error { if !pr.SignedIdentity.matchesDockerReference(image, ref) { return PolicyRequirementError(fmt.Sprintf("Signature for identity %q is not accepted", ref)) diff --git a/signature/policy_eval_sigstore_test.go b/signature/policy_eval_sigstore_test.go index 8f606c8d0..d8b7d0129 100644 --- a/signature/policy_eval_sigstore_test.go +++ b/signature/policy_eval_sigstore_test.go @@ -89,8 +89,11 @@ func TestPRSigstoreSignedFulcioPrepareTrustRoot(t *testing.T) { func TestPRSigstoreSignedPrepareTrustRoot(t *testing.T) { const testKeyPath = "fixtures/cosign.pub" + const testKeyPath2 = "fixtures/cosign2.pub" testKeyData, err := os.ReadFile(testKeyPath) require.NoError(t, err) + testKeyData2, err := os.ReadFile(testKeyPath2) + require.NoError(t, err) testFulcio, err := NewPRSigstoreSignedFulcio( PRSigstoreSignedFulcioWithCAPath("fixtures/fulcio_v1.crt.pem"), PRSigstoreSignedFulcioWithOIDCIssuer("https://github.com/login/oauth"), @@ -104,21 +107,21 @@ func TestPRSigstoreSignedPrepareTrustRoot(t *testing.T) { testIdentityOption := PRSigstoreSignedWithSignedIdentity(testIdentity) // Success with public key - for _, c := range [][]PRSigstoreSignedOption{ - { - PRSigstoreSignedWithKeyPath(testKeyPath), - testIdentityOption, - }, - { - PRSigstoreSignedWithKeyData(testKeyData), - testIdentityOption, - }, + for _, c := range []struct { + option PRSigstoreSignedOption + numKeys int + }{ + {PRSigstoreSignedWithKeyPath(testKeyPath), 1}, + {PRSigstoreSignedWithKeyPaths([]string{testKeyPath, testKeyPath2}), 2}, + {PRSigstoreSignedWithKeyData(testKeyData), 1}, + {PRSigstoreSignedWithKeyDatas([][]byte{testKeyData, testKeyData2}), 2}, } { - pr, err := newPRSigstoreSigned(c...) + pr, err := newPRSigstoreSigned(c.option, testIdentityOption) require.NoError(t, err) res, err := pr.prepareTrustRoot() require.NoError(t, err) - assert.NotNil(t, res.publicKey) + assert.NotNil(t, res.publicKeys) + assert.Len(t, res.publicKeys, c.numKeys) assert.Nil(t, res.fulcio) assert.Nil(t, res.rekorPublicKey) } @@ -131,7 +134,7 @@ func TestPRSigstoreSignedPrepareTrustRoot(t *testing.T) { require.NoError(t, err) res, err := pr.prepareTrustRoot() require.NoError(t, err) - assert.Nil(t, res.publicKey) + assert.Nil(t, res.publicKeys) assert.NotNil(t, res.fulcio) assert.NotNil(t, res.rekorPublicKey) // Success with Rekor public key @@ -141,17 +144,27 @@ func TestPRSigstoreSignedPrepareTrustRoot(t *testing.T) { PRSigstoreSignedWithRekorPublicKeyPath(testRekorPublicKeyPath), testIdentityOption, }, + { + PRSigstoreSignedWithKeyPaths([]string{testKeyPath, testKeyPath2}), + PRSigstoreSignedWithRekorPublicKeyPath(testRekorPublicKeyPath), + testIdentityOption, + }, { PRSigstoreSignedWithKeyData(testKeyData), PRSigstoreSignedWithRekorPublicKeyData(testRekorPublicKeyData), testIdentityOption, }, + { + PRSigstoreSignedWithKeyDatas([][]byte{testKeyData, testKeyData2}), + PRSigstoreSignedWithRekorPublicKeyData(testRekorPublicKeyData), + testIdentityOption, + }, } { pr, err := newPRSigstoreSigned(c...) require.NoError(t, err) res, err := pr.prepareTrustRoot() require.NoError(t, err) - assert.NotNil(t, res.publicKey) + assert.NotNil(t, res.publicKeys) assert.Nil(t, res.fulcio) assert.NotNil(t, res.rekorPublicKey) } @@ -171,10 +184,52 @@ func TestPRSigstoreSignedPrepareTrustRoot(t *testing.T) { KeyPath: "fixtures/this/does/not/exist", SignedIdentity: testIdentity, }, + { // Both KeyPath and KeyPaths specified + KeyPath: testKeyPath, + KeyPaths: []string{testKeyPath, testKeyPath2}, + SignedIdentity: testIdentity, + }, + { // Empty KeyPaths + KeyPaths: []string{}, + SignedIdentity: testIdentity, + }, + { // Invalid KeyPaths element + KeyPaths: []string{"fixtures/image.signature", testKeyPath2}, + SignedIdentity: testIdentity, + }, + { + KeyPaths: []string{testKeyPath2, "fixtures/image.signature"}, + SignedIdentity: testIdentity, + }, + { // Unusable KeyPaths element + KeyPaths: []string{"fixtures/this/does/not/exist", testKeyPath2}, + SignedIdentity: testIdentity, + }, + { + KeyPaths: []string{testKeyPath2, "fixtures/this/does/not/exist"}, + SignedIdentity: testIdentity, + }, { // Invalid public key data KeyData: []byte("this is invalid"), SignedIdentity: testIdentity, }, + { // Both KeyData and KeyDatas specified + KeyData: testKeyData, + KeyDatas: [][]byte{testKeyData, testKeyData2}, + SignedIdentity: testIdentity, + }, + { // Empty KeyDatas + KeyDatas: [][]byte{}, + SignedIdentity: testIdentity, + }, + { // Invalid KeyDatas element + KeyDatas: [][]byte{[]byte("this is invalid"), testKeyData2}, + SignedIdentity: testIdentity, + }, + { + KeyDatas: [][]byte{testKeyData, []byte("this is invalid")}, + SignedIdentity: testIdentity, + }, { // Invalid Fulcio configuration Fulcio: &prSigstoreSignedFulcio{}, RekorPublicKeyData: testKeyData, @@ -272,6 +327,8 @@ func TestPRrSigstoreSignedIsSignatureAccepted(t *testing.T) { testFulcioRekorImageSig := sigstoreSignatureFromFile(t, "fixtures/dir-img-cosign-fulcio-rekor-valid/signature-1") keyData, err := os.ReadFile("fixtures/cosign.pub") require.NoError(t, err) + keyData2, err := os.ReadFile("fixtures/cosign2.pub") + require.NoError(t, err) // prepareTrustRoot fails pr := &prSigstoreSigned{ @@ -319,16 +376,29 @@ func TestPRrSigstoreSignedIsSignatureAccepted(t *testing.T) { assertRejected(sar, err) // Successful key+Rekor use + for _, keyPaths := range [][]string{ + {"fixtures/cosign2.pub"}, + {"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) + } + + // key+Rekor, missing Rekor SET annotation pr, err = newPRSigstoreSigned( PRSigstoreSignedWithKeyPath("fixtures/cosign2.pub"), PRSigstoreSignedWithRekorPublicKeyPath("fixtures/rekor.pub"), PRSigstoreSignedWithSignedIdentity(prm), ) require.NoError(t, err) - sar, err = pr.isSignatureAccepted(context.Background(), testKeyRekorImage, testKeyRekorImageSig) - assertAccepted(sar, err) - - // key+Rekor, missing Rekor SET annotation sar, err = pr.isSignatureAccepted(context.Background(), nil, sigstoreSignatureWithoutAnnotation(t, testKeyRekorImageSig, signature.SigstoreSETAnnotationKey)) assertRejected(sar, err) @@ -339,15 +409,36 @@ func TestPRrSigstoreSignedIsSignatureAccepted(t *testing.T) { sigstoreSignatureWithModifiedAnnotation(testKeyRekorImageSig, signature.SigstoreSETAnnotationKey, "this is not a valid SET")) assertRejected(sar, err) - // Fulcio: A Rekor SET which we don’t accept (one of many reasons) - pr2, err := newPRSigstoreSigned( - PRSigstoreSignedWithKeyPath("fixtures/cosign2.pub"), - PRSigstoreSignedWithRekorPublicKeyPath("fixtures/cosign.pub"), // not rekor.pub = a key mismatch + // key+Rekor: A Rekor SET which we don’t accept (one of many reasons) + for _, keyPaths := range [][]string{ + {"fixtures/cosign2.pub"}, + {"fixtures/cosign2.pub", "fixtures/cosign.pub"}, + {"fixtures/cosign.pub", "fixtures/cosign2.pub"}, + } { + pr, err := newPRSigstoreSigned( + PRSigstoreSignedWithKeyPaths(keyPaths), + PRSigstoreSignedWithRekorPublicKeyPath("fixtures/cosign.pub"), // not rekor.pub = a key mismatch + PRSigstoreSignedWithSignedIdentity(prm), + ) + require.NoError(t, err) + // Pass a nil pointer to, kind of, test that the return value does not depend on the image. + sar, err = pr.isSignatureAccepted(context.Background(), nil, testKeyRekorImageSig) + assertRejected(sar, err) + } + // key+Rekor: A valid Rekor SET for one accepted key, but a signature with a _different_ accepted key + pr, err = newPRSigstoreSigned( + PRSigstoreSignedWithKeyPaths([]string{"fixtures/cosign.pub", "fixtures/cosign2.pub"}), + PRSigstoreSignedWithRekorPublicKeyPath("fixtures/rekor.pub"), PRSigstoreSignedWithSignedIdentity(prm), ) require.NoError(t, err) // Pass a nil pointer to, kind of, test that the return value does not depend on the image. - sar, err = pr2.isSignatureAccepted(context.Background(), nil, testKeyRekorImageSig) + // testKeyImageSig is signed by cosign.pub; the SET contains cosign2.pub. + // The SET includes the signature contents… so this actually fails on "signature in Rekor SET does not match" + // rather than accepting the SET and later failing to validate payload; we’d need a way to generate the same signature + // using two different private keys. + sar, err = pr.isSignatureAccepted(context.Background(), nil, sigstoreSignatureWithModifiedAnnotation(testKeyImageSig, + signature.SigstoreSETAnnotationKey, testKeyRekorImageSig.UntrustedAnnotations()[signature.SigstoreSETAnnotationKey])) assertRejected(sar, err) // Successful Fulcio certificate use @@ -368,7 +459,7 @@ func TestPRrSigstoreSignedIsSignatureAccepted(t *testing.T) { assertAccepted(sar, err) // Fulcio, no Rekor requirement - pr2 = &prSigstoreSigned{ + pr2 := &prSigstoreSigned{ Fulcio: fulcio, SignedIdentity: prm, } @@ -449,22 +540,23 @@ func TestPRrSigstoreSignedIsSignatureAccepted(t *testing.T) { sar, err = pr2.isSignatureAccepted(context.Background(), nil, testFulcioRekorImageSig) assertRejected(sar, err) - // Successful validation, with KeyData and KeyPath - pr, err = newPRSigstoreSigned( + // Successful validation, with KeyPath/KeyPaths/KeyData/KeyDatas + for _, opt := range []PRSigstoreSignedOption{ PRSigstoreSignedWithKeyPath("fixtures/cosign.pub"), - PRSigstoreSignedWithSignedIdentity(prm), - ) - require.NoError(t, err) - sar, err = pr.isSignatureAccepted(context.Background(), testKeyImage, testKeyImageSig) - assertAccepted(sar, err) - - pr, err = newPRSigstoreSigned( + PRSigstoreSignedWithKeyPaths([]string{"fixtures/cosign.pub", "fixtures/cosign2.pub"}), + PRSigstoreSignedWithKeyPaths([]string{"fixtures/cosign2.pub", "fixtures/cosign.pub"}), PRSigstoreSignedWithKeyData(keyData), - PRSigstoreSignedWithSignedIdentity(prm), - ) - require.NoError(t, err) - sar, err = pr.isSignatureAccepted(context.Background(), testKeyImage, testKeyImageSig) - assertAccepted(sar, err) + PRSigstoreSignedWithKeyDatas([][]byte{keyData, keyData2}), + PRSigstoreSignedWithKeyDatas([][]byte{keyData2, keyData}), + } { + pr, err := newPRSigstoreSigned( + opt, + PRSigstoreSignedWithSignedIdentity(prm), + ) + require.NoError(t, err) + sar, err = pr.isSignatureAccepted(context.Background(), testKeyImage, testKeyImageSig) + assertAccepted(sar, err) + } // A signature which does not verify pr, err = newPRSigstoreSigned( diff --git a/signature/policy_types.go b/signature/policy_types.go index 96e91a0a9..9134cac65 100644 --- a/signature/policy_types.go +++ b/signature/policy_types.go @@ -74,7 +74,7 @@ type prSignedBy struct { // KeyPath is a pathname to a local file containing the trusted key(s). Exactly one of KeyPath, KeyPaths and KeyData must be specified. KeyPath string `json:"keyPath,omitempty"` - // KeyPaths if a set of pathnames to local files containing the trusted key(s). Exactly one of KeyPath, KeyPaths and KeyData must be specified. + // KeyPaths is a set of pathnames to local files containing the trusted key(s). Exactly one of KeyPath, KeyPaths and KeyData must be specified. KeyPaths []string `json:"keyPaths,omitempty"` // KeyData contains the trusted key(s), base64-encoded. Exactly one of KeyPath, KeyPaths and KeyData must be specified. KeyData []byte `json:"keyData,omitempty"` @@ -111,13 +111,16 @@ type prSignedBaseLayer struct { type prSigstoreSigned struct { prCommon - // KeyPath is a pathname to a local file containing the trusted key. Exactly one of KeyPath, KeyData, Fulcio must be specified. + // KeyPath is a pathname to a local file containing the trusted key. Exactly one of KeyPath, KeyPaths, KeyData, KeyDatas and Fulcio must be specified. KeyPath string `json:"keyPath,omitempty"` - // KeyData contains the trusted key, base64-encoded. Exactly one of KeyPath, KeyData, Fulcio must be specified. + // KeyPaths is a set of pathnames to local files containing the trusted key(s). Exactly one of KeyPath, KeyPaths, KeyData, KeyDatas and Fulcio must be specified. + KeyPaths []string `json:"keyPaths,omitempty"` + // KeyData contains the trusted key, base64-encoded. Exactly one of KeyPath, KeyPaths, KeyData, KeyDatas and Fulcio must be specified. KeyData []byte `json:"keyData,omitempty"` - // FIXME: Multiple public keys? + // KeyDatas is a set of trusted keys, base64-encoded. Exactly one of KeyPath, KeyPaths, KeyData, KeyDatas and Fulcio must be specified. + KeyDatas [][]byte `json:"keyDatas,omitempty"` - // Fulcio specifies which Fulcio-generated certificates are accepted. Exactly one of KeyPath, KeyData, Fulcio must be specified. + // Fulcio specifies which Fulcio-generated certificates are accepted. Exactly one of KeyPath, KeyPaths, KeyData, KeyDatas and Fulcio must be specified. // If Fulcio is specified, one of RekorPublicKeyPath or RekorPublicKeyData must be specified as well. Fulcio PRSigstoreSignedFulcio `json:"fulcio,omitempty"` diff --git a/signature/sigstore/generate_test.go b/signature/sigstore/generate_test.go index 7861a9240..5eb2e204f 100644 --- a/signature/sigstore/generate_test.go +++ b/signature/sigstore/generate_test.go @@ -2,6 +2,7 @@ package sigstore import ( "context" + "crypto" "os" "path/filepath" "testing" @@ -44,7 +45,7 @@ func TestGenerateKeyPair(t *testing.T) { publicKey, err := cryptoutils.UnmarshalPEMToPublicKey(keyPair.PublicKey) require.NoError(t, err) - _, err = internal.VerifySigstorePayload(publicKey, sig.UntrustedPayload(), + _, err = internal.VerifySigstorePayload([]crypto.PublicKey{publicKey}, sig.UntrustedPayload(), sig.UntrustedAnnotations()[signature.SigstoreSignatureAnnotationKey], internal.SigstorePayloadAcceptanceRules{ ValidateSignedDockerReference: func(ref string) error { From 2a87b21a851d435047cc20da7d124eb8837edf0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 16 Aug 2024 03:17:50 +0200 Subject: [PATCH 7/9] Support accepting multiple Rekor public keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add rekorPublicKeyPaths and rekorPublicKeyDatas , similar to the primary root of trust public keys. Signed-off-by: Miloslav Trmač --- docs/containers-policy.json.5.md | 6 +- signature/fulcio_cert.go | 4 +- signature/fulcio_cert_test.go | 7 +- signature/internal/rekor_set.go | 11 +- signature/internal/rekor_set_test.go | 41 +++++-- signature/policy_config_sigstore.go | 64 +++++++++- signature/policy_config_sigstore_test.go | 102 +++++++++++++++- signature/policy_eval_sigstore.go | 42 +++---- signature/policy_eval_sigstore_test.go | 144 +++++++++++++++-------- signature/policy_types.go | 16 ++- 10 files changed, 335 insertions(+), 102 deletions(-) diff --git a/docs/containers-policy.json.5.md b/docs/containers-policy.json.5.md index 5b42e21ae..ad3a1f5db 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 4e9986422..31dfdd342 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 3fefbab85..4c9c6465c 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 e79c91cf9..604c21c08 100644 --- a/signature/internal/rekor_set.go +++ b/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") } diff --git a/signature/internal/rekor_set_test.go b/signature/internal/rekor_set_test.go index 9aa306caa..08ba69cb6 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 26e7d355b..965901e18 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 1eec5ac3c..c6ec3d801 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 60bcabeb5..9c553771c 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 d8b7d0129..b5e74c4d1 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 9134cac65..32aa1c0ad 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. From 0b425a43dfb2efcde567f3ac213a6038d4f4ca73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 20 Aug 2024 17:50:02 +0200 Subject: [PATCH 8/9] Release 5.32.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds the ability to accept sigstore signatures signed by any key from a set of several (huge thanks to @dcermak and @danishprakash for doing almost all the work), and Rekor log presence proofs signed by any key from a set of several keys. Signed-off-by: Miloslav Trmač --- version/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version/version.go b/version/version.go index 60cfbb79b..64e468725 100644 --- a/version/version.go +++ b/version/version.go @@ -11,7 +11,7 @@ const ( VersionPatch = 2 // VersionDev indicates development branch. Releases will be empty string. - VersionDev = "-dev" + VersionDev = "" ) // Version is the specification version that the package types support. From 46bde1084d62d91d42ef5042704547c2d4ad0d3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 20 Aug 2024 17:53:07 +0200 Subject: [PATCH 9/9] Bump to 5.32.3-dev MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- version/version.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version/version.go b/version/version.go index 64e468725..c0c6b47e7 100644 --- a/version/version.go +++ b/version/version.go @@ -8,10 +8,10 @@ const ( // VersionMinor is for functionality in a backwards-compatible manner VersionMinor = 32 // VersionPatch is for backwards-compatible bug fixes - VersionPatch = 2 + VersionPatch = 3 // VersionDev indicates development branch. Releases will be empty string. - VersionDev = "" + VersionDev = "-dev" ) // Version is the specification version that the package types support.