diff --git a/remotes/docker/config/hosts.go b/remotes/docker/config/hosts.go index b43ea0aa1..0ee1fd869 100644 --- a/remotes/docker/config/hosts.go +++ b/remotes/docker/config/hosts.go @@ -100,12 +100,22 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos hosts[len(hosts)-1].host = "registry-1.docker.io" } else if docker.IsLocalhost(host) { hosts[len(hosts)-1].host = host - if options.DefaultScheme == "" || options.DefaultScheme == "http" { - hosts[len(hosts)-1].scheme = "http" + if options.DefaultScheme == "" { + _, port, _ := net.SplitHostPort(host) + if port == "" || port == "443" { + // If port is default or 443, only use https + hosts[len(hosts)-1].scheme = "https" + } else { + // HTTP fallback logic will be used when protocol is ambiguous + hosts[len(hosts)-1].scheme = "http" + } - // Skipping TLS verification for localhost - var skipVerify = true - hosts[len(hosts)-1].skipVerify = &skipVerify + // When port is 80, protocol is not ambiguous + if port != "80" { + // Skipping TLS verification for localhost + var skipVerify = true + hosts[len(hosts)-1].skipVerify = &skipVerify + } } else { hosts[len(hosts)-1].scheme = options.DefaultScheme } @@ -121,13 +131,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 + // tlsConfigured indicates that TLS was configured and HTTP endpoints should // attempt to use the TLS configuration before falling back to HTTP - var explicitTLS bool + var tlsConfigured bool var defaultTLSConfig *tls.Config if options.DefaultTLS != nil { - explicitTLS = true + tlsConfigured = true defaultTLSConfig = options.DefaultTLS } else { defaultTLSConfig = &tls.Config{} @@ -166,7 +176,7 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos rhosts := make([]docker.RegistryHost, len(hosts)) for i, host := range hosts { // Allow setting for each host as well - explicitTLS := explicitTLS + explicitTLS := tlsConfigured if host.caCerts != nil || host.clientPairs != nil || host.skipVerify != nil { explicitTLS = true @@ -232,13 +242,19 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos 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. + // When TLS has been configured for the operation or host and + // the protocol from the port number is ambiguous, 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} + _, port, _ := net.SplitHostPort(host.host) + if port != "" && port != "80" { + log.G(ctx).WithField("host", host.host).Info("host will try HTTPS first since it is configured for HTTP with a TLS configuration, consider changing host to HTTPS or removing unused TLS configuration") + host.scheme = "https" + rhosts[i].Client.Transport = docker.HTTPFallback{RoundTripper: rhosts[i].Client.Transport} + } } rhosts[i].Scheme = host.scheme diff --git a/remotes/docker/config/hosts_test.go b/remotes/docker/config/hosts_test.go index 28ab41249..2641c6dac 100644 --- a/remotes/docker/config/hosts_test.go +++ b/remotes/docker/config/hosts_test.go @@ -295,27 +295,190 @@ func TestLoadCertFiles(t *testing.T) { } } -func TestLocalhostHTTPFallback(t *testing.T) { - opts := HostOptions{ - DefaultTLS: &tls.Config{ - InsecureSkipVerify: true, +func TestHTTPFallback(t *testing.T) { + for _, tc := range []struct { + host string + opts HostOptions + expectedScheme string + usesFallback bool + }{ + { + host: "localhost:8080", + opts: HostOptions{ + DefaultScheme: "http", + DefaultTLS: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + expectedScheme: "https", + usesFallback: true, + }, + { + host: "localhost:8080", + opts: HostOptions{ + DefaultScheme: "https", + DefaultTLS: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + expectedScheme: "https", + usesFallback: false, + }, + { + host: "localhost:8080", + opts: HostOptions{}, + expectedScheme: "https", + usesFallback: true, + }, + { + host: "localhost:80", + opts: HostOptions{}, + expectedScheme: "http", + usesFallback: false, + }, + { + host: "localhost:443", + opts: HostOptions{}, + expectedScheme: "https", + usesFallback: false, + }, + { + host: "localhost:80", + opts: HostOptions{ + DefaultScheme: "http", + DefaultTLS: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + expectedScheme: "http", + usesFallback: false, + }, + { + host: "localhost", + opts: HostOptions{ + DefaultScheme: "http", + DefaultTLS: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + expectedScheme: "http", + usesFallback: false, + }, + { + host: "localhost", + opts: HostOptions{ + DefaultScheme: "https", + DefaultTLS: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + expectedScheme: "https", + usesFallback: false, + }, + { + host: "localhost", + opts: HostOptions{}, + expectedScheme: "https", + usesFallback: false, + }, + { + host: "localhost:5000", + opts: HostOptions{}, + expectedScheme: "https", + usesFallback: true, + }, + { + host: "example.com", + opts: HostOptions{}, + expectedScheme: "https", + usesFallback: false, }, - } - 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") + { + host: "example.com", + opts: HostOptions{ + DefaultScheme: "http", + DefaultTLS: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + expectedScheme: "http", + usesFallback: false, + }, + { + host: "example.com:5000", + opts: HostOptions{ + DefaultScheme: "http", + DefaultTLS: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + expectedScheme: "https", + usesFallback: true, + }, + { + host: "example.com:5000", + opts: HostOptions{}, + expectedScheme: "https", + usesFallback: false, + }, + { + host: "example2.com", + opts: HostOptions{ + DefaultScheme: "http", + }, + expectedScheme: "http", + usesFallback: false, + }, + { + host: "127.0.0.254:5000", + opts: HostOptions{}, + expectedScheme: "https", + usesFallback: true, + }, + { + host: "127.0.0.254", + opts: HostOptions{}, + expectedScheme: "https", + usesFallback: false, + }, + { + host: "[::1]:5000", + opts: HostOptions{}, + expectedScheme: "https", + usesFallback: true, + }, + { + host: "::1", + opts: HostOptions{}, + expectedScheme: "https", + usesFallback: false, + }, + } { + testName := tc.host + if tc.opts.DefaultScheme != "" { + testName = testName + "-default-" + tc.opts.DefaultScheme + } + t.Run(testName, func(t *testing.T) { + ctx := logtest.WithT(context.TODO(), t) + hosts := ConfigureHosts(ctx, tc.opts) + testHosts, err := hosts(tc.host) + 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 != tc.expectedScheme { + t.Fatalf("expected %s scheme for localhost with tls config, got %q", tc.expectedScheme, testHosts[0].Scheme) + } + _, ok := testHosts[0].Client.Transport.(docker.HTTPFallback) + if tc.usesFallback && !ok { + t.Fatal("expected http fallback configured for defaulted localhost endpoint") + } else if ok && !tc.usesFallback { + t.Fatal("expected no http fallback configured for defaulted localhost endpoint") + } + }) } }