From bd9b4637a83a41f5f5be241ccfb570a4ca58c744 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Fri, 21 Jul 2017 00:42:14 -0400 Subject: [PATCH] Use specified ServerName in aggregator TLS validation --- .../pkg/util/httpstream/spdy/roundtripper.go | 22 +- .../apimachinery/pkg/util/proxy/dial.go | 3 + .../kube-aggregator/pkg/apiserver/BUILD | 1 + .../pkg/apiserver/handler_proxy_test.go | 228 +++++++++++++++++- 4 files changed, 241 insertions(+), 13 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper.go b/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper.go index ad300e28b5..12bef075da 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper.go @@ -158,15 +158,16 @@ func (s *SpdyRoundTripper) dial(req *http.Request) (net.Conn, error) { return nil, err } - if s.tlsConfig == nil { - s.tlsConfig = &tls.Config{} + tlsConfig := s.tlsConfig + switch { + case tlsConfig == nil: + tlsConfig = &tls.Config{ServerName: host} + case len(tlsConfig.ServerName) == 0: + tlsConfig = tlsConfig.Clone() + tlsConfig.ServerName = host } - if len(s.tlsConfig.ServerName) == 0 { - s.tlsConfig.ServerName = host - } - - tlsConn := tls.Client(rwc, s.tlsConfig) + tlsConn := tls.Client(rwc, tlsConfig) // need to manually call Handshake() so we can call VerifyHostname() below if err := tlsConn.Handshake(); err != nil { @@ -174,11 +175,11 @@ func (s *SpdyRoundTripper) dial(req *http.Request) (net.Conn, error) { } // Return if we were configured to skip validation - if s.tlsConfig != nil && s.tlsConfig.InsecureSkipVerify { + if tlsConfig.InsecureSkipVerify { return tlsConn, nil } - if err := tlsConn.VerifyHostname(host); err != nil { + if err := tlsConn.VerifyHostname(tlsConfig.ServerName); err != nil { return nil, err } @@ -218,6 +219,9 @@ func (s *SpdyRoundTripper) dialWithoutProxy(url *url.URL) (net.Conn, error) { if err != nil { return nil, err } + if s.tlsConfig != nil && len(s.tlsConfig.ServerName) > 0 { + host = s.tlsConfig.ServerName + } err = conn.VerifyHostname(host) if err != nil { return nil, err diff --git a/staging/src/k8s.io/apimachinery/pkg/util/proxy/dial.go b/staging/src/k8s.io/apimachinery/pkg/util/proxy/dial.go index d9fbb85b7b..e737319094 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/proxy/dial.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/proxy/dial.go @@ -94,6 +94,9 @@ func DialURL(url *url.URL, transport http.RoundTripper) (net.Conn, error) { // Verify host, _, _ := net.SplitHostPort(dialAddr) + if tlsConfig != nil && len(tlsConfig.ServerName) > 0 { + host = tlsConfig.ServerName + } if err := tlsConn.VerifyHostname(host); err != nil { tlsConn.Close() return nil, err diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/BUILD b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/BUILD index 39400b71d0..3f9b9d255b 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/BUILD +++ b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/BUILD @@ -17,6 +17,7 @@ go_test( library = ":go_default_library", tags = ["automanaged"], deps = [ + "//vendor/golang.org/x/net/websocket:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go index 482c971ed8..77ee9cd562 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go @@ -17,21 +17,23 @@ limitations under the License. package apiserver import ( + "crypto/tls" "io/ioutil" "net/http" "net/http/httptest" "net/http/httputil" + "net/url" "reflect" "strings" "testing" + "golang.org/x/net/websocket" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/authentication/user" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" - "k8s.io/kube-aggregator/pkg/apis/apiregistration" - "net/url" ) type targetHTTPHandler struct { @@ -92,7 +94,13 @@ func (r *mockedRouter) ResolveEndpoint(namespace, name string) (*url.URL, error) func TestProxyHandler(t *testing.T) { target := &targetHTTPHandler{} - targetServer := httptest.NewTLSServer(target) + targetServer := httptest.NewUnstartedServer(target) + if cert, err := tls.X509KeyPair(svcCrt, svcKey); err != nil { + t.Fatal(err) + } else { + targetServer.TLS = &tls.Config{Certificates: []tls.Certificate{cert}} + } + targetServer.StartTLS() defer targetServer.Close() tests := map[string]struct { @@ -120,7 +128,7 @@ func TestProxyHandler(t *testing.T) { expectedStatusCode: http.StatusInternalServerError, expectedBody: "missing user", }, - "proxy with user": { + "proxy with user, insecure": { user: &user.DefaultInfo{ Name: "username", Groups: []string{"one", "two"}, @@ -147,6 +155,33 @@ func TestProxyHandler(t *testing.T) { "X-Remote-Group": {"one", "two"}, }, }, + "proxy with user, cabundle": { + user: &user.DefaultInfo{ + Name: "username", + Groups: []string{"one", "two"}, + }, + path: "/request/path", + apiService: &apiregistration.APIService{ + ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"}, + Spec: apiregistration.APIServiceSpec{ + Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns"}, + Group: "foo", + Version: "v1", + CABundle: testCACrt, + }, + }, + expectedStatusCode: http.StatusOK, + expectedCalled: true, + expectedHeaders: map[string][]string{ + "X-Forwarded-Proto": {"https"}, + "X-Forwarded-Uri": {"/request/path"}, + "X-Forwarded-For": {"127.0.0.1"}, + "X-Remote-User": {"username"}, + "User-Agent": {"Go-http-client/1.1"}, + "Accept-Encoding": {"gzip"}, + "X-Remote-Group": {"one", "two"}, + }, + }, "fail on bad serving cert": { user: &user.DefaultInfo{ Name: "username", @@ -218,3 +253,188 @@ func TestProxyHandler(t *testing.T) { }() } } + +func TestProxyUpgrade(t *testing.T) { + testcases := map[string]struct { + APIService *apiregistration.APIService + ExpectError bool + ExpectCalled bool + }{ + "valid hostname + CABundle": { + APIService: &apiregistration.APIService{ + Spec: apiregistration.APIServiceSpec{ + CABundle: testCACrt, + Group: "mygroup", + Version: "v1", + Service: &apiregistration.ServiceReference{Name: "test-service", Namespace: "test-ns"}, + }, + }, + ExpectError: false, + ExpectCalled: true, + }, + "invalid hostname + insecure": { + APIService: &apiregistration.APIService{ + Spec: apiregistration.APIServiceSpec{ + InsecureSkipTLSVerify: true, + Group: "mygroup", + Version: "v1", + Service: &apiregistration.ServiceReference{Name: "invalid-service", Namespace: "invalid-ns"}, + }, + }, + ExpectError: false, + ExpectCalled: true, + }, + "invalid hostname + CABundle": { + APIService: &apiregistration.APIService{ + Spec: apiregistration.APIServiceSpec{ + CABundle: testCACrt, + Group: "mygroup", + Version: "v1", + Service: &apiregistration.ServiceReference{Name: "invalid-service", Namespace: "invalid-ns"}, + }, + }, + ExpectError: true, + ExpectCalled: false, + }, + } + + for k, tc := range testcases { + tcName := k + path := "/apis/" + tc.APIService.Spec.Group + "/" + tc.APIService.Spec.Version + "/foo" + called := false + + func() { // Cleanup after each test case. + backendHandler := http.NewServeMux() + backendHandler.Handle(path, websocket.Handler(func(ws *websocket.Conn) { + defer ws.Close() + body := make([]byte, 5) + ws.Read(body) + ws.Write([]byte("hello " + string(body))) + called = true + })) + + backendServer := httptest.NewUnstartedServer(backendHandler) + if cert, err := tls.X509KeyPair(svcCrt, svcKey); err != nil { + t.Errorf("https (valid hostname): %v", err) + return + } else { + backendServer.TLS = &tls.Config{Certificates: []tls.Certificate{cert}} + } + backendServer.StartTLS() + defer backendServer.Close() + + defer func() { + if called != tc.ExpectCalled { + t.Errorf("%s: expected called=%v, got %v", tcName, tc.ExpectCalled, called) + } + }() + + serverURL, _ := url.Parse(backendServer.URL) + proxyHandler := &proxyHandler{ + contextMapper: &fakeRequestContextMapper{user: &user.DefaultInfo{Name: "username"}}, + serviceResolver: &mockedRouter{destinationHost: serverURL.Host}, + proxyTransport: &http.Transport{}, + } + proxyHandler.updateAPIService(tc.APIService) + aggregator := httptest.NewServer(proxyHandler) + defer aggregator.Close() + + ws, err := websocket.Dial("ws://"+aggregator.Listener.Addr().String()+path, "", "http://127.0.0.1/") + if err != nil { + if !tc.ExpectError { + t.Errorf("%s: websocket dial err: %s", tcName, err) + } + return + } + defer ws.Close() + if tc.ExpectError { + t.Errorf("%s: expected websocket error, got none", tcName) + return + } + + if _, err := ws.Write([]byte("world")); err != nil { + t.Errorf("%s: write err: %s", tcName, err) + return + } + + response := make([]byte, 20) + n, err := ws.Read(response) + if err != nil { + t.Errorf("%s: read err: %s", tcName, err) + return + } + if e, a := "hello world", string(response[0:n]); e != a { + t.Errorf("%s: expected '%#v', got '%#v'", tcName, e, a) + return + } + }() + } +} + +var testCACrt = []byte(`-----BEGIN CERTIFICATE----- +MIICxDCCAaygAwIBAgIBATANBgkqhkiG9w0BAQsFADASMRAwDgYDVQQDEwd0ZXN0 +LWNhMCAXDTE3MDcyMDIxMTc1MloYDzIxMTcwNjI2MjExNzUzWjASMRAwDgYDVQQD +Ewd0ZXN0LWNhMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAuv/sT2xH +VS1/uXVNAEIwvEb2yTMbXwP6FD38LWkc37Ri7YMB9xiXEDBrbr6K1JThsqyitBxU +22QIl53LUm6I7c/vej1tdYtE2rDVuviiiRgy6omR8imVSv9vU024rgDe+nC9zTT1 +3aNKR03olCG6fkygdcZOghzlyQLhyh8LG75XdnLNksnakum2dNxQ5QIFmBKAuev3 +A069oRMNjudot+t/nFP9UDZ8dL80PNTNPF22bPsnxiau7KLZ4I0Lf7gt6yHlNcue +Fd5sqzqsw/LUFJR5Xuo1+0e7NV3SwCH5CymG6hkboM4Rf5S3EDDyXTxPbXzbQHf1 +7ksW6gjAxh4x/wIDAQABoyMwITAOBgNVHQ8BAf8EBAMCAqQwDwYDVR0TAQH/BAUw +AwEB/zANBgkqhkiG9w0BAQsFAAOCAQEATgmDrW1BjFp+Vmw6T+ojVK4lJuIoerGw +TCCqabHs6O1iWkNi5KsY6vV86tofBIEXsf6S3mV2jcBn87+CIbNHlHFKrXwmcydA +WOc0LWVqqoeqIvEcMNoWQskzmOOUDTanX9mXkirm8d8BljC351TH17rSjLGzFuNh +Cy48xyKFM7kPauNZGfCyaZsGbNJP3Keeu35dOLZMDdBJw7ZvYEUqX7MLOO+d7vlO +JGNA5jsU2uBnSo6qsjxfsbGemk2uRO0nLhplWurw+4qzA79D0lKNLtH9yTn12KZb +/kUpsOSCtLomjWwp67lQyA/yFXf897pSKMXbnIfZfIlDg51CI3U2Sw== +-----END CERTIFICATE-----`) + +// valid for hostname test-service.test-ns.svc +// signed by testCACrt +var svcCrt = []byte(`-----BEGIN CERTIFICATE----- +MIIDDDCCAfSgAwIBAgIBBDANBgkqhkiG9w0BAQsFADASMRAwDgYDVQQDEwd0ZXN0 +LWNhMCAXDTE3MDcyMDIxMjAzN1oYDzIxMTcwNjI2MjEyMDM4WjAjMSEwHwYDVQQD +Exh0ZXN0LXNlcnZpY2UudGVzdC1ucy5zdmMwggEiMA0GCSqGSIb3DQEBAQUAA4IB +DwAwggEKAoIBAQDOKgoTmlVeDhImiBLBccxdniKkS+FZSaoAEtoTvJG1wjk0ewzF +vKhjbHolJ+/qEANiQ6CpTz4hU3m/Iad6IrnmKd1jnkh9yKEaU32B2Xbh6VaV7Sca +Hv4cKWTe50sBvufZinTT8hlFcGufFlJIOLXya5t6HH1Ld7Xf2qwNqusHdmFlJko7 +0By8jhTtD7+2OAJsIPQDWfAsXxFa6LeQ/lqS2DCFnp45DirTNetXoIH8ZJvTBjak +bQuAAA3H+61gRm1blIu8/JjHYTDOcUe5pFyrFLFPgA+eIcpIbzTD61UTNhVlusV2 +eRrBr5BlRM13Zj6ZMcWp0Iiw5QI/W9QU7O4jAgMBAAGjWjBYMA4GA1UdDwEB/wQE +AwIFoDATBgNVHSUEDDAKBggrBgEFBQcDATAMBgNVHRMBAf8EAjAAMCMGA1UdEQQc +MBqCGHRlc3Qtc2VydmljZS50ZXN0LW5zLnN2YzANBgkqhkiG9w0BAQsFAAOCAQEA +kpULlml6Ct0cjOuHgDKUnTboFTUm2FHJY27p4NXUvXUMoipg/eSxk0r5JynzXrPa +jaJfY2bC45ixLjJv9irp9ER/lGYUcBQ8OHouXy+uKtA5YKHI3wYf8ITZrQwzRpsK +C5v7qDW+iyb9dn4T6qgpZvylKtkH5cH31hQooNgjZd5aEq3JSFnCM12IVqs/5kjL +NnbPXzeBQ9CHbC+mx7Mm6eSQVtGcQOy4yXFrh2/vrIB2t4gNeWaI1b+7l4MaJjV/ +kRrOirhZaJ90ow/PdYrILtEAdpeC/2Varpr3l4rIKhkkop4gfPwaFeWhG38shH3E +eG5PW2waPpxJzEnGBoAWxQ== +-----END CERTIFICATE-----`) + +var svcKey = []byte(`-----BEGIN RSA PRIVATE KEY----- +MIIEogIBAAKCAQEAzioKE5pVXg4SJogSwXHMXZ4ipEvhWUmqABLaE7yRtcI5NHsM +xbyoY2x6JSfv6hADYkOgqU8+IVN5vyGneiK55indY55IfcihGlN9gdl24elWle0n +Gh7+HClk3udLAb7n2Yp00/IZRXBrnxZSSDi18mubehx9S3e139qsDarrB3ZhZSZK +O9AcvI4U7Q+/tjgCbCD0A1nwLF8RWui3kP5aktgwhZ6eOQ4q0zXrV6CB/GSb0wY2 +pG0LgAANx/utYEZtW5SLvPyYx2EwznFHuaRcqxSxT4APniHKSG80w+tVEzYVZbrF +dnkawa+QZUTNd2Y+mTHFqdCIsOUCP1vUFOzuIwIDAQABAoIBABiX9z/DZ2+i6hNi +pCojcyev154V1zoZiYgct5snIZK3Kq/SBgIIsWW66Q9Jplsbseuk+aN46oZ7OMjO +MPZm8ho84EYj+a3XozBKyWwWDxKADW4xLjr1e4bMgVX97Xq11V6kH6+w78bS1GPT ++9jVuw7CO3fjsiawjye3JFM1Enh/NeRLEpT/oaQoWIV8b0IQB0VyqrdxWOO0rQhd +xA5w39tAZPDQ79MbMQyNWtPgBy0FuulP0GB12PrEbE+SXxsFhWViEwdB5Qx6Gqsx +KGn9vB1oaeSuuKIAjyBV0rXszrGektorDchsOY9UQi1mQsPSvvRFTM9T3qqSFIpu +oPNQLvECgYEA3ox3WJGjEve6VI4RMRt0l6ZFswNbNaHcTMPVsayqsl9KfebG+uyn +Z7TyyoCRzZZQa+3Z9jjW3hAGM9e7MG8jkeHbZpJpZv9X7eB3dgq3eZ1Zt5dyoDrU +PTdIPA2efFAf6V1ejyqH9h6RPQMeAb4uFU9nbI4rPagMxRdp5qIveIUCgYEA7Scb +0zWplDit4EUo+Fq80wzItwJZv64my8KIkEPpW3Fu6UPQvY74qyhE2fCSCwHqRpYJ +jVylyE0GIMx42kjwBgOpi4yEg8M3uMTal+Iy9SgrxZ5cPetaFpEF3Wk7/tz6ppr+ +wnZQTO2WH3YLzv7JIWVrOKuBNVfNEbguVFWw4IcCgYB54mp2uoSancySBKDLyWKo +r6raqQrqK7TQ4iyGO6/dMy1EGQF/ad8hgEu8tn+kHh/7jG/kVyruwc3z1MIze5r6 +ib00xxktDMnmgRpMLwBffdsmHq7rrGyS/lT0du0G3ocrszRXqo5+MC2RQcTMZZEt +oKhfHtn10bT0uKcKZmcjVQKBgEls2WWccMOuhM8yOowic+IYTDC1bpo1Tle6BFQ+ +YoroZQGd+IwoLv+3ORINNPppfmKaY5y7+aw5hNM025oiCQajraPCPukY0TDI6jEq +XMKgzGSkMkUNkFf6UMmLooK3Yneg94232gbnbJqTDvbo1dccMoVaPGgKpjh9QQLl +gR0TAoGACFOvhl8txfbkwLeuNeunyOPL7J4nIccthgd2ioFOr3HTou6wzN++vYTa +a3OF9jH5Z7m6X1rrwn6J1+Gw9sBme38/GeGXHigsBI/8WaTvyuppyVIXOVPoTvVf +VYsTwo5YgV1HzDkV+BNmBCw1GYcGXAElhJI+dCsgQuuU6TKzgl8= +-----END RSA PRIVATE KEY-----`)