diff --git a/client/client.go b/client/client.go index deccb4ab74..ff9efa5700 100644 --- a/client/client.go +++ b/client/client.go @@ -86,15 +86,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{ diff --git a/client/client_test.go b/client/client_test.go index 60e44dc299..eaac339658 100644 --- a/client/client_test.go +++ b/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) } } diff --git a/client/hijack.go b/client/hijack.go index f3461ecf78..dededb7af2 100644 --- a/client/hijack.go +++ b/client/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?") diff --git a/client/request.go b/client/request.go index f5c239bf25..07a12657a4 100644 --- a/client/request.go +++ b/client/request.go @@ -99,10 +99,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 @@ -113,8 +110,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) } diff --git a/client/transport.go b/client/transport.go index 43a667272d..771d76f06b 100644 --- a/client/transport.go +++ b/client/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" } diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 0ecfd08a91..7f5be145be 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -2628,7 +2628,7 @@ func (s *DockerSuite) TestRunModeUTSHost(c *check.C) { c.Assert(out, checker.Contains, runconfig.ErrConflictUTSHostname.Error()) } -func (s *DockerSuite) TestRunTLSverify(c *check.C) { +func (s *DockerSuite) TestRunTLSVerify(c *check.C) { // Remote daemons use TLS and this test is not applicable when TLS is required. testRequires(c, SameHostDaemon) if out, code, err := dockerCmdWithError("ps"); err != nil || code != 0 {