From 80c4954d9615c6d6743db4676dcfdf906cb66eb2 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Thu, 11 Feb 2016 15:45:29 -0800 Subject: [PATCH] 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 Upstream-commit: 5e8af46fda3f4e17e06726237fc6b9ab6957e3ea Component: engine --- components/engine/distribution/errors.go | 4 +++ components/engine/distribution/pull.go | 23 +++++++++++++++++ components/engine/distribution/pull_v2.go | 20 ++++++++++++--- components/engine/distribution/push.go | 22 ++++++++++++++++ components/engine/distribution/push_v2.go | 8 ++++-- components/engine/distribution/registry.go | 29 ++++++++++++++++++---- 6 files changed, 95 insertions(+), 11 deletions(-) diff --git a/components/engine/distribution/errors.go b/components/engine/distribution/errors.go index 9f9dcf6978..1cb34fdd51 100644 --- a/components/engine/distribution/errors.go +++ b/components/engine/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. diff --git a/components/engine/distribution/pull.go b/components/engine/distribution/pull.go index debe378d51..659675fd62 100644 --- a/components/engine/distribution/pull.go +++ b/components/engine/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 } } diff --git a/components/engine/distribution/pull_v2.go b/components/engine/distribution/pull_v2.go index 3d315ca413..596d1c1321 100644 --- a/components/engine/distribution/pull_v2.go +++ b/components/engine/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 { diff --git a/components/engine/distribution/push.go b/components/engine/distribution/push.go index c25f545ce0..9380f5c2ba 100644 --- a/components/engine/distribution/push.go +++ b/components/engine/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) diff --git a/components/engine/distribution/push_v2.go b/components/engine/distribution/push_v2.go index 6b9c0f245b..31b420513f 100644 --- a/components/engine/distribution/push_v2.go +++ b/components/engine/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 diff --git a/components/engine/distribution/registry.go b/components/engine/distribution/registry.go index 3b50bb2751..f0bfd8c64f 100644 --- a/components/engine/distribution/registry.go +++ b/components/engine/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 {