From 99d99415bafec08a801ca7e7994ab080e8c25b35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 26 Oct 2016 21:48:12 +0200 Subject: [PATCH 1/2] Make the existing policy_reference_match_test cases symmetric MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We expect symmetric behavior; this makes it clearer and allows us to remove a few cases from the pretty lengthy tables. Signed-off-by: Miloslav Trmač --- signature/policy_reference_match_test.go | 86 ++++++++++++------------ 1 file changed, 44 insertions(+), 42 deletions(-) diff --git a/signature/policy_reference_match_test.go b/signature/policy_reference_match_test.go index 784cd3b669..109c8ef4be 100644 --- a/signature/policy_reference_match_test.go +++ b/signature/policy_reference_match_test.go @@ -118,21 +118,19 @@ func (name nameImageTransportMock) ValidatePolicyConfigurationScope(scope string panic("unexpected call to a mock function") } -type prmTableTest struct { - imageRef, sigRef string - result bool +type prmSymmetricTableTest struct { + refA, refB string + result bool } -// Test cases for exact reference match -var prmExactMatchTestTable = []prmTableTest{ +// Test cases for exact reference match. The behavior is supposed to be symmetric. +var prmExactMatchTestTable = []prmSymmetricTableTest{ // Success, simple matches {"busybox:latest", "busybox:latest", true}, {fullRHELRef, fullRHELRef, true}, // Non-canonical reference format is canonicalized {"library/busybox:latest", "busybox:latest", true}, - {"busybox:latest", "library/busybox:latest", true}, {"docker.io/library/busybox:latest", "busybox:latest", true}, - {"busybox:latest", "docker.io/library/busybox:latest", true}, // Mismatch {"busybox:latest", "busybox:notlatest", false}, {"busybox:latest", "notbusybox:latest", false}, @@ -141,26 +139,22 @@ var prmExactMatchTestTable = []prmTableTest{ {"busybox:latest", fullRHELRef, false}, // Missing tags {"busybox", "busybox:latest", false}, - {"busybox:latest", "busybox", false}, {"busybox", "busybox", false}, // Invalid format {"UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES", "busybox:latest", false}, - {"busybox:latest", "UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES", false}, {"", "UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES", false}, // Even if they are exactly equal, invalid values are rejected. {"INVALID", "INVALID", false}, } -// Test cases for repository-only reference match -var prmRepositoryMatchTestTable = []prmTableTest{ +// Test cases for repository-only reference match. The behavior is supposed to be symmetric. +var prmRepositoryMatchTestTable = []prmSymmetricTableTest{ // Success, simple matches {"busybox:latest", "busybox:latest", true}, {fullRHELRef, fullRHELRef, true}, // Non-canonical reference format is canonicalized {"library/busybox:latest", "busybox:latest", true}, - {"busybox:latest", "library/busybox:latest", true}, {"docker.io/library/busybox:latest", "busybox:latest", true}, - {"busybox:latest", "docker.io/library/busybox:latest", true}, // The same as above, but with mismatching tags {"busybox:latest", "busybox:notlatest", true}, {fullRHELRef + "tagsuffix", fullRHELRef, true}, @@ -172,32 +166,34 @@ var prmRepositoryMatchTestTable = []prmTableTest{ {"busybox", "busybox:notlatest", true}, {fullRHELRef, untaggedRHELRef, true}, {"library/busybox", "busybox", true}, - {"busybox", "library/busybox", true}, {"docker.io/library/busybox", "busybox", true}, - {"busybox", "docker.io/library/busybox", true}, // Mismatch {"busybox:latest", "notbusybox:latest", false}, {"hostname/library/busybox:latest", "busybox:notlatest", false}, {"busybox:latest", fullRHELRef, false}, // Invalid format {"UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES", "busybox:latest", false}, - {"busybox:latest", "UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES", false}, {"", "UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES", false}, // Even if they are exactly equal, invalid values are rejected. {"INVALID", "INVALID", false}, } +func testImageAndSig(t *testing.T, prm PolicyReferenceMatch, imageRef, sigRef string, result bool) { + // This assumes that all ways to obtain a reference.Named perform equivalent validation, + // and therefore values refused by reference.ParseNamed can not happen in practice. + parsedImageRef, err := reference.ParseNamed(imageRef) + if err != nil { + return + } + res := prm.matchesDockerReference(refImageMock{parsedImageRef}, sigRef) + assert.Equal(t, result, res, fmt.Sprintf("%s vs. %s", imageRef, sigRef)) +} + func TestPRMMatchExactMatchesDockerReference(t *testing.T) { prm := NewPRMMatchExact() for _, test := range prmExactMatchTestTable { - // This assumes that all ways to obtain a reference.Named perform equivalent validation, - // and therefore values refused by reference.ParseNamed can not happen in practice. - imageRef, err := reference.ParseNamed(test.imageRef) - if err != nil { - continue - } - res := prm.matchesDockerReference(refImageMock{imageRef}, test.sigRef) - assert.Equal(t, test.result, res, fmt.Sprintf("%s vs. %s", test.imageRef, test.sigRef)) + testImageAndSig(t, prm, test.refA, test.refB, test.result) + testImageAndSig(t, prm, test.refB, test.refA, test.result) } // Even if they are signed with an empty string as a reference, unidentified images are rejected. res := prm.matchesDockerReference(refImageMock{nil}, "") @@ -207,14 +203,8 @@ func TestPRMMatchExactMatchesDockerReference(t *testing.T) { func TestPRMMatchRepositoryMatchesDockerReference(t *testing.T) { prm := NewPRMMatchRepository() for _, test := range prmRepositoryMatchTestTable { - // This assumes that all ways to obtain a reference.Named perform equivalent validation, - // and therefore values refused by reference.ParseNamed can not happen in practice. - imageRef, err := reference.ParseNamed(test.imageRef) - if err != nil { - continue - } - res := prm.matchesDockerReference(refImageMock{imageRef}, test.sigRef) - assert.Equal(t, test.result, res, fmt.Sprintf("%s vs. %s", test.imageRef, test.sigRef)) + testImageAndSig(t, prm, test.refA, test.refB, test.result) + testImageAndSig(t, prm, test.refB, test.refA, test.result) } // Even if they are signed with an empty string as a reference, unidentified images are rejected. res := prm.matchesDockerReference(refImageMock{nil}, "") @@ -262,22 +252,34 @@ func (ref forbiddenImageMock) Signatures() ([][]byte, error) { panic("unexpected call to a mock function") } +func testExactPRMAndSig(t *testing.T, prmFactory func(string) PolicyReferenceMatch, imageRef, sigRef string, result bool) { + prm := prmFactory(imageRef) + res := prm.matchesDockerReference(forbiddenImageMock{}, sigRef) + assert.Equal(t, result, res, fmt.Sprintf("%s vs. %s", imageRef, sigRef)) +} + +func prmExactReferenceFactory(ref string) PolicyReferenceMatch { + // Do not use NewPRMExactReference, we want to also test the case with an invalid DockerReference, + // even though NewPRMExactReference should never let it happen. + return &prmExactReference{DockerReference: ref} +} + func TestPRMExactReferenceMatchesDockerReference(t *testing.T) { for _, test := range prmExactMatchTestTable { - // Do not use NewPRMExactReference, we want to also test the case with an invalid DockerReference, - // even though NewPRMExactReference should never let it happen. - prm := prmExactReference{DockerReference: test.imageRef} - res := prm.matchesDockerReference(forbiddenImageMock{}, test.sigRef) - assert.Equal(t, test.result, res, fmt.Sprintf("%s vs. %s", test.imageRef, test.sigRef)) + testExactPRMAndSig(t, prmExactReferenceFactory, test.refA, test.refB, test.result) + testExactPRMAndSig(t, prmExactReferenceFactory, test.refB, test.refA, test.result) } } +func prmExactRepositoryFactory(ref string) PolicyReferenceMatch { + // Do not use NewPRMExactRepository, we want to also test the case with an invalid DockerReference, + // even though NewPRMExactRepository should never let it happen. + return &prmExactRepository{DockerRepository: ref} +} + func TestPRMExactRepositoryMatchesDockerReference(t *testing.T) { for _, test := range prmRepositoryMatchTestTable { - // Do not use NewPRMExactRepository, we want to also test the case with an invalid DockerReference, - // even though NewPRMExactRepository should never let it happen. - prm := prmExactRepository{DockerRepository: test.imageRef} - res := prm.matchesDockerReference(forbiddenImageMock{}, test.sigRef) - assert.Equal(t, test.result, res, fmt.Sprintf("%s vs. %s", test.imageRef, test.sigRef)) + testExactPRMAndSig(t, prmExactRepositoryFactory, test.refA, test.refB, test.result) + testExactPRMAndSig(t, prmExactRepositoryFactory, test.refB, test.refA, test.result) } } From 322058e563c1224b3d897e1211b84681cef6d512 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 7 Nov 2016 20:22:56 +0100 Subject: [PATCH 2/2] Add signedIdentity type:matchRepoDigestOrExact, make it the default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the new default: tag references require a signature with a matching repo:tag, digest references require a signature with a matching repo (and any tag [or digest]), with the digest itself still being validated in image.UnparsedImage, independently of signature processing. Users can still opt into strict checking by specifying matchExact in signedIdentity. Also update most tests to use matchExactOrSignedDigest, to match the default. Signed-off-by: Miloslav Trmač --- docs/policy.json.md | 38 +++----- signature/fixtures/policy.json | 5 +- signature/policy_config.go | 41 ++++++++- signature/policy_config_test.go | 108 +++++++++++++++++++---- signature/policy_eval_test.go | 2 +- signature/policy_reference_match.go | 23 +++++ signature/policy_reference_match_test.go | 91 ++++++++++++++++++- signature/policy_types.go | 15 +++- 8 files changed, 268 insertions(+), 55 deletions(-) diff --git a/docs/policy.json.md b/docs/policy.json.md index 9e705a6138..2984d38871 100644 --- a/docs/policy.json.md +++ b/docs/policy.json.md @@ -156,11 +156,19 @@ Exactly one of `keyPath` and `keyData` must be present, containing a GPG keyring The `signedIdentity` field, a JSON object, specifies what image identity the signature claims about the image. One of the following alternatives are supported: -- The identity in the signature must exactly match the image identity: +- The identity in the signature must exactly match the image identity. Note that with this, referencing an image by digest (with a signature claiming a _repository_`:`_tag_ identity) will fail. ```json {"type":"matchExact"} ``` +- If the image identity carries a tag, the identity in the signature must exactly match; + if the image identity uses a digest reference, the identity in the signature must be in the same repository as the image identity (using any tag). + + (Note that with images identified using digest references, the digest from the reference is validated even before signature verification starts.) + + ```json + {"type":"matchRepoDigestOrExact"} + ``` - The identity in the signature must be in the same repository as the image identity. This is useful e.g. to pull an image using the `:latest` tag when the image is signed with a tag specifing an exact image version. ```json @@ -185,9 +193,9 @@ One of the following alternatives are supported: } ``` -If the `signedIdentity` field is missing, it is treated as `matchExact`. +If the `signedIdentity` field is missing, it is treated as `matchRepoDigestOrExact`. -*Note*: `matchExact` and `matchRepository` can be only used if a Docker-like image identity is +*Note*: `matchExact`, `matchRepoDigestOrExact` and `matchRepository` can be only used if a Docker-like image identity is provided by the transport. In particular, the `dir:` and `oci:` transports can be only used with `exactReference` or `exactRepository`. @@ -257,27 +265,3 @@ selectively allow individual transports and scopes as desired. "default": [{"type": "insecureAcceptAnything"}] } ``` - -### A _temporary_ work-around to allow accessing any image by digest - -Usually, identities in signatures use the _repository_`:`_tag_ format, -which is not matched when pulling a specific image using a digest. -To allow such operations, a policy may set `signedIdentity` to `matchRepository`, similar -to the following fragment: - -```json - "hostname:5000/allow/pull/by/tag": [ - { - "type": "signedBy", - "keyType": "GPGKeys", - "keyPath": "/path/to/some.gpg" - "signedIdentity": { - "type": "matchRepository" - } - } - ] -``` - -*Warning*: This completely turns off tag matching for the signature check in question, -allowing also pulls by tag to accept signatures for any other tag. -A more granular solution for this situation will be provided in the future ( https://github.com/containers/image/issues/99 ). diff --git a/signature/fixtures/policy.json b/signature/fixtures/policy.json index 751f8f3d14..8e39e5c626 100644 --- a/signature/fixtures/policy.json +++ b/signature/fixtures/policy.json @@ -69,7 +69,10 @@ { "type": "signedBy", "keyType": "signedByGPGKeys", - "keyPath": "/keys/RH-key-signing-key-gpg-keyring" + "keyPath": "/keys/RH-key-signing-key-gpg-keyring", + "signedIdentity": { + "type": "matchRepoDigestOrExact" + } } ], "bogus/key-data-example": [ diff --git a/signature/policy_config.go b/signature/policy_config.go index d5e20489bd..2bd9556900 100644 --- a/signature/policy_config.go +++ b/signature/policy_config.go @@ -386,7 +386,7 @@ func (pr *prSignedBy) UnmarshalJSON(data []byte) error { return InvalidPolicyFormatError(fmt.Sprintf("Unexpected policy requirement type \"%s\"", tmp.Type)) } if signedIdentity == nil { - tmp.SignedIdentity = NewPRMMatchExact() + tmp.SignedIdentity = NewPRMMatchRepoDigestOrExact() } else { si, err := newPolicyReferenceMatchFromJSON(signedIdentity) if err != nil { @@ -501,7 +501,7 @@ func (pr *prSignedBaseLayer) UnmarshalJSON(data []byte) error { return nil } -// newPolicyRequirementFromJSON parses JSON data into a PolicyReferenceMatch implementation. +// newPolicyReferenceMatchFromJSON parses JSON data into a PolicyReferenceMatch implementation. func newPolicyReferenceMatchFromJSON(data []byte) (PolicyReferenceMatch, error) { var typeField prmCommon if err := json.Unmarshal(data, &typeField); err != nil { @@ -511,6 +511,8 @@ func newPolicyReferenceMatchFromJSON(data []byte) (PolicyReferenceMatch, error) switch typeField.Type { case prmTypeMatchExact: res = &prmMatchExact{} + case prmTypeMatchRepoDigestOrExact: + res = &prmMatchRepoDigestOrExact{} case prmTypeMatchRepository: res = &prmMatchRepository{} case prmTypeExactReference: @@ -561,6 +563,41 @@ func (prm *prmMatchExact) UnmarshalJSON(data []byte) error { return nil } +// newPRMMatchRepoDigestOrExact is NewPRMMatchRepoDigestOrExact, except it resturns the private type. +func newPRMMatchRepoDigestOrExact() *prmMatchRepoDigestOrExact { + return &prmMatchRepoDigestOrExact{prmCommon{Type: prmTypeMatchRepoDigestOrExact}} +} + +// NewPRMMatchRepoDigestOrExact returns a new "matchRepoDigestOrExact" PolicyReferenceMatch. +func NewPRMMatchRepoDigestOrExact() PolicyReferenceMatch { + return newPRMMatchRepoDigestOrExact() +} + +// Compile-time check that prmMatchRepoDigestOrExact implements json.Unmarshaler. +var _ json.Unmarshaler = (*prmMatchRepoDigestOrExact)(nil) + +// UnmarshalJSON implements the json.Unmarshaler interface. +func (prm *prmMatchRepoDigestOrExact) UnmarshalJSON(data []byte) error { + *prm = prmMatchRepoDigestOrExact{} + var tmp prmMatchRepoDigestOrExact + if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} { + switch key { + case "type": + return &tmp.Type + default: + return nil + } + }); err != nil { + return err + } + + if tmp.Type != prmTypeMatchRepoDigestOrExact { + return InvalidPolicyFormatError(fmt.Sprintf("Unexpected policy requirement type \"%s\"", tmp.Type)) + } + *prm = *newPRMMatchRepoDigestOrExact() + return nil +} + // newPRMMatchRepository is NewPRMMatchRepository, except it resturns the private type. func newPRMMatchRepository() *prmMatchRepository { return &prmMatchRepository{prmCommon{Type: prmTypeMatchRepository}} diff --git a/signature/policy_config_test.go b/signature/policy_config_test.go index 8af3003d24..c7b3278f39 100644 --- a/signature/policy_config_test.go +++ b/signature/policy_config_test.go @@ -28,7 +28,7 @@ var policyFixtureContents = &Policy{ "example.com/production": { xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "/keys/employee-gpg-keyring", - NewPRMMatchExact()), + NewPRMMatchRepoDigestOrExact()), }, "example.com/hardened": { xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, @@ -45,17 +45,17 @@ var policyFixtureContents = &Policy{ NewPRMMatchRepository()), xNewPRSignedByKeyPath(SBKeyTypeSignedByX509CAs, "/keys/public-key-signing-ca-file", - NewPRMMatchExact()), + NewPRMMatchRepoDigestOrExact()), }, "registry.access.redhat.com": { xNewPRSignedByKeyPath(SBKeyTypeSignedByGPGKeys, "/keys/RH-key-signing-key-gpg-keyring", - NewPRMMatchExact()), + NewPRMMatchRepoDigestOrExact()), }, "bogus/key-data-example": { xNewPRSignedByKeyData(SBKeyTypeSignedByGPGKeys, []byte("nonsense"), - NewPRMMatchExact()), + NewPRMMatchRepoDigestOrExact()), }, "bogus/signed-identity-example": { xNewPRSignedBaseLayer(xNewPRMExactReference("registry.access.redhat.com/rhel7/rhel:latest")), @@ -228,12 +228,12 @@ func TestPolicyUnmarshalJSON(t *testing.T) { // Start with a valid JSON. validPolicy := Policy{ Default: []PolicyRequirement{ - xNewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("abc"), NewPRMMatchExact()), + xNewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("abc"), NewPRMMatchRepoDigestOrExact()), }, Transports: map[string]PolicyTransportScopes{ "docker": { "docker.io/library/busybox": []PolicyRequirement{ - xNewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("def"), NewPRMMatchExact()), + xNewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("def"), NewPRMMatchRepoDigestOrExact()), }, "registry.access.redhat.com": []PolicyRequirement{ xNewPRSignedByKeyData(SBKeyTypeSignedByGPGKeys, []byte("RH"), NewPRMMatchRepository()), @@ -310,7 +310,7 @@ func TestPolicyTransportScopesUnmarshalJSON(t *testing.T) { // Start with a valid JSON. validPTS := PolicyTransportScopes{ "": []PolicyRequirement{ - xNewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("global"), NewPRMMatchExact()), + xNewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("global"), NewPRMMatchRepoDigestOrExact()), }, } validJSON, err := json.Marshal(validPTS) @@ -355,13 +355,13 @@ func TestPolicyTransportScopesWithTransportUnmarshalJSON(t *testing.T) { // Start with a valid JSON. validPTS := PolicyTransportScopes{ "docker.io/library/busybox": []PolicyRequirement{ - xNewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("def"), NewPRMMatchExact()), + xNewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("def"), NewPRMMatchRepoDigestOrExact()), }, "registry.access.redhat.com": []PolicyRequirement{ xNewPRSignedByKeyData(SBKeyTypeSignedByGPGKeys, []byte("RH"), NewPRMMatchRepository()), }, "": []PolicyRequirement{ - xNewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("global"), NewPRMMatchExact()), + xNewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("global"), NewPRMMatchRepoDigestOrExact()), }, } validJSON, err := json.Marshal(validPTS) @@ -440,7 +440,7 @@ func TestPolicyRequirementsUnmarshalJSON(t *testing.T) { // Start with a valid JSON. validReqs := PolicyRequirements{ - xNewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("def"), NewPRMMatchExact()), + xNewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("def"), NewPRMMatchRepoDigestOrExact()), xNewPRSignedByKeyData(SBKeyTypeSignedByGPGKeys, []byte("RH"), NewPRMMatchRepository()), } validJSON, err := json.Marshal(validReqs) @@ -632,7 +632,7 @@ func TestPRRejectUnmarshalJSON(t *testing.T) { func TestNewPRSignedBy(t *testing.T) { const testPath = "/foo/bar" testData := []byte("abc") - testIdentity := NewPRMMatchExact() + testIdentity := NewPRMMatchRepoDigestOrExact() // Success pr, err := newPRSignedBy(SBKeyTypeGPGKeys, testPath, nil, testIdentity) @@ -671,7 +671,7 @@ func TestNewPRSignedBy(t *testing.T) { func TestNewPRSignedByKeyPath(t *testing.T) { const testPath = "/foo/bar" - _pr, err := NewPRSignedByKeyPath(SBKeyTypeGPGKeys, testPath, NewPRMMatchExact()) + _pr, err := NewPRSignedByKeyPath(SBKeyTypeGPGKeys, testPath, NewPRMMatchRepoDigestOrExact()) require.NoError(t, err) pr, ok := _pr.(*prSignedBy) require.True(t, ok) @@ -681,7 +681,7 @@ func TestNewPRSignedByKeyPath(t *testing.T) { func TestNewPRSignedByKeyData(t *testing.T) { testData := []byte("abc") - _pr, err := NewPRSignedByKeyData(SBKeyTypeGPGKeys, testData, NewPRMMatchExact()) + _pr, err := NewPRSignedByKeyData(SBKeyTypeGPGKeys, testData, NewPRMMatchRepoDigestOrExact()) require.NoError(t, err) pr, ok := _pr.(*prSignedBy) require.True(t, ok) @@ -710,7 +710,7 @@ func TestPRSignedByUnmarshalJSON(t *testing.T) { testInvalidJSONInput(t, &pr) // Start with a valid JSON. - validPR, err := NewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("abc"), NewPRMMatchExact()) + validPR, err := NewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("abc"), NewPRMMatchRepoDigestOrExact()) require.NoError(t, err) validJSON, err := json.Marshal(validPR) require.NoError(t, err) @@ -722,7 +722,7 @@ func TestPRSignedByUnmarshalJSON(t *testing.T) { assert.Equal(t, validPR, &pr) // Success with KeyPath - kpPR, err := NewPRSignedByKeyPath(SBKeyTypeGPGKeys, "/foo/bar", NewPRMMatchExact()) + kpPR, err := NewPRSignedByKeyPath(SBKeyTypeGPGKeys, "/foo/bar", NewPRMMatchRepoDigestOrExact()) require.NoError(t, err) testJSON, err := json.Marshal(kpPR) require.NoError(t, err) @@ -782,7 +782,7 @@ func TestPRSignedByUnmarshalJSON(t *testing.T) { assert.Error(t, err) } // Handle "keyPath", which is not in validJSON, specially - pathPR, err := NewPRSignedByKeyPath(SBKeyTypeGPGKeys, "/foo/bar", NewPRMMatchExact()) + pathPR, err := NewPRSignedByKeyPath(SBKeyTypeGPGKeys, "/foo/bar", NewPRMMatchRepoDigestOrExact()) require.NoError(t, err) testJSON, err = json.Marshal(pathPR) require.NoError(t, err) @@ -801,6 +801,18 @@ func TestPRSignedByUnmarshalJSON(t *testing.T) { require.NoError(t, err) } + // Various ways to set signedIdentity to the default value + signedIdentityDefaultFns := []func(mSI){ + // Set signedIdentity to the default explicitly + func(v mSI) { v["signedIdentity"] = NewPRMMatchRepoDigestOrExact() }, + // Delete the signedIdentity field + func(v mSI) { delete(v, "signedIdentity") }, + } + for _, fn := range signedIdentityDefaultFns { + err = tryUnmarshalModifiedSignedBy(t, &pr, validJSON, fn) + require.NoError(t, err) + assert.Equal(t, NewPRMMatchRepoDigestOrExact(), pr.SignedIdentity) + } } func TestSBKeyTypeIsValid(t *testing.T) { @@ -945,7 +957,7 @@ func TestPRSignedBaseLayerUnmarshalJSON(t *testing.T) { func TestNewPolicyReferenceMatchFromJSON(t *testing.T) { // Sample success. Others tested in the individual PolicyReferenceMatch.UnmarshalJSON implementations. - validPRM := NewPRMMatchExact() + validPRM := NewPRMMatchRepoDigestOrExact() validJSON, err := json.Marshal(validPRM) require.NoError(t, err) prm, err := newPolicyReferenceMatchFromJSON(validJSON) @@ -1036,6 +1048,68 @@ func TestPRMMatchExactUnmarshalJSON(t *testing.T) { } } +func TestNewPRMMatchRepoDigestOrExact(t *testing.T) { + _prm := NewPRMMatchRepoDigestOrExact() + prm, ok := _prm.(*prmMatchRepoDigestOrExact) + require.True(t, ok) + assert.Equal(t, &prmMatchRepoDigestOrExact{prmCommon{prmTypeMatchRepoDigestOrExact}}, prm) +} + +func TestPRMMatchRepoDigestOrExactUnmarshalJSON(t *testing.T) { + var prm prmMatchRepoDigestOrExact + + testInvalidJSONInput(t, &prm) + + // Start with a valid JSON. + validPR := NewPRMMatchRepoDigestOrExact() + validJSON, err := json.Marshal(validPR) + require.NoError(t, err) + + // Success + prm = prmMatchRepoDigestOrExact{} + err = json.Unmarshal(validJSON, &prm) + require.NoError(t, err) + assert.Equal(t, validPR, &prm) + + // newPolicyReferenceMatchFromJSON recognizes this type + _pr, err := newPolicyReferenceMatchFromJSON(validJSON) + require.NoError(t, err) + assert.Equal(t, validPR, _pr) + + for _, invalid := range []mSI{ + // Missing "type" field + {}, + // Wrong "type" field + {"type": 1}, + {"type": "this is invalid"}, + // Extra fields + { + "type": string(prmTypeMatchRepoDigestOrExact), + "unknown": "foo", + }, + } { + testJSON, err := json.Marshal(invalid) + require.NoError(t, err) + + prm = prmMatchRepoDigestOrExact{} + err = json.Unmarshal(testJSON, &prm) + assert.Error(t, err, string(testJSON)) + } + + // Duplicated fields + for _, field := range []string{"type"} { + var tmp mSI + err := json.Unmarshal(validJSON, &tmp) + require.NoError(t, err) + + testJSON := addExtraJSONMember(t, validJSON, field, tmp[field]) + + prm = prmMatchRepoDigestOrExact{} + err = json.Unmarshal(testJSON, &prm) + assert.Error(t, err) + } +} + func TestNewPRMMatchRepository(t *testing.T) { _prm := NewPRMMatchRepository() prm, ok := _prm.(*prmMatchRepository) diff --git a/signature/policy_eval_test.go b/signature/policy_eval_test.go index e24eee8ad5..c0bfe1a39a 100644 --- a/signature/policy_eval_test.go +++ b/signature/policy_eval_test.go @@ -105,7 +105,7 @@ func (ref pcImageReferenceMock) DeleteImage(ctx *types.SystemContext) error { func TestPolicyContextRequirementsForImageRef(t *testing.T) { ktGPG := SBKeyTypeGPGKeys - prm := NewPRMMatchExact() + prm := NewPRMMatchRepoDigestOrExact() policy := &Policy{ Default: PolicyRequirements{NewPRReject()}, diff --git a/signature/policy_reference_match.go b/signature/policy_reference_match.go index aedda8d09b..ced51e6e07 100644 --- a/signature/policy_reference_match.go +++ b/signature/policy_reference_match.go @@ -36,6 +36,29 @@ func (prm *prmMatchExact) matchesDockerReference(image types.UnparsedImage, sign return signature.String() == intended.String() } +func (prm *prmMatchRepoDigestOrExact) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool { + intended, signature, err := parseImageAndDockerReference(image, signatureDockerReference) + if err != nil { + return false + } + + // Do not add default tags: image.Reference().DockerReference() should contain it already, and signatureDockerReference should be exact; so, verify that now. + if reference.IsNameOnly(signature) { + return false + } + switch intended.(type) { + case reference.NamedTagged: // Includes the case when intended has both a tag and a digest. + return signature.String() == intended.String() + case reference.Canonical: + // We don’t actually compare the manifest digest against the signature here; that happens prSignedBy.in UnparsedImage.Manifest. + // Becase UnparsedImage.Manifest verifies the intended.Digest() against the manifest, and prSignedBy verifies the signature digest against the manifest, + // we know that signature digest matches intended.Digest() (but intended.Digest() and signature digest may use different algorithms) + return signature.Name() == intended.Name() + default: // !reference.IsNameOnly(intended) + return false + } +} + func (prm *prmMatchRepository) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool { intended, signature, err := parseImageAndDockerReference(image, signatureDockerReference) if err != nil { diff --git a/signature/policy_reference_match_test.go b/signature/policy_reference_match_test.go index 109c8ef4be..2554182168 100644 --- a/signature/policy_reference_match_test.go +++ b/signature/policy_reference_match_test.go @@ -12,8 +12,10 @@ import ( ) const ( - fullRHELRef = "registry.access.redhat.com/rhel7/rhel:7.2.3" - untaggedRHELRef = "registry.access.redhat.com/rhel7/rhel" + fullRHELRef = "registry.access.redhat.com/rhel7/rhel:7.2.3" + untaggedRHELRef = "registry.access.redhat.com/rhel7/rhel" + digestSuffix = "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + digestSuffixOther = "@sha256:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" ) func TestParseImageAndDockerReference(t *testing.T) { @@ -128,18 +130,33 @@ var prmExactMatchTestTable = []prmSymmetricTableTest{ // Success, simple matches {"busybox:latest", "busybox:latest", true}, {fullRHELRef, fullRHELRef, true}, + {"busybox" + digestSuffix, "busybox" + digestSuffix, true}, // NOTE: This is not documented; signing digests is not recommended at this time. // Non-canonical reference format is canonicalized {"library/busybox:latest", "busybox:latest", true}, {"docker.io/library/busybox:latest", "busybox:latest", true}, + {"library/busybox" + digestSuffix, "busybox" + digestSuffix, true}, // Mismatch {"busybox:latest", "busybox:notlatest", false}, {"busybox:latest", "notbusybox:latest", false}, {"busybox:latest", "hostname/library/busybox:notlatest", false}, {"hostname/library/busybox:latest", "busybox:notlatest", false}, {"busybox:latest", fullRHELRef, false}, - // Missing tags + {"busybox" + digestSuffix, "notbusybox" + digestSuffix, false}, + {"busybox:latest", "busybox" + digestSuffix, false}, + {"busybox" + digestSuffix, "busybox" + digestSuffixOther, false}, + // NameOnly references {"busybox", "busybox:latest", false}, + {"busybox", "busybox" + digestSuffix, false}, {"busybox", "busybox", false}, + // References with both tags and digests: `reference.WithName` essentially drops the tag. + // This is not _particularly_ desirable but it is the semantics used throughout containers/image; at least, with the digest it is clear which image the reference means, + // even if the tag may reflect a different user intent. + // NOTE: Again, this is not documented behavior; the recommendation is to sign tags, not digests, and then tag-and-digest references won’t match the signed identity. + {"busybox:latest" + digestSuffix, "busybox:latest" + digestSuffix, true}, + {"busybox:latest" + digestSuffix, "busybox:latest" + digestSuffixOther, false}, + {"busybox:latest" + digestSuffix, "busybox:notlatest" + digestSuffix, true}, // Ugly. Do not rely on this. + {"busybox:latest" + digestSuffix, "busybox" + digestSuffix, true}, // Ugly. Do not rely on this. + {"busybox:latest" + digestSuffix, "busybox:latest", false}, // Invalid format {"UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES", "busybox:latest", false}, {"", "UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES", false}, @@ -152,9 +169,11 @@ var prmRepositoryMatchTestTable = []prmSymmetricTableTest{ // Success, simple matches {"busybox:latest", "busybox:latest", true}, {fullRHELRef, fullRHELRef, true}, + {"busybox" + digestSuffix, "busybox" + digestSuffix, true}, // NOTE: This is not documented; signing digests is not recommended at this time. // Non-canonical reference format is canonicalized {"library/busybox:latest", "busybox:latest", true}, {"docker.io/library/busybox:latest", "busybox:latest", true}, + {"library/busybox" + digestSuffix, "busybox" + digestSuffix, true}, // The same as above, but with mismatching tags {"busybox:latest", "busybox:notlatest", true}, {fullRHELRef + "tagsuffix", fullRHELRef, true}, @@ -162,15 +181,25 @@ var prmRepositoryMatchTestTable = []prmSymmetricTableTest{ {"busybox:latest", "library/busybox:notlatest", true}, {"docker.io/library/busybox:notlatest", "busybox:latest", true}, {"busybox:notlatest", "docker.io/library/busybox:latest", true}, + {"busybox:latest", "busybox" + digestSuffix, true}, + {"busybox" + digestSuffix, "busybox" + digestSuffixOther, true}, // Even this is accepted here. (This could more reasonably happen with two different digest algorithms.) // The same as above, but with defaulted tags (should not actually happen) {"busybox", "busybox:notlatest", true}, {fullRHELRef, untaggedRHELRef, true}, + {"busybox", "busybox" + digestSuffix, true}, {"library/busybox", "busybox", true}, {"docker.io/library/busybox", "busybox", true}, // Mismatch {"busybox:latest", "notbusybox:latest", false}, {"hostname/library/busybox:latest", "busybox:notlatest", false}, {"busybox:latest", fullRHELRef, false}, + {"busybox" + digestSuffix, "notbusybox" + digestSuffix, false}, + // References with both tags and digests: `reference.WithName` essentially drops the tag, and we ignore both anyway. + {"busybox:latest" + digestSuffix, "busybox:latest" + digestSuffix, true}, + {"busybox:latest" + digestSuffix, "busybox:latest" + digestSuffixOther, true}, + {"busybox:latest" + digestSuffix, "busybox:notlatest" + digestSuffix, true}, + {"busybox:latest" + digestSuffix, "busybox" + digestSuffix, true}, + {"busybox:latest" + digestSuffix, "busybox:latest", true}, // Invalid format {"UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES", "busybox:latest", false}, {"", "UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES", false}, @@ -200,6 +229,62 @@ func TestPRMMatchExactMatchesDockerReference(t *testing.T) { assert.False(t, res, `unidentified vs. ""`) } +func TestPMMMatchRepoDigestOrExactMatchesDockerReference(t *testing.T) { + prm := NewPRMMatchRepoDigestOrExact() + + // prmMatchRepoDigestOrExact is a middle ground between prmMatchExact and prmMatchRepository: + // It accepts anything prmMatchExact accepts,… + for _, test := range prmExactMatchTestTable { + if test.result == true { + testImageAndSig(t, prm, test.refA, test.refB, test.result) + testImageAndSig(t, prm, test.refB, test.refA, test.result) + } + } + // … and it rejects everything prmMatchRepository rejects. + for _, test := range prmRepositoryMatchTestTable { + if test.result == false { + testImageAndSig(t, prm, test.refA, test.refB, test.result) + testImageAndSig(t, prm, test.refB, test.refA, test.result) + } + } + + // The other cases, possibly assymetrical: + for _, test := range []struct { + imageRef, sigRef string + result bool + }{ + // Tag mismatch + {"busybox:latest", "busybox:notlatest", false}, + {fullRHELRef + "tagsuffix", fullRHELRef, false}, + {"library/busybox:latest", "busybox:notlatest", false}, + {"busybox:latest", "library/busybox:notlatest", false}, + {"docker.io/library/busybox:notlatest", "busybox:latest", false}, + {"busybox:notlatest", "docker.io/library/busybox:latest", false}, + // NameOnly references + {"busybox", "busybox:latest", false}, + {"busybox:latest", "busybox", false}, + {"busybox", "busybox" + digestSuffix, false}, + {"busybox" + digestSuffix, "busybox", false}, + {fullRHELRef, untaggedRHELRef, false}, + {"busybox", "busybox", false}, + // Tag references only accept signatures with matching tags. + {"busybox:latest", "busybox" + digestSuffix, false}, + // Digest references accept any signature with matching repository. + {"busybox" + digestSuffix, "busybox:latest", true}, + {"busybox" + digestSuffix, "busybox" + digestSuffixOther, true}, // Even this is accepted here. (This could more reasonably happen with two different digest algorithms.) + // References with both tags and digests: `reference.WithName` essentially drops the tag. + // This is not _particularly_ desirable but it is the semantics used throughout containers/image; at least, with the digest it is clear which image the reference means, + // even if the tag may reflect a different user intent. + {"busybox:latest" + digestSuffix, "busybox:latest", true}, + {"busybox:latest" + digestSuffix, "busybox:notlatest", true}, + {"busybox:latest", "busybox:latest" + digestSuffix, false}, + {"busybox:latest" + digestSuffix, "busybox:latest" + digestSuffixOther, true}, // Even this is accepted here. (This could more reasonably happen with two different digest algorithms.) + {"busybox:latest" + digestSuffix, "busybox:notlatest" + digestSuffixOther, true}, // Ugly. Do not rely on this. + } { + testImageAndSig(t, prm, test.imageRef, test.sigRef, test.result) + } +} + func TestPRMMatchRepositoryMatchesDockerReference(t *testing.T) { prm := NewPRMMatchRepository() for _, test := range prmRepositoryMatchTestTable { diff --git a/signature/policy_types.go b/signature/policy_types.go index 5775ab21dd..4cd770f11c 100644 --- a/signature/policy_types.go +++ b/signature/policy_types.go @@ -116,10 +116,11 @@ type prmCommon struct { type prmTypeIdentifier string const ( - prmTypeMatchExact prmTypeIdentifier = "matchExact" - prmTypeMatchRepository prmTypeIdentifier = "matchRepository" - prmTypeExactReference prmTypeIdentifier = "exactReference" - prmTypeExactRepository prmTypeIdentifier = "exactRepository" + prmTypeMatchExact prmTypeIdentifier = "matchExact" + prmTypeMatchRepoDigestOrExact prmTypeIdentifier = "matchRepoDigestOrExact" + prmTypeMatchRepository prmTypeIdentifier = "matchRepository" + prmTypeExactReference prmTypeIdentifier = "exactReference" + prmTypeExactRepository prmTypeIdentifier = "exactRepository" ) // prmMatchExact is a PolicyReferenceMatch with type = prmMatchExact: the two references must match exactly. @@ -127,6 +128,12 @@ type prmMatchExact struct { prmCommon } +// prmMatchRepoDigestOrExact is a PolicyReferenceMatch with type = prmMatchExactOrDigest: the two references must match exactly, +// except that digest references are also accepted if the repository name matches (regardless of tag/digest) and the signature applies to the referenced digest +type prmMatchRepoDigestOrExact struct { + prmCommon +} + // prmMatchRepository is a PolicyReferenceMatch with type = prmMatchRepository: the two references must use the same repository, may differ in the tag. type prmMatchRepository struct { prmCommon