Error on namespaced registries for credential helpers

We now error on login if repositories or repository namespaces are used
for other credential helpers than the `AuthenticationFileHelper`. On
logout we ignore them and debug log a warning that nothing has been
modified.

The functions `SetCredentials` (for login) as well as
`RemoveAuthentication` (for logout) already feature support for path
based registries for the `AuthenticationFileHelper`. This patch adds
unit tests to ensure that the support will not break in the future.

Signed-off-by: Sascha Grunert <[email protected]>

docs/containers-auth.json.5.md

@@ -24,7 +24,7 @@ is created by a `login` command from a container tool such as `podman login`,
 `buildah login` or `skopeo login`.  Each entry includes the name of the registry and then an auth
 token in the form of a base64 encoded string from the concatenation of the
 username, a colon, and the password. The registry name can additionally contain
-a path or repository name (an image name without tag or digest). The path (or
+a repository name (an image name without tag or digest) and namespaces. The path (or
 namespace) is matched in its hierarchical order when checking for available
 authentications. For example, an image pull for
 `my-registry.local/namespace/user/image:latest` will result in a lookup in

pkg/docker/config/config.go

@@ -54,10 +54,18 @@ var (
 
 // SetCredentials stores the username and password in a location
 // appropriate for sys and the users’ configuration.
+// An error will be part of the returned multierror if the provided key is
+// namespaced and a credentials helper other than the AuthenticationFileHelper
+// is used.
 // Returns a human-redable description of the location that was updated.
 // NOTE: The return value is only intended to be read by humans; its form is not an API,
 // it may change (or new forms can be added) any time.
-func SetCredentials(sys *types.SystemContext, registry, username, password string) (string, error) {
+func SetCredentials(sys *types.SystemContext, key, username, password string) (string, error) {
+	isNamespaced, err := validateKey(key)
+	if err != nil {
+		return "", err
+	}
+
 	helpers, err := sysregistriesv2.CredentialHelpers(sys)
 	if err != nil {
 		return "", err
@@ -72,33 +80,45 @@ func SetCredentials(sys *types.SystemContext, registry, username, password strin
 		// Special-case the built-in helpers for auth files.
 		case sysregistriesv2.AuthenticationFileHelper:
 			desc, err = modifyJSON(sys, func(auths *dockerConfigFile) (bool, error) {
-				if ch, exists := auths.CredHelpers[registry]; exists {
-					return false, setAuthToCredHelper(ch, registry, username, password)
+				if ch, exists := auths.CredHelpers[key]; exists {
+					if isNamespaced {
+						return false, unsupportedNamespaceErr(ch)
+					}
+					return false, setAuthToCredHelper(ch, key, username, password)
 				}
 				creds := base64.StdEncoding.EncodeToString([]byte(username + ":" + password))
 				newCreds := dockerAuthConfig{Auth: creds}
-				auths.AuthConfigs[registry] = newCreds
+				auths.AuthConfigs[key] = newCreds
 				return true, nil
 			})
 		// External helpers.
 		default:
 			desc = fmt.Sprintf("credential helper: %s", helper)
-			err = setAuthToCredHelper(helper, registry, username, password)
+
+			if isNamespaced {
+				err = unsupportedNamespaceErr(helper)
+			} else {
+				err = setAuthToCredHelper(helper, key, username, password)
+			}
 		}
 		if err != nil {
 			multiErr = multierror.Append(multiErr, err)
-			logrus.Debugf("Error storing credentials for %s in credential helper %s: %v", registry, helper, err)
+			logrus.Debugf("Error storing credentials for %s in credential helper %s: %v", key, helper, err)
 			continue
 		}
-		logrus.Debugf("Stored credentials for %s in credential helper %s", registry, helper)
+		logrus.Debugf("Stored credentials for %s in credential helper %s", key, helper)
 		return desc, nil
 	}
 	return "", multiErr
 }
 
+func unsupportedNamespaceErr(helper string) error {
+	return errors.Errorf("namespaced key is not supported for credential helper %s", helper)
+}
+
 // SetAuthentication stores the username and password in the credential helper or file
-func SetAuthentication(sys *types.SystemContext, registry, username, password string) error {
-	_, err := SetCredentials(sys, registry, username, password)
+func SetAuthentication(sys *types.SystemContext, key, username, password string) error {
+	_, err := SetCredentials(sys, key, username, password)
 	return err
 }
 
@@ -326,9 +346,14 @@ func getAuthenticationWithHomeDir(sys *types.SystemContext, registry, homeDir st
 	return auth.Username, auth.Password, nil
 }
 
-// RemoveAuthentication removes credentials for `registry` from all possible
+// RemoveAuthentication removes credentials for `key` from all possible
 // sources such as credential helpers and auth files.
-func RemoveAuthentication(sys *types.SystemContext, registry string) error {
+func RemoveAuthentication(sys *types.SystemContext, key string) error {
+	isNamespaced, err := validateKey(key)
+	if err != nil {
+		return err
+	}
+
 	helpers, err := sysregistriesv2.CredentialHelpers(sys)
 	if err != nil {
 		return err
@@ -338,17 +363,21 @@ func RemoveAuthentication(sys *types.SystemContext, registry string) error {
 	isLoggedIn := false
 
 	removeFromCredHelper := func(helper string) {
-		err := deleteAuthFromCredHelper(helper, registry)
-		if err == nil {
-			logrus.Debugf("Credentials for %q were deleted from credential helper %s", registry, helper)
-			isLoggedIn = true
-			return
-		}
-		if credentials.IsErrCredentialsNotFoundMessage(err.Error()) {
-			logrus.Debugf("Not logged in to %s with credential helper %s", registry, helper)
-			return
+		if isNamespaced {
+			logrus.Debugf("Not removing credentials: %v", unsupportedNamespaceErr(helper))
+		} else {
+			err := deleteAuthFromCredHelper(helper, key)
+			if err == nil {
+				logrus.Debugf("Credentials for %q were deleted from credential helper %s", key, helper)
+				isLoggedIn = true
+				return
+			}
+			if credentials.IsErrCredentialsNotFoundMessage(err.Error()) {
+				logrus.Debugf("Not logged in to %s with credential helper %s", key, helper)
+				return
+			}
 		}
-		multiErr = multierror.Append(multiErr, errors.Wrapf(err, "removing credentials for %s from credential helper %s", registry, helper))
+		multiErr = multierror.Append(multiErr, errors.Wrapf(err, "removing credentials for %s from credential helper %s", key, helper))
 	}
 
 	for _, helper := range helpers {
@@ -357,15 +386,15 @@ func RemoveAuthentication(sys *types.SystemContext, registry string) error {
 		// Special-case the built-in helper for auth files.
 		case sysregistriesv2.AuthenticationFileHelper:
 			_, err = modifyJSON(sys, func(auths *dockerConfigFile) (bool, error) {
-				if innerHelper, exists := auths.CredHelpers[registry]; exists {
+				if innerHelper, exists := auths.CredHelpers[key]; exists {
 					removeFromCredHelper(innerHelper)
 				}
-				if _, ok := auths.AuthConfigs[registry]; ok {
+				if _, ok := auths.AuthConfigs[key]; ok {
 					isLoggedIn = true
-					delete(auths.AuthConfigs, registry)
-				} else if _, ok := auths.AuthConfigs[normalizeRegistry(registry)]; ok {
+					delete(auths.AuthConfigs, key)
+				} else if _, ok := auths.AuthConfigs[normalizeRegistry(key)]; ok {
 					isLoggedIn = true
-					delete(auths.AuthConfigs, normalizeRegistry(registry))
+					delete(auths.AuthConfigs, normalizeRegistry(key))
 				}
 				return true, multiErr
 			})
@@ -699,18 +728,18 @@ func decodeDockerAuth(conf dockerAuthConfig) (types.DockerAuthConfig, error) {
 // to just an hostname.
 // Copied from github.com/docker/docker/registry/auth.go
 func convertToHostname(url string) string {
-	stripped := url
-	if strings.HasPrefix(url, "http://") {
-		stripped = strings.TrimPrefix(url, "http://")
-	} else if strings.HasPrefix(url, "https://") {
-		stripped = strings.TrimPrefix(url, "https://")
-	}
-
+	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://")
+	stripped = strings.TrimPrefix(stripped, "https://")
+	return stripped
+}
+
 func normalizeRegistry(registry string) string {
 	normalized := convertToHostname(registry)
 	switch normalized {
@@ -719,3 +748,15 @@ func normalizeRegistry(registry string) string {
 	}
 	return normalized
 }
+
+// validateKey verifies that the input key does not have any not allowed prefix
+// and returns an indicator if the key is namespaced.
+func validateKey(key string) (isNamespaced bool, err error) {
+	if strings.HasPrefix(key, "http://") || strings.HasPrefix(key, "https://") {
+		return isNamespaced, errors.Errorf("key %s contains http[s]:// prefix", key)
+	}
+
+	// check if the provided key contains one or more subpaths.
+	stripped := stripScheme(key)
+	return strings.ContainsRune(stripped, '/'), nil
+}

pkg/docker/config/config_test.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -675,3 +676,195 @@ func TestAuthKeysForRef(t *testing.T) {
 		require.Equal(t, tc.expected, result, tc.name)
 	}
 }
+
+func TestSetCredentials(t *testing.T) {
+	const (
+		usernamePrefix = "username-"
+		passwordPrefix = "password-"
+	)
+	getAuth := func(sys *types.SystemContext, input string) types.DockerAuthConfig {
+		ref, err := reference.ParseNamed(input)
+		require.NoError(t, err)
+		auth, err := GetCredentialsForRef(sys, ref)
+		require.NoError(t, err)
+		return auth
+	}
+
+	for _, tc := range []struct {
+		input  []string
+		assert func(*types.SystemContext, dockerConfigFile)
+	}{
+		{
+			input: []string{"quay.io"},
+			assert: func(sys *types.SystemContext, auth dockerConfigFile) {
+				assert.Len(t, auth.AuthConfigs, 1)
+				assert.NotEmpty(t, auth.AuthConfigs["quay.io"].Auth)
+			},
+		},
+		{
+			input: []string{"quay.io/a/b/c/d/image"},
+			assert: func(sys *types.SystemContext, auth dockerConfigFile) {
+				assert.Len(t, auth.AuthConfigs, 1)
+				assert.NotEmpty(t, auth.AuthConfigs["quay.io/a/b/c/d/image"].Auth)
+
+				ta := getAuth(sys, "quay.io/a/b/c/d/image")
+				assert.Equal(t, usernamePrefix+"0", ta.Username)
+				assert.Equal(t, passwordPrefix+"0", ta.Password)
+			},
+		},
+		{
+			input: []string{
+				"quay.io/a/b/c",
+				"quay.io/a/b",
+				"quay.io/a",
+				"quay.io",
+				"my-registry.local",
+				"my-registry.local",
+			},
+			assert: func(sys *types.SystemContext, auth dockerConfigFile) {
+				assert.Len(t, auth.AuthConfigs, 5)
+				assert.NotEmpty(t, auth.AuthConfigs["quay.io/a/b/c"].Auth)
+				assert.NotEmpty(t, auth.AuthConfigs["quay.io/a/b"].Auth)
+				assert.NotEmpty(t, auth.AuthConfigs["quay.io/a"].Auth)
+				assert.NotEmpty(t, auth.AuthConfigs["quay.io"].Auth)
+				assert.NotEmpty(t, auth.AuthConfigs["my-registry.local"].Auth)
+
+				ta0 := getAuth(sys, "quay.io/a/b/c")
+				assert.Equal(t, usernamePrefix+"0", ta0.Username)
+				assert.Equal(t, passwordPrefix+"0", ta0.Password)
+
+				ta1 := getAuth(sys, "quay.io/a/b")
+				assert.Equal(t, usernamePrefix+"1", ta1.Username)
+				assert.Equal(t, passwordPrefix+"1", ta1.Password)
+
+				ta2 := getAuth(sys, "quay.io/a")
+				assert.Equal(t, usernamePrefix+"2", ta2.Username)
+				assert.Equal(t, passwordPrefix+"2", ta2.Password)
+
+			},
+		},
+	} {
+		tmpFile, err := ioutil.TempFile("", "auth.json.set")
+		require.NoError(t, err)
+		defer os.RemoveAll(tmpFile.Name())
+
+		_, err = tmpFile.WriteString("{}")
+		require.NoError(t, err)
+		sys := &types.SystemContext{AuthFilePath: tmpFile.Name()}
+
+		for i, input := range tc.input {
+			_, err := SetCredentials(
+				sys,
+				input,
+				usernamePrefix+fmt.Sprint(i),
+				passwordPrefix+fmt.Sprint(i),
+			)
+			assert.NoError(t, err)
+		}
+
+		auth, err := readJSONFile(tmpFile.Name(), false)
+		require.NoError(t, err)
+
+		tc.assert(sys, auth)
+	}
+}
+
+func TestRemoveAuthentication(t *testing.T) {
+	testAuth := dockerAuthConfig{Auth: "ZXhhbXBsZTpvcmc="}
+	for _, tc := range []struct {
+		config dockerConfigFile
+		inputs []string
+		assert func(dockerConfigFile)
+	}{
+		{
+			config: dockerConfigFile{
+				AuthConfigs: map[string]dockerAuthConfig{
+					"quay.io": testAuth,
+				},
+			},
+			inputs: []string{"quay.io"},
+			assert: func(auth dockerConfigFile) {
+				assert.Len(t, auth.AuthConfigs, 0)
+			},
+		},
+		{
+			config: dockerConfigFile{
+				AuthConfigs: map[string]dockerAuthConfig{
+					"quay.io":           testAuth,
+					"my-registry.local": testAuth,
+				},
+			},
+			inputs: []string{"my-registry.local"},
+			assert: func(auth dockerConfigFile) {
+				assert.Len(t, auth.AuthConfigs, 1)
+				assert.NotEmpty(t, auth.AuthConfigs["quay.io"].Auth)
+			},
+		},
+		{
+			config: dockerConfigFile{
+				AuthConfigs: map[string]dockerAuthConfig{
+					"quay.io/a/b/c":     testAuth,
+					"quay.io/a/b":       testAuth,
+					"quay.io/a":         testAuth,
+					"quay.io":           testAuth,
+					"my-registry.local": testAuth,
+				},
+			},
+			inputs: []string{
+				"quay.io/a/b",
+				"quay.io",
+				"my-registry.local",
+			},
+			assert: func(auth dockerConfigFile) {
+				assert.Len(t, auth.AuthConfigs, 2)
+				assert.NotEmpty(t, auth.AuthConfigs["quay.io/a/b/c"].Auth)
+				assert.NotEmpty(t, auth.AuthConfigs["quay.io/a"].Auth)
+			},
+		},
+	} {
+
+		content, err := json.Marshal(&tc.config)
+		require.NoError(t, err)
+
+		tmpFile, err := ioutil.TempFile("", "auth.json")
+		require.NoError(t, err)
+		defer os.RemoveAll(tmpFile.Name())
+
+		_, err = tmpFile.Write(content)
+		require.NoError(t, err)
+
+		sys := &types.SystemContext{AuthFilePath: tmpFile.Name()}
+
+		for _, input := range tc.inputs {
+			err := RemoveAuthentication(sys, input)
+			assert.NoError(t, err)
+		}
+
+		auth, err := readJSONFile(tmpFile.Name(), false)
+		require.NoError(t, err)
+
+		tc.assert(auth)
+	}
+}
+
+func TestValidateKey(t *testing.T) {
+	for _, tc := range []struct {
+		key          string
+		shouldError  bool
+		isNamespaced bool
+	}{
+		{"my-registry.local", false, false},
+		{"https://my-registry.local", true, false},
+		{"my-registry.local/path", false, true},
+		{"quay.io/a/b/c/d", false, true},
+	} {
+
+		isNamespaced, err := validateKey(tc.key)
+		if tc.shouldError {
+			assert.Error(t, err)
+		} else {
+			assert.NoError(t, err)
+		}
+		assert.Equal(t, tc.isNamespaced, isNamespaced)
+	}
+}