From 677f711c6c8610c8c449566467de5811249a44ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= <mitr@redhat.com>
Date: Tue, 31 May 2016 02:12:28 +0200
Subject: [PATCH] Redefine Policy.Specific scopes to use fully expanded
 hostname/namespace/repo format

Using the canonical minimized format of Docker references introduces too
many ambiguities.

This also removes some validation of the scope string, but all that was
really doing was rejecting completely invalid input like uppercase.

Sadly it is not qutie obvious that we can detect and reject mistakes like
using "busybox" as a scope instead of the correct
"docker.io/library/busybox".  Perhaps require at least one dot or port
number in the host name?
---
 signature/policy_config.go      | 7 +++----
 signature/policy_config_test.go | 7 ++-----
 signature/policy_types.go       | 4 +++-
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/signature/policy_config.go b/signature/policy_config.go
index fd634783..7fc5a382 100644
--- a/signature/policy_config.go
+++ b/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
diff --git a/signature/policy_config_test.go b/signature/policy_config_test.go
index 63fd8c74..23c2f1a6 100644
--- a/signature/policy_config_test.go
+++ b/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)
diff --git a/signature/policy_types.go b/signature/policy_types.go
index c38b1e76..c57fa34d 100644
--- a/signature/policy_types.go
+++ b/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.