From c95f2b49f64396babd66e7356e17af44f5bef7fb Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Mon, 19 Jul 2021 15:08:27 +0200 Subject: [PATCH] Disable registry fallback for repository credentials As mentioned in https://github.com/containers/common/pull/659#issuecomment-875505807, we now do not normalize the registry when being in non legacy format. This means we do not match "quay.io/foo" if "quay.io/bar" is provided as credentials, because both would be normalized to "quay.io" before this patch. We also add a new corner case where auth.json has been external modified to contain an entry like "https://quay.io/v1/", which now also matches if "quay.io/foo" has been provided via the credentials. Signed-off-by: Sascha Grunert --- pkg/docker/config/config.go | 34 ++++++------ pkg/docker/config/config_test.go | 91 ++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 18 deletions(-) diff --git a/pkg/docker/config/config.go b/pkg/docker/config/config.go index cddf1bd8bd..8436741f3b 100644 --- a/pkg/docker/config/config.go +++ b/pkg/docker/config/config.go @@ -665,10 +665,10 @@ func findAuthentication(ref reference.Named, registry, path string, legacyFormat // those entries even in non-legacyFormat ~/.docker/config.json. // The docker.io registry still uses the /v1/ key with a special host name, // so account for that as well. - registry = normalizeRegistry(registry) + registry = normalizeAuthFileKey(registry, legacyFormat) normalizedAuths := map[string]dockerAuthConfig{} for k, v := range auths.AuthConfigs { - normalizedAuths[normalizeRegistry(k)] = v + normalizedAuths[normalizeAuthFileKey(k, legacyFormat)] = v } if val, exists := normalizedAuths[registry]; exists { @@ -723,29 +723,27 @@ func decodeDockerAuth(conf dockerAuthConfig) (types.DockerAuthConfig, error) { }, nil } -// convertToHostname converts a registry url which has http|https prepended -// to just an hostname. -// Copied from github.com/docker/docker/registry/auth.go -func convertToHostname(url string) string { - stripped := stripScheme(url) - nameParts := strings.SplitN(stripped, "/", 2) - return nameParts[0] -} - -// stripScheme striped the http|https scheme from the provided URL. -func stripScheme(url string) string { - stripped := strings.TrimPrefix(url, "http://") +// normalizeAuthFileKey takes a key, converts it to a host name and normalizes +// the resulting registry. +func normalizeAuthFileKey(key string, legacyFormat bool) string { + stripped := strings.TrimPrefix(key, "http://") stripped = strings.TrimPrefix(stripped, "https://") - return stripped + + if legacyFormat || stripped != key { + stripped = strings.SplitN(stripped, "/", 2)[0] + } + + return normalizeRegistry(stripped) } +// normalizeRegistry converts the provided registry if a known docker.io host +// is provided. func normalizeRegistry(registry string) string { - normalized := convertToHostname(registry) - switch normalized { + switch registry { case "registry-1.docker.io", "docker.io": return "index.docker.io" } - return normalized + return registry } // validateKey verifies that the input key does not have a prefix that is not diff --git a/pkg/docker/config/config_test.go b/pkg/docker/config/config_test.go index 8fe8b8ca52..a8ce8fdb33 100644 --- a/pkg/docker/config/config_test.go +++ b/pkg/docker/config/config_test.go @@ -886,3 +886,94 @@ func TestValidateKey(t *testing.T) { assert.Equal(t, tc.isNamespaced, isNamespaced) } } + +func TestSetGetCredentials(t *testing.T) { + const ( + username = "username" + password = "password" + ) + + tmpDir, err := ioutil.TempDir("", "auth-test-") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + + for _, tc := range []struct { + name string + set string + get string + useLegacyAPI bool + shouldAuth bool + }{ + { + name: "Should match namespace", + set: "quay.io/foo", + get: "quay.io/foo/a", + shouldAuth: true, + }, + { + name: "Should match registry if repository provided", + set: "quay.io", + get: "quay.io/foo", + shouldAuth: true, + }, + { + name: "Should not match different repository", + set: "quay.io/foo", + get: "quay.io/bar", + shouldAuth: false, + }, + { + name: "Should match legacy registry entry (new API)", + set: "https://quay.io/v1/", + get: "quay.io/foo", + shouldAuth: true, + }, + { + name: "Should match legacy registry entry (legacy API)", + set: "https://quay.io/v1/", + get: "quay.io", + shouldAuth: true, + useLegacyAPI: true, + }, + } { + + // Create a new empty SystemContext referring an empty auth.json + tmpFile, err := ioutil.TempFile("", "auth.json-") + require.NoError(t, err) + defer os.RemoveAll(tmpFile.Name()) + + sys := &types.SystemContext{} + if tc.useLegacyAPI { + sys.LegacyFormatAuthFilePath = tmpFile.Name() + _, err = tmpFile.WriteString(fmt.Sprintf( + `{"%s":{"auth":"dXNlcm5hbWU6cGFzc3dvcmQ="}}`, tc.set, + )) + } else { + sys.AuthFilePath = tmpFile.Name() + _, err = tmpFile.WriteString(fmt.Sprintf( + `{"auths":{"%s":{"auth":"dXNlcm5hbWU6cGFzc3dvcmQ="}}}`, tc.set, + )) + } + require.NoError(t, err) + + // Try to authenticate against them + var auth types.DockerAuthConfig + if !tc.useLegacyAPI { + ref, err := reference.ParseNamed(tc.get) + require.NoError(t, err) + auth, err = getCredentialsWithHomeDir(sys, ref, reference.Domain(ref), tmpDir) + require.NoError(t, err) + } else { + auth, err = getCredentialsWithHomeDir(sys, nil, tc.get, tmpDir) + require.NoError(t, err) + } + + if tc.shouldAuth { + assert.Equal(t, username, auth.Username, tc.name) + assert.Equal(t, password, auth.Password, tc.name) + } else { + assert.Empty(t, auth.Username, tc.name) + assert.Empty(t, auth.Password, tc.name) + } + } +}