Repository: containers/image
Pull request #1069 (2020-10-27 13:05:33)

subdomain matching in policy.json
Signed-off-by: Lokesh Mandvekar <lsm5@fedoraproject.org>


@mtrmac @vrothberg @ashcrow PTAL

lsm5: some comments addressed, some yet to be addressed..
lsm5: > From the tests:
> 
> > [f9f19d7](https://github.com/containers/image/commit/f9f19d75cb5cb2358e4fdc9f96f71becf6bcb1ea) - FAIL - does not have a valid DCO
> 
> How did accepting a GitHub suggestion end up with “Co-authored-by: Stephen Milner [stevem@gnulinux.net](mailto:stevem@gnulinux.net)”?

I hit `commit suggestion` and looks like that's what it ended up doing. Anyway, I'll be squashing commits, so the DCO issue will go away.

Still working on other review comments, specially the tests, so I'll update this PR soon. I'll ping you once it's ready for further review and comments.

Thanks
mtrmac: > How did accepting a GitHub suggestion end up with “Co-authored-by: Stephen Milner [stevem@gnulinux.net](mailto:stevem@gnulinux.net)”?

Oops, this actually does match the original author; I have confused the GitHub handle with someone else. I apologize.
lsm5: @mtrmac ping, looks like a few tests are failing on current origin/master with `key "1D8230F6CDB6A06716E414C1DB72F2188BB46CC8" not found`. Or is it just me?
mtrmac: > @mtrmac ping, looks like a few tests are failing on current origin/master with `key "1D8230F6CDB6A06716E414C1DB72F2188BB46CC8" not found`. Or is it just me?

@lsm5 works for me, I think (At least `go test ./copy ./signature ` works for me, and the Travis tests fail differently ; you didn’t say what is failing).

Maybe try killing gpg-agent and/or logging in again.
lsm5: > > @mtrmac ping, looks like a few tests are failing on current origin/master with `key "1D8230F6CDB6A06716E414C1DB72F2188BB46CC8" not found`. Or is it just me?
> 
> @lsm5 works for me, I think (At least `go test ./copy ./signature ` works for me, and the Travis tests fail differently ; you didn’t say what is failing).
> 
> Maybe try killing gpg-agent and/or logging in again.

ack, I rebooted and I don't see it anymore. Thanks!
lsm5: @mtrmac PTAL when you get the chance, tests pass now (but rerunning because of rebase)
lsm5: @mtrmac PTAL at the changes. Thanks.
lsm5: @mtrmac ping, PTAL
ashcrow: The updates look good to me! @mtrmac thoughts?
rhatdan: Needs a rebase.
lsm5: @mtrmac thanks a lot for the review and comments. I'll quite likely address these along with fixing test failures (post-rebase) after I'm back from break (or maybe during the break if I'm bored). See you all soon..
lsm5: @mtrmac PTAL, all comments should be addressed with the latest change. Thanks a lot!
lsm5: also updated the commit message a bit.
TomSweeneyRedHat: LGTM once @mtrmac's comments are addressed.

Patch:
diff --git a/docker/policyconfiguration/naming.go b/docker/policyconfiguration/naming.go
index 61d9aab9..94e9e5f2 100644
--- a/docker/policyconfiguration/naming.go
+++ b/docker/policyconfiguration/naming.go
@@ -52,5 +52,26 @@ func DockerReferenceNamespaces(ref reference.Named) []string {
 		}
 		name = name[:lastSlash]
 	}
+
+	// Strip port number if any, before appending to res slice.
+	// Currently, the most compatible behavior is to return
+	// example.com:8443/ns, example.com:8443, *.com.
+	// If a port number is not specified, the expected behavior would be
+	// example.com/ns, example.com, *.com
+	portNumColon := strings.Index(name, ":")
+	if portNumColon != -1 {
+		name = name[:portNumColon]
+	}
+
+	// Append wildcarded domains to res slice
+	for {
+		firstDot := strings.Index(name, ".")
+		if firstDot == -1 {
+			break
+		}
+		name = name[firstDot+1:]
+
+		res = append(res, "*."+name)
+	}
 	return res
 }
diff --git a/docs/containers-policy.json.5.md b/docs/containers-policy.json.5.md
index f0d416ee..7e1f10a7 100644
--- a/docs/containers-policy.json.5.md
+++ b/docs/containers-policy.json.5.md
@@ -65,7 +65,8 @@ The `atomic:` transport refers to images in an Atomic Registry.
 
 Supported scopes use the form _hostname_[`:`_port_][`/`_namespace_[`/`_imagestream_ [`:`_tag_]]],
 i.e. either specifying a complete name of a tagged image, or prefix denoting
-a host/namespace/image stream.
+a host/namespace/image stream or a wildcarded expression for matching all
+subdomains. For wildcarded subdomain matching, `*.example.com` is a valid case, but `example*.*.com` is not.
 
 *Note:* The _hostname_ and _port_ refer to the Docker registry host and port (the one used
 e.g. for `docker pull`), _not_ to the OpenShift API host and port.
@@ -90,7 +91,9 @@ Scopes matching individual images are named Docker references *in the fully expa
 using a tag or digest. For example, `docker.io/library/busybox:latest` (*not* `busybox:latest`).
 
 More general scopes are prefixes of individual-image scopes, and specify a repository (by omitting the tag or digest),
-a repository namespace, or a registry host (by only specifying the host name).
+a repository namespace, or a registry host (by only specifying the host name)
+or a wildcarded expression for matching all subdomains. For wildcarded subdomain
+matching, `*.example.com` is a valid case, but `example*.*.com` is not.
 
 ### `oci:`
 
@@ -253,6 +256,8 @@ selectively allow individual transports and scopes as desired.
             /* Similarly, allow installing the “official” busybox images.  Note how the fully expanded
                form, with the explicit /library/, must be used. */
             "docker.io/library/busybox": [{"type": "insecureAcceptAnything"}]
+            /* Allow installing images from all subdomains */
+            "*.temporary-project.example.com": [{"type": "insecureAcceptAnything"}]
             /* Other docker: images use the global default policy and are rejected */
         },
         "dir": {
diff --git a/go.sum b/go.sum
index f0870550..d6863c42 100644
--- a/go.sum
+++ b/go.sum
@@ -202,11 +202,13 @@ github.com/klauspost/compress v1.11.0 h1:wJbzvpYMVGG9iTI9VxpnNZfd4DzMPoCWze3GgSq
 github.com/klauspost/compress v1.11.0/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
 github.com/klauspost/compress v1.11.1 h1:bPb7nMRdOZYDrpPMTA3EInUQrdgoBinqUuSwlGdKDdE=
 github.com/klauspost/compress v1.11.1/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
+github.com/klauspost/compress v1.11.2 h1:MiK62aErc3gIiVEtyzKfeOHgW7atJb5g/KNX5m3c2nQ=
 github.com/klauspost/compress v1.11.2/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
 github.com/klauspost/compress v1.11.3 h1:dB4Bn0tN3wdCzQxnS8r06kV74qN/TAfaIS0bVE8h3jc=
 github.com/klauspost/compress v1.11.3/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
 github.com/klauspost/compress v1.11.5 h1:xNCE0uE6yvTPRS+0wGNMHPo3NIpwnk6aluQZ6R6kRcc=
 github.com/klauspost/compress v1.11.5/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
+github.com/klauspost/compress v1.11.6 h1:EgWPCW6O3n1D5n99Zq3xXBt9uCwRGvpwGOusOLNBRSQ=
 github.com/klauspost/compress v1.11.6/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
 github.com/klauspost/pgzip v1.2.3 h1:Ce2to9wvs/cuJ2b86/CKQoTYr9VHfpanYosZ0UBJqdw=
 github.com/klauspost/pgzip v1.2.3/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQKQE9RUs=
@@ -386,6 +388,7 @@ github.com/vishvananda/netlink v1.1.0/go.mod h1:cTgwzPIzzgDAYoQrMm0EdrjRUBkTqKYp
 github.com/vishvananda/netns v0.0.0-20191106174202-0a2b9b5464df/go.mod h1:JP3t17pCcGlemwknint6hfoeCVQrEMVwxRLRjXpq+BU=
 github.com/willf/bitset v1.1.11-0.20200630133818-d5bec3311243 h1:R43TdZy32XXSXjJn7M/HhALJ9imq6ztLnChfYJpVDnM=
 github.com/willf/bitset v1.1.11-0.20200630133818-d5bec3311243/go.mod h1:RjeCKbqT1RxIR/KWY6phxZiaY1IyutSBfGjNPySAYV4=
+github.com/willf/bitset v1.1.11 h1:N7Z7E9UvjW+sGsEl7k/SJrvY2reP1A07MrGuCjIOjRE=
 github.com/willf/bitset v1.1.11/go.mod h1:83CECat5yLh5zVOf4P1ErAgKA5UDvKtgyUABdr3+MjI=
 github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU=
 github.com/xeipuuv/gojsonpointer v0.0.0-20190809123943-df4f5c81cb3b h1:6cLsL+2FW6dRAdl5iMtHgRogVCff0QpRi9653YmdcJA=
diff --git a/signature/fixtures/policy.json b/signature/fixtures/policy.json
index 30b33720..893a6e8c 100644
--- a/signature/fixtures/policy.json
+++ b/signature/fixtures/policy.json
@@ -87,6 +87,36 @@
                     }
                 }
             ],
+            "*.access.redhat.com": [
+                {
+                    "type": "signedBy",
+                    "keyType": "signedByGPGKeys",
+                    "keyPath": "/keys/RH-key-signing-key-gpg-keyring",
+                    "signedIdentity": {
+                        "type": "matchRepoDigestOrExact"
+                    }
+                }
+            ],
+            "*.redhat.com": [
+                {
+                    "type": "signedBy",
+                    "keyType": "signedByGPGKeys",
+                    "keyPath": "/keys/RH-key-signing-key-gpg-keyring",
+                    "signedIdentity": {
+                        "type": "matchRepoDigestOrExact"
+                    }
+                }
+            ],
+            "*.com": [
+                {
+                    "type": "signedBy",
+                    "keyType": "signedByGPGKeys",
+                    "keyPath": "/keys/RH-key-signing-key-gpg-keyring",
+                    "signedIdentity": {
+                        "type": "matchRepoDigestOrExact"
+                    }
+                }
+            ],
             "bogus/key-data-example": [
                 {
                     "type": "signedBy",
@@ -105,4 +135,4 @@
             ]
         }
     }
-}
\ No newline at end of file
+}


Test patch:
diff --git a/docker/daemon/daemon_transport_test.go b/docker/daemon/daemon_transport_test.go
index ce1766e2..0df5a014 100644
--- a/docker/daemon/daemon_transport_test.go
+++ b/docker/daemon/daemon_transport_test.go
@@ -32,6 +32,8 @@ func TestTransportValidatePolicyConfigurationScope(t *testing.T) {
 		"registry.example.com/ns/stream",
 		"registry.example.com/ns",
 		"registry.example.com",
+		"*.example.com",
+		"*.com",
 		sha256digestHex, // Accept also unqualified hexdigest values, they are in principle possible host names.
 	} {
 		err := Transport.ValidatePolicyConfigurationScope(scope)
@@ -218,6 +220,7 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) {
 		"docker.io/library/busybox",
 		"docker.io/library",
 		"docker.io",
+		"*.io",
 	}, ref.PolicyConfigurationNamespaces())
 }
 
diff --git a/docker/docker_transport_test.go b/docker/docker_transport_test.go
index b310ce5a..c51c76ca 100644
--- a/docker/docker_transport_test.go
+++ b/docker/docker_transport_test.go
@@ -30,6 +30,7 @@ func TestTransportValidatePolicyConfigurationScope(t *testing.T) {
 		"docker.io/library/busybox",
 		"docker.io/library",
 		"docker.io",
+		"*.io",
 	} {
 		err := Transport.ValidatePolicyConfigurationScope(scope)
 		assert.NoError(t, err, scope)
@@ -147,6 +148,7 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) {
 		"docker.io/library/busybox",
 		"docker.io/library",
 		"docker.io",
+		"*.io",
 	}, ref.PolicyConfigurationNamespaces())
 }
 
diff --git a/docker/lookaside_test.go b/docker/lookaside_test.go
index 79c06141..ded4d24e 100644
--- a/docker/lookaside_test.go
+++ b/docker/lookaside_test.go
@@ -212,7 +212,7 @@ func TestLoadAndMergeConfig(t *testing.T) {
 	}, config)
 }
 
-func TestRegistryConfigurationSignaureTopLevel(t *testing.T) {
+func TestRegistryConfigurationSignatureTopLevel(t *testing.T) {
 	config := registryConfiguration{
 		DefaultDocker: &registryNamespace{SigStore: "=default", SigStoreStaging: "=default+w"},
 		Docker:        map[string]registryNamespace{},
diff --git a/docker/policyconfiguration/naming_test.go b/docker/policyconfiguration/naming_test.go
index 8c2b018c..8f70c66f 100644
--- a/docker/policyconfiguration/naming_test.go
+++ b/docker/policyconfiguration/naming_test.go
@@ -10,7 +10,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// TestDockerReference tests DockerReferenceIdentity and DockerReferenceNamespaces simulatenously
+// TestDockerReference tests DockerReferenceIdentity and DockerReferenceNamespaces simultaneously
 // to ensure they are consistent.
 func TestDockerReference(t *testing.T) {
 	sha256Digest := "@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
@@ -18,18 +18,19 @@ func TestDockerReference(t *testing.T) {
 	// and that DockerReferenceNamespaces starts with the expected value (fullName), i.e. that the two functions are
 	// consistent.
 	for inputName, expectedNS := range map[string][]string{
-		"example.com/ns/repo": {"example.com/ns/repo", "example.com/ns", "example.com"},
-		"example.com/repo":    {"example.com/repo", "example.com"},
+		"example.com/ns/repo": {"example.com/ns/repo", "example.com/ns", "example.com", "*.com"},
+		"example.com/repo":    {"example.com/repo", "example.com", "*.com"},
 		"localhost/ns/repo":   {"localhost/ns/repo", "localhost/ns", "localhost"},
 		// Note that "localhost" is special here: notlocalhost/repo is parsed as docker.io/notlocalhost.repo:
-		"localhost/repo":         {"localhost/repo", "localhost"},
-		"notlocalhost/repo":      {"docker.io/notlocalhost/repo", "docker.io/notlocalhost", "docker.io"},
-		"docker.io/ns/repo":      {"docker.io/ns/repo", "docker.io/ns", "docker.io"},
-		"docker.io/library/repo": {"docker.io/library/repo", "docker.io/library", "docker.io"},
-		"docker.io/repo":         {"docker.io/library/repo", "docker.io/library", "docker.io"},
-		"ns/repo":                {"docker.io/ns/repo", "docker.io/ns", "docker.io"},
-		"library/repo":           {"docker.io/library/repo", "docker.io/library", "docker.io"},
-		"repo":                   {"docker.io/library/repo", "docker.io/library", "docker.io"},
+		"localhost/repo":                       {"localhost/repo", "localhost"},
+		"notlocalhost/repo":                    {"docker.io/notlocalhost/repo", "docker.io/notlocalhost", "docker.io", "*.io"},
+		"docker.io/ns/repo":                    {"docker.io/ns/repo", "docker.io/ns", "docker.io", "*.io"},
+		"docker.io/library/repo":               {"docker.io/library/repo", "docker.io/library", "docker.io", "*.io"},
+		"docker.io/repo":                       {"docker.io/library/repo", "docker.io/library", "docker.io", "*.io"},
+		"ns/repo":                              {"docker.io/ns/repo", "docker.io/ns", "docker.io", "*.io"},
+		"library/repo":                         {"docker.io/library/repo", "docker.io/library", "docker.io", "*.io"},
+		"repo":                                 {"docker.io/library/repo", "docker.io/library", "docker.io", "*.io"},
+		"yet.another.example.com:8443/ns/repo": {"yet.another.example.com:8443/ns/repo", "yet.another.example.com:8443/ns", "yet.another.example.com:8443", "*.another.example.com", "*.example.com", "*.com"},
 	} {
 		for inputSuffix, mappedSuffix := range map[string]string{
 			":tag":       ":tag",
@@ -49,7 +50,13 @@ func TestDockerReference(t *testing.T) {
 			moreSpecific := identity
 			for i := range expectedNS {
 				assert.Equal(t, ns[i], expectedNS[i], fmt.Sprintf("%s item %d", fullInput, i))
-				assert.True(t, strings.HasPrefix(moreSpecific, ns[i]))
+				// Verify that expectedNS is ordered from most specific to least specific
+				if strings.HasPrefix(ns[i], "*.") {
+					// Check for subdomain matches if wildcard present
+					assert.True(t, strings.Contains(moreSpecific, ns[i][1:]))
+				} else {
+					assert.True(t, strings.HasPrefix(moreSpecific, ns[i]))
+				}
 				moreSpecific = ns[i]
 			}
 		}
diff --git a/openshift/openshift_transport_test.go b/openshift/openshift_transport_test.go
index 1cc5af7a..af8dfd86 100644
--- a/openshift/openshift_transport_test.go
+++ b/openshift/openshift_transport_test.go
@@ -25,6 +25,8 @@ func TestTransportValidatePolicyConfigurationScope(t *testing.T) {
 		"registry.example.com/ns/stream",
 		"registry.example.com/ns",
 		"registry.example.com",
+		"*.example.com",
+		"*.com",
 	} {
 		err := Transport.ValidatePolicyConfigurationScope(scope)
 		assert.NoError(t, err, scope)
@@ -112,6 +114,8 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) {
 		"registry.example.com:8443/ns/stream",
 		"registry.example.com:8443/ns",
 		"registry.example.com:8443",
+		"*.example.com",
+		"*.com",
 	}, ref.PolicyConfigurationNamespaces())
 }
 
diff --git a/signature/policy_config_test.go b/signature/policy_config_test.go
index 5a190309..f4bd6d38 100644
--- a/signature/policy_config_test.go
+++ b/signature/policy_config_test.go
@@ -62,6 +62,21 @@ var policyFixtureContents = &Policy{
 					"/keys/vendor-gpg-keyring",
 					xNewPRMRemapIdentity("private-mirror:5000/vendor-mirror", "vendor.example.com")),
 			},
+			"*.access.redhat.com": {
+				xNewPRSignedByKeyPath(SBKeyTypeSignedByGPGKeys,
+					"/keys/RH-key-signing-key-gpg-keyring",
+					NewPRMMatchRepoDigestOrExact()),
+			},
+			"*.redhat.com": {
+				xNewPRSignedByKeyPath(SBKeyTypeSignedByGPGKeys,
+					"/keys/RH-key-signing-key-gpg-keyring",
+					NewPRMMatchRepoDigestOrExact()),
+			},
+			"*.com": {
+				xNewPRSignedByKeyPath(SBKeyTypeSignedByGPGKeys,
+					"/keys/RH-key-signing-key-gpg-keyring",
+					NewPRMMatchRepoDigestOrExact()),
+			},
 			"bogus/key-data-example": {
 				xNewPRSignedByKeyData(SBKeyTypeSignedByGPGKeys,
 					[]byte("nonsense"),
diff --git a/signature/policy_eval_test.go b/signature/policy_eval_test.go
index 2f8de202..3ba2d2a8 100644
--- a/signature/policy_eval_test.go
+++ b/signature/policy_eval_test.go
@@ -155,6 +155,8 @@ func TestPolicyContextRequirementsForImageRef(t *testing.T) {
 		{"docker", ""},
 		{"docker", "unmatched"},
 		{"docker", "deep.com"},
+		{"docker", "*.very.deep.com"},
+		{"docker", "*.deep.com"},
 		{"docker", "deep.com/n1"},
 		{"docker", "deep.com/n1/n2"},
 		{"docker", "deep.com/n1/n2/n3"},
@@ -181,6 +183,9 @@ func TestPolicyContextRequirementsForImageRef(t *testing.T) {
 		{"docker", "deep.com/n1/notn2/n3/repo:tag2", "docker", "deep.com/n1"},
 		// Host name match
 		{"docker", "deep.com/notn1/n2/n3/repo:tag2", "docker", "deep.com"},
+		// Sub domain match
+		{"docker", "very.deep.com/n1/n2/n3/repo:tag2", "docker", "*.deep.com"},
+		{"docker", "not.very.deep.com/n1/n2/n3/repo:tag2", "docker", "*.very.deep.com"},
 		// Default
 		{"docker", "this.doesnt/match:anything", "docker", ""},
 		// No match within a matched transport which doesn't have a "" scope