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

@@ -21,14 +21,15 @@ Except the primary (read/write) file, other files are read-only, unless the user
 The auth.json file stores encrypted authentication information for the
 user to container image registries.  The file can have zero to many entries and
 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
-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
-`auth.json` in the following order:
+`buildah login` or `skopeo login`. Each entry either contains a single
+hostname (e.g. `docker.io`) or a namespace (e.g. `quay.io/user/image`) as a key
+and an auth token in the form of a base64 encoded string as value of `auth`. The
+token is built from the concatenation of the username, a colon, and the
+password. The registry name can additionally contain 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 `auth.json` in the following order:
 
 - `my-registry.local/namespace/user/image`
 - `my-registry.local/namespace/user`

pkg/docker/config/config.go

@@ -54,10 +54,17 @@ var (
 
 // SetCredentials stores the username and password in a location
 // appropriate for sys and the users’ configuration.
+// A valid key can be either a registry hostname or additionally a namespace if
+// the AuthenticationFileHelper is being unsed.
 // 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 +79,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 {
+				desc = fmt.Sprintf("credential helper: %s", helper)
+				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)
+// See the documentation of SetCredentials for format of "key"
+func SetAuthentication(sys *types.SystemContext, key, username, password string) error {
+	_, err := SetCredentials(sys, key, username, password)
 	return err
 }
 
@@ -326,9 +345,16 @@ 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 {
+// A valid key can be either a registry hostname or additionally a namespace if
+// the AuthenticationFileHelper is being unsed.
+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 +364,22 @@ 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)
+		if isNamespaced {
+			logrus.Debugf("Not removing credentials because namespaced keys are not supported for the credential helper: %s", helper)
 			return
+		} 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 +388,12 @@ 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 {
-					isLoggedIn = true
-					delete(auths.AuthConfigs, registry)
-				} else if _, ok := auths.AuthConfigs[normalizeRegistry(registry)]; ok {
+				if _, ok := auths.AuthConfigs[key]; ok {
 					isLoggedIn = true
-					delete(auths.AuthConfigs, normalizeRegistry(registry))
+					delete(auths.AuthConfigs, key)
 				}
 				return true, multiErr
 			})
@@ -699,18 +727,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 +747,14 @@ func normalizeRegistry(registry string) string {
 	}
 	return normalized
 }
+
+// validateKey verifies that the input key does not have a prefix that is not
+// allowed 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.
+	return strings.ContainsRune(key, '/'), nil
+}