Smarter push/pull TLS fallback

With the --insecure-registry daemon option (or talking to a registry on
a local IP), the daemon will first try TLS, and then try plaintext if
something goes wrong with the push or pull. It doesn't make sense to try
plaintext if a HTTP request went through while using TLS. This commit
changes the logic to keep track of host/port combinations where a TLS
attempt managed to do at least one HTTP request (whether the response
code indicated success or not). If the host/port responded to a HTTP
using TLS, we won't try to make plaintext HTTP requests to it.

This will result in better error messages, which sometimes ended up
showing the result of the plaintext attempt, like this:

    Error response from daemon: Get
    http://myregistrydomain.com:5000/v2/: malformed HTTP response
    "\x15\x03\x01\x00\x02\x02"

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

distribution/errors.go

@@ -31,6 +31,10 @@ type fallbackError struct {
 	// supports the v2 protocol. This is used to limit fallbacks to the v1
 	// protocol.
 	confirmedV2 bool
+	// transportOK is set to true if we managed to speak HTTP with the
+	// registry. This confirms that we're using appropriate TLS settings
+	// (or lack of TLS).
+	transportOK bool
 }
 
 // Error renders the FallbackError as a string.

distribution/pull.go

@@ -2,6 +2,7 @@ package distribution
 
 import (
 	"fmt"
+	"net/url"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/api"
@@ -109,12 +110,31 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo
 		// confirm that it was talking to a v2 registry. This will
 		// prevent fallback to the v1 protocol.
 		confirmedV2 bool
+
+		// confirmedTLSRegistries is a map indicating which registries
+		// are known to be using TLS. There should never be a plaintext
+		// retry for any of these.
+		confirmedTLSRegistries = make(map[string]struct{})
 	)
 	for _, endpoint := range endpoints {
 		if confirmedV2 && endpoint.Version == registry.APIVersion1 {
 			logrus.Debugf("Skipping v1 endpoint %s because v2 registry was detected", endpoint.URL)
 			continue
 		}
+
+		parsedURL, urlErr := url.Parse(endpoint.URL)
+		if urlErr != nil {
+			logrus.Errorf("Failed to parse endpoint URL %s", endpoint.URL)
+			continue
+		}
+
+		if parsedURL.Scheme != "https" {
+			if _, confirmedTLS := confirmedTLSRegistries[parsedURL.Host]; confirmedTLS {
+				logrus.Debugf("Skipping non-TLS endpoint %s for host/port that appears to use TLS", endpoint.URL)
+				continue
+			}
+		}
+
 		logrus.Debugf("Trying to pull %s from %s %s", repoInfo.Name(), endpoint.URL, endpoint.Version)
 
 		puller, err := newPuller(endpoint, repoInfo, imagePullConfig)
@@ -132,6 +152,9 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo
 				if fallbackErr, ok := err.(fallbackError); ok {
 					fallback = true
 					confirmedV2 = confirmedV2 || fallbackErr.confirmedV2
+					if fallbackErr.transportOK && parsedURL.Scheme == "https" {
+						confirmedTLSRegistries[parsedURL.Host] = struct{}{}
+					}
 					err = fallbackErr.err
 				}
 			}

distribution/pull_v2.go

@@ -62,7 +62,7 @@ func (p *v2Puller) Pull(ctx context.Context, ref reference.Named) (err error) {
 	p.repo, p.confirmedV2, err = NewV2Repository(ctx, p.repoInfo, p.endpoint, p.config.MetaHeaders, p.config.AuthConfig, "pull")
 	if err != nil {
 		logrus.Warnf("Error getting v2 registry: %v", err)
-		return fallbackError{err: err, confirmedV2: p.confirmedV2}
+		return err
 	}
 
 	if err = p.pullV2Repository(ctx, ref); err != nil {
@@ -71,7 +71,11 @@ func (p *v2Puller) Pull(ctx context.Context, ref reference.Named) (err error) {
 		}
 		if continueOnError(err) {
 			logrus.Errorf("Error trying v2 registry: %v", err)
-			return fallbackError{err: err, confirmedV2: p.confirmedV2}
+			return fallbackError{
+				err:         err,
+				confirmedV2: p.confirmedV2,
+				transportOK: true,
+			}
 		}
 	}
 	return err
@@ -716,12 +720,20 @@ func allowV1Fallback(err error) error {
 	case errcode.Errors:
 		if len(v) != 0 {
 			if v0, ok := v[0].(errcode.Error); ok && shouldV2Fallback(v0) {
-				return fallbackError{err: err, confirmedV2: false}
+				return fallbackError{
+					err:         err,
+					confirmedV2: false,
+					transportOK: true,
+				}
 			}
 		}
 	case errcode.Error:
 		if shouldV2Fallback(v) {
-			return fallbackError{err: err, confirmedV2: false}
+			return fallbackError{
+				err:         err,
+				confirmedV2: false,
+				transportOK: true,
+			}
 		}
 	case *url.Error:
 		if v.Err == auth.ErrNoBasicAuthCredentials {

distribution/push.go

@@ -5,6 +5,7 @@ import (
 	"compress/gzip"
 	"fmt"
 	"io"
+	"net/url"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/distribution/metadata"
@@ -119,6 +120,11 @@ func Push(ctx context.Context, ref reference.Named, imagePushConfig *ImagePushCo
 		// confirm that it was talking to a v2 registry. This will
 		// prevent fallback to the v1 protocol.
 		confirmedV2 bool
+
+		// confirmedTLSRegistries is a map indicating which registries
+		// are known to be using TLS. There should never be a plaintext
+		// retry for any of these.
+		confirmedTLSRegistries = make(map[string]struct{})
 	)
 
 	for _, endpoint := range endpoints {
@@ -127,6 +133,19 @@ func Push(ctx context.Context, ref reference.Named, imagePushConfig *ImagePushCo
 			continue
 		}
 
+		parsedURL, urlErr := url.Parse(endpoint.URL)
+		if urlErr != nil {
+			logrus.Errorf("Failed to parse endpoint URL %s", endpoint.URL)
+			continue
+		}
+
+		if parsedURL.Scheme != "https" {
+			if _, confirmedTLS := confirmedTLSRegistries[parsedURL.Host]; confirmedTLS {
+				logrus.Debugf("Skipping non-TLS endpoint %s for host/port that appears to use TLS", endpoint.URL)
+				continue
+			}
+		}
+
 		logrus.Debugf("Trying to push %s to %s %s", repoInfo.FullName(), endpoint.URL, endpoint.Version)
 
 		pusher, err := NewPusher(ref, endpoint, repoInfo, imagePushConfig)
@@ -142,6 +161,9 @@ func Push(ctx context.Context, ref reference.Named, imagePushConfig *ImagePushCo
 			default:
 				if fallbackErr, ok := err.(fallbackError); ok {
 					confirmedV2 = confirmedV2 || fallbackErr.confirmedV2
+					if fallbackErr.transportOK && parsedURL.Scheme == "https" {
+						confirmedTLSRegistries[parsedURL.Host] = struct{}{}
+					}
 					err = fallbackErr.err
 					lastErr = err
 					logrus.Errorf("Attempting next endpoint for push after error: %v", err)

distribution/push_v2.go

@@ -64,12 +64,16 @@ func (p *v2Pusher) Push(ctx context.Context) (err error) {
 	p.repo, p.pushState.confirmedV2, err = NewV2Repository(ctx, p.repoInfo, p.endpoint, p.config.MetaHeaders, p.config.AuthConfig, "push", "pull")
 	if err != nil {
 		logrus.Debugf("Error getting v2 registry: %v", err)
-		return fallbackError{err: err, confirmedV2: p.pushState.confirmedV2}
+		return err
 	}
 
 	if err = p.pushV2Repository(ctx); err != nil {
 		if continueOnError(err) {
-			return fallbackError{err: err, confirmedV2: p.pushState.confirmedV2}
+			return fallbackError{
+				err:         err,
+				confirmedV2: p.pushState.confirmedV2,
+				transportOK: true,
+			}
 		}
 	}
 	return err

distribution/registry.go

@@ -60,14 +60,18 @@ func NewV2Repository(ctx context.Context, repoInfo *registry.RepositoryInfo, end
 	endpointStr := strings.TrimRight(endpoint.URL, "/") + "/v2/"
 	req, err := http.NewRequest("GET", endpointStr, nil)
 	if err != nil {
-		return nil, false, err
+		return nil, false, fallbackError{err: err}
 	}
 	resp, err := pingClient.Do(req)
 	if err != nil {
-		return nil, false, err
+		return nil, false, fallbackError{err: err}
 	}
 	defer resp.Body.Close()
 
+	// We got a HTTP request through, so we're using the right TLS settings.
+	// From this point forward, set transportOK to true in any fallbackError
+	// we return.
+
 	v2Version := auth.APIVersion{
 		Type:    "registry",
 		Version: "2.0",
@@ -87,7 +91,11 @@ func NewV2Repository(ctx context.Context, repoInfo *registry.RepositoryInfo, end
 
 	challengeManager := auth.NewSimpleChallengeManager()
 	if err := challengeManager.AddResponse(resp); err != nil {
-		return nil, foundVersion, err
+		return nil, foundVersion, fallbackError{
+			err:         err,
+			confirmedV2: foundVersion,
+			transportOK: true,
+		}
 	}
 
 	if authConfig.RegistryToken != "" {
@@ -103,11 +111,22 @@ func NewV2Repository(ctx context.Context, repoInfo *registry.RepositoryInfo, end
 
 	repoNameRef, err := distreference.ParseNamed(repoName)
 	if err != nil {
-		return nil, foundVersion, err
+		return nil, foundVersion, fallbackError{
+			err:         err,
+			confirmedV2: foundVersion,
+			transportOK: true,
+		}
 	}
 
 	repo, err = client.NewRepository(ctx, repoNameRef, endpoint.URL, tr)
-	return repo, foundVersion, err
+	if err != nil {
+		err = fallbackError{
+			err:         err,
+			confirmedV2: foundVersion,
+			transportOK: true,
+		}
+	}
+	return
 }
 
 type existingTokenHandler struct {