From c582f7245ae3513163539d0dc2a0d6ef301ec479 Mon Sep 17 00:00:00 2001 From: rainwu Date: Fri, 7 Aug 2026 18:31:43 +0800 Subject: [PATCH] fix(login): accept equivalent registry hosts in auth creds callback Parse() appends the standard HTTPS port to the registry address, but the containerd authorizer calls the credentials callback with the request URL host, which omits the default port (or uses the registry-1.docker.io alias for Docker Hub). The strict equality check then fails and login aborts. Replace the strict equality check with an equivalence check that accepts the same hostname with the default port omitted, and the Docker Hub index.docker.io/registry-1.docker.io alias pair. Callback hosts with an explicit non-standard port must still match exactly. Fixes #3992 Refs #3245 Signed-off-by: rainwu --- pkg/cmd/login/login.go | 65 ++++++++++++++++++++------ pkg/cmd/login/login_test.go | 92 +++++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 13 deletions(-) create mode 100644 pkg/cmd/login/login_test.go diff --git a/pkg/cmd/login/login.go b/pkg/cmd/login/login.go index 773bf8edc76..89505e4e891 100644 --- a/pkg/cmd/login/login.go +++ b/pkg/cmd/login/login.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "io" + "net" "net/http" "net/url" @@ -117,19 +118,7 @@ func loginClientSide(ctx context.Context, globalOptions types.GlobalCommandOptio } dOpts = append(dOpts, dockerconfigresolver.WithHostsDirs(globalOptions.HostsDir)) - authCreds := func(acArg string) (string, string, error) { - if acArg == host { - 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 credentials.Username, credentials.Password, nil - } - return "", "", fmt.Errorf("expected acArg to be %q, got %q", host, acArg) - } - - dOpts = append(dOpts, dockerconfigresolver.WithAuthCreds(authCreds)) + dOpts = append(dOpts, dockerconfigresolver.WithAuthCreds(loginAuthCreds(ctx, host, registryURL, credentials))) ho, err := dockerconfigresolver.NewHostOptions(ctx, host, dOpts...) if err != nil { return "", err @@ -212,3 +201,53 @@ func tryLoginWithRegHost(ctx context.Context, rh docker.RegistryHost) error { return errors.New("too many 401 (probably)") } + +// loginAuthCreds returns the credentials callback handed to the containerd +// authorizer during login. +func loginAuthCreds(ctx context.Context, host string, registryURL *dockerconfigresolver.RegistryURL, credentials *dockerconfigresolver.Credentials) func(string) (string, string, error) { + return func(acArg string) (string, string, error) { + if acArg == host || isEquivalentRegistryHost(acArg, registryURL) { + 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 credentials.Username, credentials.Password, nil + } + return "", "", fmt.Errorf("expected acArg to be %q, got %q", host, acArg) + } +} + +// isEquivalentRegistryHost reports whether acArg, the host value the +// containerd authorizer passes to the credentials callback, refers to the +// same registry as registryURL, the address the user asked to log in to. +// +// Parse always appends the standard HTTPS port to registryURL when the user +// did not specify one, while the authorizer may call back with a host that +// omits the default port, or with a Docker Hub alias, in which case strict +// equality fails spuriously. +// See https://github.com/containerd/nerdctl/issues/3992 and +// https://github.com/containerd/nerdctl/issues/3245. +func isEquivalentRegistryHost(acArg string, registryURL *dockerconfigresolver.RegistryURL) bool { + acHost, acPort, err := net.SplitHostPort(acArg) + if err != nil { + // acArg carries no port + acHost, acPort = acArg, "" + } + // A callback host carrying an explicit non-standard port can only be + // equivalent by exact equality, which the caller already checked. + if acPort != "" && acPort != dockerconfigresolver.StandardHTTPSPort { + return false + } + // The user did not pass an explicit non-default port, so a callback + // host that merely omits the standard HTTPS port is equivalent. + if registryURL.Port() == dockerconfigresolver.StandardHTTPSPort && acHost == registryURL.Hostname() { + return true + } + // Docker Hub aliases: "docker.io" logins resolve to index.docker.io, + // while the actual registry endpoint is registry-1.docker.io. + if registryURL.Hostname() == "index.docker.io" && acHost == "registry-1.docker.io" { + return true + } + return false +} diff --git a/pkg/cmd/login/login_test.go b/pkg/cmd/login/login_test.go new file mode 100644 index 00000000000..956b356c09f --- /dev/null +++ b/pkg/cmd/login/login_test.go @@ -0,0 +1,92 @@ +/* + 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 login + +import ( + "context" + "testing" + + "gotest.tools/v3/assert" + + "github.com/containerd/nerdctl/v2/pkg/imgutil/dockerconfigresolver" +) + +func TestLoginAuthCredsAcceptsEquivalentHosts(t *testing.T) { + tests := []struct { + name string + address string + acArg string + wantErr bool + }{ + { + name: "exact host with standard port", + address: "harbor.example.io", + acArg: "harbor.example.io:443", + }, + { + // https://github.com/containerd/nerdctl/issues/3992 + name: "host without default port", + address: "harbor.example.io", + acArg: "harbor.example.io", + }, + { + // https://github.com/containerd/nerdctl/issues/3245 + name: "docker.io alias without port", + address: "docker.io", + acArg: "registry-1.docker.io", + }, + { + name: "docker.io alias with port", + address: "docker.io", + acArg: "registry-1.docker.io:443", + }, + { + name: "mismatched host", + address: "harbor.example.io", + acArg: "evil.example.io", + wantErr: true, + }, + { + name: "explicit non-standard port not dropped", + address: "harbor.example.io:8443", + acArg: "harbor.example.io", + wantErr: true, + }, + { + name: "different explicit port", + address: "harbor.example.io", + acArg: "harbor.example.io:8443", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + registryURL, err := dockerconfigresolver.Parse(tt.address) + assert.NilError(t, err) + credentials := &dockerconfigresolver.Credentials{Username: "user", Password: "pass"} + authCreds := loginAuthCreds(context.Background(), registryURL.Host, registryURL, credentials) + username, password, err := authCreds(tt.acArg) + if tt.wantErr { + assert.ErrorContains(t, err, "expected acArg") + return + } + assert.NilError(t, err) + assert.Equal(t, "user", username) + assert.Equal(t, "pass", password) + }) + } +}