From 0afef56feae48d68bccb42ae067189a5ec46339b Mon Sep 17 00:00:00 2001 From: apostasie Date: Sun, 11 Aug 2024 13:46:50 -0700 Subject: [PATCH] Auth code refactor: credstore and registry URL Signed-off-by: apostasie --- cmd/nerdctl/login_linux_test.go | 440 +++++++++++++----- cmd/nerdctl/logout.go | 62 +-- pkg/cmd/login/login.go | 131 ++---- pkg/cmd/logout/logout.go | 46 ++ .../dockerconfigresolver/credentialsstore.go | 181 +++++++ .../credentialsstore_test.go | 384 +++++++++++++++ pkg/imgutil/dockerconfigresolver/defaults.go | 51 ++ .../dockerconfigresolver.go | 96 +--- .../dockerconfigresolver_util.go | 50 -- .../dockerconfigresolver/registryurl.go | 147 ++++++ .../dockerconfigresolver/registryurl_test.go | 189 ++++++++ .../testregistry/testregistry_linux.go | 21 +- pkg/testutil/testutil.go | 2 +- 13 files changed, 1415 insertions(+), 385 deletions(-) create mode 100644 pkg/cmd/logout/logout.go create mode 100644 pkg/imgutil/dockerconfigresolver/credentialsstore.go create mode 100644 pkg/imgutil/dockerconfigresolver/credentialsstore_test.go create mode 100644 pkg/imgutil/dockerconfigresolver/defaults.go delete mode 100644 pkg/imgutil/dockerconfigresolver/dockerconfigresolver_util.go create mode 100644 pkg/imgutil/dockerconfigresolver/registryurl.go create mode 100644 pkg/imgutil/dockerconfigresolver/registryurl_test.go diff --git a/cmd/nerdctl/login_linux_test.go b/cmd/nerdctl/login_linux_test.go index 4429b4480ea..360a6aaa12c 100644 --- a/cmd/nerdctl/login_linux_test.go +++ b/cmd/nerdctl/login_linux_test.go @@ -14,32 +14,28 @@ limitations under the License. */ +// https://docs.docker.com/reference/cli/dockerd/#insecure-registries +// Local registries, whose IP address falls in the 127.0.0.0/8 range, are automatically marked as insecure as of Docker 1.3.2. +// It isn't recommended to rely on this, as it may change in the future. +// "--insecure" means that either the certificates are untrusted, or that the protocol is plain http + package main import ( - "crypto/rand" - "encoding/base64" "fmt" "net" "os" "strconv" "testing" + "gotest.tools/v3/icmd" + + "github.com/containerd/nerdctl/v2/pkg/imgutil/dockerconfigresolver" "github.com/containerd/nerdctl/v2/pkg/testutil" "github.com/containerd/nerdctl/v2/pkg/testutil/testca" "github.com/containerd/nerdctl/v2/pkg/testutil/testregistry" - "gotest.tools/v3/icmd" ) -func safeRandomString(n int) string { - b := make([]byte, n) - _, _ = rand.Read(b) - // XXX WARNING there is something in the registry (or more likely in the way we generate htpasswd files) - // that is broken and does not resist truly random strings - // return string(b) - return base64.URLEncoding.EncodeToString(b) -} - type Client struct { args []string configPath string @@ -56,269 +52,485 @@ func (ag *Client) WithHostsDir(hostDirs string) *Client { } func (ag *Client) WithCredentials(username, password string) *Client { - ag.args = append(ag.args, "--username", username, "--password", password) + if username != "" { + ag.args = append(ag.args, "--username", username) + } + if password != "" { + ag.args = append(ag.args, "--password", password) + } + return ag +} + +func (ag *Client) WithConfigPath(value string) *Client { + ag.configPath = value return ag } +func (ag *Client) GetConfigPath() string { + return ag.configPath +} + func (ag *Client) Run(base *testutil.Base, host string) *testutil.Cmd { if ag.configPath == "" { ag.configPath, _ = os.MkdirTemp(base.T.TempDir(), "docker-config") } - args := append([]string{"--debug-full", "login"}, ag.args...) + args := []string{"login"} + if base.Target == "nerdctl" { + args = append(args, "--debug-full") + } + args = append(args, ag.args...) icmdCmd := icmd.Command(base.Binary, append(base.Args, append(args, host)...)...) icmdCmd.Env = append(base.Env, "HOME="+os.Getenv("HOME"), "DOCKER_CONFIG="+ag.configPath) + return &testutil.Cmd{ Cmd: icmdCmd, Base: base, } } -func TestLogin(t *testing.T) { +func TestLoginPersistence(t *testing.T) { + base := testutil.NewBase(t) + t.Parallel() + + // Retrieve from the store + testCases := []struct { + auth string + }{ + { + "basic", + }, + { + "token", + }, + } + + for _, tc := range testCases { + t.Run(fmt.Sprintf("Server %s", tc.auth), func(t *testing.T) { + t.Parallel() + + username := testregistry.SafeRandomString(30) + "∞" + password := testregistry.SafeRandomString(30) + ":∞" + + // Add the requested authentication + var auth testregistry.Auth + var dependentCleanup func(error) + + auth = &testregistry.NoAuth{} + if tc.auth == "basic" { + auth = &testregistry.BasicAuth{ + Username: username, + Password: password, + } + } else if tc.auth == "token" { + authCa := testca.New(base.T) + as := testregistry.NewAuthServer(base, authCa, 0, username, password, false) + auth = &testregistry.TokenAuth{ + Address: as.Scheme + "://" + net.JoinHostPort(as.IP.String(), strconv.Itoa(as.Port)), + CertPath: as.CertPath, + } + dependentCleanup = as.Cleanup + } + + // Start the registry with the requested options + reg := testregistry.NewRegistry(base, nil, 0, auth, dependentCleanup) + + // Register registry cleanup + t.Cleanup(func() { + reg.Cleanup(nil) + }) + + // First, login successfully + c := (&Client{}). + WithCredentials(username, password) + + c.Run(base, fmt.Sprintf("localhost:%d", reg.Port)). + AssertOK() + + // Now, log in successfully without passing any explicit credentials + nc := (&Client{}). + WithConfigPath(c.GetConfigPath()) + nc.Run(base, fmt.Sprintf("localhost:%d", reg.Port)). + AssertOK() + + // Now fail while using invalid credentials + nc.WithCredentials("invalid", "invalid"). + Run(base, fmt.Sprintf("localhost:%d", reg.Port)). + AssertFail() + + // And login again without, reverting to the last saved good state + nc = (&Client{}). + WithConfigPath(c.GetConfigPath()) + + nc.Run(base, fmt.Sprintf("localhost:%d", reg.Port)). + AssertOK() + }) + } +} + +/* +func TestAgainstNoAuth(t *testing.T) { + base := testutil.NewBase(t) + t.Parallel() + + // Start the registry with the requested options + reg := testregistry.NewRegistry(base, nil, 0, &testregistry.NoAuth{}, nil) + + // Register registry cleanup + t.Cleanup(func() { + reg.Cleanup(nil) + }) + + c := (&Client{}). + WithCredentials("invalid", "invalid") + + c.Run(base, fmt.Sprintf("localhost:%d", reg.Port)). + AssertOK() + + content, _ := os.ReadFile(filepath.Join(c.configPath, "config.json")) + fmt.Println(string(content)) + + c.Run(base, fmt.Sprintf("localhost:%d", reg.Port)). + AssertFail() + +} + +*/ + +func TestLoginAgainstVariants(t *testing.T) { // Skip docker, because Docker doesn't have `--hosts-dir` nor `insecure-registry` option + // This will test access to a wide variety of servers, with or without TLS, with basic or token authentication testutil.DockerIncompatible(t) base := testutil.NewBase(t) t.Parallel() - testregistry.EnsureImages(base) - testCases := []struct { - port int - tls bool - auth string - insecure bool + port int + tls bool + auth string }{ + // Basic auth, no TLS { 80, false, "basic", - true, }, { 443, false, "basic", - true, }, { 0, false, "basic", - true, }, + // Token auth, no TLS { 80, - true, - "basic", false, + "token", }, { 443, - true, - "basic", false, + "token", }, { 0, - true, - "basic", false, + "token", }, + // Basic auth, with TLS { 80, - false, - "token", true, + "basic", }, { 443, - false, - "token", true, + "basic", }, { 0, - false, - "token", true, + "basic", }, + // Token auth, with TLS { 80, true, "token", - false, }, { 443, true, "token", - false, }, { 0, true, "token", - false, }, } + // Iterate through all cases, that will present a variety of port (80, 443, random), TLS (yes or no), and authentication (basic, token) type combinations for _, tc := range testCases { - // Since we have a lock mechanism for acquiring ports, we can just parallelize everything - t.Run(fmt.Sprintf("Login against registry with tls: %t port: %d auth: %s", tc.tls, tc.port, tc.auth), func(t *testing.T) { + port := tc.port + tls := tc.tls + auth := tc.auth + + t.Run(fmt.Sprintf("Login against `tls: %t port: %d auth: %s`", tls, port, auth), func(t *testing.T) { // Tests with fixed ports should not be parallelized (although the port locking mechanism will prevent conflicts) - // as their children are, and this might deadlock given how Parallel works - if tc.port == 0 { + // as their children tests are parallelized, and this might deadlock given the way `Parallel` works + if port == 0 { t.Parallel() } - // Generate credentials so that we never cross hit another test registry (spiced up with unicode) - // Note that the grammar for basic auth does not allow colons in usernames, while token auth allows it - username := safeRandomString(30) + "∞" - password := safeRandomString(30) + ":∞" + // Generate credentials that are specific to each registry, so that we never cross hit another one + username := testregistry.SafeRandomString(30) + "∞" + password := testregistry.SafeRandomString(30) + ":∞" // Get a CA if we want TLS var ca *testca.CA - if tc.tls { + if tls { ca = testca.New(base.T) } - // Add the requested authentication - var auth testregistry.Auth - auth = &testregistry.NoAuth{} + // Add the requested authenticator + var authenticator testregistry.Auth var dependentCleanup func(error) - if tc.auth == "basic" { - auth = &testregistry.BasicAuth{ + + authenticator = &testregistry.NoAuth{} + if auth == "basic" { + authenticator = &testregistry.BasicAuth{ Username: username, Password: password, } - } else if tc.auth == "token" { + } else if auth == "token" { authCa := ca - // We could be on !tls - still need a ca to sign jwt + // We could be on !tls, meaning no ca - but we still need a CA to sign jwt tokens if authCa == nil { authCa = testca.New(base.T) } - as := testregistry.NewAuthServer(base, authCa, 0, username, password, tc.tls) - auth = &testregistry.TokenAuth{ + as := testregistry.NewAuthServer(base, authCa, 0, username, password, tls) + authenticator = &testregistry.TokenAuth{ Address: as.Scheme + "://" + net.JoinHostPort(as.IP.String(), strconv.Itoa(as.Port)), CertPath: as.CertPath, } dependentCleanup = as.Cleanup } - // Start the registry - reg := testregistry.NewRegistry(base, ca, tc.port, auth, dependentCleanup) + // Start the registry with the requested options + reg := testregistry.NewRegistry(base, ca, port, authenticator, dependentCleanup) - // Attach our cleanup function + // Register registry cleanup t.Cleanup(func() { reg.Cleanup(nil) }) + // Any registry is reachable through its ip+port, and localhost variants regHosts := []string{ net.JoinHostPort(reg.IP.String(), strconv.Itoa(reg.Port)), + net.JoinHostPort("localhost", strconv.Itoa(reg.Port)), + net.JoinHostPort("127.0.0.1", strconv.Itoa(reg.Port)), + // TODO: ipv6 + // net.JoinHostPort("::1", strconv.Itoa(reg.Port)), } - // XXX seems like omitting ports is broken on main currently - // (plus the hosts.toml resolution is not good either) - // XXX we should also add hostname here (maybe use the container name?) - // Obviously also need to add localhost to the mix once we fix behavior - /* - if reg.Port == 443 || reg.Port == 80 { - regHosts = append(regHosts, reg.IP.String()) - } - */ + // Registries that use port 443 also allow access without specifying a port + if reg.Port == 443 { + regHosts = append(regHosts, reg.IP.String()) + regHosts = append(regHosts, "localhost") + regHosts = append(regHosts, "127.0.0.1") + // TODO: ipv6 + // regHosts = append(regHosts, "::1") + } + // Iterate through these hosts access points, and create a test per-variant for _, value := range regHosts { regHost := value t.Run(regHost, func(t *testing.T) { t.Parallel() - t.Run("Valid credentials (no certs) ", func(t *testing.T) { + // 1. test with valid credentials but no access to the CA + t.Run("1. valid credentials (no CA) ", func(t *testing.T) { t.Parallel() + c := (&Client{}). WithCredentials(username, password) - // Fail without insecure - c.Run(base, regHost).AssertFail() + rl, _ := dockerconfigresolver.Parse(regHost) + // a. Insecure flag not being set + // TODO: remove specialization when we fix the localhost mess + if rl.IsLocalhost() && !tls { + c.Run(base, regHost). + AssertOK() + } else { + c.Run(base, regHost). + AssertFail() + } + + // b. Insecure flag set to false + // TODO: remove specialization when we fix the localhost mess + if !rl.IsLocalhost() { + (&Client{}). + WithCredentials(username, password). + WithInsecure(false). + Run(base, regHost). + AssertFail() + } - // Succeed with insecure - c.WithInsecure(true). - Run(base, regHost).AssertOK() + // c. Insecure flag set to true + // TODO: remove specialization when we fix the localhost mess + if !rl.IsLocalhost() || !tls { + (&Client{}). + WithCredentials(username, password). + WithInsecure(true). + Run(base, regHost). + AssertOK() + } }) - t.Run("Valid credentials (with certs)", func(t *testing.T) { + // 2. test with valid credentials AND access to the CA + t.Run("2. valid credentials (with access to server CA)", func(t *testing.T) { t.Parallel() + + rl, _ := dockerconfigresolver.Parse(regHost) + + // a. Insecure flag not being set c := (&Client{}). WithCredentials(username, password). WithHostsDir(reg.HostsDir) - if tc.insecure { - c.Run(base, regHost).AssertFail() + if tls || rl.IsLocalhost() { + c.Run(base, regHost). + AssertOK() + } else { + c.Run(base, regHost). + AssertFail() + } + + // b. Insecure flag set to false + if tls { + c.WithInsecure(false). + Run(base, regHost). + AssertOK() } else { - c.Run(base, regHost).AssertOK() + // TODO: remove specialization when we fix the localhost mess + if !rl.IsLocalhost() { + c.WithInsecure(false). + Run(base, regHost). + AssertFail() + } } + // c. Insecure flag set to true c.WithInsecure(true). - Run(base, regHost).AssertOK() + Run(base, regHost). + AssertOK() }) - t.Run("Valid credentials (with certs), any variant", func(t *testing.T) { + t.Run("3. valid credentials, any url variant, should always succeed", func(t *testing.T) { t.Parallel() c := (&Client{}). WithCredentials(username, password). WithHostsDir(reg.HostsDir). // Just use insecure here for all servers - it does not matter for what we are testing here + // in this case, which is whether we can successfully log in against any of these variants WithInsecure(true) - c.Run(base, "http://"+regHost).AssertOK() - c.Run(base, "https://"+regHost).AssertOK() - c.Run(base, "http://"+regHost+"/whatever?foo=bar;foo:bar#foo=bar").AssertOK() - c.Run(base, "https://"+regHost+"/whatever?foo=bar&bar=foo;foo=foo+bar:bar#foo=bar").AssertOK() + // TODO: remove specialization when we fix the localhost mess + rl, _ := dockerconfigresolver.Parse(regHost) + if !rl.IsLocalhost() || !tls { + c.Run(base, "http://"+regHost).AssertOK() + c.Run(base, "https://"+regHost).AssertOK() + c.Run(base, "http://"+regHost+"/whatever?foo=bar;foo:bar#foo=bar").AssertOK() + c.Run(base, "https://"+regHost+"/whatever?foo=bar&bar=foo;foo=foo+bar:bar#foo=bar").AssertOK() + } }) - t.Run("Wrong pass (no certs)", func(t *testing.T) { + t.Run("4. wrong password should always fail", func(t *testing.T) { t.Parallel() - c := (&Client{}). - WithCredentials(username, "invalid") - c.Run(base, regHost).AssertFail() + (&Client{}). + WithCredentials(username, "invalid"). + WithHostsDir(reg.HostsDir). + Run(base, regHost). + AssertFail() - c.WithInsecure(true). - Run(base, regHost).AssertFail() - }) + (&Client{}). + WithCredentials(username, "invalid"). + WithHostsDir(reg.HostsDir). + WithInsecure(false). + Run(base, regHost). + AssertFail() - t.Run("Wrong user (no certs)", func(t *testing.T) { - t.Parallel() - c := (&Client{}). - WithCredentials("invalid", password) + (&Client{}). + WithCredentials(username, "invalid"). + WithHostsDir(reg.HostsDir). + WithInsecure(true). + Run(base, regHost). + AssertFail() - c.Run(base, regHost).AssertFail() + (&Client{}). + WithCredentials(username, "invalid"). + Run(base, regHost). + AssertFail() - c.WithInsecure(true). - Run(base, regHost).AssertFail() + (&Client{}). + WithCredentials(username, "invalid"). + WithInsecure(false). + Run(base, regHost). + AssertFail() + + (&Client{}). + WithCredentials(username, "invalid"). + WithInsecure(true). + Run(base, regHost). + AssertFail() }) - t.Run("Wrong pass (with certs)", func(t *testing.T) { + t.Run("5. wrong username should always fail", func(t *testing.T) { t.Parallel() - c := (&Client{}). - WithCredentials(username, "invalid"). - WithHostsDir(reg.HostsDir) - c.Run(base, regHost).AssertFail() + (&Client{}). + WithCredentials("invalid", password). + WithHostsDir(reg.HostsDir). + Run(base, regHost). + AssertFail() - c.WithInsecure(true). - Run(base, regHost).AssertFail() - }) + (&Client{}). + WithCredentials("invalid", password). + WithHostsDir(reg.HostsDir). + WithInsecure(false). + Run(base, regHost). + AssertFail() - t.Run("Wrong user (with certs)", func(t *testing.T) { - t.Parallel() - c := (&Client{}). + (&Client{}). WithCredentials("invalid", password). - WithHostsDir(reg.HostsDir) + WithHostsDir(reg.HostsDir). + WithInsecure(true). + Run(base, regHost). + AssertFail() - c.Run(base, regHost).AssertFail() + (&Client{}). + WithCredentials("invalid", password). + Run(base, regHost). + AssertFail() - c.WithInsecure(true). - Run(base, regHost).AssertFail() + (&Client{}). + WithCredentials("invalid", password). + WithInsecure(false). + Run(base, regHost). + AssertFail() + + (&Client{}). + WithCredentials("invalid", password). + WithInsecure(true). + Run(base, regHost). + AssertFail() }) }) } diff --git a/cmd/nerdctl/logout.go b/cmd/nerdctl/logout.go index a08c2bdd0a8..e88953ff580 100644 --- a/cmd/nerdctl/logout.go +++ b/cmd/nerdctl/logout.go @@ -17,15 +17,14 @@ package main import ( - "fmt" - - "github.com/containerd/nerdctl/v2/pkg/imgutil/dockerconfigresolver" - dockercliconfig "github.com/docker/cli/cli/config" "github.com/spf13/cobra" + + "github.com/containerd/log" + "github.com/containerd/nerdctl/v2/pkg/cmd/logout" ) func newLogoutCommand() *cobra.Command { - var logoutCommand = &cobra.Command{ + return &cobra.Command{ Use: "logout [flags] [SERVER]", Args: cobra.MaximumNArgs(1), Short: "Log out from a container registry", @@ -34,62 +33,33 @@ func newLogoutCommand() *cobra.Command { SilenceUsage: true, SilenceErrors: true, } - return logoutCommand } -// code inspired from XXX func logoutAction(cmd *cobra.Command, args []string) error { - serverAddress := dockerconfigresolver.IndexServer - isDefaultRegistry := true - if len(args) >= 1 { - serverAddress = args[0] - isDefaultRegistry = false - } - - var ( - regsToLogout = []string{serverAddress} - hostnameAddress = serverAddress - ) - - if !isDefaultRegistry { - hostnameAddress = dockerconfigresolver.ConvertToHostname(serverAddress) - // the tries below are kept for backward compatibility where a user could have - // saved the registry in one of the following format. - regsToLogout = append(regsToLogout, hostnameAddress, "http://"+hostnameAddress, "https://"+hostnameAddress) + logoutServer := "" + if len(args) > 0 { + logoutServer = args[0] } - fmt.Fprintf(cmd.OutOrStdout(), "Removing login credentials for %s\n", hostnameAddress) - - dockerConfigFile, err := dockercliconfig.Load("") + errGroup, err := logout.Logout(cmd.Context(), logoutServer) if err != nil { - return err + log.L.WithError(err).Errorf("Failed to erase credentials for: %s", logoutServer) } - errs := make(map[string]error) - for _, r := range regsToLogout { - if err := dockerConfigFile.GetCredentialsStore(r).Erase(r); err != nil { - errs[r] = err + if errGroup != nil { + log.L.Error("None of the following entries could be found") + for _, v := range errGroup { + log.L.Errorf("%s", v) } } - // if at least one removal succeeded, report success. Otherwise report errors - if len(errs) == len(regsToLogout) { - fmt.Fprintln(cmd.ErrOrStderr(), "WARNING: could not erase credentials:") - for k, v := range errs { - fmt.Fprintf(cmd.OutOrStdout(), "%s: %s\n", k, v) - } - } - - return nil + return err } func logoutShellComplete(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - dockerConfigFile, err := dockercliconfig.Load("") + candidates, err := logout.ShellCompletion() if err != nil { return nil, cobra.ShellCompDirectiveError } - candidates := []string{} - for key := range dockerConfigFile.AuthConfigs { - candidates = append(candidates, key) - } + return candidates, cobra.ShellCompDirectiveNoFileComp } diff --git a/pkg/cmd/login/login.go b/pkg/cmd/login/login.go index e2906b14ed7..41912799416 100644 --- a/pkg/cmd/login/login.go +++ b/pkg/cmd/login/login.go @@ -27,9 +27,6 @@ import ( "os" "strings" - dockercliconfig "github.com/docker/cli/cli/config" - dockercliconfigtypes "github.com/docker/cli/cli/config/types" - "github.com/docker/docker/api/types/registry" "golang.org/x/net/context/ctxhttp" "golang.org/x/term" @@ -47,106 +44,64 @@ Configure a credential helper to remove this warning. See https://docs.docker.com/engine/reference/commandline/login/#credentials-store ` -type isFileStore interface { - IsFileStore() bool - GetFilename() string -} - func Login(ctx context.Context, options types.LoginCommandOptions, stdout io.Writer) error { - var serverAddress string - if options.ServerAddress == "" || options.ServerAddress == "docker.io" || options.ServerAddress == "index.docker.io" || options.ServerAddress == "registry-1.docker.io" { - serverAddress = dockerconfigresolver.IndexServer - } else { - serverAddress = options.ServerAddress + registryURL, err := dockerconfigresolver.Parse(options.ServerAddress) + if err != nil { + return err + } + + credStore, err := dockerconfigresolver.NewCredentialsStore("") + if err != nil { + return err } var responseIdentityToken string - isDefaultRegistry := serverAddress == dockerconfigresolver.IndexServer - authConfig, err := GetDefaultAuthConfig(options.Username == "" && options.Password == "", serverAddress, isDefaultRegistry) - if authConfig == nil { - authConfig = ®istry.AuthConfig{ServerAddress: serverAddress} - } - if err == nil && authConfig.Username != "" && authConfig.Password != "" { - // login With StoreCreds - responseIdentityToken, err = loginClientSide(ctx, options.GOptions, *authConfig) + credentials, err := credStore.Retrieve(registryURL, options.Username == "" && options.Password == "") + credentials.IdentityToken = "" + + if err == nil && credentials.Username != "" && credentials.Password != "" { + responseIdentityToken, err = loginClientSide(ctx, options.GOptions, registryURL, credentials) } - if err != nil || authConfig.Username == "" || authConfig.Password == "" { - err = ConfigureAuthentication(authConfig, options.Username, options.Password) + if err != nil || credentials.Username == "" || credentials.Password == "" { + err = configureAuthentication(credentials, options.Username, options.Password) if err != nil { return err } - responseIdentityToken, err = loginClientSide(ctx, options.GOptions, *authConfig) + responseIdentityToken, err = loginClientSide(ctx, options.GOptions, registryURL, credentials) if err != nil { return err } } if responseIdentityToken != "" { - authConfig.Password = "" - authConfig.IdentityToken = responseIdentityToken - } - - dockerConfigFile, err := dockercliconfig.Load("") - if err != nil { - return err + credentials.Password = "" + credentials.IdentityToken = responseIdentityToken } - creds := dockerConfigFile.GetCredentialsStore(serverAddress) - - store, isFile := creds.(isFileStore) // Display a warning if we're storing the users password (not a token) and credentials store type is file. - if isFile && authConfig.Password != "" { - _, err = fmt.Fprintln(stdout, fmt.Sprintf(unencryptedPasswordWarning, store.GetFilename())) + storageFileLocation := credStore.FileStorageLocation(registryURL) + if storageFileLocation != "" && credentials.Password != "" { + _, err = fmt.Fprintln(stdout, fmt.Sprintf(unencryptedPasswordWarning, storageFileLocation)) if err != nil { return err } } - if err := creds.Store(dockercliconfigtypes.AuthConfig(*(authConfig))); err != nil { + err = credStore.Store(registryURL, credentials) + if err != nil { return fmt.Errorf("error saving credentials: %w", err) } - fmt.Fprintln(stdout, "Login Succeeded") - - return nil -} + _, err = fmt.Fprintln(stdout, "Login Succeeded") -// GetDefaultAuthConfig gets the default auth config given a serverAddress. -// If credentials for given serverAddress exists in the credential store, the configuration will be populated with values in it. -// Code from github.com/docker/cli/cli/command (v20.10.3). -func GetDefaultAuthConfig(checkCredStore bool, serverAddress string, isDefaultRegistry bool) (*registry.AuthConfig, error) { - if !isDefaultRegistry { - var err error - serverAddress, err = convertToHostname(serverAddress) - if err != nil { - return nil, err - } - } - authconfig := dockercliconfigtypes.AuthConfig{} - if checkCredStore { - dockerConfigFile, err := dockercliconfig.Load("") - if err != nil { - return nil, err - } - authconfig, err = dockerConfigFile.GetAuthConfig(serverAddress) - if err != nil { - return nil, err - } - } - authconfig.ServerAddress = serverAddress - authconfig.IdentityToken = "" - res := registry.AuthConfig(authconfig) - return &res, nil + return err } -func loginClientSide(ctx context.Context, globalOptions types.GlobalCommandOptions, auth registry.AuthConfig) (string, error) { - host, err := convertToHostname(auth.ServerAddress) - if err != nil { - return "", err - } +func loginClientSide(ctx context.Context, globalOptions types.GlobalCommandOptions, registryURL *dockerconfigresolver.RegistryURL, credentials *dockerconfigresolver.Credentials) (string, error) { + host := registryURL.Host var dOpts []dockerconfigresolver.Opt if globalOptions.InsecureRegistry { log.G(ctx).Warnf("skipping verifying HTTPS certs for %q", host) @@ -156,12 +111,12 @@ func loginClientSide(ctx context.Context, globalOptions types.GlobalCommandOptio authCreds := func(acArg string) (string, string, error) { if acArg == host { - if auth.RegistryToken != "" { + if credentials.RegistryToken != "" { // Even containerd/CRI does not support RegistryToken as of v1.4.3, // so, nobody is actually using RegistryToken? log.G(ctx).Warnf("RegistryToken (for %q) is not supported yet (FIXME)", host) } - return auth.Username, auth.Password, nil + return credentials.Username, credentials.Password, nil } return "", "", fmt.Errorf("expected acArg to be %q, got %q", host, acArg) } @@ -250,10 +205,9 @@ func tryLoginWithRegHost(ctx context.Context, rh docker.RegistryHost) error { return errors.New("too many 401 (probably)") } -func ConfigureAuthentication(authConfig *registry.AuthConfig, username, password string) error { - authConfig.Username = strings.TrimSpace(authConfig.Username) +func configureAuthentication(credentials *dockerconfigresolver.Credentials, username, password string) error { if username = strings.TrimSpace(username); username == "" { - username = authConfig.Username + username = credentials.Username } if username == "" { fmt.Print("Enter Username: ") @@ -280,8 +234,8 @@ func ConfigureAuthentication(authConfig *registry.AuthConfig, username, password return fmt.Errorf("error: Password is Required") } - authConfig.Username = username - authConfig.Password = password + credentials.Username = username + credentials.Password = password return nil } @@ -303,22 +257,3 @@ func readUsername() (string, error) { return username, nil } - -func convertToHostname(serverAddress string) (string, error) { - // Ensure that URL contains scheme for a good parsing process - if strings.Contains(serverAddress, "://") { - u, err := url.Parse(serverAddress) - if err != nil { - return "", err - } - serverAddress = u.Host - } else { - u, err := url.Parse("https://" + serverAddress) - if err != nil { - return "", err - } - serverAddress = u.Host - } - - return serverAddress, nil -} diff --git a/pkg/cmd/logout/logout.go b/pkg/cmd/logout/logout.go new file mode 100644 index 00000000000..99a4b77f852 --- /dev/null +++ b/pkg/cmd/logout/logout.go @@ -0,0 +1,46 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package logout + +import ( + "context" + + "github.com/containerd/nerdctl/v2/pkg/imgutil/dockerconfigresolver" +) + +func Logout(ctx context.Context, logoutServer string) (map[string]error, error) { + reg, err := dockerconfigresolver.Parse(logoutServer) + if err != nil { + return nil, err + } + + credentialsStore, err := dockerconfigresolver.NewCredentialsStore("") + if err != nil { + return nil, err + } + + return credentialsStore.Erase(reg) +} + +func ShellCompletion() ([]string, error) { + credentialsStore, err := dockerconfigresolver.NewCredentialsStore("") + if err != nil { + return nil, err + } + + return credentialsStore.ShellCompletion(), nil +} diff --git a/pkg/imgutil/dockerconfigresolver/credentialsstore.go b/pkg/imgutil/dockerconfigresolver/credentialsstore.go new file mode 100644 index 00000000000..2afd353d4ba --- /dev/null +++ b/pkg/imgutil/dockerconfigresolver/credentialsstore.go @@ -0,0 +1,181 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package dockerconfigresolver + +import ( + "errors" + "fmt" + "strings" + + "github.com/containerd/log" + "github.com/docker/cli/cli/config" + "github.com/docker/cli/cli/config/configfile" + "github.com/docker/cli/cli/config/types" +) + +type Credentials = types.AuthConfig + +// NewCredentialsStore returns a CredentialsStore from a directory +// If path is left empty, the default docker `~/.docker/config.json` will be used +// In case the docker call fails, we wrap the error with ErrUnableToInstantiate +func NewCredentialsStore(path string) (*CredentialsStore, error) { + dockerConfigFile, err := config.Load(path) + if err != nil { + return nil, errors.Join(ErrUnableToInstantiate, err) + } + + return &CredentialsStore{ + dockerConfigFile: dockerConfigFile, + }, nil +} + +// CredentialsStore is an abstraction in front of docker config API manipulation +// exposing just the limited functions we need and hiding away url normalization / identifiers magic, and handling of +// backward compatibility +type CredentialsStore struct { + dockerConfigFile *configfile.ConfigFile +} + +// Erase will remove any and all stored credentials for that registry namespace (including all legacy variants) +// If we do not find at least ONE variant matching the namespace, this will error with ErrUnableToErase +func (cs *CredentialsStore) Erase(registryURL *RegistryURL) (map[string]error, error) { + // Get all associated identifiers for that registry including legacy ones and variants + logoutList := registryURL.AllIdentifiers() + + // Iterate through and delete them one by one + errs := make(map[string]error) + for _, serverAddress := range logoutList { + if err := cs.dockerConfigFile.GetCredentialsStore(serverAddress).Erase(serverAddress); err != nil { + errs[serverAddress] = err + } + } + + // If we succeeded removing at least one, it is a success. + // The only error condition is if we failed removing *everything* - meaning there was no such credential information + // in whatever format - or the store is broken. + if len(errs) == len(logoutList) { + return errs, ErrUnableToErase + } + + return nil, nil +} + +// Store will save credentials for a given registry +// On error, ErrUnableToStore +func (cs *CredentialsStore) Store(registryURL *RegistryURL, credentials *Credentials) error { + // We just overwrite the server property here with the host + // Whether it was one of the variants, or was not set at all (see for example Amazon ECR, https://github.com/containerd/nerdctl/issues/733 + // - which is likely a bug in docker) it doesn't matter. + // This is the credentials that were returned for that host, by the docker credentials store. + if registryURL.Namespace != nil { + credentials.ServerAddress = fmt.Sprintf("%s%s?%s", registryURL.Host, registryURL.Path, registryURL.RawQuery) + } else { + credentials.ServerAddress = registryURL.Host + } + + // XXX future namespaced url likely require special handling here + if err := cs.dockerConfigFile.GetCredentialsStore(registryURL.CanonicalIdentifier()).Store(*(credentials)); err != nil { + return errors.Join(ErrUnableToStore, err) + } + + return nil +} + +// ShellCompletion will return candidate strings for nerdctl logout +func (cs *CredentialsStore) ShellCompletion() []string { + candidates := []string{} + for key := range cs.dockerConfigFile.AuthConfigs { + candidates = append(candidates, key) + } + + return candidates +} + +// FileStorageLocation will return the file where credentials are stored for a given registry, or the empty string +// if it is stored / to be stored in a different place (like an OS keychain, with docker credential helpers) +func (cs *CredentialsStore) FileStorageLocation(registryURL *RegistryURL) string { + if store, isFile := (cs.dockerConfigFile.GetCredentialsStore(registryURL.CanonicalIdentifier())).(isFileStore); isFile { + return store.GetFilename() + } + + return "" +} + +// Retrieve gets existing credentials from the store for a certain registry. +// If none are found, an empty Credentials struct is returned. +// If we hard-fail reading from the store, indicative of a broken system, we wrap the error with ErrUnableToRetrieve +func (cs *CredentialsStore) Retrieve(registryURL *RegistryURL, checkCredStore bool) (*Credentials, error) { + var err error + returnedCredentials := &Credentials{} + + // As long as we depend on .ServerAddress, make sure it is populated correctly + // It does not matter what was stored - the docker cli clearly has issues with this + // What matters is that the credentials retrieved from the docker credentials store are *for that registryURL* + // and that is what ServerAddress should point to + defer func() { + if registryURL.Namespace != nil { + returnedCredentials.ServerAddress = fmt.Sprintf("%s%s?%s", registryURL.Host, registryURL.Path, registryURL.RawQuery) + } else { + returnedCredentials.ServerAddress = registryURL.Host + } + }() + + if !checkCredStore { + return returnedCredentials, nil + } + + log.L.Debugf("retrieving credentials from store for %s", registryURL.CanonicalIdentifier()) + + // Get the legacy variants (w/o scheme or port), and iterate over until we find one with credentials + variants := registryURL.AllIdentifiers() + + log.L.Debugf("all identifiers %v", variants) + + for _, identifier := range variants { + log.L.Debugf("looking up %s", identifier) + var credentials types.AuthConfig + // Note that Get does not raise an error on ENOENT + credentials, err = cs.dockerConfigFile.GetCredentialsStore(identifier).Get(identifier) + if err != nil { + continue + } + returnedCredentials = &credentials + // Clean-up the username + returnedCredentials.Username = strings.TrimSpace(returnedCredentials.Username) + // Stop here if we found credentials with this variant + if returnedCredentials.IdentityToken != "" || + returnedCredentials.Username != "" || + returnedCredentials.Password != "" || + returnedCredentials.RegistryToken != "" { + log.L.Debugf("found credentials %v", returnedCredentials) + break + } + } + + // (Last non nil) credential store error gets wrapped into ErrUnableToRetrieve + if err != nil { + err = errors.Join(ErrUnableToRetrieve, err) + } + + return returnedCredentials, err +} + +// isFileStore is an internal mock interface purely meant to help identify that the docker credential backend is a filesystem one +type isFileStore interface { + IsFileStore() bool + GetFilename() string +} diff --git a/pkg/imgutil/dockerconfigresolver/credentialsstore_test.go b/pkg/imgutil/dockerconfigresolver/credentialsstore_test.go new file mode 100644 index 00000000000..db8f93c9c13 --- /dev/null +++ b/pkg/imgutil/dockerconfigresolver/credentialsstore_test.go @@ -0,0 +1,384 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package dockerconfigresolver + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + "gotest.tools/v3/assert" +) + +func createTempDir(t *testing.T, mode os.FileMode) string { + tmpDir, err := os.MkdirTemp(t.TempDir(), "docker-config") + if err != nil { + t.Fatal(err) + } + err = os.Chmod(tmpDir, mode) + if err != nil { + t.Fatal(err) + } + return tmpDir +} + +func TestBrokenCredentialsStore(t *testing.T) { + if runtime.GOOS == "freebsd" { + // It is unclear why these tests are failing on FreeBSD, and if it is a problem with Vagrant or differences + // with FreeBSD + // Anyhow, this test is about extreme cases & conditions (filesystem errors wrt credentials loading). + t.Skip("skipping broken credential store tests for freebsd") + } + + testCases := []struct { + description string + setup func() string + errorNew error + errorRead error + errorWrite error + }{ + { + description: "Pointing DOCKER_CONFIG at a non-existent directory inside an unreadable directory will prevent instantiation", + setup: func() string { + tmpDir := createTempDir(t, 0000) + return filepath.Join(tmpDir, "doesnotexistcantcreate") + }, + errorNew: ErrUnableToInstantiate, + }, + { + description: "Pointing DOCKER_CONFIG at a non-existent directory inside a read-only directory will prevent saving credentials", + setup: func() string { + tmpDir := createTempDir(t, 0500) + return filepath.Join(tmpDir, "doesnotexistcantcreate") + }, + errorWrite: ErrUnableToStore, + }, + { + description: "Pointing DOCKER_CONFIG at an unreadable directory will prevent instantiation", + setup: func() string { + return createTempDir(t, 0000) + }, + errorNew: ErrUnableToInstantiate, + }, + { + description: "Pointing DOCKER_CONFIG at a read-only directory will prevent saving credentials", + setup: func() string { + return createTempDir(t, 0500) + }, + errorWrite: ErrUnableToStore, + }, + { + description: "Pointing DOCKER_CONFIG at a directory containing am unparsable `config.json` will prevent instantiation", + setup: func() string { + tmpDir := createTempDir(t, 0700) + err := os.WriteFile(filepath.Join(tmpDir, "config.json"), []byte("porked"), 0600) + if err != nil { + t.Fatal(err) + } + return tmpDir + }, + errorNew: ErrUnableToInstantiate, + }, + { + description: "Pointing DOCKER_CONFIG at a file instead of a directory will prevent instantiation", + setup: func() string { + tmpDir := createTempDir(t, 0700) + fd, err := os.OpenFile(filepath.Join(tmpDir, "isafile"), os.O_CREATE, 0600) + if err != nil { + t.Fatal(err) + } + err = fd.Close() + if err != nil { + t.Fatal(err) + } + return filepath.Join(tmpDir, "isafile") + }, + errorNew: ErrUnableToInstantiate, + }, + { + description: "Pointing DOCKER_CONFIG at a directory containing a `config.json` directory will prevent instantiation", + setup: func() string { + tmpDir := createTempDir(t, 0700) + err := os.Mkdir(filepath.Join(tmpDir, "config.json"), 0600) + if err != nil { + t.Fatal(err) + } + return tmpDir + }, + errorNew: ErrUnableToInstantiate, + }, + // FIXME: this test works in Lima, but does not work on github action + // It is unclear why at this point. + /* + { + description: "Pointing DOCKER_CONFIG at a directory containing a `config.json` dangling symlink will prevent saving credentials", + setup: func() string { + tmpDir := createTempDir(t, 0700) + err := os.Symlink("doesnotexist", filepath.Join(tmpDir, "config.json")) + if err != nil { + t.Fatal(err) + } + return tmpDir + }, + errorWrite: ErrUnableToStore, + }, + */ + { + description: "Pointing DOCKER_CONFIG at a directory containing an unreadable, valid `config.json` file will prevent instantiation", + setup: func() string { + tmpDir := createTempDir(t, 0700) + err := os.WriteFile(filepath.Join(tmpDir, "config.json"), []byte("{}"), 0600) + if err != nil { + t.Fatal(err) + } + err = os.Chmod(filepath.Join(tmpDir, "config.json"), 0000) + if err != nil { + t.Fatal(err) + } + return tmpDir + }, + errorNew: ErrUnableToInstantiate, + }, + { + description: "Pointing DOCKER_CONFIG at a directory containing a read-only, valid `config.json` file will NOT prevent saving credentials", + setup: func() string { + tmpDir := createTempDir(t, 0700) + err := os.WriteFile(filepath.Join(tmpDir, "config.json"), []byte("{}"), 0600) + if err != nil { + t.Fatal(err) + } + err = os.Chmod(filepath.Join(tmpDir, "config.json"), 0400) + if err != nil { + t.Fatal(err) + } + return tmpDir + }, + }, + } + + t.Run("Broken Docker Config testing", func(t *testing.T) { + registryURL, err := Parse("registry") + if err != nil { + t.Fatal(err) + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + directory := tc.setup() + cs, err := NewCredentialsStore(directory) + assert.ErrorIs(t, err, tc.errorNew) + if err != nil { + return + } + + var af *Credentials + af, err = cs.Retrieve(registryURL, true) + assert.ErrorIs(t, err, tc.errorRead) + + err = cs.Store(registryURL, af) + assert.ErrorIs(t, err, tc.errorWrite) + }) + } + }) +} + +func writeContent(t *testing.T, content string) string { + t.Helper() + tmpDir := createTempDir(t, 0700) + err := os.WriteFile(filepath.Join(tmpDir, "config.json"), []byte(content), 0600) + if err != nil { + t.Fatal(err) + } + return tmpDir +} + +func TestWorkingCredentialsStore(t *testing.T) { + testCases := []struct { + description string + setup func() string + username string + password string + }{ + { + description: "Reading credentials from `auth` using canonical identifier", + username: "username", + password: "password", + setup: func() string { + content := `{ + "auths": { + "someregistry:443": { + "auth": "dXNlcm5hbWU6cGFzc3dvcmQ=" + } + } + }` + return writeContent(t, content) + }, + }, + { + description: "Reading from legacy / alternative identifiers: someregistry", + username: "username", + setup: func() string { + content := `{ + "auths": { + "someregistry": { + "username": "username" + } + } + }` + return writeContent(t, content) + }, + }, + { + description: "Reading from legacy / alternative identifiers: http://someregistry", + username: "username", + setup: func() string { + content := `{ + "auths": { + "http://someregistry": { + "username": "username" + } + } + }` + return writeContent(t, content) + }, + }, + { + description: "Reading from legacy / alternative identifiers: https://someregistry", + username: "username", + setup: func() string { + content := `{ + "auths": { + "https://someregistry": { + "username": "username" + } + } + }` + return writeContent(t, content) + }, + }, + { + description: "Reading from legacy / alternative identifiers: http://someregistry:443", + username: "username", + setup: func() string { + content := `{ + "auths": { + "http://someregistry:443": { + "username": "username" + } + } + }` + return writeContent(t, content) + }, + }, + { + description: "Reading from legacy / alternative identifiers: https://someregistry:443", + username: "username", + setup: func() string { + content := `{ + "auths": { + "https://someregistry:443": { + "username": "username" + } + } + }` + return writeContent(t, content) + }, + }, + { + description: "Canonical form is preferred over legacy forms", + username: "pick", + setup: func() string { + content := `{ + "auths": { + "http://someregistry:443": { + "username": "ignore" + }, + "https://someregistry:443": { + "username": "ignore" + }, + "someregistry": { + "username": "ignore" + }, + "someregistry:443": { + "serveraddress": "bla", + "username": "pick" + }, + "http://someregistry": { + "username": "ignore" + }, + "https://someregistry": { + "username": "ignore" + } + } +}` + return writeContent(t, content) + }, + }, + } + + t.Run("Working credentials store", func(t *testing.T) { + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + registryURL, err := Parse("someregistry") + if err != nil { + t.Fatal(err) + } + cs, err := NewCredentialsStore(tc.setup()) + if err != nil { + t.Fatal(err) + } + + var af *Credentials + af, err = cs.Retrieve(registryURL, true) + assert.ErrorIs(t, err, nil) + assert.Equal(t, af.Username, tc.username) + assert.Equal(t, af.ServerAddress, "someregistry:443") + assert.Equal(t, af.Password, tc.password) + }) + } + }) + + t.Run("Namespaced host", func(t *testing.T) { + server := "somehost.com/path?ns=someregistry.com" + registryURL, err := Parse(server) + if err != nil { + t.Fatal(err) + } + + content := `{ + "auths": { + "nerdctl-experimental://someregistry.com:443/host/somehost.com:443/path": { + "username": "username" + } + } + }` + dir := writeContent(t, content) + cs, err := NewCredentialsStore(dir) + if err != nil { + t.Fatal(err) + } + + var af *Credentials + af, err = cs.Retrieve(registryURL, true) + assert.ErrorIs(t, err, nil) + assert.Equal(t, af.Username, "username") + assert.Equal(t, af.ServerAddress, "somehost.com:443/path?ns=someregistry.com") + + }) +} diff --git a/pkg/imgutil/dockerconfigresolver/defaults.go b/pkg/imgutil/dockerconfigresolver/defaults.go new file mode 100644 index 00000000000..db78f3f70f7 --- /dev/null +++ b/pkg/imgutil/dockerconfigresolver/defaults.go @@ -0,0 +1,51 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package dockerconfigresolver + +import "errors" + +type scheme string + +const ( + standardHTTPSPort = "443" + schemeHTTP scheme = "http" + schemeHTTPS scheme = "https" + // schemeNerdctlExperimental is currently provisional, to unlock namespace based host authentication + // This may change or break without notice, and you should have no expectations that credentials saved like that + // will be supported in the future + schemeNerdctlExperimental scheme = "nerdctl-experimental" + // See https://github.com/moby/moby/blob/v27.1.1/registry/config.go#L42-L48 + //nolint:misspell + // especially Sebastiaan comments on future domain consolidation + dockerIndexServer = "https://index.docker.io/v1/" + // The query parameter that containerd will slap on namespaced hosts + namespaceQueryParameter = "ns" +) + +// Errors returned by the credentials store +var ( + ErrUnableToInstantiate = errors.New("unable to instantiate docker credentials store") + ErrUnableToErase = errors.New("unable to erase credentials") + ErrUnableToStore = errors.New("unable to store credentials") + ErrUnableToRetrieve = errors.New("unable to retrieve credentials") +) + +// Errors returned by `Parse` +var ( + ErrUnparsableURL = errors.New("unparsable registry URL") + ErrUnsupportedScheme = errors.New("unsupported scheme in registry URL") +) diff --git a/pkg/imgutil/dockerconfigresolver/dockerconfigresolver.go b/pkg/imgutil/dockerconfigresolver/dockerconfigresolver.go index af59898e93f..47a7be13c29 100644 --- a/pkg/imgutil/dockerconfigresolver/dockerconfigresolver.go +++ b/pkg/imgutil/dockerconfigresolver/dockerconfigresolver.go @@ -20,7 +20,6 @@ import ( "context" "crypto/tls" "errors" - "fmt" "os" "github.com/containerd/containerd/v2/core/remotes" @@ -28,9 +27,6 @@ import ( dockerconfig "github.com/containerd/containerd/v2/core/remotes/docker/config" "github.com/containerd/errdefs" "github.com/containerd/log" - dockercliconfig "github.com/docker/cli/cli/config" - "github.com/docker/cli/cli/config/credentials" - dockercliconfigtypes "github.com/docker/cli/cli/config/types" ) var PushTracker = docker.NewInMemoryTracker() @@ -169,85 +165,35 @@ type AuthCreds func(string) (string, string, error) // NewAuthCreds returns AuthCreds that uses $DOCKER_CONFIG/config.json . // AuthCreds can be nil. func NewAuthCreds(refHostname string) (AuthCreds, error) { - // Load does not raise an error on ENOENT - dockerConfigFile, err := dockercliconfig.Load("") + // Note: does not raise an error on ENOENT + credStore, err := NewCredentialsStore("") if err != nil { return nil, err } - // DefaultHost converts "docker.io" to "registry-1.docker.io", - // which is wanted by credFunc . - credFuncExpectedHostname, err := docker.DefaultHost(refHostname) - if err != nil { - return nil, err - } + credFunc := func(host string) (string, string, error) { + rHost, err := Parse(host) + if err != nil { + return "", "", err + } - var credFunc AuthCreds + ac, err := credStore.Retrieve(rHost, true) + if err != nil { + return "", "", err + } - authConfigHostnames := []string{refHostname} - if refHostname == "docker.io" || refHostname == "registry-1.docker.io" { - // "docker.io" appears as ""https://index.docker.io/v1/" in ~/.docker/config.json . - // Unlike other registries, we have to pass the full URL to GetAuthConfig. - authConfigHostnames = append([]string{IndexServer}, refHostname) - } + if ac.IdentityToken != "" { + return "", ac.IdentityToken, nil + } - for _, authConfigHostname := range authConfigHostnames { - // GetAuthConfig does not raise an error on ENOENT - ac, err := dockerConfigFile.GetAuthConfig(authConfigHostname) - if err != nil { - log.L.WithError(err).Warnf("cannot get auth config for authConfigHostname=%q (refHostname=%q)", - authConfigHostname, refHostname) - } else { - // When refHostname is "docker.io": - // - credFuncExpectedHostname: "registry-1.docker.io" - // - credFuncArg: "registry-1.docker.io" - // - authConfigHostname: "https://index.docker.io/v1/" (IndexServer) - // - ac.ServerAddress: "https://index.docker.io/v1/". - if !isAuthConfigEmpty(ac) { - if ac.ServerAddress == "" { - // This can happen with Amazon ECR: https://github.com/containerd/nerdctl/issues/733 - log.L.Debugf("failed to get ac.ServerAddress for authConfigHostname=%q (refHostname=%q)", - authConfigHostname, refHostname) - } else if authConfigHostname == IndexServer { - if ac.ServerAddress != IndexServer { - return nil, fmt.Errorf("expected ac.ServerAddress (%q) to be %q", ac.ServerAddress, IndexServer) - } - } else { - acsaHostname := credentials.ConvertToHostname(ac.ServerAddress) - if acsaHostname != authConfigHostname { - return nil, fmt.Errorf("expected the hostname part of ac.ServerAddress (%q) to be authConfigHostname=%q, got %q", - ac.ServerAddress, authConfigHostname, acsaHostname) - } - } - - if ac.RegistryToken != "" { - // Even containerd/CRI does not support RegistryToken as of v1.4.3, - // so, nobody is actually using RegistryToken? - log.L.Warnf("ac.RegistryToken (for %q) is not supported yet (FIXME)", authConfigHostname) - } - - credFunc = func(credFuncArg string) (string, string, error) { - // credFuncArg should be like "registry-1.docker.io" - if credFuncArg != credFuncExpectedHostname { - return "", "", fmt.Errorf("expected credFuncExpectedHostname=%q (refHostname=%q), got credFuncArg=%q", - credFuncExpectedHostname, refHostname, credFuncArg) - } - if ac.IdentityToken != "" { - return "", ac.IdentityToken, nil - } - return ac.Username, ac.Password, nil - } - break - } + if ac.RegistryToken != "" { + // Even containerd/CRI does not support RegistryToken as of v1.4.3, + // so, nobody is actually using RegistryToken? + log.L.Warnf("ac.RegistryToken (for %q) is not supported yet (FIXME)", rHost.Host) } - } - // credsFunc can be nil here - return credFunc, nil -} -func isAuthConfigEmpty(ac dockercliconfigtypes.AuthConfig) bool { - if ac.IdentityToken != "" || ac.Username != "" || ac.Password != "" || ac.RegistryToken != "" { - return false + return ac.Username, ac.Password, nil } - return true + + return credFunc, nil } diff --git a/pkg/imgutil/dockerconfigresolver/dockerconfigresolver_util.go b/pkg/imgutil/dockerconfigresolver/dockerconfigresolver_util.go deleted file mode 100644 index b452425e7ba..00000000000 --- a/pkg/imgutil/dockerconfigresolver/dockerconfigresolver_util.go +++ /dev/null @@ -1,50 +0,0 @@ -/* - Copyright The containerd Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -/* - Portions from https://github.com/moby/moby/blob/v20.10.18/registry/auth.go#L154-L167 - Copyright (C) Docker/Moby authors. - Licensed under the Apache License, Version 2.0 - NOTICE: https://github.com/moby/moby/blob/v20.10.18/NOTICE -*/ - -package dockerconfigresolver - -import ( - "strings" -) - -// IndexServer is used for user auth and image search -// -// From https://github.com/moby/moby/blob/v20.10.18/registry/config.go#L36-L39 -const IndexServer = "https://index.docker.io/v1/" - -// ConvertToHostname converts a registry url which has http|https prepended -// to just an hostname. -// -// From https://github.com/moby/moby/blob/v20.10.18/registry/auth.go#L154-L167 -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://") - } - - nameParts := strings.SplitN(stripped, "/", 2) - - return nameParts[0] -} diff --git a/pkg/imgutil/dockerconfigresolver/registryurl.go b/pkg/imgutil/dockerconfigresolver/registryurl.go new file mode 100644 index 00000000000..c4542df471a --- /dev/null +++ b/pkg/imgutil/dockerconfigresolver/registryurl.go @@ -0,0 +1,147 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package dockerconfigresolver + +import ( + "errors" + "fmt" + "net" + "net/url" + "strings" + + "github.com/containerd/log" +) + +// Parse will return a normalized Docker Registry url from the provided string address +func Parse(address string) (*RegistryURL, error) { + log.L.Debugf("parsing docker registry URL %q", address) + var err error + // No address or address as docker.io? Default to standardized index + if address == "" || address == "docker.io" { + log.L.Debugf("normalized to %q", dockerIndexServer) + address = dockerIndexServer + } + // If it has no scheme, slap one just so we can parse + if !strings.Contains(address, "://") { + address = fmt.Sprintf("%s://%s", schemeHTTPS, address) + } + // Parse it + u, err := url.Parse(address) + if err != nil { + log.L.Debug("unparsable - giving up") + return nil, errors.Join(ErrUnparsableURL, err) + } + sch := scheme(u.Scheme) + // Scheme is entirely disregarded anyhow, so, just drop it all and set to https + if sch == schemeHTTP { + log.L.Debug("changing http to https") + u.Scheme = string(schemeHTTPS) + } else if sch != schemeHTTPS && sch != schemeNerdctlExperimental { + log.L.Debugf("unrecognized scheme %q", sch) + // Docker is wildly buggy when it comes to non-http schemes. Being more defensive. + return nil, ErrUnsupportedScheme + } + // If it has no port, add the standard port explicitly + if u.Port() == "" { + log.L.Debug("adding standard port") + u.Host = u.Hostname() + ":" + standardHTTPSPort + } + reg := &RegistryURL{URL: *u} + queryParams := u.Query() + nsQuery := queryParams.Get(namespaceQueryParameter) + if nsQuery != "" { + log.L.Debugf("this is a namespaced url, parsing namespace %q", nsQuery) + reg.Namespace, err = Parse(nsQuery) + if err != nil { + return nil, err + } + } + return reg, nil +} + +// RegistryURL is a struct that represents a registry namespace or host, meant specifically to deal with +// credentials storage and retrieval inside Docker config file. +type RegistryURL struct { + url.URL + Namespace *RegistryURL +} + +// CanonicalIdentifier returns the identifier expected to be used to save credentials to docker auth config +func (rn *RegistryURL) CanonicalIdentifier() string { + log.L.Debugf("retrieving canonical identifier for %q", rn.URL.String()) + // If it is the docker index over https, port 443, on the /v1/ path, we use the docker fully qualified identifier + if rn.Scheme == string(schemeHTTPS) && rn.Hostname() == "index.docker.io" && rn.Path == "/v1/" && rn.Port() == standardHTTPSPort || + rn.URL.String() == dockerIndexServer { + log.L.Debugf("assimilated to docker %q", dockerIndexServer) + return dockerIndexServer + } + // Otherwise, for anything else, we use the hostname+port part + identifier := rn.Host + // If this is a namespaced entry, wrap it, and slap the path as well, as hosts are allowed to be non-compliant + if rn.Namespace != nil { + log.L.Debug("namespaced identifier") + identifier = fmt.Sprintf("%s://%s/host/%s%s", schemeNerdctlExperimental, rn.Namespace.CanonicalIdentifier(), identifier, rn.Path) + } + log.L.Debugf("final value: %q", identifier) + return identifier +} + +// AllIdentifiers returns a list of identifiers that may have been used to save credentials, +// accounting for legacy formats including scheme, with and without ports +func (rn *RegistryURL) AllIdentifiers() []string { + canonicalID := rn.CanonicalIdentifier() + fullList := []string{ + // This is rn.Host, and always have a port (see parsing) + canonicalID, + } + // If the canonical identifier points to Docker Hub, or is one of our experimental ids, there is no alternative / legacy id + if canonicalID == dockerIndexServer || rn.Namespace != nil { + return fullList + } + + // Docker behavior: if the domain was index.docker.io over 443, we are allowed to additionally read the canonical + // docker credentials + if rn.Hostname() == "index.docker.io" && rn.Port() == standardHTTPSPort { + fullList = append(fullList, dockerIndexServer) + } + + // Add legacy variants + fullList = append(fullList, + fmt.Sprintf("%s://%s", schemeHTTPS, rn.Host), + fmt.Sprintf("%s://%s", schemeHTTP, rn.Host), + ) + + // Note that docker does not try to be smart wrt explicit port vs. implied port + // If standard port, allow retrieving credentials from the variant without a port as well + if rn.Port() == standardHTTPSPort { + fullList = append( + fullList, + rn.Hostname(), + fmt.Sprintf("%s://%s", schemeHTTPS, rn.Hostname()), + fmt.Sprintf("%s://%s", schemeHTTP, rn.Hostname()), + ) + } + + return fullList +} + +func (rn *RegistryURL) IsLocalhost() bool { + // Containerd exposes both a IsLocalhost and a MatchLocalhost method + // There does not seem to be a clear reason for the duplication, nor the differences in implementation. + // Either way, they both reparse the host with net.SplitHostPort, which is unnecessary here + return rn.Hostname() == "localhost" || net.ParseIP(rn.Hostname()).IsLoopback() +} diff --git a/pkg/imgutil/dockerconfigresolver/registryurl_test.go b/pkg/imgutil/dockerconfigresolver/registryurl_test.go new file mode 100644 index 00000000000..02bb0eb64c4 --- /dev/null +++ b/pkg/imgutil/dockerconfigresolver/registryurl_test.go @@ -0,0 +1,189 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package dockerconfigresolver + +import ( + "testing" + + "gotest.tools/v3/assert" +) + +func TestURLParsingAndID(t *testing.T) { + tests := []struct { + address string + error error + identifier string + allIDs []string + isLocalhost bool + }{ + { + address: "∞://", + error: ErrUnparsableURL, + }, + { + address: "whatever://", + error: ErrUnsupportedScheme, + }, + { + address: "https://index.docker.io/v1/", + identifier: "https://index.docker.io/v1/", + allIDs: []string{"https://index.docker.io/v1/"}, + }, + { + address: "index.docker.io", + identifier: "index.docker.io:443", + allIDs: []string{ + "index.docker.io:443", + "https://index.docker.io/v1/", + "https://index.docker.io:443", "http://index.docker.io:443", + "index.docker.io", "https://index.docker.io", "http://index.docker.io", + }, + }, + { + address: "index.docker.io/whatever", + identifier: "index.docker.io:443", + allIDs: []string{ + "index.docker.io:443", + "https://index.docker.io/v1/", + "https://index.docker.io:443", "http://index.docker.io:443", + "index.docker.io", "https://index.docker.io", "http://index.docker.io", + }, + }, + { + address: "http://index.docker.io", + identifier: "index.docker.io:443", + allIDs: []string{ + "index.docker.io:443", + "https://index.docker.io/v1/", + "https://index.docker.io:443", "http://index.docker.io:443", + "index.docker.io", "https://index.docker.io", "http://index.docker.io", + }, + }, + { + address: "index.docker.io:80", + identifier: "index.docker.io:80", + allIDs: []string{ + "index.docker.io:80", + "https://index.docker.io:80", "http://index.docker.io:80", + }, + }, + { + address: "index.docker.io:8080", + identifier: "index.docker.io:8080", + allIDs: []string{ + "index.docker.io:8080", + "https://index.docker.io:8080", "http://index.docker.io:8080", + }, + }, + { + address: "foo.docker.io", + identifier: "foo.docker.io:443", + allIDs: []string{ + "foo.docker.io:443", "https://foo.docker.io:443", "http://foo.docker.io:443", + "foo.docker.io", "https://foo.docker.io", "http://foo.docker.io", + }, + }, + { + address: "docker.io", + identifier: "https://index.docker.io/v1/", + allIDs: []string{"https://index.docker.io/v1/"}, + }, + { + address: "docker.io/whatever", + identifier: "docker.io:443", + allIDs: []string{ + "docker.io:443", "https://docker.io:443", "http://docker.io:443", + "docker.io", "https://docker.io", "http://docker.io", + }, + }, + { + address: "http://docker.io", + identifier: "docker.io:443", + allIDs: []string{ + "docker.io:443", "https://docker.io:443", "http://docker.io:443", + "docker.io", "https://docker.io", "http://docker.io", + }, + }, + { + address: "docker.io:80", + identifier: "docker.io:80", + allIDs: []string{ + "docker.io:80", + "https://docker.io:80", "http://docker.io:80", + }, + }, + { + address: "docker.io:8080", + identifier: "docker.io:8080", + allIDs: []string{ + "docker.io:8080", + "https://docker.io:8080", "http://docker.io:8080", + }, + }, + { + address: "anything/whatever?u=v&w=y;foo=bar#frag=o", + identifier: "anything:443", + allIDs: []string{ + "anything:443", "https://anything:443", "http://anything:443", + "anything", "https://anything", "http://anything", + }, + }, + { + address: "https://registry-host.com/subpath/something?bar=bar&ns=registry-namespace.com&foo=foo", + identifier: "nerdctl-experimental://registry-namespace.com:443/host/registry-host.com:443/subpath/something", + allIDs: []string{ + "nerdctl-experimental://registry-namespace.com:443/host/registry-host.com:443/subpath/something", + }, + }, + { + address: "localhost:1234", + identifier: "localhost:1234", + allIDs: []string{ + "localhost:1234", "https://localhost:1234", "http://localhost:1234", + }, + }, + { + address: "127.0.0.1:1234", + identifier: "127.0.0.1:1234", + allIDs: []string{ + "127.0.0.1:1234", "https://127.0.0.1:1234", "http://127.0.0.1:1234", + }, + }, + { + address: "[::1]:1234", + identifier: "[::1]:1234", + allIDs: []string{ + "[::1]:1234", "https://[::1]:1234", "http://[::1]:1234", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.address, func(t *testing.T) { + reg, err := Parse(tc.address) + assert.ErrorIs(t, err, tc.error) + if err == nil { + assert.Equal(t, reg.CanonicalIdentifier(), tc.identifier) + allIDs := reg.AllIdentifiers() + assert.Equal(t, len(allIDs), len(tc.allIDs)) + for k, v := range tc.allIDs { + assert.Equal(t, allIDs[k], v) + } + } + }) + } +} diff --git a/pkg/testutil/testregistry/testregistry_linux.go b/pkg/testutil/testregistry/testregistry_linux.go index 811176c4c3c..14a3b7204f6 100644 --- a/pkg/testutil/testregistry/testregistry_linux.go +++ b/pkg/testutil/testregistry/testregistry_linux.go @@ -17,6 +17,8 @@ package testregistry import ( + "crypto/rand" + "encoding/base64" "fmt" "net" "os" @@ -339,7 +341,7 @@ func NewRegistry(base *testutil.Base, ca *testca.CA, port int, auth Auth, boundC var cert *testca.Cert if ca != nil { scheme = "https" - cert = ca.NewCert(hostIP.String(), "127.0.0.1") + cert = ca.NewCert(hostIP.String(), "127.0.0.1", "localhost", "::1") args = append(args, "--env", "REGISTRY_HTTP_TLS_CERTIFICATE=/registry/domain.crt", "--env", "REGISTRY_HTTP_TLS_KEY=/registry/domain.key", @@ -394,6 +396,10 @@ func NewRegistry(base *testutil.Base, ca *testca.CA, port int, auth Auth, boundC if err != nil { return "", err } + err = generateCertsd(hDir, ca.CertPath, "localhost", port) + if err != nil { + return "", err + } if port == 443 { err = generateCertsd(hDir, ca.CertPath, hostIP.String(), 0) if err != nil { @@ -403,6 +409,10 @@ func NewRegistry(base *testutil.Base, ca *testca.CA, port int, auth Auth, boundC if err != nil { return "", err } + err = generateCertsd(hDir, ca.CertPath, "localhost", 0) + if err != nil { + return "", err + } } } @@ -468,3 +478,12 @@ func NewWithBasicAuth(base *testutil.Base, user, pass string, port int, tls bool } return NewRegistry(base, ca, port, auth, nil) } + +func SafeRandomString(n int) string { + b := make([]byte, n) + _, _ = rand.Read(b) + // XXX WARNING there is something in the registry (or more likely in the way we generate htpasswd files) + // that is broken and does not resist truly random strings + // return string(b) + return base64.URLEncoding.EncodeToString(b) +} diff --git a/pkg/testutil/testutil.go b/pkg/testutil/testutil.go index f42b4861e8e..dd3f6f41d8f 100644 --- a/pkg/testutil/testutil.go +++ b/pkg/testutil/testutil.go @@ -392,7 +392,7 @@ func (c *Cmd) AssertOK() { func (c *Cmd) AssertFail() { c.Base.T.Helper() res := c.runIfNecessary() - assert.Assert(c.Base.T, res.ExitCode != 0) + assert.Assert(c.Base.T, res.ExitCode != 0, res) } func (c *Cmd) AssertExitCode(exitCode int) {