Merge pull request #127 from carolynvs/fix-skip-tls

Match insecure registry behavior from Docker

go.mod

@@ -9,6 +9,7 @@ require (
 	github.com/docker/distribution v2.8.1+incompatible
 	github.com/docker/docker v20.10.17+incompatible
 	github.com/docker/go v1.5.1-1
+	github.com/hashicorp/go-multierror v1.1.1
 	github.com/opencontainers/go-digest v1.0.0
 	github.com/opencontainers/image-spec v1.0.3-0.20211202183452-c5a74bcca799
 	github.com/sirupsen/logrus v1.8.1
@@ -33,7 +34,6 @@ require (
 	github.com/google/go-cmp v0.5.6 // indirect
 	github.com/gorilla/mux v1.8.0 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
-	github.com/hashicorp/go-multierror v1.1.1 // indirect
 	github.com/inconshreveable/mousetrap v1.0.0 // indirect
 	github.com/klauspost/compress v1.15.1 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect

remotes/fixup.go

@@ -251,11 +251,11 @@ func pushByDigest(ctx context.Context, target reference.Named, baseImage *bundle
 func resolveImage(ctx context.Context, target reference.Named, baseImage *bundle.BaseImage, cfg fixupConfig) (imageFixupInfo, bool, bool, error) {
 	sourceImageRef, err := ref(baseImage.Image)
 	if err != nil {
-		return imageFixupInfo{}, false, false, fmt.Errorf("failed to resolve image: invalid source ref %s: %v", baseImage.Image, err)
+		return imageFixupInfo{}, false, false, fmt.Errorf("failed to resolve image: invalid source ref %s: %w", baseImage.Image, err)
 	}
 	_, descriptor, err := cfg.resolver.Resolve(ctx, sourceImageRef.String())
 	if err != nil {
-		return imageFixupInfo{}, false, false, fmt.Errorf("failed to resolve image %s: %v", sourceImageRef.String(), err)
+		return imageFixupInfo{}, false, false, fmt.Errorf("failed to resolve image %s: %w", sourceImageRef.String(), err)
 	}
 	return imageFixupInfo{
 		targetRepo:         target,

remotes/resolver.go

@@ -5,6 +5,7 @@ import (
 	"crypto/tls"
 	"fmt"
 	"net/http"
+	"strings"
 
 	"github.com/containerd/containerd/remotes"
 	"github.com/containerd/containerd/remotes/docker"
@@ -14,59 +15,55 @@ import (
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
+// multiRegistryResolver is an OCI registry resolver that accepts a list of
+// insecure registries. It will skip TLS validation for registries that are secured with TLS
+// use plain http for unsecured registries and any registry that is exposed on a loopback ip address.
 type multiRegistryResolver struct {
-	plainHTTP           remotes.Resolver
-	secure              remotes.Resolver
-	skipTLS             remotes.Resolver
+	resolver            remotes.Resolver
 	plainHTTPRegistries map[string]struct{}
 	skipTLSRegistries   map[string]struct{}
-}
-
-func (r *multiRegistryResolver) resolveImplementation(image string) (remotes.Resolver, error) {
-	ref, err := reference.ParseNormalizedNamed(image)
-	if err != nil {
-		return nil, err
-	}
-	repoInfo, err := registry.ParseRepositoryInfo(ref)
-	if err != nil {
-		return nil, err
-	}
-	if _, plainHTTP := r.plainHTTPRegistries[repoInfo.Index.Name]; plainHTTP {
-		return r.plainHTTP, nil
-	}
-	if _, skipTLS := r.skipTLSRegistries[repoInfo.Index.Name]; skipTLS {
-		return r.skipTLS, nil
-	}
-	return r.secure, nil
+	authorizer          docker.Authorizer
+	skipTLSClient       *http.Client
+	skipTLSAuthorizer   docker.Authorizer
 }
 
 func (r *multiRegistryResolver) Resolve(ctx context.Context, ref string) (name string, desc ocispec.Descriptor, err error) {
-	impl, err := r.resolveImplementation(ref)
-	if err != nil {
-		return "", ocispec.Descriptor{}, err
+	name, desc, err = r.resolver.Resolve(ctx, ref)
+
+	// Add some extra context to the poor error message
+	// which is returned when you forget to specify that the registry
+	// uses an insecure TLS certificate
+	// Example: pulling from host localhost:55027 failed with status code [manifests sha256:464c8a63f292a07fb0ea2bf2cf636dafe38bf74d0536879fb9ec4611f2168067]: 400 Bad Request
+	if err != nil && strings.Contains(err.Error(), "400 Bad Request") {
+		ref, otherErr := reference.ParseNormalizedNamed(ref)
+		if otherErr != nil {
+			return
+		}
+		repoInfo, otherErr := registry.ParseRepositoryInfo(ref)
+		if otherErr != nil {
+			return
+		}
+
+		// Check if the registry is not flagged with skipTLS, which is one common explanation for this error
+		if _, skipTLS := r.skipTLSRegistries[repoInfo.Index.Name]; !skipTLS {
+			err = fmt.Errorf("possible attempt to access an insecure registry without skipping TLS verification detected: %w", err)
+		}
 	}
-	return impl.Resolve(ctx, ref)
+
+	return
 }
 
 func (r *multiRegistryResolver) Fetcher(ctx context.Context, ref string) (remotes.Fetcher, error) {
-	impl, err := r.resolveImplementation(ref)
-	if err != nil {
-		return nil, err
-	}
-	return impl.Fetcher(ctx, ref)
+	return r.resolver.Fetcher(ctx, ref)
 }
 
 func (r *multiRegistryResolver) Pusher(ctx context.Context, ref string) (remotes.Pusher, error) {
-	impl, err := r.resolveImplementation(ref)
-	if err != nil {
-		return nil, err
-	}
-	return impl.Pusher(ctx, ref)
+	return r.resolver.Pusher(ctx, ref)
 }
 
 // CreateResolver creates a docker registry resolver, using the local docker CLI credentials
-func CreateResolver(cfg *configfile.ConfigFile, plainHTTPRegistries ...string) remotes.Resolver {
-	authorizer := docker.NewAuthorizer(nil, func(hostName string) (string, string, error) {
+func CreateResolver(cfg *configfile.ConfigFile, insecureRegistries ...string) remotes.Resolver {
+	authCreds := docker.WithAuthCreds(func(hostName string) (string, string, error) {
 		if hostName == registry.DefaultV2Registry.Host {
 			hostName = registry.IndexServer
 		}
@@ -88,39 +85,18 @@ func CreateResolver(cfg *configfile.ConfigFile, plainHTTPRegistries ...string) r
 		},
 	}
 
-	skipTLSAuthorizer := docker.NewAuthorizer(clientSkipTLS, func(hostName string) (string, string, error) {
-		if hostName == registry.DefaultV2Registry.Host {
-			hostName = registry.IndexServer
-		}
-		a, err := cfg.GetAuthConfig(hostName)
-		if err != nil {
-			return "", "", err
-		}
-		if a.IdentityToken != "" {
-			return "", a.IdentityToken, nil
-		}
-		return a.Username, a.Password, nil
-	})
-
 	result := &multiRegistryResolver{
-		plainHTTP: docker.NewResolver(docker.ResolverOptions{
-			Authorizer: authorizer,
-			PlainHTTP:  true,
-		}),
-		secure: docker.NewResolver(docker.ResolverOptions{
-			Authorizer: authorizer,
-			PlainHTTP:  false,
-		}),
-		skipTLS: docker.NewResolver(docker.ResolverOptions{
-			Authorizer: skipTLSAuthorizer,
-			PlainHTTP:  false,
-			Client:     clientSkipTLS,
-		}),
+		authorizer:          docker.NewDockerAuthorizer(authCreds),
+		skipTLSClient:       clientSkipTLS,
+		skipTLSAuthorizer:   docker.NewDockerAuthorizer(authCreds, docker.WithAuthClient(clientSkipTLS)),
 		plainHTTPRegistries: make(map[string]struct{}),
 		skipTLSRegistries:   make(map[string]struct{}),
 	}
 
-	for _, r := range plainHTTPRegistries {
+	// Determine ahead of time how each registry is insecure
+	// 1. It uses TLS but has a bad cert
+	// 2. It doesn't use TLS
+	for _, r := range insecureRegistries {
 		pingURL := fmt.Sprintf("https://%s/v2/", r)
 		resp, err := clientSkipTLS.Get(pingURL)
 		if err == nil {
@@ -131,5 +107,47 @@ func CreateResolver(cfg *configfile.ConfigFile, plainHTTPRegistries ...string) r
 		}
 	}
 
+	result.resolver = docker.NewResolver(docker.ResolverOptions{
+		Hosts: result.configureHosts(),
+	})
+
 	return result
 }
+
+func (r *multiRegistryResolver) configureHosts() docker.RegistryHosts {
+	return func(host string) ([]docker.RegistryHost, error) {
+		config := docker.RegistryHost{
+			Client:       http.DefaultClient,
+			Authorizer:   r.authorizer,
+			Host:         host,
+			Scheme:       "https",
+			Path:         "/v2",
+			Capabilities: docker.HostCapabilityPull | docker.HostCapabilityResolve | docker.HostCapabilityPush,
+		}
+
+		if _, skipTLS := r.skipTLSRegistries[host]; skipTLS {
+			config.Client = r.skipTLSClient
+			config.Authorizer = r.skipTLSAuthorizer
+		} else if _, plainHTTP := r.plainHTTPRegistries[host]; plainHTTP {
+			config.Scheme = "http"
+		} else {
+			// Default to plain http for localhost
+			match, err := docker.MatchLocalhost(host)
+			if err != nil {
+				return nil, err
+			}
+			if match {
+				config.Scheme = "http"
+			}
+		}
+
+		// If this is not set, then we aren't prompted to authenticate to Docker Hub,
+		// which causes the returned content type to be text/html instead of the
+		// specialized content types for images and manifests
+		if host == "docker.io" {
+			config.Host = "registry-1.docker.io"
+		}
+
+		return []docker.RegistryHost{config}, nil
+	}
+}