Skip to content

Commit d48ceb6

Browse files
dmcgowandmcgowan
committed
Avoid TLS fallback when protocol is not ambiguous
The TLS fallback should only be used when the protocol is ambiguous due to provided TLS configurations and defaulting to http. Do not add TLS configurations when defaulting to http. When the port is 80 or will be defaulted to 80, there is no protocol ambiguity and TLS fallback should not be used. Signed-off-by: Derek McGowan <[email protected]>
1 parent fa4ae46 commit d48ceb6

2 files changed

Lines changed: 213 additions & 34 deletions

File tree

‎remotes/docker/config/hosts.go‎

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,22 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos
100100
hosts[len(hosts)-1].host = "registry-1.docker.io"
101101
} else if docker.IsLocalhost(host) {
102102
hosts[len(hosts)-1].host = host
103-
if options.DefaultScheme == "" || options.DefaultScheme == "http" {
104-
hosts[len(hosts)-1].scheme = "http"
103+
if options.DefaultScheme == "" {
104+
_, port, _ := net.SplitHostPort(host)
105+
if port == "" || port == "443" {
106+
// If port is default or 443, only use https
107+
hosts[len(hosts)-1].scheme = "https"
108+
} else {
109+
// HTTP fallback logic will be used when protocol is ambiguous
110+
hosts[len(hosts)-1].scheme = "http"
111+
}
105112

106-
// Skipping TLS verification for localhost
107-
var skipVerify = true
108-
hosts[len(hosts)-1].skipVerify = &skipVerify
113+
// When port is 80, protocol is not ambiguous
114+
if port != "80" {
115+
// Skipping TLS verification for localhost
116+
var skipVerify = true
117+
hosts[len(hosts)-1].skipVerify = &skipVerify
118+
}
109119
} else {
110120
hosts[len(hosts)-1].scheme = options.DefaultScheme
111121
}
@@ -121,13 +131,13 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos
121131
hosts[len(hosts)-1].capabilities = docker.HostCapabilityPull | docker.HostCapabilityResolve | docker.HostCapabilityPush
122132
}
123133

124-
// explicitTLS indicates that TLS was explicitly configured and HTTP endpoints should
134+
// tlsConfigured indicates that TLS was configured and HTTP endpoints should
125135
// attempt to use the TLS configuration before falling back to HTTP
126-
var explicitTLS bool
136+
var tlsConfigured bool
127137

128138
var defaultTLSConfig *tls.Config
129139
if options.DefaultTLS != nil {
130-
explicitTLS = true
140+
tlsConfigured = true
131141
defaultTLSConfig = options.DefaultTLS
132142
} else {
133143
defaultTLSConfig = &tls.Config{}
@@ -166,7 +176,7 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos
166176
rhosts := make([]docker.RegistryHost, len(hosts))
167177
for i, host := range hosts {
168178
// Allow setting for each host as well
169-
explicitTLS := explicitTLS
179+
explicitTLS := tlsConfigured
170180

171181
if host.caCerts != nil || host.clientPairs != nil || host.skipVerify != nil {
172182
explicitTLS = true
@@ -232,13 +242,19 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos
232242
rhosts[i].Authorizer = authorizer
233243
}
234244

235-
// When TLS has been explicitly configured for the operation or host, use the
236-
// docker.HTTPFallback roundtripper to catch TLS errors and re-attempt the request as http.
237-
// This allows preference for https when configured but also catches TLS errors early enough
238-
// in the request to avoid sending the request twice or consuming the request body.
245+
// When TLS has been configured for the operation or host and
246+
// the protocol from the port number is ambiguous, use the
247+
// docker.HTTPFallback roundtripper to catch TLS errors and re-attempt the
248+
// request as http. This allows preference for https when configured but
249+
// also catches TLS errors early enough in the request to avoid sending
250+
// the request twice or consuming the request body.
239251
if host.scheme == "http" && explicitTLS {
240-
host.scheme = "https"
241-
rhosts[i].Client.Transport = docker.HTTPFallback{RoundTripper: rhosts[i].Client.Transport}
252+
_, port, _ := net.SplitHostPort(host.host)
253+
if port != "" && port != "80" {
254+
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")
255+
host.scheme = "https"
256+
rhosts[i].Client.Transport = docker.HTTPFallback{RoundTripper: rhosts[i].Client.Transport}
257+
}
242258
}
243259

244260
rhosts[i].Scheme = host.scheme

‎remotes/docker/config/hosts_test.go‎

Lines changed: 182 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -295,27 +295,190 @@ func TestLoadCertFiles(t *testing.T) {
295295
}
296296
}
297297

298-
func TestLocalhostHTTPFallback(t *testing.T) {
299-
opts := HostOptions{
300-
DefaultTLS: &tls.Config{
301-
InsecureSkipVerify: true,
298+
func TestHTTPFallback(t *testing.T) {
299+
for _, tc := range []struct {
300+
host string
301+
opts HostOptions
302+
expectedScheme string
303+
usesFallback bool
304+
}{
305+
{
306+
host: "localhost:8080",
307+
opts: HostOptions{
308+
DefaultScheme: "http",
309+
DefaultTLS: &tls.Config{
310+
InsecureSkipVerify: true,
311+
},
312+
},
313+
expectedScheme: "https",
314+
usesFallback: true,
315+
},
316+
{
317+
host: "localhost:8080",
318+
opts: HostOptions{
319+
DefaultScheme: "https",
320+
DefaultTLS: &tls.Config{
321+
InsecureSkipVerify: true,
322+
},
323+
},
324+
expectedScheme: "https",
325+
usesFallback: false,
326+
},
327+
{
328+
host: "localhost:8080",
329+
opts: HostOptions{},
330+
expectedScheme: "https",
331+
usesFallback: true,
332+
},
333+
{
334+
host: "localhost:80",
335+
opts: HostOptions{},
336+
expectedScheme: "http",
337+
usesFallback: false,
338+
},
339+
{
340+
host: "localhost:443",
341+
opts: HostOptions{},
342+
expectedScheme: "https",
343+
usesFallback: false,
344+
},
345+
{
346+
host: "localhost:80",
347+
opts: HostOptions{
348+
DefaultScheme: "http",
349+
DefaultTLS: &tls.Config{
350+
InsecureSkipVerify: true,
351+
},
352+
},
353+
expectedScheme: "http",
354+
usesFallback: false,
355+
},
356+
{
357+
host: "localhost",
358+
opts: HostOptions{
359+
DefaultScheme: "http",
360+
DefaultTLS: &tls.Config{
361+
InsecureSkipVerify: true,
362+
},
363+
},
364+
expectedScheme: "http",
365+
usesFallback: false,
366+
},
367+
{
368+
host: "localhost",
369+
opts: HostOptions{
370+
DefaultScheme: "https",
371+
DefaultTLS: &tls.Config{
372+
InsecureSkipVerify: true,
373+
},
374+
},
375+
expectedScheme: "https",
376+
usesFallback: false,
377+
},
378+
{
379+
host: "localhost",
380+
opts: HostOptions{},
381+
expectedScheme: "https",
382+
usesFallback: false,
383+
},
384+
{
385+
host: "localhost:5000",
386+
opts: HostOptions{},
387+
expectedScheme: "https",
388+
usesFallback: true,
389+
},
390+
{
391+
host: "example.com",
392+
opts: HostOptions{},
393+
expectedScheme: "https",
394+
usesFallback: false,
302395
},
303-
}
304-
hosts := ConfigureHosts(context.TODO(), opts)
305396

306-
testHosts, err := hosts("localhost:8080")
307-
if err != nil {
308-
t.Fatal(err)
309-
}
310-
if len(testHosts) != 1 {
311-
t.Fatalf("expected a single host for localhost config, got %d hosts", len(testHosts))
312-
}
313-
if testHosts[0].Scheme != "https" {
314-
t.Fatalf("expected https scheme for localhost with tls config, got %q", testHosts[0].Scheme)
315-
}
316-
_, ok := testHosts[0].Client.Transport.(docker.HTTPFallback)
317-
if !ok {
318-
t.Fatal("expected http fallback configured for defaulted localhost endpoint")
397+
{
398+
host: "example.com",
399+
opts: HostOptions{
400+
DefaultScheme: "http",
401+
DefaultTLS: &tls.Config{
402+
InsecureSkipVerify: true,
403+
},
404+
},
405+
expectedScheme: "http",
406+
usesFallback: false,
407+
},
408+
{
409+
host: "example.com:5000",
410+
opts: HostOptions{
411+
DefaultScheme: "http",
412+
DefaultTLS: &tls.Config{
413+
InsecureSkipVerify: true,
414+
},
415+
},
416+
expectedScheme: "https",
417+
usesFallback: true,
418+
},
419+
{
420+
host: "example.com:5000",
421+
opts: HostOptions{},
422+
expectedScheme: "https",
423+
usesFallback: false,
424+
},
425+
{
426+
host: "example2.com",
427+
opts: HostOptions{
428+
DefaultScheme: "http",
429+
},
430+
expectedScheme: "http",
431+
usesFallback: false,
432+
},
433+
{
434+
host: "127.0.0.254:5000",
435+
opts: HostOptions{},
436+
expectedScheme: "https",
437+
usesFallback: true,
438+
},
439+
{
440+
host: "127.0.0.254",
441+
opts: HostOptions{},
442+
expectedScheme: "https",
443+
usesFallback: false,
444+
},
445+
{
446+
host: "[::1]:5000",
447+
opts: HostOptions{},
448+
expectedScheme: "https",
449+
usesFallback: true,
450+
},
451+
{
452+
host: "::1",
453+
opts: HostOptions{},
454+
expectedScheme: "https",
455+
usesFallback: false,
456+
},
457+
} {
458+
testName := tc.host
459+
if tc.opts.DefaultScheme != "" {
460+
testName = testName + "-default-" + tc.opts.DefaultScheme
461+
}
462+
t.Run(testName, func(t *testing.T) {
463+
ctx := logtest.WithT(context.TODO(), t)
464+
hosts := ConfigureHosts(ctx, tc.opts)
465+
testHosts, err := hosts(tc.host)
466+
if err != nil {
467+
t.Fatal(err)
468+
}
469+
if len(testHosts) != 1 {
470+
t.Fatalf("expected a single host for localhost config, got %d hosts", len(testHosts))
471+
}
472+
if testHosts[0].Scheme != tc.expectedScheme {
473+
t.Fatalf("expected %s scheme for localhost with tls config, got %q", tc.expectedScheme, testHosts[0].Scheme)
474+
}
475+
_, ok := testHosts[0].Client.Transport.(docker.HTTPFallback)
476+
if tc.usesFallback && !ok {
477+
t.Fatal("expected http fallback configured for defaulted localhost endpoint")
478+
} else if ok && !tc.usesFallback {
479+
t.Fatal("expected no http fallback configured for defaulted localhost endpoint")
480+
}
481+
})
319482
}
320483
}
321484

0 commit comments

Comments
 (0)