Merge pull request #9188 from dmcgowan/backport-1.7-localhost-http-fallback

estesp · web-flow · commit 781d39521a37 · 2023-10-04T08:54:44.000-04:00
[release/1.7] remotes: always try to establish tls connection when tls configured
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()
