From 03f19da21c336a823749c2d268f0585d2ef5bedc Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Thu, 14 Apr 2022 09:59:19 -0700 Subject: [PATCH] the http2RoundTripper should only use http2 Signed-off-by: Margo Crawford --- .../concierge/impersonator/impersonator.go | 11 ++++- .../impersonator/impersonator_test.go | 48 ++++++++++++++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/internal/concierge/impersonator/impersonator.go b/internal/concierge/impersonator/impersonator.go index 828d94e9..70155dba 100644 --- a/internal/concierge/impersonator/impersonator.go +++ b/internal/concierge/impersonator/impersonator.go @@ -472,7 +472,7 @@ func newImpersonationReverseProxyFunc(restConfig *rest.Config) (func(*genericapi return nil, fmt.Errorf("could not get http/1.1 anonymous round tripper: %w", err) } - http2RoundTripper, err := getTransportForProtocol(restConfig, "h2") // TODO figure out why this leads to still supporting http1 + http2RoundTripper, err := getTransportForProtocol(restConfig, "h2") if err != nil { return nil, fmt.Errorf("could not get http/2.0 round tripper: %w", err) } @@ -812,6 +812,15 @@ func getTransportForProtocol(restConfig *rest.Config, protocol string) (http.Rou return nil, fmt.Errorf("could not build transport: %w", err) } + // For clients that support http2, transport.New calls http2.ConfigureTransports, + // which configures with both h2 and http/1.1, + // even when you explicitly only ask for h2. + // Override that change. + cfg, err := utilnet.TLSClientConfig(rt) + if err != nil { + return nil, fmt.Errorf("could not extract TLS config: %w", err) + } + cfg.NextProtos = []string{protocol} if err := kubeclient.AssertSecureTransport(rt); err != nil { return nil, err // make sure we only use a secure TLS config } diff --git a/internal/concierge/impersonator/impersonator_test.go b/internal/concierge/impersonator/impersonator_test.go index 6eedd950..cf68a510 100644 --- a/internal/concierge/impersonator/impersonator_test.go +++ b/internal/concierge/impersonator/impersonator_test.go @@ -5,6 +5,8 @@ package impersonator import ( "context" + "crypto/tls" + "crypto/x509" "fmt" "math/rand" "net" @@ -701,7 +703,28 @@ func TestImpersonator(t *testing.T) { testKubeAPIServerWasCalled := false var testKubeAPIServerSawHeaders http.Header testKubeAPIServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - tlsserver.AssertTLS(t, r, ptls.Secure) + tlsConfigFunc := func(rootCAs *x509.CertPool) *tls.Config { + // Requests to get configmaps, flowcontrol requests, and healthz requests + // are not done by our http round trippers that specify only one protocol + // (either http1.1 or http2, not both). + // For all other requests from the impersonator, if it is not an upgrade + // request it should only be using http2. + // If it is an upgrade request it should only be using http1.1, but that + // is covered by the AssertTLS function. + secure := ptls.Secure(rootCAs) + switch r.URL.Path { + case "/api/v1/namespaces/kube-system/configmaps", + "/apis/flowcontrol.apiserver.k8s.io/v1beta2/prioritylevelconfigurations", + "/apis/flowcontrol.apiserver.k8s.io/v1beta2/flowschemas", + "/healthz": + default: + if !httpstream.IsUpgradeRequest(r) { + secure.NextProtos = []string{secure.NextProtos[0]} + } + } + return secure + } + tlsserver.AssertTLS(t, r, tlsConfigFunc) switch r.URL.Path { case "/api/v1/namespaces/kube-system/configmaps": @@ -1780,7 +1803,28 @@ func TestImpersonatorHTTPHandler(t *testing.T) { testKubeAPIServerWasCalled := false testKubeAPIServerSawHeaders := http.Header{} testKubeAPIServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - tlsserver.AssertTLS(t, r, ptls.Secure) + tlsConfigFunc := func(rootCAs *x509.CertPool) *tls.Config { + // Requests to get configmaps, flowcontrol requests, and healthz requests + // are not done by our http round trippers that specify only one protocol + // (either http1.1 or http2, not both). + // For all other requests from the impersonator, if it is not an upgrade + // request it should only be using http2. + // If it is an upgrade request it should only be using http1.1, but that + // is covered by the AssertTLS function. + secure := ptls.Secure(rootCAs) + switch r.URL.Path { + case "/api/v1/namespaces/kube-system/configmaps", + "/apis/flowcontrol.apiserver.k8s.io/v1beta2/prioritylevelconfigurations", + "/apis/flowcontrol.apiserver.k8s.io/v1beta2/flowschemas", + "/healthz": + default: + if !httpstream.IsUpgradeRequest(r) { + secure.NextProtos = []string{secure.NextProtos[0]} + } + } + return secure + } + tlsserver.AssertTLS(t, r, tlsConfigFunc) testKubeAPIServerWasCalled = true testKubeAPIServerSawHeaders = r.Header