From 4b3806fec9567e8676ddad5f955c9984041d6b8a 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 1/5] 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 beb5d0673e..e77b3c4199 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 2157f6be8963aa44650740714e687308afea3ee9 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 2/5] 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 4851650778..99d3a1e01a 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 1bc6565641e4106ddf54565db409a3cadba4f467 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 3/5] 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 896ca5a60d..e5c9329185 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 99d3a1e01a..610a820806 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 ab622de4344afbec9e42a998bee93b416515dafc 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 4/5] 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 5ee1156793..dd34ec4f63 100644 --- a/signature/internal/sigstore_payload.go +++ b/signature/internal/sigstore_payload.go @@ -165,24 +165,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 32d9aab3d645bc156db9711e00f3bbcb2d07b6b5 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 5/5] 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 909d04afd0..5b42e21aed 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 dd34ec4f63..90a81dc1c4 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" @@ -166,35 +167,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 61b011ffe6..d6be57d78a 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 0000000000..ea2aa90773 --- /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 e77b3c4199..26e7d355bb 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 d71ae5f895..1eec5ac3c5 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 610a820806..60bcabeb56 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 8f606c8d0e..d8b7d01299 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 96e91a0a9c..9134cac652 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 7861a92401..5eb2e204f9 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 {