diff --git a/docs/containers-policy.json.5.md b/docs/containers-policy.json.5.md index 703bb8396..8611bd714 100644 --- a/docs/containers-policy.json.5.md +++ b/docs/containers-policy.json.5.md @@ -156,13 +156,14 @@ This requirement requires an image to be signed using “simple signing” with "type": "signedBy", "keyType": "GPGKeys", /* The only currently supported value */ "keyPath": "/path/to/local/keyring/file", + "keyPaths": ["/path/to/local/keyring/file1","/path/to/local/keyring/file2"…], "keyData": "base64-encoded-keyring-data", "signedIdentity": identity_requirement } ``` -Exactly one of `keyPath` and `keyData` must be present, containing a GPG keyring of one or more public keys. Only signatures made by these keys are accepted. +Exactly one of `keyPath`, `keyPaths` and `keyData` must be present, containing a GPG keyring of one or more public keys. Only signatures made by these keys are accepted. The `signedIdentity` field, a JSON object, specifies what image identity the signature claims about the image. One of the following alternatives are supported: diff --git a/signature/fixtures/policy.json b/signature/fixtures/policy.json index 8d488feb0..22d68d8d1 100644 --- a/signature/fixtures/policy.json +++ b/signature/fixtures/policy.json @@ -75,6 +75,13 @@ } } ], + "registry.redhat.io/beta": [ + { + "type": "signedBy", + "keyType": "GPGKeys", + "keyPaths": ["/keys/RH-production-signing-key-gpg-keyring", "/keys/RH-beta-signing-key-gpg-keyring"] + } + ], "private-mirror:5000/vendor-mirror": [ { "type": "signedBy", diff --git a/signature/fixtures/public-key-1.gpg b/signature/fixtures/public-key-1.gpg new file mode 100644 index 000000000..c97f2e0ad Binary files /dev/null and b/signature/fixtures/public-key-1.gpg differ diff --git a/signature/fixtures/public-key-2.gpg b/signature/fixtures/public-key-2.gpg new file mode 100644 index 000000000..c15d06b4a Binary files /dev/null and b/signature/fixtures/public-key-2.gpg differ diff --git a/signature/mechanism.go b/signature/mechanism.go index 249b5a1fe..1d3fe0fdc 100644 --- a/signature/mechanism.go +++ b/signature/mechanism.go @@ -65,7 +65,7 @@ func NewGPGSigningMechanism() (SigningMechanism, error) { // of these keys. // The caller must call .Close() on the returned SigningMechanism. func NewEphemeralGPGSigningMechanism(blob []byte) (SigningMechanism, []string, error) { - return newEphemeralGPGSigningMechanism(blob) + return newEphemeralGPGSigningMechanism([][]byte{blob}) } // gpgUntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION, diff --git a/signature/mechanism_gpgme.go b/signature/mechanism_gpgme.go index 54060af34..2b2a7ad86 100644 --- a/signature/mechanism_gpgme.go +++ b/signature/mechanism_gpgme.go @@ -33,10 +33,10 @@ func newGPGSigningMechanismInDirectory(optionalDir string) (signingMechanismWith } // newEphemeralGPGSigningMechanism returns a new GPG/OpenPGP signing mechanism which -// recognizes _only_ public keys from the supplied blob, and returns the identities +// recognizes _only_ public keys from the supplied blobs, and returns the identities // of these keys. // The caller must call .Close() on the returned SigningMechanism. -func newEphemeralGPGSigningMechanism(blob []byte) (signingMechanismWithPassphrase, []string, error) { +func newEphemeralGPGSigningMechanism(blobs [][]byte) (signingMechanismWithPassphrase, []string, error) { dir, err := os.MkdirTemp("", "containers-ephemeral-gpg-") if err != nil { return nil, nil, err @@ -55,9 +55,13 @@ func newEphemeralGPGSigningMechanism(blob []byte) (signingMechanismWithPassphras ctx: ctx, ephemeralDir: dir, } - keyIdentities, err := mech.importKeysFromBytes(blob) - if err != nil { - return nil, nil, err + keyIdentities := []string{} + for _, blob := range blobs { + ki, err := mech.importKeysFromBytes(blob) + if err != nil { + return nil, nil, err + } + keyIdentities = append(keyIdentities, ki...) } removeDir = false diff --git a/signature/mechanism_openpgp.go b/signature/mechanism_openpgp.go index 8798ee2cd..5d6c1ac83 100644 --- a/signature/mechanism_openpgp.go +++ b/signature/mechanism_openpgp.go @@ -63,14 +63,19 @@ func newGPGSigningMechanismInDirectory(optionalDir string) (signingMechanismWith // recognizes _only_ public keys from the supplied blob, and returns the identities // of these keys. // The caller must call .Close() on the returned SigningMechanism. -func newEphemeralGPGSigningMechanism(blob []byte) (signingMechanismWithPassphrase, []string, error) { +func newEphemeralGPGSigningMechanism(blobs [][]byte) (signingMechanismWithPassphrase, []string, error) { m := &openpgpSigningMechanism{ keyring: openpgp.EntityList{}, } - keyIdentities, err := m.importKeysFromBytes(blob) - if err != nil { - return nil, nil, err + keyIdentities := []string{} + for _, blob := range blobs { + ki, err := m.importKeysFromBytes(blob) + if err != nil { + return nil, nil, err + } + keyIdentities = append(keyIdentities, ki...) } + return m, keyIdentities, nil } diff --git a/signature/mechanism_test.go b/signature/mechanism_test.go index 6c2b3393d..340d8c7e5 100644 --- a/signature/mechanism_test.go +++ b/signature/mechanism_test.go @@ -135,7 +135,7 @@ func TestNewEphemeralGPGSigningMechanism(t *testing.T) { assert.Equal(t, TestKeyFingerprint, signingFingerprint, version) } - // Two keys: Read the binary-format pubring.gpg, and concatenate it twice. + // Two keys in a keyring: Read the binary-format pubring.gpg, and concatenate it twice. // (Using two copies of public-key.gpg, in the ASCII-armored format, works with // gpgmeSigningMechanism but not openpgpSigningMechanism.) keyBlob, err = os.ReadFile("./fixtures/pubring.gpg") @@ -145,6 +145,16 @@ func TestNewEphemeralGPGSigningMechanism(t *testing.T) { defer mech.Close() assert.Equal(t, []string{TestKeyFingerprint, TestKeyFingerprintWithPassphrase, TestKeyFingerprint, TestKeyFingerprintWithPassphrase}, keyIdentities) + // Two keys from two blobs: + keyBlob1, err := os.ReadFile("./fixtures/public-key-1.gpg") + require.NoError(t, err) + keyBlob2, err := os.ReadFile("./fixtures/public-key-2.gpg") + require.NoError(t, err) + mech, keyIdentities, err = newEphemeralGPGSigningMechanism([][]byte{keyBlob1, keyBlob2}) + require.NoError(t, err) + defer mech.Close() + assert.Equal(t, []string{TestKeyFingerprint, TestKeyFingerprintWithPassphrase}, keyIdentities) + // Invalid input: This is, sadly, accepted anyway by GPG, just returns no keys. // For openpgpSigningMechanism we can detect this and fail. mech, keyIdentities, err = NewEphemeralGPGSigningMechanism([]byte("This is invalid")) diff --git a/signature/policy_config.go b/signature/policy_config.go index d3c07d03b..8c6f603cb 100644 --- a/signature/policy_config.go +++ b/signature/policy_config.go @@ -316,12 +316,22 @@ func (pr *prReject) UnmarshalJSON(data []byte) error { } // newPRSignedBy returns a new prSignedBy if parameters are valid. -func newPRSignedBy(keyType sbKeyType, keyPath string, keyData []byte, signedIdentity PolicyReferenceMatch) (*prSignedBy, error) { +func newPRSignedBy(keyType sbKeyType, keyPath string, keyPaths []string, keyData []byte, signedIdentity PolicyReferenceMatch) (*prSignedBy, error) { if !keyType.IsValid() { return nil, InvalidPolicyFormatError(fmt.Sprintf("invalid keyType \"%s\"", keyType)) } - if len(keyPath) > 0 && len(keyData) > 0 { - return nil, InvalidPolicyFormatError("keyType and keyData cannot be used simultaneously") + keySources := 0 + if keyPath != "" { + keySources++ + } + if keyPaths != nil { + keySources++ + } + if keyData != nil { + keySources++ + } + if keySources != 1 { + return nil, InvalidPolicyFormatError("exactly one of keyPath, keyPaths and keyData must be specified") } if signedIdentity == nil { return nil, InvalidPolicyFormatError("signedIdentity not specified") @@ -330,6 +340,7 @@ func newPRSignedBy(keyType sbKeyType, keyPath string, keyData []byte, signedIden prCommon: prCommon{Type: prTypeSignedBy}, KeyType: keyType, KeyPath: keyPath, + KeyPaths: keyPaths, KeyData: keyData, SignedIdentity: signedIdentity, }, nil @@ -337,7 +348,7 @@ func newPRSignedBy(keyType sbKeyType, keyPath string, keyData []byte, signedIden // newPRSignedByKeyPath is NewPRSignedByKeyPath, except it returns the private type. func newPRSignedByKeyPath(keyType sbKeyType, keyPath string, signedIdentity PolicyReferenceMatch) (*prSignedBy, error) { - return newPRSignedBy(keyType, keyPath, nil, signedIdentity) + return newPRSignedBy(keyType, keyPath, nil, nil, signedIdentity) } // NewPRSignedByKeyPath returns a new "signedBy" PolicyRequirement using a KeyPath @@ -345,9 +356,19 @@ func NewPRSignedByKeyPath(keyType sbKeyType, keyPath string, signedIdentity Poli return newPRSignedByKeyPath(keyType, keyPath, signedIdentity) } +// newPRSignedByKeyPaths is NewPRSignedByKeyPaths, except it returns the private type. +func newPRSignedByKeyPaths(keyType sbKeyType, keyPaths []string, signedIdentity PolicyReferenceMatch) (*prSignedBy, error) { + return newPRSignedBy(keyType, "", keyPaths, nil, signedIdentity) +} + +// NewPRSignedByKeyPaths returns a new "signedBy" PolicyRequirement using KeyPaths +func NewPRSignedByKeyPaths(keyType sbKeyType, keyPaths []string, signedIdentity PolicyReferenceMatch) (PolicyRequirement, error) { + return newPRSignedByKeyPaths(keyType, keyPaths, signedIdentity) +} + // newPRSignedByKeyData is NewPRSignedByKeyData, except it returns the private type. func newPRSignedByKeyData(keyType sbKeyType, keyData []byte, signedIdentity PolicyReferenceMatch) (*prSignedBy, error) { - return newPRSignedBy(keyType, "", keyData, signedIdentity) + return newPRSignedBy(keyType, "", nil, keyData, signedIdentity) } // NewPRSignedByKeyData returns a new "signedBy" PolicyRequirement using a KeyData @@ -362,7 +383,7 @@ var _ json.Unmarshaler = (*prSignedBy)(nil) func (pr *prSignedBy) UnmarshalJSON(data []byte) error { *pr = prSignedBy{} var tmp prSignedBy - var gotKeyPath, gotKeyData = false, false + var gotKeyPath, gotKeyPaths, gotKeyData = false, false, false var signedIdentity json.RawMessage if err := internal.ParanoidUnmarshalJSONObject(data, func(key string) interface{} { switch key { @@ -373,6 +394,9 @@ func (pr *prSignedBy) 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 @@ -401,16 +425,16 @@ func (pr *prSignedBy) UnmarshalJSON(data []byte) error { var res *prSignedBy var err error switch { - case gotKeyPath && gotKeyData: - return InvalidPolicyFormatError("keyPath and keyData cannot be used simultaneously") - case gotKeyPath && !gotKeyData: + case gotKeyPath && !gotKeyPaths && !gotKeyData: res, err = newPRSignedByKeyPath(tmp.KeyType, tmp.KeyPath, tmp.SignedIdentity) - case !gotKeyPath && gotKeyData: + case !gotKeyPath && gotKeyPaths && !gotKeyData: + res, err = newPRSignedByKeyPaths(tmp.KeyType, tmp.KeyPaths, tmp.SignedIdentity) + case !gotKeyPath && !gotKeyPaths && gotKeyData: res, err = newPRSignedByKeyData(tmp.KeyType, tmp.KeyData, tmp.SignedIdentity) - case !gotKeyPath && !gotKeyData: - return InvalidPolicyFormatError("At least one of keyPath and keyData mus be specified") - default: // Coverage: This should never happen - return fmt.Errorf("Impossible keyPath/keyData presence combination!?") + case !gotKeyPath && !gotKeyPaths && !gotKeyData: + return InvalidPolicyFormatError("Exactly one of keyPath, keyPaths and keyData must be specified, none of them present") + default: + return fmt.Errorf("Exactly one of keyPath, keyPaths and keyData must be specified, more than one present") } if err != nil { return err @@ -582,7 +606,7 @@ func (pr *prSigstoreSigned) UnmarshalJSON(data []byte) error { case !gotKeyPath && gotKeyData: res, err = newPRSigstoreSignedKeyData(tmp.KeyData, tmp.SignedIdentity) case !gotKeyPath && !gotKeyData: - return InvalidPolicyFormatError("At least one of keyPath and keyData mus be specified") + return InvalidPolicyFormatError("At least one of keyPath and keyData must be specified") default: // Coverage: This should never happen return fmt.Errorf("Impossible keyPath/keyData presence combination!?") } diff --git a/signature/policy_config_test.go b/signature/policy_config_test.go index 5572869bb..a8e9a76a1 100644 --- a/signature/policy_config_test.go +++ b/signature/policy_config_test.go @@ -3,6 +3,7 @@ package signature import ( "bytes" "encoding/json" + "fmt" "os" "path/filepath" "testing" @@ -70,6 +71,11 @@ var policyFixtureContents = &Policy{ "/keys/RH-key-signing-key-gpg-keyring", NewPRMMatchRepoDigestOrExact()), }, + "registry.redhat.io/beta": { + xNewPRSignedByKeyPaths(SBKeyTypeGPGKeys, + []string{"/keys/RH-production-signing-key-gpg-keyring", "/keys/RH-beta-signing-key-gpg-keyring"}, + newPRMMatchRepoDigestOrExact()), + }, "private-mirror:5000/vendor-mirror": { xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "/keys/vendor-gpg-keyring", @@ -342,15 +348,17 @@ func (d policyJSONUmarshallerTests) run(t *testing.T) { assertJSONUnmarshalFromObjectFails(t, invalid, dest) } // Various ways to corrupt the JSON - for _, fn := range d.breakFns { - var tmp mSI - err := json.Unmarshal(validJSON, &tmp) - require.NoError(t, err) + for index, fn := range d.breakFns { + t.Run(fmt.Sprintf("breakFns[%d]", index), func(t *testing.T) { + var tmp mSI + err := json.Unmarshal(validJSON, &tmp) + require.NoError(t, err) - fn(tmp) + fn(tmp) - dest := d.newDest() - assertJSONUnmarshalFromObjectFails(t, tmp, dest) + dest := d.newDest() + assertJSONUnmarshalFromObjectFails(t, tmp, dest) + }) } // Duplicated fields @@ -376,6 +384,15 @@ func xNewPRSignedByKeyPath(keyType sbKeyType, keyPath string, signedIdentity Pol return pr } +// xNewPRSignedByKeyPaths is like NewPRSignedByKeyPaths, except it must not fail. +func xNewPRSignedByKeyPaths(keyType sbKeyType, keyPaths []string, signedIdentity PolicyReferenceMatch) PolicyRequirement { + pr, err := NewPRSignedByKeyPaths(keyType, keyPaths, signedIdentity) + if err != nil { + panic("xNewPRSignedByKeyPaths failed") + } + return pr +} + // xNewPRSignedByKeyData is like NewPRSignedByKeyData, except it must not fail. func xNewPRSignedByKeyData(keyType sbKeyType, keyData []byte, signedIdentity PolicyReferenceMatch) PolicyRequirement { pr, err := NewPRSignedByKeyData(keyType, keyData, signedIdentity) @@ -705,41 +722,62 @@ func TestPRRejectUnmarshalJSON(t *testing.T) { func TestNewPRSignedBy(t *testing.T) { const testPath = "/foo/bar" + testPaths := []string{"/path/1", "/path/2"} testData := []byte("abc") testIdentity := NewPRMMatchRepoDigestOrExact() // Success - pr, err := newPRSignedBy(SBKeyTypeGPGKeys, testPath, nil, testIdentity) + pr, err := newPRSignedBy(SBKeyTypeGPGKeys, testPath, nil, nil, testIdentity) require.NoError(t, err) assert.Equal(t, &prSignedBy{ prCommon: prCommon{prTypeSignedBy}, KeyType: SBKeyTypeGPGKeys, KeyPath: testPath, + KeyPaths: nil, + KeyData: nil, + SignedIdentity: testIdentity, + }, pr) + pr, err = newPRSignedBy(SBKeyTypeGPGKeys, "", testPaths, nil, testIdentity) + require.NoError(t, err) + assert.Equal(t, &prSignedBy{ + prCommon: prCommon{prTypeSignedBy}, + KeyType: SBKeyTypeGPGKeys, + KeyPath: "", + KeyPaths: testPaths, KeyData: nil, SignedIdentity: testIdentity, }, pr) - pr, err = newPRSignedBy(SBKeyTypeGPGKeys, "", testData, testIdentity) + pr, err = newPRSignedBy(SBKeyTypeGPGKeys, "", nil, testData, testIdentity) require.NoError(t, err) assert.Equal(t, &prSignedBy{ prCommon: prCommon{prTypeSignedBy}, KeyType: SBKeyTypeGPGKeys, KeyPath: "", + KeyPaths: nil, KeyData: testData, SignedIdentity: testIdentity, }, pr) // Invalid keyType - _, err = newPRSignedBy(sbKeyType(""), testPath, nil, testIdentity) + _, err = newPRSignedBy(sbKeyType(""), testPath, nil, nil, testIdentity) assert.Error(t, err) - _, err = newPRSignedBy(sbKeyType("this is invalid"), testPath, nil, testIdentity) + _, err = newPRSignedBy(sbKeyType("this is invalid"), testPath, nil, nil, testIdentity) assert.Error(t, err) - // Both keyPath and keyData specified - _, err = newPRSignedBy(SBKeyTypeGPGKeys, testPath, testData, testIdentity) + // Invalid keyPath/keyPaths/keyData combinations + _, err = newPRSignedBy(SBKeyTypeGPGKeys, testPath, testPaths, testData, testIdentity) + assert.Error(t, err) + _, err = newPRSignedBy(SBKeyTypeGPGKeys, testPath, testPaths, nil, testIdentity) + assert.Error(t, err) + _, err = newPRSignedBy(SBKeyTypeGPGKeys, testPath, nil, testData, testIdentity) + assert.Error(t, err) + _, err = newPRSignedBy(SBKeyTypeGPGKeys, "", testPaths, testData, testIdentity) + assert.Error(t, err) + _, err = newPRSignedBy(SBKeyTypeGPGKeys, "", nil, nil, testIdentity) assert.Error(t, err) // Invalid signedIdentity - _, err = newPRSignedBy(SBKeyTypeGPGKeys, testPath, nil, nil) + _, err = newPRSignedBy(SBKeyTypeGPGKeys, testPath, nil, nil, nil) assert.Error(t, err) } @@ -753,6 +791,16 @@ func TestNewPRSignedByKeyPath(t *testing.T) { // Failure cases tested in TestNewPRSignedBy. } +func TestNewPRSignedByKeyPaths(t *testing.T) { + testPaths := []string{"/path/1", "/path/2"} + _pr, err := NewPRSignedByKeyPaths(SBKeyTypeGPGKeys, testPaths, NewPRMMatchRepoDigestOrExact()) + require.NoError(t, err) + pr, ok := _pr.(*prSignedBy) + require.True(t, ok) + assert.Equal(t, testPaths, pr.KeyPaths) + // Failure cases tested in TestNewPRSignedBy. +} + func TestNewPRSignedByKeyData(t *testing.T) { testData := []byte("abc") _pr, err := NewPRSignedByKeyData(SBKeyTypeGPGKeys, testData, NewPRMMatchRepoDigestOrExact()) @@ -796,13 +844,19 @@ func TestPRSignedByUnmarshalJSON(t *testing.T) { func(v mSI) { delete(v, "keyType") }, // Invalid "keyType" field func(v mSI) { v["keyType"] = "this is invalid" }, - // Both "keyPath" and "keyData" is missing + // All three of "keyPath", "keyPaths" and "keyData" are missing func(v mSI) { delete(v, "keyData") }, - // Both "keyPath" and "keyData" is present + // All three of "keyPath", "keyPaths" and "keyData" are present + func(v mSI) { v["keyPath"] = "/foo/bar"; v["keyPaths"] = []string{"/1", "/2"} }, + // Two of "keyPath", "keyPaths" and "keyData" are present + func(v mSI) { v["keyPath"] = "/foo/bar"; v["keyPaths"] = []string{"/1", "/2"}; delete(v, "keyData") }, func(v mSI) { v["keyPath"] = "/foo/bar" }, + func(v mSI) { v["keyPaths"] = []string{"/1", "/2"} }, // Invalid "keyPath" field func(v mSI) { delete(v, "keyData"); v["keyPath"] = 1 }, - func(v mSI) { v["type"] = "this is invalid" }, + // Invalid "keyPaths" field + func(v mSI) { delete(v, "keyData"); v["keyPaths"] = 1 }, + func(v mSI) { delete(v, "keyData"); v["keyPaths"] = []int{1} }, // Invalid "keyData" field func(v mSI) { v["keyData"] = 1 }, func(v mSI) { v["keyData"] = "this is invalid base64" }, @@ -825,6 +879,17 @@ func TestPRSignedByUnmarshalJSON(t *testing.T) { }, duplicateFields: []string{"type", "keyType", "keyPath", "signedIdentity"}, }.run(t) + // Test the keyPaths-specific aspects + policyJSONUmarshallerTests{ + newDest: func() json.Unmarshaler { return &prSignedBy{} }, + newValidObject: func() (interface{}, error) { + return NewPRSignedByKeyPaths(SBKeyTypeGPGKeys, []string{"/1", "/2"}, NewPRMMatchRepoDigestOrExact()) + }, + otherJSONParser: func(validJSON []byte) (interface{}, error) { + return newPolicyRequirementFromJSON(validJSON) + }, + duplicateFields: []string{"type", "keyType", "keyPaths", "signedIdentity"}, + }.run(t) var pr prSignedBy diff --git a/signature/policy_eval_signedby.go b/signature/policy_eval_signedby.go index b04e57382..ef98b8b83 100644 --- a/signature/policy_eval_signedby.go +++ b/signature/policy_eval_signedby.go @@ -25,23 +25,38 @@ func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image priva return sarRejected, nil, fmt.Errorf(`Unknown "keyType" value "%s"`, string(pr.KeyType)) } - if pr.KeyPath != "" && pr.KeyData != nil { - return sarRejected, nil, errors.New(`Internal inconsistency: both "keyPath" and "keyData" specified`) - } // FIXME: move this to per-context initialization - var data []byte - if pr.KeyData != nil { - data = pr.KeyData - } else { + var data [][]byte + keySources := 0 + if pr.KeyPath != "" { + keySources++ d, err := os.ReadFile(pr.KeyPath) if err != nil { return sarRejected, nil, err } - data = d + 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} + } + if keySources != 1 { + return sarRejected, nil, errors.New(`Internal inconsistency: not exactly one of "keyPath", "keyPaths" and "keyData" specified`) } // FIXME: move this to per-context initialization - mech, trustedIdentities, err := NewEphemeralGPGSigningMechanism(data) + mech, trustedIdentities, err := newEphemeralGPGSigningMechanism(data) if err != nil { return sarRejected, nil, err } diff --git a/signature/policy_eval_signedby_test.go b/signature/policy_eval_signedby_test.go index e234617b7..0e9bb83f7 100644 --- a/signature/policy_eval_signedby_test.go +++ b/signature/policy_eval_signedby_test.go @@ -55,25 +55,33 @@ func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) { testImage := dirImageMock(t, "fixtures/dir-img-valid", "testing/manifest:latest") testImageSig, err := os.ReadFile("fixtures/dir-img-valid/signature-1") require.NoError(t, err) - - // Successful validation, with KeyData and KeyPath - pr, err := NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) - require.NoError(t, err) - sar, parsedSig, err := pr.isSignatureAuthorAccepted(context.Background(), testImage, testImageSig) - assertSARAccepted(t, sar, parsedSig, err, Signature{ - DockerManifestDigest: TestImageManifestDigest, - DockerReference: "testing/manifest:latest", - }) - keyData, err := os.ReadFile("fixtures/public-key.gpg") require.NoError(t, err) - pr, err = NewPRSignedByKeyData(ktGPG, keyData, prm) - require.NoError(t, err) - sar, parsedSig, err = pr.isSignatureAuthorAccepted(context.Background(), testImage, testImageSig) - assertSARAccepted(t, sar, parsedSig, err, Signature{ - DockerManifestDigest: TestImageManifestDigest, - DockerReference: "testing/manifest:latest", - }) + + // Successful validation, with KeyPath, KeyPaths and KeyData. + for _, fn := range []func() (PolicyRequirement, error){ + func() (PolicyRequirement, error) { + return NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm) + }, + // Test the files in both orders, to make sure the correct public keys accepted in either position. + func() (PolicyRequirement, error) { + return NewPRSignedByKeyPaths(ktGPG, []string{"fixtures/public-key-1.gpg", "fixtures/public-key-1.gpg"}, prm) + }, + func() (PolicyRequirement, error) { + return NewPRSignedByKeyPaths(ktGPG, []string{"fixtures/public-key-2.gpg", "fixtures/public-key-1.gpg"}, prm) + }, + func() (PolicyRequirement, error) { + return NewPRSignedByKeyData(ktGPG, keyData, prm) + }, + } { + pr, err := fn() + require.NoError(t, err) + sar, parsedSig, err := pr.isSignatureAuthorAccepted(context.Background(), testImage, testImageSig) + assertSARAccepted(t, sar, parsedSig, err, Signature{ + DockerManifestDigest: TestImageManifestDigest, + DockerReference: "testing/manifest:latest", + }) + } // Unimplemented and invalid KeyType values for _, keyType := range []sbKeyType{SBKeyTypeSignedByGPGKeys, @@ -84,7 +92,7 @@ func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) { // Do not use NewPRSignedByKeyData, because it would reject invalid values. pr := &prSignedBy{ KeyType: keyType, - KeyData: []byte("abc"), + KeyData: keyData, SignedIdentity: prm, } // Pass nil pointers to, kind of, test that the return value does not depend on the parameters. @@ -92,31 +100,49 @@ func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) { assertSARRejected(t, sar, parsedSig, err) } - // Both KeyPath and KeyData set. Do not use NewPRSignedBy*, because it would reject this. - prSB := &prSignedBy{ - KeyType: ktGPG, - KeyPath: "/foo/bar", - KeyData: []byte("abc"), - SignedIdentity: prm, + // Invalid KeyPath/KeyPaths/KeyData combinations. + for _, fn := range []func() (PolicyRequirement, error){ + // Two or more of KeyPath, KeyPaths and KeyData set. Do not use NewPRSignedBy*, because it would reject this. + func() (PolicyRequirement, error) { + return &prSignedBy{KeyType: ktGPG, KeyPath: "fixtures/public-key.gpg", KeyPaths: []string{"fixtures/public-key-1.gpg", "fixtures/public-key-2.gpg"}, KeyData: keyData, SignedIdentity: prm}, nil + }, + func() (PolicyRequirement, error) { + return &prSignedBy{KeyType: ktGPG, KeyPath: "fixtures/public-key.gpg", KeyPaths: []string{"fixtures/public-key-1.gpg", "fixtures/public-key-2.gpg"}, SignedIdentity: prm}, nil + }, + func() (PolicyRequirement, error) { + return &prSignedBy{KeyType: ktGPG, KeyPath: "fixtures/public-key.gpg", KeyData: keyData, SignedIdentity: prm}, nil + }, + func() (PolicyRequirement, error) { + return &prSignedBy{KeyType: ktGPG, KeyPaths: []string{"fixtures/public-key-1.gpg", "fixtures/public-key-2.gpg"}, KeyData: keyData, SignedIdentity: prm}, nil + }, + // None of KeyPath, KeyPaths and KeyData set. Do not use NewPRSignedBy*, because it would reject this. + func() (PolicyRequirement, error) { + return &prSignedBy{KeyType: ktGPG, SignedIdentity: prm}, nil + }, + func() (PolicyRequirement, error) { // Invalid KeyPath + return NewPRSignedByKeyPath(ktGPG, "/this/does/not/exist", prm) + }, + func() (PolicyRequirement, error) { // Invalid KeyPaths + return NewPRSignedByKeyPaths(ktGPG, []string{"/this/does/not/exist"}, prm) + }, + func() (PolicyRequirement, error) { // One of the KeyPaths is invalid + return NewPRSignedByKeyPaths(ktGPG, []string{"fixtures/public-key.gpg", "/this/does/not/exist"}, prm) + }, + } { + pr, err := fn() + require.NoError(t, err) + // Pass nil pointers to, kind of, test that the return value does not depend on the parameters. + sar, parsedSig, err := pr.isSignatureAuthorAccepted(context.Background(), nil, nil) + assertSARRejected(t, sar, parsedSig, err) } - // Pass nil pointers to, kind of, test that the return value does not depend on the parameters. - sar, parsedSig, err = prSB.isSignatureAuthorAccepted(context.Background(), nil, nil) - assertSARRejected(t, sar, parsedSig, err) - - // Invalid KeyPath - pr, err = NewPRSignedByKeyPath(ktGPG, "/this/does/not/exist", prm) - require.NoError(t, err) - // Pass nil pointers to, kind of, test that the return value does not depend on the parameters. - sar, parsedSig, err = pr.isSignatureAuthorAccepted(context.Background(), nil, nil) - assertSARRejected(t, sar, parsedSig, err) // Errors initializing the temporary GPG directory and mechanism are not obviously easy to reach. // KeyData has no public keys. - pr, err = NewPRSignedByKeyData(ktGPG, []byte{}, prm) + pr, err := NewPRSignedByKeyData(ktGPG, []byte{}, prm) require.NoError(t, err) // Pass nil pointers to, kind of, test that the return value does not depend on the parameters. - sar, parsedSig, err = pr.isSignatureAuthorAccepted(context.Background(), nil, nil) + sar, parsedSig, err := pr.isSignatureAuthorAccepted(context.Background(), nil, nil) assertSARRejectedPolicyRequirement(t, sar, parsedSig, err) // A signature which does not GPG verify diff --git a/signature/policy_types.go b/signature/policy_types.go index 68dcd9892..9e837452a 100644 --- a/signature/policy_types.go +++ b/signature/policy_types.go @@ -67,14 +67,16 @@ type prReject struct { type prSignedBy struct { prCommon - // KeyType specifies what kind of key reference KeyPath/KeyData is. + // KeyType specifies what kind of key reference KeyPath/KeyPaths/KeyData is. // Acceptable values are “GPGKeys” | “signedByGPGKeys” “X.509Certificates” | “signedByX.509CAs” // FIXME: eventually also support GPGTOFU, X.509TOFU, with KeyPath only KeyType sbKeyType `json:"keyType"` - // KeyPath is a pathname to a local file containing the trusted key(s). Exactly one of KeyPath and KeyData must be specified. + // 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"` - // KeyData contains the trusted key(s), base64-encoded. Exactly one of KeyPath and KeyData must be specified. + // 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 []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"` // SignedIdentity specifies what image identity the signature must be claiming about the image.