diff --git a/remotes/docker/authorizer.go b/remotes/docker/authorizer.go index eaa0e5dbdbcd..517a643fad0c 100644 --- a/remotes/docker/authorizer.go +++ b/remotes/docker/authorizer.go @@ -194,15 +194,15 @@ func (a *dockerAuthorizer) AddResponses(ctx context.Context, responses []*http.R return err } - if username != "" && secret != "" { - common := auth.TokenOptions{ - Username: username, - Secret: secret, - } - - a.handlers[host] = newAuthHandler(a.client, a.header, c.Scheme, common) - return nil + if username == "" || secret == "" { + return fmt.Errorf("%w: no basic auth credentials", ErrInvalidAuthorization) } + + a.handlers[host] = newAuthHandler(a.client, a.header, c.Scheme, auth.TokenOptions{ + Username: username, + Secret: secret, + }) + return nil } } return fmt.Errorf("failed to find supported auth scheme: %w", errdefs.ErrNotImplemented) diff --git a/remotes/docker/resolver.go b/remotes/docker/resolver.go index 7c9d5241aa9c..6bd70d410540 100644 --- a/remotes/docker/resolver.go +++ b/remotes/docker/resolver.go @@ -34,6 +34,7 @@ import ( "github.com/containerd/containerd/reference" "github.com/containerd/containerd/remotes" "github.com/containerd/containerd/remotes/docker/schema1" + remoteerrors "github.com/containerd/containerd/remotes/errors" "github.com/containerd/containerd/version" digest "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -308,11 +309,11 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp if resp.StatusCode > 399 { // Set firstErr when encountering the first non-404 status code. if firstErr == nil { - firstErr = fmt.Errorf("pulling from host %s failed with status code %v: %v", host.Host, u, resp.Status) + firstErr = remoteerrors.NewUnexpectedStatusErr(resp) } continue // try another host } - return "", ocispec.Descriptor{}, fmt.Errorf("pulling from host %s failed with unexpected status code %v: %v", host.Host, u, resp.Status) + return "", ocispec.Descriptor{}, remoteerrors.NewUnexpectedStatusErr(resp) } size := resp.ContentLength contentType := getManifestMediaType(resp) diff --git a/remotes/docker/resolver_test.go b/remotes/docker/resolver_test.go index 1d7e9d9758f9..e8d4e7b44dc7 100644 --- a/remotes/docker/resolver_test.go +++ b/remotes/docker/resolver_test.go @@ -34,6 +34,7 @@ import ( "github.com/containerd/containerd/remotes" "github.com/containerd/containerd/remotes/docker/auth" + remoteerrors "github.com/containerd/containerd/remotes/errors" digest "github.com/opencontainers/go-digest" specs "github.com/opencontainers/image-spec/specs-go" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -210,6 +211,14 @@ func TestPostBasicAuthTokenResolver(t *testing.T) { runBasicTest(t, "testname", withTokenServer(th, creds)) } +func TestBasicAuthResolver(t *testing.T) { + creds := func(string) (string, string, error) { + return "totallyvaliduser", "totallyvalidpassword", nil + } + + runBasicTest(t, "testname", withBasicAuthServer(creds)) +} + func TestBadTokenResolver(t *testing.T) { th := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPost { @@ -231,7 +240,7 @@ func TestBadTokenResolver(t *testing.T) { defer close() resolver := NewResolver(ro) - image := fmt.Sprintf("%s/doesntmatter:sometatg", base) + image := fmt.Sprintf("%s/doesntmatter:sometag", base) _, _, err := resolver.Resolve(ctx, image) if err == nil { @@ -242,6 +251,59 @@ func TestBadTokenResolver(t *testing.T) { } } +func TestMissingBasicAuthResolver(t *testing.T) { + creds := func(string) (string, string, error) { + return "", "", nil + } + + ctx := context.Background() + h := newContent(ocispec.MediaTypeImageManifest, []byte("not anything parse-able")) + + base, ro, close := withBasicAuthServer(creds)(logHandler{t, h}) + defer close() + + resolver := NewResolver(ro) + image := fmt.Sprintf("%s/doesntmatter:sometag", base) + + _, _, err := resolver.Resolve(ctx, image) + if err == nil { + t.Fatal("Expected error getting token with inssufficient scope") + } + if !errors.Is(err, ErrInvalidAuthorization) { + t.Fatal(err) + } + if !strings.Contains(err.Error(), "no basic auth credentials") { + t.Fatalf("expected \"no basic auth credentials\" message, got %s", err.Error()) + } +} + +func TestWrongBasicAuthResolver(t *testing.T) { + creds := func(string) (string, string, error) { + return "totallyvaliduser", "definitelythewrongpassword", nil + } + + ctx := context.Background() + h := newContent(ocispec.MediaTypeImageManifest, []byte("not anything parse-able")) + + base, ro, close := withBasicAuthServer(creds)(logHandler{t, h}) + defer close() + + resolver := NewResolver(ro) + image := fmt.Sprintf("%s/doesntmatter:sometag", base) + + _, _, err := resolver.Resolve(ctx, image) + if err == nil { + t.Fatal("Expected error getting token with inssufficient scope") + } + var rerr remoteerrors.ErrUnexpectedStatus + if !errors.As(err, &rerr) { + t.Fatal(err) + } + if rerr.StatusCode != 403 { + t.Fatalf("expected 403 status code, got %d", rerr.StatusCode) + } +} + func TestHostFailureFallbackResolver(t *testing.T) { sf := func(h http.Handler) (string, ResolverOptions, func()) { s := httptest.NewServer(h) @@ -543,6 +605,37 @@ func withTokenServer(th http.Handler, creds func(string) (string, string, error) } } +func withBasicAuthServer(creds func(string) (string, string, error)) func(h http.Handler) (string, ResolverOptions, func()) { + return func(h http.Handler) (string, ResolverOptions, func()) { + // Wrap with basic auth + wrapped := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + user, password, ok := r.BasicAuth() + if ok { + if user != "totallyvaliduser" || password != "totallyvalidpassword" { + rw.WriteHeader(http.StatusForbidden) + rw.Write([]byte(`{"errors":[{"code":"DENIED"}]}`)) + return + } + } else { + authHeader := "Basic realm=\"testserver\"" + rw.Header().Set("WWW-Authenticate", authHeader) + rw.WriteHeader(http.StatusUnauthorized) + return + } + h.ServeHTTP(rw, r) + }) + + base, options, close := tlsServer(wrapped) + options.Hosts = ConfigureDefaultRegistries( + WithClient(options.Client), + WithAuthorizer(NewDockerAuthorizer( + WithAuthCreds(creds), + )), + ) + return base, options, close + } +} + func tlsServer(h http.Handler) (string, ResolverOptions, func()) { s := httptest.NewUnstartedServer(h) s.StartTLS() @@ -558,6 +651,7 @@ func tlsServer(h http.Handler) (string, ResolverOptions, func()) { }, }, } + options := ResolverOptions{ Hosts: ConfigureDefaultRegistries(WithClient(client)), // Set deprecated field for tests to use for configuration diff --git a/remotes/errors/errors.go b/remotes/errors/errors.go index 67ccb23df6e8..f60ff0fc286c 100644 --- a/remotes/errors/errors.go +++ b/remotes/errors/errors.go @@ -33,7 +33,7 @@ type ErrUnexpectedStatus struct { } func (e ErrUnexpectedStatus) Error() string { - return fmt.Sprintf("unexpected status: %s", e.Status) + return fmt.Sprintf("unexpected status from %s request to %s: %s", e.RequestMethod, e.RequestURL, e.Status) } // NewUnexpectedStatusErr creates an ErrUnexpectedStatus from HTTP response