Merge pull request #88 from mtrmac/policy-eval

Add a policy evaluation library

signature/docker.go

@@ -28,16 +28,33 @@ func SignDockerManifest(manifest []byte, dockerReference string, mech SigningMec
 // using mech.
 func VerifyDockerManifestSignature(unverifiedSignature, unverifiedManifest []byte,
 	expectedDockerReference string, mech SigningMechanism, expectedKeyIdentity string) (*Signature, error) {
-	expectedManifestDigest, err := utils.ManifestDigest(unverifiedManifest)
+	sig, err := verifyAndExtractSignature(mech, unverifiedSignature, signatureAcceptanceRules{
+		validateKeyIdentity: func(keyIdentity string) error {
+			if keyIdentity != expectedKeyIdentity {
+				return InvalidSignatureError{msg: fmt.Sprintf("Signature by %s does not match expected fingerprint %s", keyIdentity, expectedKeyIdentity)}
+			}
+			return nil
+		},
+		validateSignedDockerReference: func(signedDockerReference string) error {
+			if signedDockerReference != expectedDockerReference {
+				return InvalidSignatureError{msg: fmt.Sprintf("Docker reference %s does not match %s",
+					signedDockerReference, expectedDockerReference)}
+			}
+			return nil
+		},
+		validateSignedDockerManifestDigest: func(signedDockerManifestDigest string) error {
+			matches, err := utils.ManifestMatchesDigest(unverifiedManifest, signedDockerManifestDigest)
+			if err != nil {
+				return err
+			}
+			if !matches {
+				return InvalidSignatureError{msg: fmt.Sprintf("Signature for docker digest %s does not match", signedDockerManifestDigest, signedDockerManifestDigest)}
+			}
+			return nil
+		},
+	})
 	if err != nil {
 		return nil, err
 	}
-	sig, err := verifyAndExtractSignature(mech, unverifiedSignature, expectedKeyIdentity, expectedDockerReference)
-	if err != nil {
-		return nil, err
-	}
-	if sig.DockerManifestDigest != expectedManifestDigest {
-		return nil, InvalidSignatureError{msg: fmt.Sprintf("Docker manifest digest %s does not match %s", sig.DockerManifestDigest, expectedManifestDigest)}
-	}
 	return sig, nil
 }

signature/docker_test.go

@@ -72,6 +72,11 @@ func TestVerifyDockerManifestSignature(t *testing.T) {
 	assert.Error(t, err)
 	assert.Nil(t, sig)
 
+	// Docker reference mismatch
+	sig, err = VerifyDockerManifestSignature(signature, manifest, "example.com/doesnt/match", mech, TestKeyFingerprint)
+	assert.Error(t, err)
+	assert.Nil(t, sig)
+
 	// Docker manifest digest mismatch
 	sig, err = VerifyDockerManifestSignature(signature, []byte("unexpected manifest"), TestImageSignatureReference, mech, TestKeyFingerprint)
 	assert.Error(t, err)

signature/fixtures/dir-img-manifest-digest-error/manifest.json

@@ -0,0 +1 @@
+../v2s1-invalid-signatures.manifest.json

signature/fixtures/dir-img-manifest-digest-error/signature-1

@@ -0,0 +1 @@
+../dir-img-valid/signature-1

signature/fixtures/dir-img-mixed/manifest.json

@@ -0,0 +1 @@
+../dir-img-valid/manifest.json

signature/fixtures/dir-img-mixed/signature-1

@@ -0,0 +1 @@
+../invalid-blob.signature

signature/fixtures/dir-img-mixed/signature-2

@@ -0,0 +1 @@
+../dir-img-valid/signature-1

signature/fixtures/dir-img-modified-manifest/manifest.json

@@ -0,0 +1,27 @@
+{
+    "schemaVersion": 2,
+    "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
+    "config": {
+        "mediaType": "application/vnd.docker.container.image.v1+json",
+        "size": 7023,
+        "digest": "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7"
+    },
+    "layers": [
+        {
+            "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
+            "size": 32654,
+            "digest": "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f"
+        },
+        {
+            "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
+            "size": 16724,
+            "digest": "sha256:3c3a4604a545cdc127456d94e421cd355bca5b528f4a9c1905b15da2eb4a4c6b"
+        },
+        {
+            "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
+            "size": 73109,
+            "digest": "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736"
+        }
+    ],
+    "extra": "this manifest has been modified"
+}

signature/fixtures/dir-img-modified-manifest/signature-1

@@ -0,0 +1 @@
+../dir-img-valid/signature-1

signature/fixtures/dir-img-no-manifest/signature-1

@@ -0,0 +1 @@
+../dir-img-valid/signature-1

signature/fixtures/dir-img-unsigned/manifest.json

@@ -0,0 +1 @@
+../dir-img-valid/manifest.json

signature/fixtures/dir-img-valid-2/manifest.json

@@ -0,0 +1 @@
+../dir-img-valid/manifest.json

signature/fixtures/dir-img-valid-2/signature-1

@@ -0,0 +1 @@
+../dir-img-valid/signature-1

signature/fixtures/dir-img-valid/manifest.json

@@ -0,0 +1 @@
+../image.manifest.json

signature/mechanism_test.go

@@ -138,5 +138,12 @@ func TestGPGSigningMechanismVerify(t *testing.T) {
 	require.NoError(t, err)
 	content, signingFingerprint, err = mech.Verify(signature)
 	assertSigningError(t, content, signingFingerprint, err)
+
+	// Valid signature with an unknown key
+	signature, err = ioutil.ReadFile("./fixtures/unknown-key.signature")
+	require.NoError(t, err)
+	content, signingFingerprint, err = mech.Verify(signature)
+	assertSigningError(t, content, signingFingerprint, err)
+
 	// The various GPG/GPGME failures cases are not obviously easy to reach.
 }

signature/policy_config.go

@@ -88,10 +88,9 @@ func (m *policySpecificMap) UnmarshalJSON(data []byte) error {
 	// So, use a temporary map of pointers-to-slices and convert.
 	tmpMap := map[string]*PolicyRequirements{}
 	if err := paranoidUnmarshalJSONObject(data, func(key string) interface{} {
-		// Check that the scope format is at least plausible.
-		if _, err := reference.ParseNamed(key); err != nil {
-			return nil // FIXME? This returns an "Unknown key" error instead of saying that the format is invalid.
-		}
+		// FIXME? We might want to validate the scope format.
+		// Note that reference.ParseNamed is unsuitable; it would understand "example.com" as
+		// "docker.io/library/example.com"
 		// paranoidUnmarshalJSONObject detects key duplication for us, check just to be safe.
 		if _, ok := tmpMap[key]; ok {
 			return nil

signature/policy_config_test.go

@@ -160,7 +160,7 @@ func TestPolicyUnmarshalJSON(t *testing.T) {
 			xNewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("abc"), NewPRMMatchExact()),
 		},
 		Specific: map[string]PolicyRequirements{
-			"library/busybox": []PolicyRequirement{
+			"docker.io/library/busybox": []PolicyRequirement{
 				xNewPRSignedByKeyData(SBKeyTypeGPGKeys, []byte("def"), NewPRMMatchExact()),
 			},
 			"registry.access.redhat.com": []PolicyRequirement{
@@ -191,11 +191,8 @@ func TestPolicyUnmarshalJSON(t *testing.T) {
 		func(v mSI) { v["specific"] = []string{} },
 		// "default" is an invalid PolicyRequirements
 		func(v mSI) { v["default"] = PolicyRequirements{} },
-		// Invalid scope name in "specific". Uppercase is invalid in Docker reference components.
-		// Get valid PolicyRequirements by copying them from  "library/buxybox".
-		func(v mSI) { x(v, "specific")["INVALIDUPPERCASE"] = x(v, "specific")["library/busybox"] },
 		// A field in "specific" is an invalid PolicyRequirements
-		func(v mSI) { x(v, "specific")["library/busybox"] = PolicyRequirements{} },
+		func(v mSI) { x(v, "specific")["docker.io/library/busybox"] = PolicyRequirements{} },
 	}
 	for _, fn := range breakFns {
 		err = tryUnmarshalModifiedPolicy(t, &p, validJSON, fn)

signature/policy_eval.go

@@ -0,0 +1,334 @@
+// This defines the top-level policy evaluation API.
+// To the extent possible, the interface of the fuctions provided
+// here is intended to be completely unambiguous, and stable for users
+// to rely on.
+
+package signature
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/Sirupsen/logrus"
+	distreference "github.com/docker/distribution/reference"
+	"github.com/projectatomic/skopeo/reference"
+	"github.com/projectatomic/skopeo/types"
+)
+
+// PolicyRequirementError is an explanatory text for rejecting a signature or an image.
+type PolicyRequirementError string
+
+func (err PolicyRequirementError) Error() string {
+	return string(err)
+}
+
+// signatureAcceptanceResult is the principal value returned by isSignatureAuthorAccepted.
+type signatureAcceptanceResult string
+
+const (
+	sarAccepted signatureAcceptanceResult = "sarAccepted"
+	sarRejected signatureAcceptanceResult = "sarRejected"
+	sarUnknown  signatureAcceptanceResult = "sarUnknown"
+)
+
+// PolicyRequirement is a rule which must be satisfied by at least one of the signatures of an image.
+// The type is public, but its definition is private.
+type PolicyRequirement interface {
+	// FIXME: For speed, we should support creating per-context state (not stored in the PolicyRequirement), to cache
+	// costly initialization like creating temporary GPG home directories and reading files.
+	// Setup() (someState, error)
+	// Then, the operations below would be done on the someState object, not directly on a PolicyRequirement.
+
+	// isSignatureAuthorAccepted, given an image and a signature blob, returns:
+	// - sarAccepted if the signature has been verified against the appropriate public key
+	//   (where "appropriate public key" may depend on the contents of the signature);
+	//   in that case a parsed Signature should be returned.
+	// - sarRejected if the signature has not been verified;
+	//   in that case error must be non-nil, and should be an PolicyRequirementError if evaluation
+	//   succeeded but the result was rejection.
+	// - sarUnknown if if this PolicyRequirement does not deal with signatures.
+	//   NOTE: sarUnknown should not be returned if this PolicyRequirement should make a decision but something failed.
+	//   Returning sarUnknown and a non-nil error value is invalid.
+	// WARNING: This makes the signature contents acceptable for futher processing,
+	// but it does not necessarily mean that the contents of the signature are
+	// consistent with local policy.
+	// For example:
+	// - Do not use a true value to determine whether to run
+	//   a container based on this image; use IsRunningImageAllowed instead.
+	// - Just because a signature is accepted does not automatically mean the contents of the
+	//   signature are authorized to run code as root, or to affect system or cluster configuration.
+	isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error)
+
+	// isRunningImageAllowed returns true if the requirement allows running an image.
+	// If it returns false, err must be non-nil, and should be an PolicyRequirementError if evaluation
+	// succeeded but the result was rejection.
+	// WARNING: This validates signatures and the manifest, but does not download or validate the
+	// layers. Users must validate that the layers match their expected digests.
+	isRunningImageAllowed(image types.Image) (bool, error)
+}
+
+// PolicyReferenceMatch specifies a set of image identities accepted in PolicyRequirement.
+// The type is public, but its implementation is private.
+type PolicyReferenceMatch interface {
+	// matchesDockerReference decides whether a specific image identity is accepted for an image
+	// (or, usually, for the image's IntendedDockerReference()),
+	matchesDockerReference(image types.Image, signatureDockerReference string) bool
+}
+
+// PolicyContext encapsulates a policy and possible cached state
+// for speeding up its evaluation.
+type PolicyContext struct {
+	Policy *Policy
+	state  policyContextState // Internal consistency checking
+}
+
+// policyContextState is used internally to verify the users are not misusing a PolicyContext.
+type policyContextState string
+
+const (
+	pcInvalid      policyContextState = ""
+	pcInitializing policyContextState = "Initializing"
+	pcReady        policyContextState = "Ready"
+	pcInUse        policyContextState = "InUse"
+	pcDestroying   policyContextState = "Destroying"
+	pcDestroyed    policyContextState = "Destroyed"
+)
+
+// changeContextState changes pc.state, or fails if the state is unexpected
+func (pc *PolicyContext) changeState(expected, new policyContextState) error {
+	if pc.state != expected {
+		return fmt.Errorf(`"Invalid PolicyContext state, expected "%s", found "%s"`, expected, pc.state)
+	}
+	pc.state = new
+	return nil
+}
+
+// NewPolicyContext sets up and initializes a context for the specified policy.
+// The policy must not be modified while the context exists. FIXME: make a deep copy?
+// If this function succeeds, the caller should call PolicyContext.Destroy() when done.
+func NewPolicyContext(policy *Policy) (*PolicyContext, error) {
+	pc := &PolicyContext{Policy: policy, state: pcInitializing}
+	// FIXME: initialize
+	if err := pc.changeState(pcInitializing, pcReady); err != nil {
+		// Huh?! This should never fail, we didn't give the pointer to anybody.
+		// Just give up and leave unclean state around.
+		return nil, err
+	}
+	return pc, nil
+}
+
+// Destroy should be called when the user of the context is done with it.
+func (pc *PolicyContext) Destroy() error {
+	if err := pc.changeState(pcReady, pcDestroying); err != nil {
+		return err
+	}
+	// FIXME: destroy
+	return pc.changeState(pcDestroying, pcDestroyed)
+}
+
+// fullyExpandedDockerReference converts a reference.Named into a fully expanded format;
+// i.e. soft of an opposite to ref.String(), which is a fully canonicalized/minimized format.
+// This is guaranteed to be the same as reference.FullName(), with a tag or digest appended, if available.
+// FIXME? This feels like it should be provided by skopeo/reference.
+func fullyExpandedDockerReference(ref reference.Named) (string, error) {
+	res := ref.FullName()
+	tagged, isTagged := ref.(distreference.Tagged)
+	digested, isDigested := ref.(distreference.Digested)
+	// A github.com/distribution/reference value can have a tag and a digest at the same time!
+	// skopeo/reference does not handle that, so fail.
+	// FIXME? Should we support that?
+	switch {
+	case isTagged && isDigested:
+		// Coverage: This should currently not happen, the way skopeo/reference sets up types,
+		// isTagged and isDigested is mutually exclusive.
+		return "", fmt.Errorf("Names with both a tag and digest are not currently supported")
+	case isTagged:
+		res = res + ":" + tagged.Tag()
+	case isDigested:
+		res = res + "@" + digested.Digest().String()
+	default:
+		// res is already OK.
+	}
+	return res, nil
+}
+
+// requirementsForImage selects the appropriate requirements for image.
+func (pc *PolicyContext) requirementsForImage(image types.Image) (PolicyRequirements, error) {
+	imageIdentity := image.IntendedDockerReference()
+	// We don't technically need to parse it first in order to match the full name:tag,
+	// but do so anyway to ensure that the intended identity really does follow that
+	// format, or at least that it is not demonstrably wrong.
+	ref, err := reference.ParseNamed(imageIdentity)
+	if err != nil {
+		return nil, err
+	}
+	ref = reference.WithDefaultTag(ref)
+
+	// Look for a full match.
+	fullyExpanded, err := fullyExpandedDockerReference(ref)
+	if err != nil { // Coverage: This cannot currently happen.
+		return nil, err
+	}
+	if req, ok := pc.Policy.Specific[fullyExpanded]; ok {
+		logrus.Debugf(" Using specific policy section %s", fullyExpanded)
+		return req, nil
+	}
+
+	// Look for a match of the repository, and then of the possible parent
+	// namespaces. Note that this only happens on the expanded host names
+	// and repository names, i.e. "busybox" is looked up as "docker.io/library/busybox",
+	// then in its parent "docker.io/library"; in none of "busybox",
+	// un-namespaced "library" nor in "" implicitly representing "library/".
+	//
+	// ref.FullName() == ref.Hostname() + "/" + ref.RemoteName(), so the last
+	// iteration matches the host name (for any namespace).
+	name := ref.FullName()
+	for {
+		if req, ok := pc.Policy.Specific[name]; ok {
+			logrus.Debugf(" Using specific policy section %s", name)
+			return req, nil
+		}
+
+		lastSlash := strings.LastIndex(name, "/")
+		if lastSlash == -1 {
+			break
+		}
+		name = name[:lastSlash]
+	}
+
+	logrus.Debugf(" Using default policy section")
+	return pc.Policy.Default, nil
+}
+
+// GetSignaturesWithAcceptedAuthor returns those signatures from an image
+// for which the policy accepts the author (and which have been successfully
+// verified).
+// NOTE: This may legitimately return an empty list and no error, if the image
+// has no signatures or only invalid signatures.
+// WARNING: This makes the signature contents acceptable for futher processing,
+// but it does not necessarily mean that the contents of the signature are
+// consistent with local policy.
+// For example:
+// - Do not use a an existence of an accepted signature to determine whether to run
+//   a container based on this image; use IsRunningImageAllowed instead.
+// - Just because a signature is accepted does not automatically mean the contents of the
+//   signature are authorized to run code as root, or to affect system or cluster configuration.
+func (pc *PolicyContext) GetSignaturesWithAcceptedAuthor(image types.Image) (sigs []*Signature, finalErr error) {
+	if err := pc.changeState(pcReady, pcInUse); err != nil {
+		return nil, err
+	}
+	defer func() {
+		if err := pc.changeState(pcInUse, pcReady); err != nil {
+			sigs = nil
+			finalErr = err
+		}
+	}()
+
+	logrus.Debugf("GetSignaturesWithAcceptedAuthor for image %s", image.IntendedDockerReference())
+
+	reqs, err := pc.requirementsForImage(image)
+	if err != nil {
+		return nil, err
+	}
+
+	// FIXME: rename Signatures to UnverifiedSignatures
+	unverifiedSignatures, err := image.Signatures()
+	if err != nil {
+		return nil, err
+	}
+
+	res := make([]*Signature, 0, len(unverifiedSignatures))
+	for sigNumber, sig := range unverifiedSignatures {
+		var acceptedSig *Signature // non-nil if accepted
+		rejected := false
+		// FIXME? Say more about the contents of the signature, i.e. parse it even before verification?!
+		logrus.Debugf("Evaluating signature %d:", sigNumber)
+	interpretingReqs:
+		for reqNumber, req := range reqs {
+			// FIXME: Log the requirement itself? For now, we use just the number.
+			// FIXME: supply state
+			switch res, as, err := req.isSignatureAuthorAccepted(image, sig); res {
+			case sarAccepted:
+				if as == nil { // Coverage: this should never happen
+					logrus.Debugf(" Requirement %d: internal inconsistency: sarAccepted but no parsed contents", reqNumber)
+					rejected = true
+					break interpretingReqs
+				}
+				logrus.Debugf(" Requirement %d: signature accepted", reqNumber)
+				if acceptedSig == nil {
+					acceptedSig = as
+				} else if *as != *acceptedSig { // Coverage: this should never happen
+					// Huh?! Two ways of verifying the same signature blob resulted in two different parses of its already accepted contents?
+					logrus.Debugf(" Requirement %d: internal inconsistency: sarAccepted but different parsed contents", reqNumber)
+					rejected = true
+					acceptedSig = nil
+					break interpretingReqs
+				}
+			case sarRejected:
+				logrus.Debugf(" Requirement %d: signature rejected: %s", reqNumber, err.Error())
+				rejected = true
+				break interpretingReqs
+			case sarUnknown:
+				if err != nil { // Coverage: this should never happen
+					logrus.Debugf(" Requirement %d: internal inconsistency: sarUnknown but an error message %s", reqNumber, err.Error())
+					rejected = true
+					break interpretingReqs
+				}
+				logrus.Debugf(" Requirement %d: signature state unknown, continuing", reqNumber)
+			default: // Coverage: this should never happen
+				logrus.Debugf(" Requirement %d: internal inconsistency: unknown result %#v", reqNumber, string(res))
+				rejected = true
+				break interpretingReqs
+			}
+		}
+		// This also handles the (invalid) case of empty reqs, by rejecting the signature.
+		if acceptedSig != nil && !rejected {
+			logrus.Debugf(" Overall: OK, signature accepted")
+			res = append(res, acceptedSig)
+		} else {
+			logrus.Debugf(" Overall: Signature not accepted")
+		}
+	}
+	return res, nil
+}
+
+// IsRunningImageAllowed returns true iff the policy allows running the image.
+// If it returns false, err must be non-nil, and should be an PolicyRequirementError if evaluation
+// succeeded but the result was rejection.
+// WARNING: This validates signatures and the manifest, but does not download or validate the
+// layers. Users must validate that the layers match their expected digests.
+func (pc *PolicyContext) IsRunningImageAllowed(image types.Image) (res bool, finalErr error) {
+	if err := pc.changeState(pcReady, pcInUse); err != nil {
+		return false, err
+	}
+	defer func() {
+		if err := pc.changeState(pcInUse, pcReady); err != nil {
+			res = false
+			finalErr = err
+		}
+	}()
+
+	logrus.Debugf("IsRunningImageAllowed for image %s", image.IntendedDockerReference())
+
+	reqs, err := pc.requirementsForImage(image)
+	if err != nil {
+		return false, err
+	}
+
+	if len(reqs) == 0 {
+		return false, PolicyRequirementError("List of verification policy requirements must not be empty")
+	}
+
+	for reqNumber, req := range reqs {
+		// FIXME: supply state
+		allowed, err := req.isRunningImageAllowed(image)
+		if !allowed {
+			logrus.Debugf("Requirement %d: denied, done", reqNumber)
+			return false, err
+		}
+		logrus.Debugf(" Requirement %d: allowed", reqNumber)
+	}
+	// We have tested that len(reqs) != 0, so at least one req must have explicitly allowed this image.
+	logrus.Debugf("Overall: allowed")
+	return true, nil
+}

signature/policy_eval_baselayer.go

@@ -0,0 +1,18 @@
+// Policy evaluation for prSignedBaseLayer.
+
+package signature
+
+import (
+	"github.com/Sirupsen/logrus"
+	"github.com/projectatomic/skopeo/types"
+)
+
+func (pr *prSignedBaseLayer) isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error) {
+	return sarUnknown, nil, nil
+}
+
+func (pr *prSignedBaseLayer) isRunningImageAllowed(image types.Image) (bool, error) {
+	// FIXME? Reject this at policy parsing time already?
+	logrus.Errorf("signedBaseLayer not implemented yet!")
+	return false, PolicyRequirementError("signedBaseLayer not implemented yet!")
+}

signature/policy_eval_baselayer_test.go

@@ -0,0 +1,24 @@
+package signature
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestPRSignedBaseLayerIsSignatureAuthorAccepted(t *testing.T) {
+	pr, err := NewPRSignedBaseLayer(NewPRMMatchRepository())
+	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(nil, nil)
+	assertSARUnknown(t, sar, parsedSig, err)
+}
+
+func TestPRSignedBaseLayerIsRunningImageAllowed(t *testing.T) {
+	// This will obviously need to change after signedBaseLayer is implemented.
+	pr, err := NewPRSignedBaseLayer(NewPRMMatchRepository())
+	require.NoError(t, err)
+	// Pass a nil pointer to, kind of, test that the return value does not depend on the image.
+	res, err := pr.isRunningImageAllowed(nil)
+	assertRunningRejectedPolicyRequirement(t, res, err)
+}

signature/policy_eval_signedby.go

@@ -0,0 +1,137 @@
+// Policy evaluation for prSignedBy.
+
+package signature
+
+import (
+	"errors"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"strings"
+
+	"github.com/projectatomic/skopeo/docker/utils"
+	"github.com/projectatomic/skopeo/types"
+)
+
+func (pr *prSignedBy) isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error) {
+	switch pr.KeyType {
+	case SBKeyTypeGPGKeys:
+	case SBKeyTypeSignedByGPGKeys, SBKeyTypeX509Certificates, SBKeyTypeSignedByX509CAs:
+		// FIXME? Reject this at policy parsing time already?
+		return sarRejected, nil, fmt.Errorf(`"Unimplemented "keyType" value "%s"`, string(pr.KeyType))
+	default:
+		// This should never happen, newPRSignedBy ensures KeyType.IsValid()
+		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 {
+		d, err := ioutil.ReadFile(pr.KeyPath)
+		if err != nil {
+			return sarRejected, nil, err
+		}
+		data = d
+	}
+
+	// FIXME: move this to per-context initialization
+	dir, err := ioutil.TempDir("", "skopeo-signedBy-")
+	if err != nil {
+		return sarRejected, nil, err
+	}
+	defer os.RemoveAll(dir)
+	mech, err := newGPGSigningMechanismInDirectory(dir)
+	if err != nil {
+		return sarRejected, nil, err
+	}
+
+	trustedIdentities, err := mech.ImportKeysFromBytes(data)
+	if err != nil {
+		return sarRejected, nil, err
+	}
+	if len(trustedIdentities) == 0 {
+		return sarRejected, nil, PolicyRequirementError("No public keys imported")
+	}
+
+	signature, err := verifyAndExtractSignature(mech, sig, signatureAcceptanceRules{
+		validateKeyIdentity: func(keyIdentity string) error {
+			for _, trustedIdentity := range trustedIdentities {
+				if keyIdentity == trustedIdentity {
+					return nil
+				}
+			}
+			// Coverage: We use a private GPG home directory and only import trusted keys, so this should
+			// not be reachable.
+			return PolicyRequirementError(fmt.Sprintf("Signature by key %s is not accepted", keyIdentity))
+		},
+		validateSignedDockerReference: func(ref string) error {
+			if !pr.SignedIdentity.matchesDockerReference(image, ref) {
+				return PolicyRequirementError(fmt.Sprintf("Signature for identity %s is not accepted", ref))
+			}
+			return nil
+		},
+		validateSignedDockerManifestDigest: func(digest string) error {
+			manifest, err := image.Manifest()
+			if err != nil {
+				return err
+			}
+			digestMatches, err := utils.ManifestMatchesDigest(manifest, digest)
+			if err != nil {
+				return err
+			}
+			if !digestMatches {
+				return PolicyRequirementError(fmt.Sprintf("Signature for digest %s does not match", digest))
+			}
+			return nil
+		},
+	})
+	if err != nil {
+		return sarRejected, nil, err
+	}
+
+	return sarAccepted, signature, nil
+}
+
+func (pr *prSignedBy) isRunningImageAllowed(image types.Image) (bool, error) {
+	sigs, err := image.Signatures()
+	if err != nil {
+		return false, err
+	}
+	var rejections []error
+	for _, s := range sigs {
+		var reason error
+		switch res, _, err := pr.isSignatureAuthorAccepted(image, s); res {
+		case sarAccepted:
+			// One accepted signature is enough.
+			return true, nil
+		case sarRejected:
+			reason = err
+		case sarUnknown:
+			// Huh?! This should not happen at all; treat it as any other invalid value.
+			fallthrough
+		default:
+			reason = fmt.Errorf(`Internal error: Unexpected signature verification result "%s"`, string(res))
+		}
+		rejections = append(rejections, reason)
+	}
+	var summary error
+	switch len(rejections) {
+	case 0:
+		summary = PolicyRequirementError("A signature was required, but no signature exists")
+	case 1:
+		summary = rejections[0]
+	default:
+		var msgs []string
+		for _, e := range rejections {
+			msgs = append(msgs, e.Error())
+		}
+		summary = PolicyRequirementError(fmt.Sprintf("None of the signatures were accepted, reasons: %s",
+			strings.Join(msgs, "; ")))
+	}
+	return false, summary
+}

signature/policy_eval_signedby_test.go

@@ -0,0 +1,239 @@
+package signature
+
+import (
+	"io/ioutil"
+	"os"
+	"path"
+	"testing"
+
+	"github.com/projectatomic/skopeo/directory"
+	"github.com/projectatomic/skopeo/docker"
+	"github.com/projectatomic/skopeo/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// dirImageMock returns a types.Image for a directory, claiming a specified intendedDockerReference.
+func dirImageMock(dir, intendedDockerReference string) types.Image {
+	return docker.GenericImageFromSource(&dirImageSourceMock{
+		ImageSource:             directory.NewDirImageSource(dir),
+		intendedDockerReference: intendedDockerReference,
+	})
+}
+
+// dirImageSourceMock inherits dirImageSource, but overrides its IntendedDockerReference method.
+type dirImageSourceMock struct {
+	types.ImageSource
+	intendedDockerReference string
+}
+
+func (d *dirImageSourceMock) IntendedDockerReference() string {
+	return d.intendedDockerReference
+}
+
+func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) {
+	ktGPG := SBKeyTypeGPGKeys
+	prm := NewPRMMatchExact()
+	testImage := dirImageMock("fixtures/dir-img-valid", "testing/manifest:latest")
+	testImageSig, err := ioutil.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(testImage, testImageSig)
+	assertSARAccepted(t, sar, parsedSig, err, Signature{
+		DockerManifestDigest: TestImageManifestDigest,
+		DockerReference:      "testing/manifest:latest",
+	})
+
+	keyData, err := ioutil.ReadFile("fixtures/public-key.gpg")
+	require.NoError(t, err)
+	pr, err = NewPRSignedByKeyData(ktGPG, keyData, prm)
+	require.NoError(t, err)
+	sar, parsedSig, err = pr.isSignatureAuthorAccepted(testImage, testImageSig)
+	assertSARAccepted(t, sar, parsedSig, err, Signature{
+		DockerManifestDigest: TestImageManifestDigest,
+		DockerReference:      "testing/manifest:latest",
+	})
+
+	// Unimplemented and invalid KeyType values
+	for _, keyType := range []sbKeyType{SBKeyTypeSignedByGPGKeys,
+		SBKeyTypeX509Certificates,
+		SBKeyTypeSignedByX509CAs,
+		sbKeyType("This is invalid"),
+	} {
+		// Do not use NewPRSignedByKeyData, because it would reject invalid values.
+		pr := &prSignedBy{
+			KeyType:        keyType,
+			KeyData:        []byte("abc"),
+			SignedIdentity: prm,
+		}
+		// Pass nil pointers to, kind of, test that the return value does not depend on the parameters.
+		sar, parsedSig, err := pr.isSignatureAuthorAccepted(nil, nil)
+		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,
+	}
+	// Pass nil pointers to, kind of, test that the return value does not depend on the parameters.
+	sar, parsedSig, err = prSB.isSignatureAuthorAccepted(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(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)
+	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(nil, nil)
+	assertSARRejectedPolicyRequirement(t, sar, parsedSig, err)
+
+	// A signature which does not GPG verify
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
+	require.NoError(t, err)
+	// Pass a nil pointer to, kind of, test that the return value does not depend on the image parmater..
+	sar, parsedSig, err = pr.isSignatureAuthorAccepted(nil, []byte("invalid signature"))
+	assertSARRejected(t, sar, parsedSig, err)
+
+	// A valid signature using an unknown key.
+	// (This is (currently?) rejected through the "mech.Verify fails" path, not the "!identityFound" path,
+	// because we use a temporary directory and only import the trusted keys.)
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
+	require.NoError(t, err)
+	sig, err := ioutil.ReadFile("fixtures/unknown-key.signature")
+	require.NoError(t, err)
+	// Pass a nil pointer to, kind of, test that the return value does not depend on the image parmater..
+	sar, parsedSig, err = pr.isSignatureAuthorAccepted(nil, sig)
+	assertSARRejected(t, sar, parsedSig, err)
+
+	// A valid signature of an invalid JSON.
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
+	require.NoError(t, err)
+	sig, err = ioutil.ReadFile("fixtures/invalid-blob.signature")
+	require.NoError(t, err)
+	// Pass a nil pointer to, kind of, test that the return value does not depend on the image parmater..
+	sar, parsedSig, err = pr.isSignatureAuthorAccepted(nil, sig)
+	assertSARRejected(t, sar, parsedSig, err)
+	assert.IsType(t, InvalidSignatureError{}, err)
+
+	// A valid signature with a rejected identity.
+	nonmatchingPRM, err := NewPRMExactReference("this/doesnt:match")
+	require.NoError(t, err)
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", nonmatchingPRM)
+	require.NoError(t, err)
+	sar, parsedSig, err = pr.isSignatureAuthorAccepted(testImage, testImageSig)
+	assertSARRejectedPolicyRequirement(t, sar, parsedSig, err)
+
+	// Error reading image manifest
+	image := dirImageMock("fixtures/dir-img-no-manifest", "testing/manifest:latest")
+	sig, err = ioutil.ReadFile("fixtures/dir-img-no-manifest/signature-1")
+	require.NoError(t, err)
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
+	require.NoError(t, err)
+	sar, parsedSig, err = pr.isSignatureAuthorAccepted(image, sig)
+	assertSARRejected(t, sar, parsedSig, err)
+
+	// Error computing manifest digest
+	image = dirImageMock("fixtures/dir-img-manifest-digest-error", "testing/manifest:latest")
+	sig, err = ioutil.ReadFile("fixtures/dir-img-manifest-digest-error/signature-1")
+	require.NoError(t, err)
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
+	require.NoError(t, err)
+	sar, parsedSig, err = pr.isSignatureAuthorAccepted(image, sig)
+	assertSARRejected(t, sar, parsedSig, err)
+
+	// A valid signature with a non-matching manifest
+	image = dirImageMock("fixtures/dir-img-modified-manifest", "testing/manifest:latest")
+	sig, err = ioutil.ReadFile("fixtures/dir-img-modified-manifest/signature-1")
+	require.NoError(t, err)
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
+	require.NoError(t, err)
+	sar, parsedSig, err = pr.isSignatureAuthorAccepted(image, sig)
+	assertSARRejectedPolicyRequirement(t, sar, parsedSig, err)
+}
+
+// createInvalidSigDir creates a directory suitable for dirImageMock, in which image.Signatures()
+// fails.
+// The caller should eventually call os.RemoveAll on the returned path.
+func createInvalidSigDir(t *testing.T) string {
+	dir, err := ioutil.TempDir("", "skopeo-test-unreadable-signature")
+	require.NoError(t, err)
+	err = ioutil.WriteFile(path.Join(dir, "manifest.json"), []byte("{}"), 0644)
+	require.NoError(t, err)
+	// Creating a 000-permissions file would work for unprivileged accounts, but root (in particular,
+	// in the Docker container we use for testing) would still have access.  So, create a symlink
+	// pointing to itself, to cause an ELOOP. (Note that a symlink pointing to a nonexistent file would be treated
+	// just like a nonexistent signature file, and not an error.)
+	err = os.Symlink("signature-1", path.Join(dir, "signature-1"))
+	require.NoError(t, err)
+	return dir
+}
+
+func TestPRSignedByIsRunningImageAllowed(t *testing.T) {
+	ktGPG := SBKeyTypeGPGKeys
+	prm := NewPRMMatchExact()
+
+	// A simple success case: single valid signature.
+	image := dirImageMock("fixtures/dir-img-valid", "testing/manifest:latest")
+	pr, err := NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
+	require.NoError(t, err)
+	allowed, err := pr.isRunningImageAllowed(image)
+	assertRunningAllowed(t, allowed, err)
+
+	// Error reading signatures
+	invalidSigDir := createInvalidSigDir(t)
+	defer os.RemoveAll(invalidSigDir)
+	image = dirImageMock(invalidSigDir, "testing/manifest:latest")
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
+	require.NoError(t, err)
+	allowed, err = pr.isRunningImageAllowed(image)
+	assertRunningRejected(t, allowed, err)
+
+	// No signatures
+	image = dirImageMock("fixtures/dir-img-unsigned", "testing/manifest:latest")
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
+	require.NoError(t, err)
+	allowed, err = pr.isRunningImageAllowed(image)
+	assertRunningRejectedPolicyRequirement(t, allowed, err)
+
+	// 1 invalid signature: use dir-img-valid, but a non-matching Docker reference
+	image = dirImageMock("fixtures/dir-img-valid", "testing/manifest:notlatest")
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
+	require.NoError(t, err)
+	allowed, err = pr.isRunningImageAllowed(image)
+	assertRunningRejectedPolicyRequirement(t, allowed, err)
+
+	// 2 valid signatures
+	image = dirImageMock("fixtures/dir-img-valid-2", "testing/manifest:latest")
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
+	require.NoError(t, err)
+	allowed, err = pr.isRunningImageAllowed(image)
+	assertRunningAllowed(t, allowed, err)
+
+	// One invalid, one valid signature (in this order)
+	image = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:latest")
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
+	require.NoError(t, err)
+	allowed, err = pr.isRunningImageAllowed(image)
+	assertRunningAllowed(t, allowed, err)
+
+	// 2 invalid signatures: use dir-img-valid-2, but a non-matching Docker reference
+	image = dirImageMock("fixtures/dir-img-valid-2", "testing/manifest:notlatest")
+	pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key.gpg", prm)
+	require.NoError(t, err)
+	allowed, err = pr.isRunningImageAllowed(image)
+	assertRunningRejectedPolicyRequirement(t, allowed, err)
+}

signature/policy_eval_simple.go

@@ -0,0 +1,25 @@
+// Policy evaluation for the various simple PolicyRequirement types.
+
+package signature
+
+import "github.com/projectatomic/skopeo/types"
+
+func (pr *prInsecureAcceptAnything) isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error) {
+	// prInsecureAcceptAnything semantics: Every image is allowed to run,
+	// but this does not consider the signature as verified.
+	return sarUnknown, nil, nil
+}
+
+func (pr *prInsecureAcceptAnything) isRunningImageAllowed(image types.Image) (bool, error) {
+	return true, nil
+}
+
+func (pr *prReject) isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error) {
+	// FIXME? Name the image, or better the matched scope in Policy.Specific.
+	return sarRejected, nil, PolicyRequirementError("Any signatures for these images are rejected by policy.")
+}
+
+func (pr *prReject) isRunningImageAllowed(image types.Image) (bool, error) {
+	// FIXME? Name the image, or better the matched scope in Policy.Specific.
+	return false, PolicyRequirementError("Running these images is rejected by policy.")
+}

signature/policy_eval_simple_test.go

@@ -0,0 +1,32 @@
+package signature
+
+import "testing"
+
+func TestPRInsecureAcceptAnythingIsSignatureAuthorAccepted(t *testing.T) {
+	pr := NewPRInsecureAcceptAnything()
+	// Pass nil pointers to, kind of, test that the return value does not depend on the parameters.
+	sar, parsedSig, err := pr.isSignatureAuthorAccepted(nil, nil)
+	assertSARUnknown(t, sar, parsedSig, err)
+}
+
+func TestPRInsecureAcceptAnythingIsRunningImageAllowed(t *testing.T) {
+	pr := NewPRInsecureAcceptAnything()
+	// Pass a nil pointer to, kind of, test that the return value does not depend on the image.
+	res, err := pr.isRunningImageAllowed(nil)
+	assertRunningAllowed(t, res, err)
+}
+
+func TestPRRejectIsSignatureAuthorAccepted(t *testing.T) {
+	pr := NewPRReject()
+	// Pass nil pointers to, kind of, test that the return value does not depend on the parameters.
+	sar, parsedSig, err := pr.isSignatureAuthorAccepted(nil, nil)
+	assertSARRejectedPolicyRequirement(t, sar, parsedSig, err)
+}
+
+func TestPRRejectIsRunningImageAllowed(t *testing.T) {
+	// This will obviously need to change after this is implemented.
+	pr := NewPRReject()
+	// Pass a nil pointer to, kind of, test that the return value does not depend on the image.
+	res, err := pr.isRunningImageAllowed(nil)
+	assertRunningRejectedPolicyRequirement(t, res, err)
+}

signature/policy_eval_test.go

@@ -0,0 +1,504 @@
+package signature
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/projectatomic/skopeo/reference"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPolicyRequirementError(t *testing.T) {
+	// A stupid test just to keep code coverage
+	s := "test"
+	err := PolicyRequirementError(s)
+	assert.Equal(t, s, err.Error())
+}
+
+func TestPolicyContextChangeState(t *testing.T) {
+	pc, err := NewPolicyContext(&Policy{Default: PolicyRequirements{NewPRReject()}})
+	require.NoError(t, err)
+	defer pc.Destroy()
+
+	require.Equal(t, pcReady, pc.state)
+	err = pc.changeState(pcReady, pcInUse)
+	require.NoError(t, err)
+
+	err = pc.changeState(pcReady, pcInUse)
+	require.Error(t, err)
+
+	// Return state to pcReady to allow pc.Destroy to clean up.
+	err = pc.changeState(pcInUse, pcReady)
+	require.NoError(t, err)
+}
+
+func TestPolicyContextNewDestroy(t *testing.T) {
+	pc, err := NewPolicyContext(&Policy{Default: PolicyRequirements{NewPRReject()}})
+	require.NoError(t, err)
+	assert.Equal(t, pcReady, pc.state)
+
+	err = pc.Destroy()
+	require.NoError(t, err)
+	assert.Equal(t, pcDestroyed, pc.state)
+
+	// Trying to destroy when not pcReady
+	pc, err = NewPolicyContext(&Policy{Default: PolicyRequirements{NewPRReject()}})
+	require.NoError(t, err)
+	err = pc.changeState(pcReady, pcInUse)
+	require.NoError(t, err)
+	err = pc.Destroy()
+	require.Error(t, err)
+	assert.Equal(t, pcInUse, pc.state) // The state, and hopefully nothing else, has changed.
+
+	err = pc.changeState(pcInUse, pcReady)
+	require.NoError(t, err)
+	err = pc.Destroy()
+	assert.NoError(t, err)
+}
+
+func TestFullyExpandedDockerReference(t *testing.T) {
+	sha256Digest := "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
+	// Test both that fullyExpandedDockerReference returns the expected value (fullName+suffix),
+	// and that .FullName returns the expected value (fullName), i.e. that the two functions are
+	// consistent.
+	for inputName, fullName := range map[string]string{
+		"example.com/ns/repo": "example.com/ns/repo",
+		"example.com/repo":    "example.com/repo",
+		"localhost/ns/repo":   "localhost/ns/repo",
+		// Note that "localhost" is special here: notlocalhost/repo is be parsed as docker.io/notlocalhost.repo:
+		"localhost/repo":         "localhost/repo",
+		"notlocalhost/repo":      "docker.io/notlocalhost/repo",
+		"docker.io/ns/repo":      "docker.io/ns/repo",
+		"docker.io/library/repo": "docker.io/library/repo",
+		"docker.io/repo":         "docker.io/library/repo",
+		"ns/repo":                "docker.io/ns/repo",
+		"library/repo":           "docker.io/library/repo",
+		"repo":                   "docker.io/library/repo",
+	} {
+		for inputSuffix, mappedSuffix := range map[string]string{
+			":tag":       ":tag",
+			sha256Digest: sha256Digest,
+			"":           "",
+			// A github.com/distribution/reference value can have a tag and a digest at the same time!
+			// github.com/skopeo/reference handles that by dropping the tag. That is not obviously the
+			// right thing to do, but it is at least reasonable, so test that we keep behaving reasonably.
+			// This test case should not be construed to make this an API promise.
+			":tag" + sha256Digest: sha256Digest,
+		} {
+			fullInput := inputName + inputSuffix
+			ref, err := reference.ParseNamed(fullInput)
+			require.NoError(t, err)
+			assert.Equal(t, fullName, ref.FullName(), fullInput)
+			expanded, err := fullyExpandedDockerReference(ref)
+			require.NoError(t, err)
+			assert.Equal(t, fullName+mappedSuffix, expanded, fullInput)
+		}
+	}
+}
+
+func TestPolicyContextRequirementsForImage(t *testing.T) {
+	ktGPG := SBKeyTypeGPGKeys
+	prm := NewPRMMatchExact()
+
+	policy := &Policy{
+		Default:  PolicyRequirements{NewPRReject()},
+		Specific: map[string]PolicyRequirements{},
+	}
+	// Just put _something_ into the Specific map for the keys we care about, and make it pairwise
+	// distinct so that we can compare the values and show them when debugging the tests.
+	for _, scope := range []string{
+		"unmatched",
+		"hostname.com",
+		"hostname.com/namespace",
+		"hostname.com/namespace/repo",
+		"hostname.com/namespace/repo:latest",
+		"hostname.com/namespace/repo:tag2",
+		"localhost",
+		"localhost/namespace",
+		"localhost/namespace/repo",
+		"localhost/namespace/repo:latest",
+		"localhost/namespace/repo:tag2",
+		"deep.com",
+		"deep.com/n1",
+		"deep.com/n1/n2",
+		"deep.com/n1/n2/n3",
+		"deep.com/n1/n2/n3/repo",
+		"deep.com/n1/n2/n3/repo:tag2",
+		"docker.io",
+		"docker.io/library",
+		"docker.io/library/busybox",
+		"docker.io/namespaceindocker",
+		"docker.io/namespaceindocker/repo",
+		"docker.io/namespaceindocker/repo:tag2",
+		// Note: these non-fully-expanded repository names are not matched against canonical (shortened)
+		// Docker names; they are instead parsed as starting with hostnames.
+		"busybox",
+		"library/busybox",
+		"namespaceindocker",
+		"namespaceindocker/repo",
+		"namespaceindocker/repo:tag2",
+	} {
+		policy.Specific[scope] = PolicyRequirements{xNewPRSignedByKeyData(ktGPG, []byte(scope), prm)}
+	}
+
+	pc, err := NewPolicyContext(policy)
+	require.NoError(t, err)
+
+	for input, matched := range map[string]string{
+		// Full match
+		"hostname.com/namespace/repo:latest": "hostname.com/namespace/repo:latest",
+		"hostname.com/namespace/repo:tag2":   "hostname.com/namespace/repo:tag2",
+		"hostname.com/namespace/repo":        "hostname.com/namespace/repo:latest",
+		"localhost/namespace/repo:latest":    "localhost/namespace/repo:latest",
+		"localhost/namespace/repo:tag2":      "localhost/namespace/repo:tag2",
+		"localhost/namespace/repo":           "localhost/namespace/repo:latest",
+		"deep.com/n1/n2/n3/repo:tag2":        "deep.com/n1/n2/n3/repo:tag2",
+		// Repository match
+		"hostname.com/namespace/repo:notlatest": "hostname.com/namespace/repo",
+		"localhost/namespace/repo:notlatest":    "localhost/namespace/repo",
+		"deep.com/n1/n2/n3/repo:nottag2":        "deep.com/n1/n2/n3/repo",
+		// Namespace match
+		"hostname.com/namespace/notrepo:latest": "hostname.com/namespace",
+		"localhost/namespace/notrepo:latest":    "localhost/namespace",
+		"deep.com/n1/n2/n3/notrepo:tag2":        "deep.com/n1/n2/n3",
+		"deep.com/n1/n2/notn3/repo:tag2":        "deep.com/n1/n2",
+		"deep.com/n1/notn2/n3/repo:tag2":        "deep.com/n1",
+		// Host name match
+		"hostname.com/notnamespace/repo:latest": "hostname.com",
+		"localhost/notnamespace/repo:latest":    "localhost",
+		"deep.com/notn1/n2/n3/repo:tag2":        "deep.com",
+		// Default
+		"this.doesnt/match:anything":            "",
+		"this.doesnt/match-anything/defaulttag": "",
+
+		// docker.io canonizalication effects
+		"docker.io/library/busybox": "docker.io/library/busybox",
+		"library/busybox":           "docker.io/library/busybox",
+		"busybox":                   "docker.io/library/busybox",
+		"docker.io/library/somethinginlibrary":     "docker.io/library",
+		"library/somethinginlibrary":               "docker.io/library",
+		"somethinginlibrary":                       "docker.io/library",
+		"docker.io/namespaceindocker/repo:tag2":    "docker.io/namespaceindocker/repo:tag2",
+		"namespaceindocker/repo:tag2":              "docker.io/namespaceindocker/repo:tag2",
+		"docker.io/namespaceindocker/repo:nottag2": "docker.io/namespaceindocker/repo",
+		"namespaceindocker/repo:nottag2":           "docker.io/namespaceindocker/repo",
+		"docker.io/namespaceindocker/notrepo:tag2": "docker.io/namespaceindocker",
+		"namespaceindocker/notrepo:tag2":           "docker.io/namespaceindocker",
+		"docker.io/notnamespaceindocker/repo:tag2": "docker.io",
+		"notnamespaceindocker/repo:tag2":           "docker.io",
+	} {
+		var expected PolicyRequirements
+		if matched != "" {
+			e, ok := policy.Specific[matched]
+			require.True(t, ok, fmt.Sprintf("case %s: expected reqs not found", input))
+			expected = e
+		} else {
+			expected = policy.Default
+		}
+
+		reqs, err := pc.requirementsForImage(refImageMock(input))
+		require.NoError(t, err)
+		comment := fmt.Sprintf("case %s: %#v", input, reqs[0])
+		// Do not sue assert.Equal, which would do a deep contents comparison; we want to compare
+		// the pointers. Also, == does not work on slices; so test that the slices start at the
+		// same element and have the same length.
+		assert.True(t, &(reqs[0]) == &(expected[0]), comment)
+		assert.True(t, len(reqs) == len(expected), comment)
+	}
+
+	// Invalid reference format
+	_, err = pc.requirementsForImage(refImageMock("UPPERCASEISINVALID"))
+	assert.Error(t, err)
+}
+
+func TestPolicyContextGetSignaturesWithAcceptedAuthor(t *testing.T) {
+	expectedSig := &Signature{
+		DockerManifestDigest: TestImageManifestDigest,
+		DockerReference:      "testing/manifest:latest",
+	}
+
+	pc, err := NewPolicyContext(&Policy{
+		Default: PolicyRequirements{NewPRReject()},
+		Specific: map[string]PolicyRequirements{
+			"docker.io/testing/manifest:latest": {
+				xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "fixtures/public-key.gpg", NewPRMMatchExact()),
+			},
+			"docker.io/testing/manifest:twoAccepts": {
+				xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "fixtures/public-key.gpg", NewPRMMatchRepository()),
+				xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "fixtures/public-key.gpg", NewPRMMatchRepository()),
+			},
+			"docker.io/testing/manifest:acceptReject": {
+				xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "fixtures/public-key.gpg", NewPRMMatchRepository()),
+				NewPRReject(),
+			},
+			"docker.io/testing/manifest:acceptUnknown": {
+				xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "fixtures/public-key.gpg", NewPRMMatchRepository()),
+				xNewPRSignedBaseLayer(NewPRMMatchRepository()),
+			},
+			"docker.io/testing/manifest:rejectUnknown": {
+				NewPRReject(),
+				xNewPRSignedBaseLayer(NewPRMMatchRepository()),
+			},
+			"docker.io/testing/manifest:unknown": {
+				xNewPRSignedBaseLayer(NewPRMMatchRepository()),
+			},
+			"docker.io/testing/manifest:unknown2": {
+				NewPRInsecureAcceptAnything(),
+			},
+			"docker.io/testing/manifest:invalidEmptyRequirements": {},
+		},
+	})
+	require.NoError(t, err)
+	defer pc.Destroy()
+
+	// Success
+	image := dirImageMock("fixtures/dir-img-valid", "testing/manifest:latest")
+	sigs, err := pc.GetSignaturesWithAcceptedAuthor(image)
+	require.NoError(t, err)
+	assert.Equal(t, []*Signature{expectedSig}, sigs)
+
+	// Two signatures
+	// FIXME? Use really different signatures for this?
+	image = dirImageMock("fixtures/dir-img-valid-2", "testing/manifest:latest")
+	sigs, err = pc.GetSignaturesWithAcceptedAuthor(image)
+	require.NoError(t, err)
+	assert.Equal(t, []*Signature{expectedSig, expectedSig}, sigs)
+
+	// No signatures
+	image = dirImageMock("fixtures/dir-img-unsigned", "testing/manifest:latest")
+	sigs, err = pc.GetSignaturesWithAcceptedAuthor(image)
+	require.NoError(t, err)
+	assert.Empty(t, sigs)
+
+	// Only invalid signatures
+	image = dirImageMock("fixtures/dir-img-modified-manifest", "testing/manifest:latest")
+	sigs, err = pc.GetSignaturesWithAcceptedAuthor(image)
+	require.NoError(t, err)
+	assert.Empty(t, sigs)
+
+	// 1 invalid, 1 valid signature (in this order)
+	image = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:latest")
+	sigs, err = pc.GetSignaturesWithAcceptedAuthor(image)
+	require.NoError(t, err)
+	assert.Equal(t, []*Signature{expectedSig}, sigs)
+
+	// Two sarAccepted results for one signature
+	image = dirImageMock("fixtures/dir-img-valid", "testing/manifest:twoAccepts")
+	sigs, err = pc.GetSignaturesWithAcceptedAuthor(image)
+	require.NoError(t, err)
+	assert.Equal(t, []*Signature{expectedSig}, sigs)
+
+	// sarAccepted+sarRejected for a signature
+	image = dirImageMock("fixtures/dir-img-valid", "testing/manifest:acceptReject")
+	sigs, err = pc.GetSignaturesWithAcceptedAuthor(image)
+	require.NoError(t, err)
+	assert.Empty(t, sigs)
+
+	// sarAccepted+sarUnknown for a signature
+	image = dirImageMock("fixtures/dir-img-valid", "testing/manifest:acceptUnknown")
+	sigs, err = pc.GetSignaturesWithAcceptedAuthor(image)
+	require.NoError(t, err)
+	assert.Equal(t, []*Signature{expectedSig}, sigs)
+
+	// sarRejected+sarUnknown for a signature
+	image = dirImageMock("fixtures/dir-img-valid", "testing/manifest:rejectUnknown")
+	sigs, err = pc.GetSignaturesWithAcceptedAuthor(image)
+	require.NoError(t, err)
+	assert.Empty(t, sigs)
+
+	// sarUnknown only
+	image = dirImageMock("fixtures/dir-img-valid", "testing/manifest:unknown")
+	sigs, err = pc.GetSignaturesWithAcceptedAuthor(image)
+	require.NoError(t, err)
+	assert.Empty(t, sigs)
+
+	image = dirImageMock("fixtures/dir-img-valid", "testing/manifest:unknown2")
+	sigs, err = pc.GetSignaturesWithAcceptedAuthor(image)
+	require.NoError(t, err)
+	assert.Empty(t, sigs)
+
+	// Empty list of requirements (invalid)
+	image = dirImageMock("fixtures/dir-img-valid", "testing/manifest:invalidEmptyRequirements")
+	sigs, err = pc.GetSignaturesWithAcceptedAuthor(image)
+	require.NoError(t, err)
+	assert.Empty(t, sigs)
+
+	// Failures: Make sure we return nil sigs.
+
+	// Unexpected state (context already destroyed)
+	destroyedPC, err := NewPolicyContext(pc.Policy)
+	require.NoError(t, err)
+	err = destroyedPC.Destroy()
+	require.NoError(t, err)
+	image = dirImageMock("fixtures/dir-img-valid", "testing/manifest:latest")
+	sigs, err = destroyedPC.GetSignaturesWithAcceptedAuthor(image)
+	assert.Error(t, err)
+	assert.Nil(t, sigs)
+	// Not testing the pcInUse->pcReady transition, that would require custom PolicyRequirement
+	// implementations meddling with the state, or threads. This is for catching trivial programmer
+	// mistakes only, anyway.
+
+	// Invalid IntendedDockerReference value
+	image = dirImageMock("fixtures/dir-img-valid", "UPPERCASEISINVALID")
+	sigs, err = pc.GetSignaturesWithAcceptedAuthor(image)
+	assert.Error(t, err)
+	assert.Nil(t, sigs)
+
+	// Error reading signatures.
+	invalidSigDir := createInvalidSigDir(t)
+	defer os.RemoveAll(invalidSigDir)
+	image = dirImageMock(invalidSigDir, "testing/manifest:latest")
+	sigs, err = pc.GetSignaturesWithAcceptedAuthor(image)
+	assert.Error(t, err)
+	assert.Nil(t, sigs)
+}
+
+func TestPolicyContextIsRunningImageAllowed(t *testing.T) {
+	pc, err := NewPolicyContext(&Policy{
+		Default: PolicyRequirements{NewPRReject()},
+		Specific: map[string]PolicyRequirements{
+			"docker.io/testing/manifest:latest": {
+				xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "fixtures/public-key.gpg", NewPRMMatchExact()),
+			},
+			"docker.io/testing/manifest:twoAllows": {
+				xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "fixtures/public-key.gpg", NewPRMMatchRepository()),
+				xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "fixtures/public-key.gpg", NewPRMMatchRepository()),
+			},
+			"docker.io/testing/manifest:allowDeny": {
+				xNewPRSignedByKeyPath(SBKeyTypeGPGKeys, "fixtures/public-key.gpg", NewPRMMatchRepository()),
+				NewPRReject(),
+			},
+			"docker.io/testing/manifest:reject": {
+				NewPRReject(),
+			},
+			"docker.io/testing/manifest:acceptAnything": {
+				NewPRInsecureAcceptAnything(),
+			},
+			"docker.io/testing/manifest:invalidEmptyRequirements": {},
+		},
+	})
+	require.NoError(t, err)
+	defer pc.Destroy()
+
+	// Success
+	image := dirImageMock("fixtures/dir-img-valid", "testing/manifest:latest")
+	res, err := pc.IsRunningImageAllowed(image)
+	assertRunningAllowed(t, res, err)
+
+	// Two signatures
+	// FIXME? Use really different signatures for this?
+	image = dirImageMock("fixtures/dir-img-valid-2", "testing/manifest:latest")
+	res, err = pc.IsRunningImageAllowed(image)
+	assertRunningAllowed(t, res, err)
+
+	// No signatures
+	image = dirImageMock("fixtures/dir-img-unsigned", "testing/manifest:latest")
+	res, err = pc.IsRunningImageAllowed(image)
+	assertRunningRejectedPolicyRequirement(t, res, err)
+
+	// Only invalid signatures
+	image = dirImageMock("fixtures/dir-img-modified-manifest", "testing/manifest:latest")
+	res, err = pc.IsRunningImageAllowed(image)
+	assertRunningRejectedPolicyRequirement(t, res, err)
+
+	// 1 invalid, 1 valid signature (in this order)
+	image = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:latest")
+	res, err = pc.IsRunningImageAllowed(image)
+	assertRunningAllowed(t, res, err)
+
+	// Two allowed results
+	image = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:twoAllows")
+	res, err = pc.IsRunningImageAllowed(image)
+	assertRunningAllowed(t, res, err)
+
+	// Allow + deny results
+	image = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:allowDeny")
+	res, err = pc.IsRunningImageAllowed(image)
+	assertRunningRejectedPolicyRequirement(t, res, err)
+
+	// prReject works
+	image = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:reject")
+	res, err = pc.IsRunningImageAllowed(image)
+	assertRunningRejectedPolicyRequirement(t, res, err)
+
+	// prInsecureAcceptAnything works
+	image = dirImageMock("fixtures/dir-img-mixed", "testing/manifest:acceptAnything")
+	res, err = pc.IsRunningImageAllowed(image)
+	assertRunningAllowed(t, res, err)
+
+	// Empty list of requirements (invalid)
+	image = dirImageMock("fixtures/dir-img-valid", "testing/manifest:invalidEmptyRequirements")
+	res, err = pc.IsRunningImageAllowed(image)
+	assertRunningRejectedPolicyRequirement(t, res, err)
+
+	// Unexpected state (context already destroyed)
+	destroyedPC, err := NewPolicyContext(pc.Policy)
+	require.NoError(t, err)
+	err = destroyedPC.Destroy()
+	require.NoError(t, err)
+	image = dirImageMock("fixtures/dir-img-valid", "testing/manifest:latest")
+	res, err = destroyedPC.IsRunningImageAllowed(image)
+	assertRunningRejected(t, res, err)
+	// Not testing the pcInUse->pcReady transition, that would require custom PolicyRequirement
+	// implementations meddling with the state, or threads. This is for catching trivial programmer
+	// mistakes only, anyway.
+
+	// Invalid IntendedDockerReference value
+	image = dirImageMock("fixtures/dir-img-valid", "UPPERCASEISINVALID")
+	res, err = pc.IsRunningImageAllowed(image)
+	assertRunningRejected(t, res, err)
+}
+
+// Helpers for validating PolicyRequirement.isSignatureAuthorAccepted results:
+
+// assertSARRejected verifies that isSignatureAuthorAccepted returns a consistent sarRejected result
+// with the expected signature.
+func assertSARAccepted(t *testing.T, sar signatureAcceptanceResult, parsedSig *Signature, err error, expectedSig Signature) {
+	assert.Equal(t, sarAccepted, sar)
+	assert.Equal(t, &expectedSig, parsedSig)
+	assert.NoError(t, err)
+}
+
+// assertSARRejected verifies that isSignatureAuthorAccepted returns a consistent sarRejected result.
+func assertSARRejected(t *testing.T, sar signatureAcceptanceResult, parsedSig *Signature, err error) {
+	assert.Equal(t, sarRejected, sar)
+	assert.Nil(t, parsedSig)
+	assert.Error(t, err)
+}
+
+// assertSARRejectedPolicyRequiremnt verifies that isSignatureAuthorAccepted returns a consistent sarRejected resul,
+// and that the returned error is a PolicyRequirementError..
+func assertSARRejectedPolicyRequirement(t *testing.T, sar signatureAcceptanceResult, parsedSig *Signature, err error) {
+	assertSARRejected(t, sar, parsedSig, err)
+	assert.IsType(t, PolicyRequirementError(""), err)
+}
+
+// assertSARRejected verifies that isSignatureAuthorAccepted returns a consistent sarUnknown result.
+func assertSARUnknown(t *testing.T, sar signatureAcceptanceResult, parsedSig *Signature, err error) {
+	assert.Equal(t, sarUnknown, sar)
+	assert.Nil(t, parsedSig)
+	assert.NoError(t, err)
+}
+
+// Helpers for validating PolicyRequirement.isRunningImageAllowed results:
+
+// assertRunningAllowed verifies that isRunningImageAllowed returns a consistent true result
+func assertRunningAllowed(t *testing.T, allowed bool, err error) {
+	assert.Equal(t, true, allowed)
+	assert.NoError(t, err)
+}
+
+// assertRunningRejected verifies that isRunningImageAllowed returns a consistent false result
+func assertRunningRejected(t *testing.T, allowed bool, err error) {
+	assert.Equal(t, false, allowed)
+	assert.Error(t, err)
+}
+
+// assertRunningRejectedPolicyRequirement verifies that isRunningImageAllowed returns a consistent false result
+// and that the returned error is a PolicyRequirementError.
+func assertRunningRejectedPolicyRequirement(t *testing.T, allowed bool, err error) {
+	assertRunningRejected(t, allowed, err)
+	assert.IsType(t, PolicyRequirementError(""), err)
+}

signature/policy_reference_match.go

@@ -0,0 +1,61 @@
+// PolicyReferenceMatch implementations.
+
+package signature
+
+import (
+	"github.com/projectatomic/skopeo/reference"
+	"github.com/projectatomic/skopeo/types"
+)
+
+// parseDockerReferences converts two reference strings into parsed entities, failing on any error
+func parseDockerReferences(s1, s2 string) (reference.Named, reference.Named, error) {
+	r1, err := reference.ParseNamed(s1)
+	if err != nil {
+		return nil, nil, err
+	}
+	r2, err := reference.ParseNamed(s2)
+	if err != nil {
+		return nil, nil, err
+	}
+	return r1, r2, nil
+}
+
+func (prm *prmMatchExact) matchesDockerReference(image types.Image, signatureDockerReference string) bool {
+	intended, signature, err := parseDockerReferences(image.IntendedDockerReference(), signatureDockerReference)
+	if err != nil {
+		return false
+	}
+	// Do not add default tags: image.IntendedDockerReference() has it added already per its construction, and signatureDockerReference should be exact; so, verify that now.
+	if reference.IsNameOnly(intended) || reference.IsNameOnly(signature) {
+		return false
+	}
+	return signature.String() == intended.String()
+}
+
+func (prm *prmMatchRepository) matchesDockerReference(image types.Image, signatureDockerReference string) bool {
+	intended, signature, err := parseDockerReferences(image.IntendedDockerReference(), signatureDockerReference)
+	if err != nil {
+		return false
+	}
+	return signature.Name() == intended.Name()
+}
+
+func (prm *prmExactReference) matchesDockerReference(image types.Image, signatureDockerReference string) bool {
+	intended, signature, err := parseDockerReferences(prm.DockerReference, signatureDockerReference)
+	if err != nil {
+		return false
+	}
+	// prm.DockerReference and signatureDockerReference should be exact; so, verify that now.
+	if reference.IsNameOnly(intended) || reference.IsNameOnly(signature) {
+		return false
+	}
+	return signature.String() == intended.String()
+}
+
+func (prm *prmExactRepository) matchesDockerReference(image types.Image, signatureDockerReference string) bool {
+	intended, signature, err := parseDockerReferences(prm.DockerRepository, signatureDockerReference)
+	if err != nil {
+		return false
+	}
+	return signature.Name() == intended.Name()
+}

signature/policy_reference_match_test.go

@@ -0,0 +1,202 @@
+package signature
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/projectatomic/skopeo/types"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	fullRHELRef     = "registry.access.redhat.com/rhel7/rhel:7.2.3"
+	untaggedRHELRef = "registry.access.redhat.com/rhel7/rhel"
+)
+
+func TestParseDockerReferences(t *testing.T) {
+	const (
+		ok1  = "busybox"
+		ok2  = fullRHELRef
+		bad1 = "UPPERCASE_IS_INVALID_IN_DOCKER_REFERENCES"
+		bad2 = ""
+	)
+
+	// Success
+	r1, r2, err := parseDockerReferences(ok1, ok2)
+	require.NoError(t, err)
+	assert.Equal(t, ok1, r1.String())
+	assert.Equal(t, ok2, r2.String())
+
+	// Failures
+	for _, refs := range [][]string{
+		{bad1, ok2},
+		{ok1, bad2},
+		{bad1, bad2},
+	} {
+		_, _, err := parseDockerReferences(refs[0], refs[1])
+		assert.Error(t, err)
+	}
+}
+
+// refImageMock is a mock of types.Image which returns itself in IntendedDockerReference.
+type refImageMock string
+
+func (ref refImageMock) IntendedDockerReference() string {
+	return string(ref)
+}
+func (ref refImageMock) Manifest() ([]byte, error) {
+	panic("unexpected call to a mock function")
+}
+func (ref refImageMock) ManifestMatchesDigest(expectedDigest string) (bool, error) {
+	panic("unexpected call to a mock function")
+}
+func (ref refImageMock) Signatures() ([][]byte, error) {
+	panic("unexpected call to a mock function")
+}
+func (ref refImageMock) Layers(layers ...string) error {
+	panic("unexpected call to a mock function")
+}
+func (ref refImageMock) Inspect() (*types.ImageInspectInfo, error) {
+	panic("unexpected call to a mock function")
+}
+func (ref refImageMock) DockerTar() ([]byte, error) {
+	panic("unexpected call to a mock function")
+}
+func (ref refImageMock) GetRepositoryTags() ([]string, error) {
+	panic("unexpected call to a mock function")
+}
+
+type prmTableTest struct {
+	imageRef, sigRef string
+	result           bool
+}
+
+// Test cases for exact reference match
+var prmExactMatchTestTable = []prmTableTest{
+	// 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},
+	{"busybox:latest", "hostname/library/busybox:notlatest", false},
+	{"hostname/library/busybox:latest", "busybox:notlatest", false},
+	{"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{
+	// 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},
+	{"library/busybox:latest", "busybox:notlatest", true},
+	{"busybox:latest", "library/busybox:notlatest", true},
+	{"docker.io/library/busybox:notlatest", "busybox:latest", true},
+	{"busybox:notlatest", "docker.io/library/busybox:latest", true},
+	// The same as above, but with defaulted tags (should not actually happen)
+	{"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 TestPRMMatchExactMatchesDockerReference(t *testing.T) {
+	prm := NewPRMMatchExact()
+	for _, test := range prmExactMatchTestTable {
+		res := prm.matchesDockerReference(refImageMock(test.imageRef), test.sigRef)
+		assert.Equal(t, test.result, res, fmt.Sprintf("%s vs. %s", test.imageRef, test.sigRef))
+	}
+}
+
+func TestPRMMatchRepositoryMatchesDockerReference(t *testing.T) {
+	prm := NewPRMMatchRepository()
+	for _, test := range prmRepositoryMatchTestTable {
+		res := prm.matchesDockerReference(refImageMock(test.imageRef), test.sigRef)
+		assert.Equal(t, test.result, res, fmt.Sprintf("%s vs. %s", test.imageRef, test.sigRef))
+	}
+}
+
+// forbiddenImageMock is a mock of types.Image which ensures IntendedDockerReference is not called
+type forbiddenImageMock string
+
+func (ref forbiddenImageMock) IntendedDockerReference() string {
+	panic("unexpected call to a mock function")
+}
+func (ref forbiddenImageMock) Manifest() ([]byte, error) {
+	panic("unexpected call to a mock function")
+}
+func (ref forbiddenImageMock) ManifestMatchesDigest(expectedDigest string) (bool, error) {
+	panic("unexpected call to a mock function")
+}
+func (ref forbiddenImageMock) Signatures() ([][]byte, error) {
+	panic("unexpected call to a mock function")
+}
+func (ref forbiddenImageMock) Layers(layers ...string) error {
+	panic("unexpected call to a mock function")
+}
+func (ref forbiddenImageMock) Inspect() (*types.ImageInspectInfo, error) {
+	panic("unexpected call to a mock function")
+}
+func (ref forbiddenImageMock) DockerTar() ([]byte, error) {
+	panic("unexpected call to a mock function")
+}
+func (ref forbiddenImageMock) GetRepositoryTags() ([]string, error) {
+	panic("unexpected call to a mock function")
+}
+
+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))
+	}
+}
+
+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))
+	}
+}

signature/policy_types.go

@@ -11,7 +11,9 @@ type Policy struct {
 	// Default applies to any image which does not have a matching policy in Specific.
 	Default PolicyRequirements `json:"default"`
 	// Specific applies to images matching scope, the map key.
-	// Scope is registry/server, namespace in a registry, single repository.
+	// Scope is hostname[/zero/or/more/namespaces[/repository[:tag|@digest]]]; note that in order to be
+	// unambiguous, this must use a fully expanded format, e.g. "docker.io/library/busybox" or
+	// "docker.io/library", not "busybox" or "library".
 	// FIXME: Scope syntax - should it be namespaced docker:something ? Or, in the worst case, a composite object (we couldn't use a JSON map)
 	// Most specific scope wins, duplication is prohibited (hard failure).
 	// Defaults to an empty map if not specified.
@@ -24,7 +26,6 @@ type PolicyRequirements []PolicyRequirement
 
 // PolicyRequirement is a rule which must be satisfied by at least one of the signatures of an image.
 // The type is public, but its definition is private.
-type PolicyRequirement interface{} // Will be expanded and moved elsewhere later.
 
 // prCommon is the common type field in a JSON encoding of PolicyRequirement.
 type prCommon struct {
@@ -99,7 +100,6 @@ type prSignedBaseLayer struct {
 
 // PolicyReferenceMatch specifies a set of image identities accepted in PolicyRequirement.
 // The type is public, but its implementation is private.
-type PolicyReferenceMatch interface{} // Will be expanded and moved elsewhere later.
 
 // prmCommon is the common type field in a JSON encoding of PolicyReferenceMatch.
 type prmCommon struct {

signature/signature.go

@@ -155,26 +155,37 @@ func (s privateSignature) sign(mech SigningMechanism, keyIdentity string) ([]byt
 	return mech.Sign(json, keyIdentity)
 }
 
-// verifyAndExtractSignature verifies that signature has been signed by expectedKeyIdentity
-// using mech for expectedDockerReference, and returns it (without matching its contents to an image).
-func verifyAndExtractSignature(mech SigningMechanism, unverifiedSignature []byte,
-	expectedKeyIdentity, expectedDockerReference string) (*Signature, error) {
+// signatureAcceptanceRules specifies how to decide whether an untrusted signature is acceptable.
+// We centralize the actual parsing and data extraction in verifyAndExtractSignature; this supplies
+// the policy.  We use an object instead of supplying func parameters to verifyAndExtractSignature
+// because all of the functions have the same type, so there is a risk of exchanging the functions;
+// named members of this struct are more explicit.
+type signatureAcceptanceRules struct {
+	validateKeyIdentity                func(string) error
+	validateSignedDockerReference      func(string) error
+	validateSignedDockerManifestDigest func(string) error
+}
+
+// verifyAndExtractSignature verifies that unverifiedSignature has been signed, and that its principial components
+// match expected values, both as specified by rules, and returns it
+func verifyAndExtractSignature(mech SigningMechanism, unverifiedSignature []byte, rules signatureAcceptanceRules) (*Signature, error) {
 	signed, keyIdentity, err := mech.Verify(unverifiedSignature)
 	if err != nil {
 		return nil, err
 	}
-	if keyIdentity != expectedKeyIdentity {
-		return nil, InvalidSignatureError{msg: fmt.Sprintf("Signature by %s does not match expected fingerprint %s", keyIdentity, expectedKeyIdentity)}
+	if err := rules.validateKeyIdentity(keyIdentity); err != nil {
+		return nil, err
 	}
 
 	var unmatchedSignature privateSignature
 	if err := json.Unmarshal(signed, &unmatchedSignature); err != nil {
 		return nil, InvalidSignatureError{msg: err.Error()}
 	}
-
-	if unmatchedSignature.DockerReference != expectedDockerReference {
-		return nil, InvalidSignatureError{msg: fmt.Sprintf("Docker reference %s does not match %s",
-			unmatchedSignature.DockerReference, expectedDockerReference)}
+	if err := rules.validateSignedDockerManifestDigest(unmatchedSignature.DockerManifestDigest); err != nil {
+		return nil, err
+	}
+	if err := rules.validateSignedDockerReference(unmatchedSignature.DockerReference); err != nil {
+		return nil, err
 	}
 	signature := unmatchedSignature.Signature // Policy OK.
 	return &signature, nil

signature/signature_test.go

@@ -2,6 +2,7 @@ package signature
 
 import (
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"testing"
 
@@ -149,7 +150,26 @@ func TestSign(t *testing.T) {
 	signature, err := sig.sign(mech, TestKeyFingerprint)
 	require.NoError(t, err)
 
-	verified, err := verifyAndExtractSignature(mech, signature, TestKeyFingerprint, sig.DockerReference)
+	verified, err := verifyAndExtractSignature(mech, signature, signatureAcceptanceRules{
+		validateKeyIdentity: func(keyIdentity string) error {
+			if keyIdentity != TestKeyFingerprint {
+				return fmt.Errorf("Unexpected keyIdentity")
+			}
+			return nil
+		},
+		validateSignedDockerReference: func(signedDockerReference string) error {
+			if signedDockerReference != sig.DockerReference {
+				return fmt.Errorf("Unexpected signedDockerReference")
+			}
+			return nil
+		},
+		validateSignedDockerManifestDigest: func(signedDockerManifestDigest string) error {
+			if signedDockerManifestDigest != sig.DockerManifestDigest {
+				return fmt.Errorf("Unexpected signedDockerManifestDigest")
+			}
+			return nil
+		},
+	})
 	require.NoError(t, err)
 
 	assert.Equal(t, sig.Signature, *verified)
@@ -167,40 +187,102 @@ func TestVerifyAndExtractSignature(t *testing.T) {
 	mech, err := newGPGSigningMechanismInDirectory(testGPGHomeDirectory)
 	require.NoError(t, err)
 
+	type triple struct{ keyIdentity, signedDockerReference, signedDockerManifestDigest string }
+	var wanted, recorded triple
+	// recordingRules are a plausible signatureAcceptanceRules implementations, but equally
+	// importantly record that we are passing the correct values to the rule callbacks.
+	recordingRules := signatureAcceptanceRules{
+		validateKeyIdentity: func(keyIdentity string) error {
+			recorded.keyIdentity = keyIdentity
+			if keyIdentity != wanted.keyIdentity {
+				return fmt.Errorf("keyIdentity mismatch")
+			}
+			return nil
+		},
+		validateSignedDockerReference: func(signedDockerReference string) error {
+			recorded.signedDockerReference = signedDockerReference
+			if signedDockerReference != wanted.signedDockerReference {
+				return fmt.Errorf("signedDockerReference mismatch")
+			}
+			return nil
+		},
+		validateSignedDockerManifestDigest: func(signedDockerManifestDigest string) error {
+			recorded.signedDockerManifestDigest = signedDockerManifestDigest
+			if signedDockerManifestDigest != wanted.signedDockerManifestDigest {
+				return fmt.Errorf("signedDockerManifestDigest mismatch")
+			}
+			return nil
+		},
+	}
+
 	signature, err := ioutil.ReadFile("./fixtures/image.signature")
 	require.NoError(t, err)
+	signatureData := triple{
+		keyIdentity:                TestKeyFingerprint,
+		signedDockerReference:      TestImageSignatureReference,
+		signedDockerManifestDigest: TestImageManifestDigest,
+	}
 
 	// Successful verification
-	sig, err := verifyAndExtractSignature(mech, signature, TestKeyFingerprint, TestImageSignatureReference)
+	wanted = signatureData
+	recorded = triple{}
+	sig, err := verifyAndExtractSignature(mech, signature, recordingRules)
 	require.NoError(t, err)
 	assert.Equal(t, TestImageSignatureReference, sig.DockerReference)
 	assert.Equal(t, TestImageManifestDigest, sig.DockerManifestDigest)
+	assert.Equal(t, signatureData, recorded)
 
 	// For extra paranoia, test that we return a nil signature object on error.
 
 	// Completely invalid signature.
-	sig, err = verifyAndExtractSignature(mech, []byte{}, TestKeyFingerprint, TestImageSignatureReference)
+	recorded = triple{}
+	sig, err = verifyAndExtractSignature(mech, []byte{}, recordingRules)
 	assert.Error(t, err)
 	assert.Nil(t, sig)
+	assert.Equal(t, triple{}, recorded)
 
-	sig, err = verifyAndExtractSignature(mech, []byte("invalid signature"), TestKeyFingerprint, TestImageSignatureReference)
+	recorded = triple{}
+	sig, err = verifyAndExtractSignature(mech, []byte("invalid signature"), recordingRules)
 	assert.Error(t, err)
 	assert.Nil(t, sig)
+	assert.Equal(t, triple{}, recorded)
 
-	// Valid signature of non-JSON
+	// Valid signature of non-JSON: asked for keyIdentity, only
 	invalidBlobSignature, err := ioutil.ReadFile("./fixtures/invalid-blob.signature")
 	require.NoError(t, err)
-	sig, err = verifyAndExtractSignature(mech, invalidBlobSignature, TestKeyFingerprint, TestImageSignatureReference)
+	recorded = triple{}
+	sig, err = verifyAndExtractSignature(mech, invalidBlobSignature, recordingRules)
 	assert.Error(t, err)
 	assert.Nil(t, sig)
+	assert.Equal(t, triple{keyIdentity: signatureData.keyIdentity}, recorded)
 
-	// Valid signature with a wrong key
-	sig, err = verifyAndExtractSignature(mech, signature, "unexpected fingerprint", TestImageSignatureReference)
+	// Valid signature with a wrong key: asked for keyIdentity, only
+	wanted = signatureData
+	wanted.keyIdentity = "unexpected fingerprint"
+	recorded = triple{}
+	sig, err = verifyAndExtractSignature(mech, signature, recordingRules)
 	assert.Error(t, err)
 	assert.Nil(t, sig)
+	assert.Equal(t, triple{keyIdentity: signatureData.keyIdentity}, recorded)
+
+	// Valid signature with a wrong manifest digest: asked for keyIdentity and signedDockerManifestDigest
+	wanted = signatureData
+	wanted.signedDockerManifestDigest = "invalid digest"
+	recorded = triple{}
+	sig, err = verifyAndExtractSignature(mech, signature, recordingRules)
+	assert.Error(t, err)
+	assert.Nil(t, sig)
+	assert.Equal(t, triple{
+		keyIdentity:                signatureData.keyIdentity,
+		signedDockerManifestDigest: signatureData.signedDockerManifestDigest,
+	}, recorded)
 
 	// Valid signature with a wrong image reference
-	sig, err = verifyAndExtractSignature(mech, signature, TestKeyFingerprint, "unexpected docker reference")
+	wanted = signatureData
+	wanted.signedDockerReference = "unexpected docker reference"
+	recorded = triple{}
+	sig, err = verifyAndExtractSignature(mech, signature, recordingRules)
 	assert.Error(t, err)
 	assert.Nil(t, sig)
+	assert.Equal(t, signatureData, recorded)
 }