From fcdf2c491545aa316644b3d9b583237169db1e73 Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Mon, 1 Aug 2022 14:07:16 -0500 Subject: [PATCH] Match insecure registry behavior from Docker This tweaks how we create a resolver so that we handle plain http connections, and TLS validation the same way as Docker. * When a loopback address is specified as insecure, that should take precedence over it being plain http. So if I'm running a local registry with self-signed certificates, it should use the skipTLS http client, and not attempt to use plain http. * All loopback addresses, not just localhost and 127.0.0.1, should default to plain http unless specified as insecure. The impact is that you only need to specify that a registry is insecure when: * You used self-signed certificates * It uses plain http on a non-loopback address When I was looking at the docker library we are using to create a resolver, I saw that we were using deprecated functionality. By using the Hosts function that is called per registry to determine how to connect to that registry, I was able to simplify the resolver and not need to create 3 separate resolvers. Signed-off-by: Carolyn Van Slyck --- go.mod | 2 +- remotes/fixup.go | 4 +- remotes/resolver.go | 148 +++++++++++++++++++++++++------------------- 3 files changed, 86 insertions(+), 68 deletions(-) diff --git a/go.mod b/go.mod index bee2749..a5d6be5 100644 --- a/go.mod +++ b/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 diff --git a/remotes/fixup.go b/remotes/fixup.go index 18cc3be..23159ca 100644 --- a/remotes/fixup.go +++ b/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, diff --git a/remotes/resolver.go b/remotes/resolver.go index ddb000d..548521b 100644 --- a/remotes/resolver.go +++ b/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 + } +}