diff --git a/pkg/util/httpstream/spdy/roundtripper.go b/pkg/util/httpstream/spdy/roundtripper.go index a054d30c233..aab46ead3ff 100644 --- a/pkg/util/httpstream/spdy/roundtripper.go +++ b/pkg/util/httpstream/spdy/roundtripper.go @@ -53,6 +53,10 @@ type SpdyRoundTripper struct { // Dialer is the dialer used to connect. Used if non-nil. Dialer *net.Dialer + + // proxier knows which proxy to use given a request, defaults to http.ProxyFromEnvironment + // Used primarily for mocking the proxy discovery in tests. + proxier func(req *http.Request) (*url.URL, error) } // NewRoundTripper creates a new SpdyRoundTripper that will use @@ -70,7 +74,11 @@ func NewSpdyRoundTripper(tlsConfig *tls.Config) *SpdyRoundTripper { // dial dials the host specified by req, using TLS if appropriate, optionally // using a proxy server if one is configured via environment variables. func (s *SpdyRoundTripper) dial(req *http.Request) (net.Conn, error) { - proxyURL, err := http.ProxyFromEnvironment(req) + proxier := s.proxier + if proxier == nil { + proxier = http.ProxyFromEnvironment + } + proxyURL, err := proxier(req) if err != nil { return nil, err } @@ -106,7 +114,7 @@ func (s *SpdyRoundTripper) dial(req *http.Request) (net.Conn, error) { return rwc, nil } - host, _, err := net.SplitHostPort(req.URL.Host) + host, _, err := net.SplitHostPort(targetHost) if err != nil { return nil, err } @@ -122,6 +130,11 @@ func (s *SpdyRoundTripper) dial(req *http.Request) (net.Conn, error) { return nil, err } + // Return if we were configured to skip validation + if s.tlsConfig != nil && s.tlsConfig.InsecureSkipVerify { + return tlsConn, nil + } + if err := tlsConn.VerifyHostname(host); err != nil { return nil, err } diff --git a/pkg/util/httpstream/spdy/roundtripper_test.go b/pkg/util/httpstream/spdy/roundtripper_test.go index 7ca1d42ad34..1ce07c3689b 100644 --- a/pkg/util/httpstream/spdy/roundtripper_test.go +++ b/pkg/util/httpstream/spdy/roundtripper_test.go @@ -22,20 +22,48 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" "testing" + "github.com/elazarl/goproxy" "k8s.io/kubernetes/pkg/util/httpstream" ) func TestRoundTripAndNewConnection(t *testing.T) { - localhostPool := x509.NewCertPool() if !localhostPool.AppendCertsFromPEM(localhostCert) { t.Errorf("error setting up localhostCert pool") } + httpsServerInvalidHostname := func(h http.Handler) *httptest.Server { + cert, err := tls.X509KeyPair(exampleCert, exampleKey) + if err != nil { + t.Errorf("https (invalid hostname): proxy_test: %v", err) + } + ts := httptest.NewUnstartedServer(h) + ts.TLS = &tls.Config{ + Certificates: []tls.Certificate{cert}, + } + ts.StartTLS() + return ts + } + + httpsServerValidHostname := func(h http.Handler) *httptest.Server { + cert, err := tls.X509KeyPair(localhostCert, localhostKey) + if err != nil { + t.Errorf("https (valid hostname): proxy_test: %v", err) + } + ts := httptest.NewUnstartedServer(h) + ts.TLS = &tls.Config{ + Certificates: []tls.Certificate{cert}, + } + ts.StartTLS() + return ts + } + testCases := map[string]struct { serverFunc func(http.Handler) *httptest.Server + proxyServerFunc func(http.Handler) *httptest.Server clientTLS *tls.Config serverConnectionHeader string serverUpgradeHeader string @@ -78,37 +106,85 @@ func TestRoundTripAndNewConnection(t *testing.T) { shouldError: false, }, "https (invalid hostname + InsecureSkipVerify)": { - serverFunc: func(h http.Handler) *httptest.Server { - cert, err := tls.X509KeyPair(exampleCert, exampleKey) - if err != nil { - t.Errorf("https (invalid hostname): proxy_test: %v", err) - } - ts := httptest.NewUnstartedServer(h) - ts.TLS = &tls.Config{ - Certificates: []tls.Certificate{cert}, - } - ts.StartTLS() - return ts - }, + serverFunc: httpsServerInvalidHostname, clientTLS: &tls.Config{InsecureSkipVerify: true}, serverConnectionHeader: "Upgrade", serverUpgradeHeader: "SPDY/3.1", serverStatusCode: http.StatusSwitchingProtocols, shouldError: false, }, + "https (invalid hostname + hostname verification)": { + serverFunc: httpsServerInvalidHostname, + clientTLS: &tls.Config{InsecureSkipVerify: false}, + serverConnectionHeader: "Upgrade", + serverUpgradeHeader: "SPDY/3.1", + serverStatusCode: http.StatusSwitchingProtocols, + shouldError: true, + }, "https (valid hostname + RootCAs)": { - serverFunc: func(h http.Handler) *httptest.Server { - cert, err := tls.X509KeyPair(localhostCert, localhostKey) - if err != nil { - t.Errorf("https (valid hostname): proxy_test: %v", err) - } - ts := httptest.NewUnstartedServer(h) - ts.TLS = &tls.Config{ - Certificates: []tls.Certificate{cert}, - } - ts.StartTLS() - return ts - }, + serverFunc: httpsServerValidHostname, + clientTLS: &tls.Config{RootCAs: localhostPool}, + serverConnectionHeader: "Upgrade", + serverUpgradeHeader: "SPDY/3.1", + serverStatusCode: http.StatusSwitchingProtocols, + shouldError: false, + }, + "proxied http->http": { + serverFunc: httptest.NewServer, + proxyServerFunc: httptest.NewServer, + serverConnectionHeader: "Upgrade", + serverUpgradeHeader: "SPDY/3.1", + serverStatusCode: http.StatusSwitchingProtocols, + shouldError: false, + }, + "proxied https (invalid hostname + InsecureSkipVerify) -> http": { + serverFunc: httptest.NewServer, + proxyServerFunc: httpsServerInvalidHostname, + clientTLS: &tls.Config{InsecureSkipVerify: true}, + serverConnectionHeader: "Upgrade", + serverUpgradeHeader: "SPDY/3.1", + serverStatusCode: http.StatusSwitchingProtocols, + shouldError: false, + }, + "proxied https (invalid hostname + hostname verification) -> http": { + serverFunc: httptest.NewServer, + proxyServerFunc: httpsServerInvalidHostname, + clientTLS: &tls.Config{InsecureSkipVerify: false}, + serverConnectionHeader: "Upgrade", + serverUpgradeHeader: "SPDY/3.1", + serverStatusCode: http.StatusSwitchingProtocols, + shouldError: true, // fails because the client doesn't trust the proxy + }, + "proxied https (valid hostname + RootCAs) -> http": { + serverFunc: httptest.NewServer, + proxyServerFunc: httpsServerValidHostname, + clientTLS: &tls.Config{RootCAs: localhostPool}, + serverConnectionHeader: "Upgrade", + serverUpgradeHeader: "SPDY/3.1", + serverStatusCode: http.StatusSwitchingProtocols, + shouldError: false, + }, + "proxied https (invalid hostname + InsecureSkipVerify) -> https (invalid hostname)": { + serverFunc: httpsServerInvalidHostname, + proxyServerFunc: httpsServerInvalidHostname, + clientTLS: &tls.Config{InsecureSkipVerify: true}, + serverConnectionHeader: "Upgrade", + serverUpgradeHeader: "SPDY/3.1", + serverStatusCode: http.StatusSwitchingProtocols, + shouldError: false, // works because the test proxy ignores TLS errors + }, + "proxied https (invalid hostname + hostname verification) -> https (invalid hostname)": { + serverFunc: httpsServerInvalidHostname, + proxyServerFunc: httpsServerInvalidHostname, + clientTLS: &tls.Config{InsecureSkipVerify: false}, + serverConnectionHeader: "Upgrade", + serverUpgradeHeader: "SPDY/3.1", + serverStatusCode: http.StatusSwitchingProtocols, + shouldError: true, // fails because the client doesn't trust the proxy + }, + "proxied https (valid hostname + RootCAs) -> https (valid hostname + RootCAs)": { + serverFunc: httpsServerValidHostname, + proxyServerFunc: httpsServerValidHostname, clientTLS: &tls.Config{RootCAs: localhostPool}, serverConnectionHeader: "Upgrade", serverUpgradeHeader: "SPDY/3.1", @@ -149,20 +225,46 @@ func TestRoundTripAndNewConnection(t *testing.T) { // TODO: Uncomment when fix #19254 // defer server.Close() + serverURL, err := url.Parse(server.URL) + if err != nil { + t.Fatalf("%s: Error creating request: %s", k, err) + } req, err := http.NewRequest("GET", server.URL, nil) if err != nil { t.Fatalf("%s: Error creating request: %s", k, err) } - spdyTransport := NewRoundTripper(testCase.clientTLS) + spdyTransport := NewSpdyRoundTripper(testCase.clientTLS) + + var proxierCalled bool + var proxyCalledWithHost string + if testCase.proxyServerFunc != nil { + proxyHandler := goproxy.NewProxyHttpServer() + proxyHandler.OnRequest().HandleConnectFunc(func(host string, ctx *goproxy.ProxyCtx) (*goproxy.ConnectAction, string) { + proxyCalledWithHost = host + return goproxy.OkConnect, host + }) + proxy := testCase.proxyServerFunc(proxyHandler) + + spdyTransport.proxier = func(proxierReq *http.Request) (*url.URL, error) { + proxyURL, err := url.Parse(proxy.URL) + if err != nil { + return nil, err + } + proxierCalled = true + return proxyURL, nil + } + // TODO: Uncomment when fix #19254 + // defer proxy.Close() + } + client := &http.Client{Transport: spdyTransport} resp, err := client.Do(req) - if err != nil { - t.Fatalf("%s: unexpected error from client.Do: %s", k, err) + var conn httpstream.Connection + if err == nil { + conn, err = spdyTransport.NewConnection(resp) } - - conn, err := spdyTransport.NewConnection(resp) haveErr := err != nil if e, a := testCase.shouldError, haveErr; e != a { t.Fatalf("%s: shouldError=%t, got %t: %v", k, e, a, err) @@ -200,6 +302,15 @@ func TestRoundTripAndNewConnection(t *testing.T) { if e, a := "hello", string(b[0:n]); e != a { t.Fatalf("%s: expected '%s', got '%s'", k, e, a) } + + if testCase.proxyServerFunc != nil { + if !proxierCalled { + t.Fatalf("%s: Expected to use a proxy but proxier in SpdyRoundTripper wasn't called", k) + } + if proxyCalledWithHost != serverURL.Host { + t.Fatalf("%s: Expected to see a call to the proxy for backend %q, got %q", k, serverURL.Host, proxyCalledWithHost) + } + } } }