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 784cd3b669..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) { @@ -118,49 +120,60 @@ 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}, + {"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}, - {"busybox:latest", "library/busybox:latest", true}, {"docker.io/library/busybox:latest", "busybox:latest", true}, - {"busybox:latest", "docker.io/library/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:latest", "busybox", 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}, - {"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}, + {"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}, - {"busybox:latest", "library/busybox:latest", true}, {"docker.io/library/busybox:latest", "busybox:latest", true}, - {"busybox:latest", "docker.io/library/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}, @@ -168,53 +181,115 @@ var prmRepositoryMatchTestTable = []prmTableTest{ {"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}, - {"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}, + {"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}, - {"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}, "") 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 { - // 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 +337,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) } } 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