Merge pull request #26804 from stevvooe/clear-tlsconfig-unix-socket

client: pedantic checking of tlsconfig

client.go

@@ -131,15 +131,16 @@ func NewClient(host string, version string, client *http.Client, httpHeaders map
 		return nil, err
 	}
 
-	if client == nil {
-		client = &http.Client{}
-	}
-
-	if client.Transport == nil {
-		// setup the transport, if not already present
+	if client != nil {
+		if _, ok := client.Transport.(*http.Transport); !ok {
+			return nil, fmt.Errorf("unable to verify TLS configuration, invalid transport %v", client.Transport)
+		}
+	} else {
 		transport := new(http.Transport)
 		sockets.ConfigureTransport(transport, proto, addr)
-		client.Transport = transport
+		client = &http.Client{
+			Transport: transport,
+		}
 	}
 
 	return &Client{

client_test.go

@@ -40,6 +40,20 @@ func TestNewEnvClient(t *testing.T) {
 			},
 			expectedVersion: DefaultVersion,
 		},
+		{
+			envs: map[string]string{
+				"DOCKER_CERT_PATH":  "testdata/",
+				"DOCKER_TLS_VERIFY": "1",
+			},
+			expectedVersion: DefaultVersion,
+		},
+		{
+			envs: map[string]string{
+				"DOCKER_CERT_PATH": "testdata/",
+				"DOCKER_HOST":      "https://notaunixsocket",
+			},
+			expectedVersion: DefaultVersion,
+		},
 		{
 			envs: map[string]string{
 				"DOCKER_HOST": "host",
@@ -69,7 +83,9 @@ func TestNewEnvClient(t *testing.T) {
 		recoverEnvs := setupEnvs(t, c.envs)
 		apiclient, err := NewEnvClient()
 		if c.expectedError != "" {
-			if err == nil || err.Error() != c.expectedError {
+			if err == nil {
+				t.Errorf("expected an error for %v", c)
+			} else if err.Error() != c.expectedError {
 				t.Errorf("expected an error %s, got %s, for %v", c.expectedError, err.Error(), c)
 			}
 		} else {
@@ -81,6 +97,19 @@ func TestNewEnvClient(t *testing.T) {
 				t.Errorf("expected %s, got %s, for %v", c.expectedVersion, version, c)
 			}
 		}
+
+		if c.envs["DOCKER_TLS_VERIFY"] != "" {
+			// pedantic checking that this is handled correctly
+			tr := apiclient.client.Transport.(*http.Transport)
+			if tr.TLSClientConfig == nil {
+				t.Errorf("no tls config found when DOCKER_TLS_VERIFY enabled")
+			}
+
+			if tr.TLSClientConfig.InsecureSkipVerify {
+				t.Errorf("tls verification should be enabled")
+			}
+		}
+
 		recoverEnvs(t)
 	}
 }

hijack.go

@@ -47,12 +47,7 @@ func (cli *Client) postHijacked(ctx context.Context, path string, query url.Valu
 	req.Header.Set("Connection", "Upgrade")
 	req.Header.Set("Upgrade", "tcp")
 
-	tlsConfig, err := resolveTLSConfig(cli.client.Transport)
-	if err != nil {
-		return types.HijackedResponse{}, err
-	}
-
-	conn, err := dial(cli.proto, cli.addr, tlsConfig)
+	conn, err := dial(cli.proto, cli.addr, resolveTLSConfig(cli.client.Transport))
 	if err != nil {
 		if strings.Contains(err.Error(), "connection refused") {
 			return types.HijackedResponse{}, fmt.Errorf("Cannot connect to the Docker daemon. Is 'docker daemon' running on this host?")

request.go

@@ -100,10 +100,7 @@ func (cli *Client) sendClientRequest(ctx context.Context, method, path string, q
 		req.Host = "docker"
 	}
 
-	scheme, err := resolveScheme(cli.client.Transport)
-	if err != nil {
-		return serverResp, err
-	}
+	scheme := resolveScheme(cli.client.Transport)
 
 	req.URL.Host = cli.addr
 	req.URL.Scheme = scheme
@@ -114,8 +111,7 @@ func (cli *Client) sendClientRequest(ctx context.Context, method, path string, q
 
 	resp, err := ctxhttp.Do(ctx, cli.client, req)
 	if err != nil {
-
-		if scheme == "https" && strings.Contains(err.Error(), "malformed HTTP response") {
+		if scheme != "https" && strings.Contains(err.Error(), "malformed HTTP response") {
 			return serverResp, fmt.Errorf("%v.\n* Are you trying to connect to a TLS-enabled daemon without TLS?", err)
 		}
 

transport.go

@@ -18,14 +18,12 @@ func (tf transportFunc) RoundTrip(req *http.Request) (*http.Response, error) {
 
 // resolveTLSConfig attempts to resolve the tls configuration from the
 // RoundTripper.
-func resolveTLSConfig(transport http.RoundTripper) (*tls.Config, error) {
+func resolveTLSConfig(transport http.RoundTripper) *tls.Config {
 	switch tr := transport.(type) {
 	case *http.Transport:
-		return tr.TLSClientConfig, nil
-	case transportFunc:
-		return nil, nil // detect this type for testing.
+		return tr.TLSClientConfig
 	default:
-		return nil, errTLSConfigUnavailable
+		return nil
 	}
 }
 
@@ -37,15 +35,11 @@ func resolveTLSConfig(transport http.RoundTripper) (*tls.Config, error) {
 // Unfortunately, the model of having a host-ish/url-thingy as the connection
 // string has us confusing protocol and transport layers. We continue doing
 // this to avoid breaking existing clients but this should be addressed.
-func resolveScheme(transport http.RoundTripper) (string, error) {
-	c, err := resolveTLSConfig(transport)
-	if err != nil {
-		return "", err
-	}
-
+func resolveScheme(transport http.RoundTripper) string {
+	c := resolveTLSConfig(transport)
 	if c != nil {
-		return "https", nil
+		return "https"
 	}
 
-	return "http", nil
+	return "http"
 }