Skip to content

Commit

Permalink
Merge pull request #2526 from mtrmac/sigstore-multi-rekor
Browse files Browse the repository at this point in the history 
Add support for accepting multiple Rekor public keys
  • Loading branch information
@TomSweeneyRedHat
TomSweeneyRedHat authored Aug 20, 2024
2 parents e207baa + 7bdb48b commit 6692ffc
Show file tree
Hide file tree
Showing 10 changed files with 335 additions and 102 deletions.
6 changes: 4 additions & 2 deletions docs/containers-policy.json.5.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,9 @@ This requirement requires an image to be signed using a sigstore signature with
"subjectEmail", "[email protected]",
},
"rekorPublicKeyPath": "/path/to/local/public/key/file",
"rekorPublicKeyPaths": ["/path/to/local/public/key/one","/path/to/local/public/key/two"],
"rekorPublicKeyData": "base64-encoded-public-key-data",
"rekorPublicKeyDatas": ["base64-encoded-public-key-one-data","base64-encoded-public-key-two-data"],
"signedIdentity": identity_requirement
}
```
Expand All @@ -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).
Expand Down
4 changes: 2 additions & 2 deletions signature/fulcio_cert.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions signature/fulcio_cert_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -450,7 +451,7 @@ func TestVerifyRekorFulcio(t *testing.T) {
require.NoError(t, err)

// Success
pk, err := verifyRekorFulcio(rekorKeyECDSA, &fulcioTrustRoot{
pk, err := verifyRekorFulcio(rekorKeysECDSA, &fulcioTrustRoot{
caCertificates: caCertificates,
oidcIssuer: "https://github.com/login/oauth",
subjectEmail: "[email protected]",
Expand All @@ -459,7 +460,7 @@ func TestVerifyRekorFulcio(t *testing.T) {
assertPublicKeyMatchesCert(t, certBytes, pk)

// Rekor failure
pk, err = verifyRekorFulcio(rekorKeyECDSA, &fulcioTrustRoot{
pk, err = verifyRekorFulcio(rekorKeysECDSA, &fulcioTrustRoot{
caCertificates: caCertificates,
oidcIssuer: "https://github.com/login/oauth",
subjectEmail: "[email protected]",
Expand All @@ -468,7 +469,7 @@ func TestVerifyRekorFulcio(t *testing.T) {
assert.Nil(t, pk)

// Fulcio failure
pk, err = verifyRekorFulcio(rekorKeyECDSA, &fulcioTrustRoot{
pk, err = verifyRekorFulcio(rekorKeysECDSA, &fulcioTrustRoot{
caCertificates: caCertificates,
oidcIssuer: "https://github.com/login/oauth",
subjectEmail: "[email protected]",
Expand Down
11 changes: 9 additions & 2 deletions signature/internal/rekor_set.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (p UntrustedRekorPayload) MarshalJSON() ([]byte, error) {

// VerifyRekorSET verifies that unverifiedRekorSET is correctly signed by publicKey and matches the rest of the data.
// Returns bundle upload time on success.
func VerifyRekorSET(publicKey *ecdsa.PublicKey, unverifiedRekorSET []byte, unverifiedKeyOrCertBytes []byte, unverifiedBase64Signature string, unverifiedPayloadBytes []byte) (time.Time, error) {
func VerifyRekorSET(publicKeys []*ecdsa.PublicKey, unverifiedRekorSET []byte, unverifiedKeyOrCertBytes []byte, unverifiedBase64Signature string, unverifiedPayloadBytes []byte) (time.Time, error) {
// FIXME: Should the publicKey parameter hard-code ecdsa?

// == Parse SET bytes
Expand All @@ -127,7 +127,14 @@ func VerifyRekorSET(publicKey *ecdsa.PublicKey, unverifiedRekorSET []byte, unver
return time.Time{}, NewInvalidSignatureError(fmt.Sprintf("canonicalizing Rekor SET JSON: %v", err))
}
untrustedSETPayloadHash := sha256.Sum256(untrustedSETPayloadCanonicalBytes)
if !ecdsa.VerifyASN1(publicKey, untrustedSETPayloadHash[:], untrustedSET.UntrustedSignedEntryTimestamp) {
publicKeymatched := false
for _, pk := range publicKeys {
if ecdsa.VerifyASN1(pk, untrustedSETPayloadHash[:], untrustedSET.UntrustedSignedEntryTimestamp) {
publicKeymatched = true
break
}
}
if !publicKeymatched {
return time.Time{}, NewInvalidSignatureError("cryptographic signature verification of Rekor SET failed")
}

Expand Down
41 changes: 28 additions & 13 deletions signature/internal/rekor_set_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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)

Expand All @@ -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(`{}`)
Expand All @@ -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)

Expand Down Expand Up @@ -379,15 +394,15 @@ func TestVerifyRekorSET(t *testing.T) {
UntrustedPayload: json.RawMessage(testPayload),
})
require.NoError(t, err)
tm, err = VerifyRekorSET(&testKey.PublicKey, testSET, cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
tm, err = VerifyRekorSET(testPublicKeys, testSET, cosignCertBytes, string(cosignSigBase64), cosignPayloadBytes)
assert.Error(t, err)
assert.Zero(t, tm)
}

// Invalid unverifiedBase64Signature parameter
truncatedBase64 := cosignSigBase64
truncatedBase64 = truncatedBase64[:len(truncatedBase64)-1]
tm, err = VerifyRekorSET(cosignRekorKeyECDSA, cosignSETBytes, cosignCertBytes,
tm, err = VerifyRekorSET(cosignRekorKeysECDSA, cosignSETBytes, cosignCertBytes,
string(truncatedBase64), cosignPayloadBytes)
assert.Error(t, err)
assert.Zero(t, tm)
Expand All @@ -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)
Expand Down
64 changes: 59 additions & 5 deletions signature/policy_config_sigstore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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:
Expand Down Expand Up @@ -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...)
Expand Down
Loading

0 comments on commit 6692ffc

Please sign in to comment.