remotes: always try to establish tls connection when tls configured

When a endpoint is configured for http and has a tls configuration, always try to the tls connection and fallback to http when the tls connections fails from receiving an http response. This fixes an issue with default localhost endpoints which get defaulted to http with insecure tls also configured but are using tls. Signed-off-by: Derek McGowan <[email protected]> (cherry picked from commit 79772a0) Signed-off-by: Derek McGowan <[email protected]>
containerd · Oct 3, 2023 · 7779ce6 · 7779ce6
1 parent c12225c
commit 7779ce6
Show file tree

Hide file tree

Showing 4 changed files with 104 additions and 6 deletions.
diff --git a/remotes/docker/config/hosts.go b/remotes/docker/config/hosts.go
@@ -122,8 +122,13 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos
 			hosts[len(hosts)-1].capabilities = docker.HostCapabilityPull | docker.HostCapabilityResolve | docker.HostCapabilityPush
 		}
 
+		// explicitTLS indicates that TLS was explicitly configured and HTTP endpoints should
+		// attempt to use the TLS configuration before falling back to HTTP
+		var explicitTLS bool
+
 		var defaultTLSConfig *tls.Config
 		if options.DefaultTLS != nil {
+			explicitTLS = true
 			defaultTLSConfig = options.DefaultTLS
 		} else {
 			defaultTLSConfig = &tls.Config{}
@@ -161,14 +166,11 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos
 
 		rhosts := make([]docker.RegistryHost, len(hosts))
 		for i, host := range hosts {
-
-			rhosts[i].Scheme = host.scheme
-			rhosts[i].Host = host.host
-			rhosts[i].Path = host.path
-			rhosts[i].Capabilities = host.capabilities
-			rhosts[i].Header = host.header
+			// Allow setting for each host as well
+			explicitTLS := explicitTLS
 
 			if host.caCerts != nil || host.clientPairs != nil || host.skipVerify != nil {
+				explicitTLS = true
 				tr := defaultTransport.Clone()
 				tlsConfig := tr.TLSClientConfig
 				if host.skipVerify != nil {
@@ -232,6 +234,21 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos
 				rhosts[i].Client = client
 				rhosts[i].Authorizer = authorizer
 			}
+
+			// When TLS has been explicitly configured for the operation or host, use the
+			// docker.HTTPFallback roundtripper to catch TLS errors and re-attempt the request as http.
+			// This allows preference for https when configured but also catches TLS errors early enough
+			// in the request to avoid sending the request twice or consuming the request body.
+			if host.scheme == "http" && explicitTLS {
+				host.scheme = "https"
+				rhosts[i].Client.Transport = docker.HTTPFallback{RoundTripper: rhosts[i].Client.Transport}
+			}
+
+			rhosts[i].Scheme = host.scheme
+			rhosts[i].Host = host.host
+			rhosts[i].Path = host.path
+			rhosts[i].Capabilities = host.capabilities
+			rhosts[i].Header = host.header
 		}
 
 		return rhosts, nil

diff --git a/remotes/docker/config/hosts_test.go b/remotes/docker/config/hosts_test.go
@@ -19,6 +19,7 @@ package config
 import (
 	"bytes"
 	"context"
+	"crypto/tls"
 	"fmt"
 	"net/http"
 	"os"
@@ -284,6 +285,30 @@ func TestLoadCertFiles(t *testing.T) {
 	}
 }
 
+func TestLocalhostHTTPFallback(t *testing.T) {
+	opts := HostOptions{
+		DefaultTLS: &tls.Config{
+			InsecureSkipVerify: true,
+		},
+	}
+	hosts := ConfigureHosts(context.TODO(), opts)
+
+	testHosts, err := hosts("localhost:8080")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(testHosts) != 1 {
+		t.Fatalf("expected a single host for localhost config, got %d hosts", len(testHosts))
+	}
+	if testHosts[0].Scheme != "https" {
+		t.Fatalf("expected https scheme for localhost with tls config, got %q", testHosts[0].Scheme)
+	}
+	_, ok := testHosts[0].Client.Transport.(docker.HTTPFallback)
+	if !ok {
+		t.Fatal("expected http fallback configured for defaulted localhost endpoint")
+	}
+}
+
 func compareRegistryHost(j, k docker.RegistryHost) bool {
 	if j.Scheme != k.Scheme {
 		return false

diff --git a/remotes/docker/resolver.go b/remotes/docker/resolver.go
@@ -18,6 +18,7 @@ package docker
 
 import (
 	"context"
+	"crypto/tls"
 	"errors"
 	"fmt"
 	"io"
@@ -707,3 +708,27 @@ func IsLocalhost(host string) bool {
 	ip := net.ParseIP(host)
 	return ip.IsLoopback()
 }
+
+// HTTPFallback is an http.RoundTripper which allows fallback from https to http
+// for registry endpoints with configurations for both http and TLS, such as
+// defaulted localhost endpoints.
+type HTTPFallback struct {
+	http.RoundTripper
+}
+
+func (f HTTPFallback) RoundTrip(r *http.Request) (*http.Response, error) {
+	resp, err := f.RoundTripper.RoundTrip(r)
+	var tlsErr tls.RecordHeaderError
+	if errors.As(err, &tlsErr) && string(tlsErr.RecordHeader[:]) == "HTTP/" {
+		// server gave HTTP response to HTTPS client
+		plainHTTPUrl := *r.URL
+		plainHTTPUrl.Scheme = "http"
+
+		plainHTTPRequest := *r
+		plainHTTPRequest.URL = &plainHTTPUrl
+
+		return f.RoundTripper.RoundTrip(&plainHTTPRequest)
+	}
+
+	return resp, err
+}
diff --git a/remotes/docker/resolver_test.go b/remotes/docker/resolver_test.go
@@ -26,6 +26,7 @@ import (
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"strconv"
 	"strings"
 	"testing"
@@ -332,6 +333,36 @@ func TestHostTLSFailureFallbackResolver(t *testing.T) {
 	runBasicTest(t, "testname", sf)
 }
 
+func TestHTTPFallbackResolver(t *testing.T) {
+	sf := func(h http.Handler) (string, ResolverOptions, func()) {
+		s := httptest.NewServer(h)
+		u, err := url.Parse(s.URL)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		client := &http.Client{
+			Transport: HTTPFallback{http.DefaultTransport},
+		}
+		options := ResolverOptions{
+			Hosts: func(host string) ([]RegistryHost, error) {
+				return []RegistryHost{
+					{
+						Client:       client,
+						Host:         u.Host,
+						Scheme:       "https",
+						Path:         "/v2",
+						Capabilities: HostCapabilityPull | HostCapabilityResolve | HostCapabilityPush,
+					},
+				}, nil
+			},
+		}
+		return u.Host, options, s.Close
+	}
+
+	runBasicTest(t, "testname", sf)
+}
+
 func TestResolveProxy(t *testing.T) {
 	var (
 		ctx  = context.Background()