Skip to content

Commit 51df21d

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]> (cherry picked from commit d48ceb6) Signed-off-by: Derek McGowan <[email protected]>
1 parent bf6f562 commit 51df21d

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
@@ -101,12 +101,22 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos
101101
hosts[len(hosts)-1].host = "registry-1.docker.io"
102102
} else if docker.IsLocalhost(host) {
103103
hosts[len(hosts)-1].host = host
104-
if options.DefaultScheme == "" || options.DefaultScheme == "http" {
105-
hosts[len(hosts)-1].scheme = "http"
104+
if options.DefaultScheme == "" {
105+
_, port, _ := net.SplitHostPort(host)
106+
if port == "" || port == "443" {
107+
// If port is default or 443, only use https
108+
hosts[len(hosts)-1].scheme = "https"
109+
} else {
110+
// HTTP fallback logic will be used when protocol is ambiguous
111+
hosts[len(hosts)-1].scheme = "http"
112+
}
106113

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

125-
// explicitTLS indicates that TLS was explicitly configured and HTTP endpoints should
135+
// tlsConfigured indicates that TLS was configured and HTTP endpoints should
126136
// attempt to use the TLS configuration before falling back to HTTP
127-
var explicitTLS bool
137+
var tlsConfigured bool
128138

129139
var defaultTLSConfig *tls.Config
130140
if options.DefaultTLS != nil {
131-
explicitTLS = true
141+
tlsConfigured = true
132142
defaultTLSConfig = options.DefaultTLS
133143
} else {
134144
defaultTLSConfig = &tls.Config{}
@@ -167,7 +177,7 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos
167177
rhosts := make([]docker.RegistryHost, len(hosts))
168178
for i, host := range hosts {
169179
// Allow setting for each host as well
170-
explicitTLS := explicitTLS
180+
explicitTLS := tlsConfigured
171181

172182
if host.caCerts != nil || host.clientPairs != nil || host.skipVerify != nil {
173183
explicitTLS = true
@@ -235,13 +245,19 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos
235245
rhosts[i].Authorizer = authorizer
236246
}
237247

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

247263
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
@@ -285,27 +285,190 @@ func TestLoadCertFiles(t *testing.T) {
285285
}
286286
}
287287

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

296-
testHosts, err := hosts("localhost:8080")
297-
if err != nil {
298-
t.Fatal(err)
299-
}
300-
if len(testHosts) != 1 {
301-
t.Fatalf("expected a single host for localhost config, got %d hosts", len(testHosts))
302-
}
303-
if testHosts[0].Scheme != "https" {
304-
t.Fatalf("expected https scheme for localhost with tls config, got %q", testHosts[0].Scheme)
305-
}
306-
_, ok := testHosts[0].Client.Transport.(docker.HTTPFallback)
307-
if !ok {
308-
t.Fatal("expected http fallback configured for defaulted localhost endpoint")
387+
{
388+
host: "example.com",
389+
opts: HostOptions{
390+
DefaultScheme: "http",
391+
DefaultTLS: &tls.Config{
392+
InsecureSkipVerify: true,
393+
},
394+
},
395+
expectedScheme: "http",
396+
usesFallback: false,
397+
},
398+
{
399+
host: "example.com:5000",
400+
opts: HostOptions{
401+
DefaultScheme: "http",
402+
DefaultTLS: &tls.Config{
403+
InsecureSkipVerify: true,
404+
},
405+
},
406+
expectedScheme: "https",
407+
usesFallback: true,
408+
},
409+
{
410+
host: "example.com:5000",
411+
opts: HostOptions{},
412+
expectedScheme: "https",
413+
usesFallback: false,
414+
},
415+
{
416+
host: "example2.com",
417+
opts: HostOptions{
418+
DefaultScheme: "http",
419+
},
420+
expectedScheme: "http",
421+
usesFallback: false,
422+
},
423+
{
424+
host: "127.0.0.254:5000",
425+
opts: HostOptions{},
426+
expectedScheme: "https",
427+
usesFallback: true,
428+
},
429+
{
430+
host: "127.0.0.254",
431+
opts: HostOptions{},
432+
expectedScheme: "https",
433+
usesFallback: false,
434+
},
435+
{
436+
host: "[::1]:5000",
437+
opts: HostOptions{},
438+
expectedScheme: "https",
439+
usesFallback: true,
440+
},
441+
{
442+
host: "::1",
443+
opts: HostOptions{},
444+
expectedScheme: "https",
445+
usesFallback: false,
446+
},
447+
} {
448+
testName := tc.host
449+
if tc.opts.DefaultScheme != "" {
450+
testName = testName + "-default-" + tc.opts.DefaultScheme
451+
}
452+
t.Run(testName, func(t *testing.T) {
453+
ctx := logtest.WithT(context.TODO(), t)
454+
hosts := ConfigureHosts(ctx, tc.opts)
455+
testHosts, err := hosts(tc.host)
456+
if err != nil {
457+
t.Fatal(err)
458+
}
459+
if len(testHosts) != 1 {
460+
t.Fatalf("expected a single host for localhost config, got %d hosts", len(testHosts))
461+
}
462+
if testHosts[0].Scheme != tc.expectedScheme {
463+
t.Fatalf("expected %s scheme for localhost with tls config, got %q", tc.expectedScheme, testHosts[0].Scheme)
464+
}
465+
_, ok := testHosts[0].Client.Transport.(docker.HTTPFallback)
466+
if tc.usesFallback && !ok {
467+
t.Fatal("expected http fallback configured for defaulted localhost endpoint")
468+
} else if ok && !tc.usesFallback {
469+
t.Fatal("expected no http fallback configured for defaulted localhost endpoint")
470+
}
471+
})
309472
}
310473
}
311474

0 commit comments

Comments
 (0)