diff --git a/cmd/pinniped/cmd/kubeconfig_test.go b/cmd/pinniped/cmd/kubeconfig_test.go index 4b30456f4..1af712dcf 100644 --- a/cmd/pinniped/cmd/kubeconfig_test.go +++ b/cmd/pinniped/cmd/kubeconfig_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package cmd @@ -18,6 +18,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" kubetesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/clientcmd" + "k8s.io/utils/ptr" conciergev1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1" configv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/config/v1alpha1" @@ -27,6 +28,7 @@ import ( "go.pinniped.dev/internal/here" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/testlogger" + "go.pinniped.dev/internal/testutil/tlsserver" ) func TestGetKubeconfig(t *testing.T) { @@ -3198,7 +3200,7 @@ func TestGetKubeconfig(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { var issuerEndpointPtr *string - issuerCABundle, issuerEndpoint := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) { + testServer, testServerCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") switch r.URL.Path { case "/.well-known/openid-configuration": @@ -3226,8 +3228,8 @@ func TestGetKubeconfig(t *testing.T) { default: t.Fatalf("tried to call issuer at a path that wasn't one of the expected discovery endpoints.") } - }) - issuerEndpointPtr = &issuerEndpoint + }), nil) + issuerEndpointPtr = ptr.To(testServer.URL) testLog := testlogger.NewLegacy(t) //nolint:staticcheck // old test with lots of log statements cmd := kubeconfigCommand(kubeconfigDeps{ @@ -3248,7 +3250,7 @@ func TestGetKubeconfig(t *testing.T) { } fake := fakeconciergeclientset.NewSimpleClientset() if tt.conciergeObjects != nil { - fake = fakeconciergeclientset.NewSimpleClientset(tt.conciergeObjects(issuerCABundle, issuerEndpoint)...) + fake = fakeconciergeclientset.NewSimpleClientset(tt.conciergeObjects(string(testServerCA), testServer.URL)...) } if len(tt.conciergeReactions) > 0 { fake.ReactionChain = append(tt.conciergeReactions, fake.ReactionChain...) @@ -3263,7 +3265,7 @@ func TestGetKubeconfig(t *testing.T) { cmd.SetOut(&stdout) cmd.SetErr(&stderr) - cmd.SetArgs(tt.args(issuerCABundle, issuerEndpoint)) + cmd.SetArgs(tt.args(string(testServerCA), testServer.URL)) err := cmd.Execute() if tt.wantError { @@ -3274,19 +3276,19 @@ func TestGetKubeconfig(t *testing.T) { var expectedLogs []string if tt.wantLogs != nil { - expectedLogs = tt.wantLogs(issuerCABundle, issuerEndpoint) + expectedLogs = tt.wantLogs(string(testServerCA), testServer.URL) } testLog.Expect(expectedLogs) expectedStdout := "" if tt.wantStdout != nil { - expectedStdout = tt.wantStdout(issuerCABundle, issuerEndpoint) + expectedStdout = tt.wantStdout(string(testServerCA), testServer.URL) } require.Equal(t, expectedStdout, stdout.String(), "unexpected stdout") actualStderr := stderr.String() if tt.wantStderr != nil { - testutil.RequireErrorString(t, actualStderr, tt.wantStderr(issuerCABundle, issuerEndpoint)) + testutil.RequireErrorString(t, actualStderr, tt.wantStderr(string(testServerCA), testServer.URL)) } else { require.Empty(t, actualStderr, "unexpected stderr") } diff --git a/internal/concierge/impersonator/impersonator_test.go b/internal/concierge/impersonator/impersonator_test.go index b424e6a21..783ae8e07 100644 --- a/internal/concierge/impersonator/impersonator_test.go +++ b/internal/concierge/impersonator/impersonator_test.go @@ -692,7 +692,7 @@ func TestImpersonator(t *testing.T) { // will proxy incoming calls to this fake server. testKubeAPIServerWasCalled := false var testKubeAPIServerSawHeaders http.Header - testKubeAPIServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + testKubeAPIServer, testKubeAPIServerCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { 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 @@ -815,7 +815,7 @@ func TestImpersonator(t *testing.T) { // Create the client config that the impersonation server should use to talk to the Kube API server. testKubeAPIServerKubeconfig := rest.Config{ Host: testKubeAPIServer.URL, - TLSClientConfig: rest.TLSClientConfig{CAData: tlsserver.TLSTestServerCA(testKubeAPIServer)}, + TLSClientConfig: rest.TLSClientConfig{CAData: testKubeAPIServerCA}, } // Punch out just enough stuff to make New actually run without error. @@ -1806,7 +1806,7 @@ func TestImpersonatorHTTPHandler(t *testing.T) { testKubeAPIServerWasCalled := false testKubeAPIServerSawHeaders := http.Header{} - testKubeAPIServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + testKubeAPIServer, testKubeAPIServerCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { 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 @@ -1842,7 +1842,7 @@ func TestImpersonatorHTTPHandler(t *testing.T) { testKubeAPIServerKubeconfig := rest.Config{ Host: testKubeAPIServer.URL, BearerToken: "some-service-account-token", - TLSClientConfig: rest.TLSClientConfig{CAData: tlsserver.TLSTestServerCA(testKubeAPIServer)}, + TLSClientConfig: rest.TLSClientConfig{CAData: testKubeAPIServerCA}, } if tt.restConfig == nil { tt.restConfig = &testKubeAPIServerKubeconfig diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go index 951db2bfa..eda1fd78c 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go @@ -66,7 +66,6 @@ const ( reasonInvalidTLSConfiguration = "InvalidTLSConfiguration" reasonInvalidDiscoveryProbe = "InvalidDiscoveryProbe" reasonInvalidAuthenticator = "InvalidAuthenticator" - reasonInvalidTokenSigningFailure = "InvalidTokenSigningFailure" reasonInvalidCouldNotFetchJWKS = "InvalidCouldNotFetchJWKS" msgUnableToValidate = "unable to validate; see other conditions for details" diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index 3e1868f22..188ce5398 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go @@ -76,7 +76,7 @@ func TestController(t *testing.T) { distributedGroups := []string{"some-distributed-group-1", "some-distributed-group-2"} goodMux := http.NewServeMux() - goodOIDCIssuerServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + goodOIDCIssuerServer, _ := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { tlsserver.AssertTLS(t, r, ptls.Default) goodMux.ServeHTTP(w, r) }), tlsserver.RecordTLSHello) @@ -174,7 +174,7 @@ func TestController(t *testing.T) { })) badMuxInvalidJWKSURI := http.NewServeMux() - badOIDCIssuerServerInvalidJWKSURI := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + badOIDCIssuerServerInvalidJWKSURI, _ := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { tlsserver.AssertTLS(t, r, ptls.Default) badMuxInvalidJWKSURI.ServeHTTP(w, r) }), tlsserver.RecordTLSHello) @@ -185,7 +185,7 @@ func TestController(t *testing.T) { })) badMuxInvalidJWKSURIScheme := http.NewServeMux() - badOIDCIssuerServerInvalidJWKSURIScheme := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + badOIDCIssuerServerInvalidJWKSURIScheme, _ := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { tlsserver.AssertTLS(t, r, ptls.Default) badMuxInvalidJWKSURIScheme.ServeHTTP(w, r) }), tlsserver.RecordTLSHello) @@ -196,7 +196,7 @@ func TestController(t *testing.T) { })) jwksFetchShouldFailMux := http.NewServeMux() - jwksFetchShouldFailServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + jwksFetchShouldFailServer, _ := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { tlsserver.AssertTLS(t, r, ptls.Default) jwksFetchShouldFailMux.ServeHTTP(w, r) }), tlsserver.RecordTLSHello) diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go index a6ebd96a9..2ad717a9f 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go @@ -10,7 +10,7 @@ import ( "crypto/x509" "fmt" "net/url" - "os" + "time" k8sauthv1beta1 "k8s.io/api/authentication/v1beta1" "k8s.io/apimachinery/pkg/api/equality" @@ -19,10 +19,8 @@ import ( errorsutil "k8s.io/apimachinery/pkg/util/errors" k8snetutil "k8s.io/apimachinery/pkg/util/net" "k8s.io/apiserver/pkg/authentication/authenticator" - webhookutil "k8s.io/apiserver/pkg/util/webhook" "k8s.io/apiserver/plugin/pkg/authenticator/token/webhook" - "k8s.io/client-go/tools/clientcmd" - clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + "k8s.io/client-go/rest" "k8s.io/klog/v2" "k8s.io/utils/clock" @@ -34,7 +32,9 @@ import ( "go.pinniped.dev/internal/controller/authenticator/authncache" "go.pinniped.dev/internal/controller/conditionsutil" "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/endpointaddr" + "go.pinniped.dev/internal/kubeclient" "go.pinniped.dev/internal/plog" ) @@ -48,9 +48,7 @@ const ( reasonSuccess = "Success" reasonNotReady = "NotReady" reasonUnableToValidate = "UnableToValidate" - reasonUnableToCreateTempFile = "UnableToCreateTempFile" - reasonUnableToMarshallKubeconfig = "UnableToMarshallKubeconfig" - reasonUnableToLoadKubeconfig = "UnableToLoadKubeconfig" + reasonUnableToCreateClient = "UnableToCreateClient" reasonUnableToInstantiateWebhook = "UnableToInstantiateWebhook" reasonInvalidTLSConfiguration = "InvalidTLSConfiguration" reasonInvalidEndpointURL = "InvalidEndpointURL" @@ -66,18 +64,16 @@ func New( webhooks authinformers.WebhookAuthenticatorInformer, clock clock.Clock, log plog.Logger, - tlsDialerFunc func(network string, addr string, config *tls.Config) (*tls.Conn, error), ) controllerlib.Controller { return controllerlib.New( controllerlib.Config{ Name: controllerName, Syncer: &webhookCacheFillerController{ - cache: cache, - client: client, - webhooks: webhooks, - clock: clock, - log: log.WithName(controllerName), - tlsDialerFunc: tlsDialerFunc, + cache: cache, + client: client, + webhooks: webhooks, + clock: clock, + log: log.WithName(controllerName), }, }, controllerlib.WithInformer( @@ -89,12 +85,11 @@ func New( } type webhookCacheFillerController struct { - cache *authncache.Cache - webhooks authinformers.WebhookAuthenticatorInformer - client conciergeclientset.Interface - clock clock.Clock - log plog.Logger - tlsDialerFunc func(network string, addr string, config *tls.Config) (*tls.Conn, error) + cache *authncache.Cache + webhooks authinformers.WebhookAuthenticatorInformer + client conciergeclientset.Interface + clock clock.Clock + log plog.Logger } // Sync implements controllerlib.Syncer. @@ -105,25 +100,25 @@ func (c *webhookCacheFillerController) Sync(ctx controllerlib.Context) error { return nil } if err != nil { + // no unit test for this failure return fmt.Errorf("failed to get WebhookAuthenticator %s/%s: %w", ctx.Key.Namespace, ctx.Key.Name, err) } conditions := make([]*metav1.Condition, 0) - specCopy := obj.Spec.DeepCopy() var errs []error - certPool, pemBytes, conditions, tlsBundleOk := c.validateTLSBundle(specCopy.TLS, conditions) - endpointHostPort, conditions, endpointOk := c.validateEndpoint(specCopy.Endpoint, conditions) + certPool, pemBytes, conditions, tlsBundleOk := c.validateTLSBundle(obj.Spec.TLS, conditions) + endpointHostPort, conditions, endpointOk := c.validateEndpoint(obj.Spec.Endpoint, conditions) okSoFar := tlsBundleOk && endpointOk conditions, tlsNegotiateErr := c.validateConnection(certPool, endpointHostPort, conditions, okSoFar) errs = append(errs, tlsNegotiateErr) okSoFar = okSoFar && tlsNegotiateErr == nil webhookAuthenticator, conditions, err := newWebhookAuthenticator( - specCopy.Endpoint, + // Note that we use the whole URL when constructing the webhook client, + // not just the host and port that ew validated above. We need the path, etc. + obj.Spec.Endpoint, pemBytes, - os.CreateTemp, - clientcmd.WriteToFile, conditions, okSoFar, ) @@ -152,10 +147,8 @@ func (c *webhookCacheFillerController) Sync(ctx controllerlib.Context) error { // newWebhookAuthenticator creates a webhook from the provided API server url and caBundle // used to validate TLS connections. func newWebhookAuthenticator( - endpoint string, + endpointURL string, pemBytes []byte, - tempfileFunc func(string, string) (*os.File, error), - marshalFunc func(clientcmdapi.Config, string) error, conditions []*metav1.Condition, prereqOk bool, ) (*webhook.WebhookTokenAuthenticator, []*metav1.Condition, error) { @@ -168,39 +161,6 @@ func newWebhookAuthenticator( }) return nil, conditions, nil } - temp, err := tempfileFunc("", "pinniped-webhook-kubeconfig-*") - if err != nil { - errText := "unable to create temporary file" - msg := fmt.Sprintf("%s: %s", errText, err.Error()) - conditions = append(conditions, &metav1.Condition{ - Type: typeAuthenticatorValid, - Status: metav1.ConditionFalse, - Reason: reasonUnableToCreateTempFile, - Message: msg, - }) - return nil, conditions, fmt.Errorf("%s: %w", errText, err) - } - defer func() { _ = os.Remove(temp.Name()) }() - - cluster := &clientcmdapi.Cluster{Server: endpoint} - cluster.CertificateAuthorityData = pemBytes - - kubeconfig := clientcmdapi.NewConfig() - kubeconfig.Clusters["anonymous-cluster"] = cluster - kubeconfig.Contexts["anonymous"] = &clientcmdapi.Context{Cluster: "anonymous-cluster"} - kubeconfig.CurrentContext = "anonymous" - - if err := marshalFunc(*kubeconfig, temp.Name()); err != nil { - errText := "unable to marshal kubeconfig" - msg := fmt.Sprintf("%s: %s", errText, err.Error()) - conditions = append(conditions, &metav1.Condition{ - Type: typeAuthenticatorValid, - Status: metav1.ConditionFalse, - Reason: reasonUnableToMarshallKubeconfig, - Message: msg, - }) - return nil, conditions, fmt.Errorf("%s: %w", errText, err) - } // We use v1beta1 instead of v1 since v1beta1 is more prevalent in our desired // integration points. @@ -215,40 +175,30 @@ func newWebhookAuthenticator( // custom proxy stuff used by the API server. var customDial k8snetutil.DialFunc - // TODO refactor this code to directly construct the rest.Config - // ideally we would keep rest config generation contained to the kubeclient package - // but this will require some form of a new WithTLSConfigFunc kubeclient.Option - // ex: - // _, caBundle, err := pinnipedauthenticator.CABundle(spec.TLS) - // ... - // restConfig := &rest.Config{ - // Host: spec.Endpoint, - // TLSClientConfig: rest.TLSClientConfig{CAData: caBundle}, - // // copied from k8s.io/apiserver/pkg/util/webhook - // Timeout: 30 * time.Second, - // QPS: -1, - // } - // client, err := kubeclient.New(kubeclient.WithConfig(restConfig), kubeclient.WithTLSConfigFunc(ptls.Default)) - // ... - // then use client.JSONConfig as clientConfig - clientConfig, err := webhookutil.LoadKubeconfig(temp.Name(), customDial) + restConfig := &rest.Config{ + Host: endpointURL, + TLSClientConfig: rest.TLSClientConfig{CAData: pemBytes}, + + // The remainder of these settings are copied from webhookutil.LoadKubeconfig in k8s.io/apiserver/pkg/util/webhook. + Dial: customDial, + Timeout: 30 * time.Second, + QPS: -1, + } + + client, err := kubeclient.New(kubeclient.WithConfig(restConfig), kubeclient.WithTLSConfigFunc(ptls.Default)) if err != nil { - // no unit test for this failure. - errText := "unable to load kubeconfig" + errText := "unable to create client for this webhook" msg := fmt.Sprintf("%s: %s", errText, err.Error()) conditions = append(conditions, &metav1.Condition{ Type: typeAuthenticatorValid, Status: metav1.ConditionFalse, - Reason: reasonUnableToLoadKubeconfig, + Reason: reasonUnableToCreateClient, Message: msg, }) return nil, conditions, fmt.Errorf("%s: %w", errText, err) } - // this uses a http client that does not honor our TLS config - // TODO: fix when we pick up https://github.com/kubernetes/kubernetes/pull/106155 - // NOTE: looks like the above was merged on Mar 18, 2022 - webhookA, err := webhook.New(clientConfig, version, implicitAuds, *webhook.DefaultRetryBackoff()) + webhookAuthenticator, err := webhook.New(client.JSONConfig, version, implicitAuds, *webhook.DefaultRetryBackoff()) if err != nil { // no unit test for this failure. errText := "unable to instantiate webhook" @@ -261,6 +211,7 @@ func newWebhookAuthenticator( }) return nil, conditions, fmt.Errorf("%s: %w", errText, err) } + msg := "authenticator initialized" conditions = append(conditions, &metav1.Condition{ Type: typeAuthenticatorValid, @@ -268,7 +219,8 @@ func newWebhookAuthenticator( Reason: reasonSuccess, Message: msg, }) - return webhookA, conditions, nil + + return webhookAuthenticator, conditions, nil } func (c *webhookCacheFillerController) validateConnection(certPool *x509.CertPool, endpointHostPort *endpointaddr.HostPort, conditions []*metav1.Condition, prereqOk bool) ([]*metav1.Condition, error) { @@ -282,11 +234,7 @@ func (c *webhookCacheFillerController) validateConnection(certPool *x509.CertPoo return conditions, nil } - conn, err := c.tlsDialerFunc("tcp", endpointHostPort.Endpoint(), &tls.Config{ - MinVersion: tls.VersionTLS12, - // If certPool is nil then RootCAs will be set to nil and TLS will use the host's root CA set automatically. - RootCAs: certPool, - }) + conn, err := tls.Dial("tcp", endpointHostPort.Endpoint(), ptls.Default(certPool)) if err != nil { errText := "cannot dial server" @@ -303,6 +251,7 @@ func (c *webhookCacheFillerController) validateConnection(certPool *x509.CertPoo // this error should never be significant err = conn.Close() if err != nil { + // no unit test for this failure c.log.Error("error closing dialer", err) } @@ -310,7 +259,7 @@ func (c *webhookCacheFillerController) validateConnection(certPool *x509.CertPoo Type: typeWebhookConnectionValid, Status: metav1.ConditionTrue, Reason: reasonSuccess, - Message: "tls verified", + Message: "successfully dialed webhook server", }) return conditions, nil } @@ -381,7 +330,7 @@ func (c *webhookCacheFillerController) validateEndpoint(endpoint string, conditi Type: typeEndpointURLValid, Status: metav1.ConditionTrue, Reason: reasonSuccess, - Message: "endpoint is a valid URL", + Message: "spec.endpoint is a valid URL", }) return &endpointHostPort, conditions, true } diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go index 081603236..a846c9577 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go @@ -16,20 +16,17 @@ import ( "net/http" "net/http/httptest" "net/url" - "os" "testing" "time" - "github.com/google/go-cmp/cmp" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + authenticationv1beta1 "k8s.io/api/authentication/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" coretesting "k8s.io/client-go/testing" - "k8s.io/client-go/tools/clientcmd" - clientcmdapi "k8s.io/client-go/tools/clientcmd/api" clocktesting "k8s.io/utils/clock/testing" + "k8s.io/utils/ptr" auth1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1" pinnipedfake "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned/fake" @@ -91,56 +88,40 @@ func TestController(t *testing.T) { ) require.NoError(t, err) - hostAsLocalhostMux := http.NewServeMux() - hostAsLocalhostWebhookServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - tlsserver.AssertTLS(t, r, ptls.Default) - hostAsLocalhostMux.ServeHTTP(w, r) - }), func(thisServer *httptest.Server) { - thisTLSConfig := ptls.Default(nil) - thisTLSConfig.Certificates = []tls.Certificate{ - *hostAsLocalhostServingCert, - } - thisServer.TLS = thisTLSConfig + hostAsLocalhostWebhookServer, _ := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // only expecting dials, which will not get into handler func + }), func(s *httptest.Server) { + s.TLS.Certificates = []tls.Certificate{*hostAsLocalhostServingCert} + tlsserver.AssertEveryTLSHello(t, s, ptls.Default) // assert on every hello because we are only expecting dials }) - hostAs127001Mux := http.NewServeMux() - hostAs127001WebhookServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - tlsserver.AssertTLS(t, r, ptls.Default) - hostAs127001Mux.ServeHTTP(w, r) - }), func(thisServer *httptest.Server) { - thisTLSConfig := ptls.Default(nil) - thisTLSConfig.Certificates = []tls.Certificate{ - *hostAs127001ServingCert, - } - thisServer.TLS = thisTLSConfig + hostAs127001WebhookServer, _ := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // only expecting dials, which will not get into handler func + }), func(s *httptest.Server) { + s.TLS.Certificates = []tls.Certificate{*hostAs127001ServingCert} + tlsserver.AssertEveryTLSHello(t, s, ptls.Default) // assert on every hello because we are only expecting dials }) - localWithExampleDotComMux := http.NewServeMux() - hostLocalWithExampleDotComCertServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - tlsserver.AssertTLS(t, r, ptls.Default) - localWithExampleDotComMux.ServeHTTP(w, r) - }), func(thisServer *httptest.Server) { - thisTLSConfig := ptls.Default(nil) - thisTLSConfig.Certificates = []tls.Certificate{ - *localButExampleDotComServerCert, - } - thisServer.TLS = thisTLSConfig + hostLocalWithExampleDotComCertServer, _ := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // only expecting dials, which will not get into handler func + }), func(s *httptest.Server) { + s.TLS.Certificates = []tls.Certificate{*localButExampleDotComServerCert} + tlsserver.AssertEveryTLSHello(t, s, ptls.Default) // assert on every hello because we are only expecting dials }) - goodMux := http.NewServeMux() - hostGoodDefaultServingCertServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - tlsserver.AssertTLS(t, r, ptls.Default) - goodMux.ServeHTTP(w, r) - }), tlsserver.RecordTLSHello) - goodMux.Handle("/some/webhook", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - _, err := fmt.Fprintf(w, `{"something": "%s"}`, "something-for-response") - require.NoError(t, err) - })) - goodMux.Handle("/nothing/here", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hostLocalIPv6Server, ipv6CA := tlsserver.TestServerIPv6(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}), tlsserver.RecordTLSHello) + + mux := http.NewServeMux() + mux.Handle("/nothing/here", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // note that we are only dialing, so we shouldn't actually get here w.WriteHeader(http.StatusNotFound) - fmt.Fprint(w, "404 nothing here") + _, _ = fmt.Fprint(w, "404 nothing here") })) + hostGoodDefaultServingCertServer, _ := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mux.ServeHTTP(w, r) + }), func(s *httptest.Server) { + tlsserver.AssertEveryTLSHello(t, s, ptls.Default) // assert on every hello because we are only expecting dials + }) goodWebhookDefaultServingCertEndpoint := hostGoodDefaultServingCertServer.URL goodWebhookDefaultServingCertEndpointBut404 := goodWebhookDefaultServingCertEndpoint + "/nothing/here" @@ -270,7 +251,7 @@ func TestController(t *testing.T) { ObservedGeneration: observedGeneration, LastTransitionTime: time, Reason: "Success", - Message: "tls verified", + Message: "successfully dialed webhook server", } } unknownWebhookConnectionValid := func(time metav1.Time, observedGeneration int64) metav1.Condition { @@ -321,7 +302,7 @@ func TestController(t *testing.T) { ObservedGeneration: observedGeneration, LastTransitionTime: time, Reason: "Success", - Message: "endpoint is a valid URL", + Message: "spec.endpoint is a valid URL", } } sadEndpointURLValid := func(issuer string, time metav1.Time, observedGeneration int64) metav1.Condition { @@ -383,14 +364,13 @@ func TestController(t *testing.T) { webhooks []runtime.Object // for modifying the clients to hack in arbitrary api responses configClient func(*pinnipedfake.Clientset) - tlsDialerFunc func(network string, addr string, config *tls.Config) (*tls.Conn, error) wantSyncLoopErr testutil.RequireErrorStringFunc wantLogs []map[string]any wantActions func() []coretesting.Action wantCacheEntries int }{ { - name: "404: WebhookAuthenticator not found will abort sync loop, no status conditions", + name: "Sync: WebhookAuthenticator not found will abort sync loop, no status conditions", syncKey: controllerlib.Key{Name: "test-name"}, wantLogs: []map[string]any{ { @@ -546,6 +526,63 @@ func TestController(t *testing.T) { }, wantCacheEntries: 1, }, + { + name: "Sync: valid WebhookAuthenticator with IPV6 and CA: will complete sync loop successfully with success conditions and ready phase", + syncKey: controllerlib.Key{Name: "test-name"}, + webhooks: []runtime.Object{ + &auth1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: func() auth1alpha1.WebhookAuthenticatorSpec { + ipv6 := goodWebhookAuthenticatorSpecWithCA.DeepCopy() + ipv6.Endpoint = hostLocalIPv6Server.URL + ipv6.TLS = ptr.To(auth1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString(ipv6CA), + }) + return *ipv6 + }(), + }, + }, + wantLogs: []map[string]any{ + { + "level": "info", + "timestamp": "2099-08-08T13:57:36.123456Z", + "logger": "webhookcachefiller-controller", + "message": "added new webhook authenticator", + "endpoint": hostLocalIPv6Server.URL, + "webhook": map[string]interface{}{ + "name": "test-name", + }, + }, + }, + wantActions: func() []coretesting.Action { + updateStatusAction := coretesting.NewUpdateAction(webhookAuthenticatorGVR, "", &auth1alpha1.WebhookAuthenticator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + }, + Spec: func() auth1alpha1.WebhookAuthenticatorSpec { + ipv6 := goodWebhookAuthenticatorSpecWithCA.DeepCopy() + ipv6.Endpoint = hostLocalIPv6Server.URL + ipv6.TLS = ptr.To(auth1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString(ipv6CA), + }) + return *ipv6 + }(), + Status: auth1alpha1.WebhookAuthenticatorStatus{ + Conditions: allHappyConditionsSuccess(hostLocalIPv6Server.URL, frozenMetav1Now, 0), + Phase: "Ready", + }, + }) + updateStatusAction.Subresource = "status" + return []coretesting.Action{ + coretesting.NewListAction(webhookAuthenticatorGVR, webhookAuthenticatorGVK, "", metav1.ListOptions{}), + coretesting.NewWatchAction(webhookAuthenticatorGVR, "", metav1.ListOptions{}), + updateStatusAction, + } + }, + wantCacheEntries: 1, + }, { name: "Sync: valid WebhookAuthenticator without CA: loop will fail to cache the authenticator, will write failed and unknown status conditions, and will enqueue resync", syncKey: controllerlib.Key{Name: "test-name"}, @@ -716,9 +753,6 @@ func TestController(t *testing.T) { { name: "validateEndpoint: should error if endpoint cannot be parsed", syncKey: controllerlib.Key{Name: "test-name"}, - tlsDialerFunc: func(network string, addr string, config *tls.Config) (*tls.Conn, error) { - return nil, errors.New("IPv6 test fake error") - }, webhooks: []runtime.Object{ &auth1alpha1.WebhookAuthenticator{ ObjectMeta: metav1.ObjectMeta{ @@ -859,14 +893,14 @@ func TestController(t *testing.T) { Name: "test-name", }, Spec: auth1alpha1.WebhookAuthenticatorSpec{ - Endpoint: fmt.Sprintf("%s:%s", "https://localhost", localhostURL.Port()), + Endpoint: fmt.Sprintf("https://localhost:%s", localhostURL.Port()), TLS: &auth1alpha1.TLSSpec{ // CA Bundle for validating the server's certs CertificateAuthorityData: base64.StdEncoding.EncodeToString(caForLocalhostAsHostname.Bundle()), }, }, Status: auth1alpha1.WebhookAuthenticatorStatus{ - Conditions: allHappyConditionsSuccess(fmt.Sprintf("%s:%s", "https://localhost", localhostURL.Port()), frozenMetav1Now, 0), + Conditions: allHappyConditionsSuccess(fmt.Sprintf("https://localhost:%s", localhostURL.Port()), frozenMetav1Now, 0), Phase: "Ready", }, }, @@ -877,7 +911,7 @@ func TestController(t *testing.T) { "timestamp": "2099-08-08T13:57:36.123456Z", "logger": "webhookcachefiller-controller", "message": "added new webhook authenticator", - "endpoint": fmt.Sprintf("%s:%s", "https://localhost", localhostURL.Port()), + "endpoint": fmt.Sprintf("https://localhost:%s", localhostURL.Port()), "webhook": map[string]interface{}{ "name": "test-name", }, @@ -894,14 +928,6 @@ func TestController(t *testing.T) { { name: "validateConnection: IPv6 address with port: should call dialer func with correct arguments", syncKey: controllerlib.Key{Name: "test-name"}, - tlsDialerFunc: func(network string, addr string, config *tls.Config) (*tls.Conn, error) { - assert.Equal(t, "tcp", network) - assert.Equal(t, "[0:0:0:0:0:0:0:1]:4242", addr) - assert.True(t, caForLocalhostAs127001.Pool().Equal(config.RootCAs)) - assert.Equal(t, uint16(tls.VersionTLS12), config.MinVersion) - - return nil, errors.New("IPv6 test fake error to skip real dial in prod code, this is actually success") - }, webhooks: []runtime.Object{ &auth1alpha1.WebhookAuthenticator{ ObjectMeta: metav1.ObjectMeta{ @@ -930,7 +956,7 @@ func TestController(t *testing.T) { Conditions: conditionstestutil.Replace( allHappyConditionsSuccess("https://[0:0:0:0:0:0:0:1]:4242/some/fake/path", frozenMetav1Now, 0), []metav1.Condition{ - sadWebhookConnectionValidWithMessage(frozenMetav1Now, 0, "cannot dial server: IPv6 test fake error to skip real dial in prod code, this is actually success"), + sadWebhookConnectionValidWithMessage(frozenMetav1Now, 0, "cannot dial server: dial tcp [::1]:4242: connect: connection refused"), sadReadyCondition(frozenMetav1Now, 0), unknownAuthenticatorValid(frozenMetav1Now, 0), }, @@ -945,20 +971,12 @@ func TestController(t *testing.T) { updateStatusAction, } }, - wantSyncLoopErr: testutil.WantExactErrorString(`cannot dial server: IPv6 test fake error to skip real dial in prod code, this is actually success`), + wantSyncLoopErr: testutil.WantExactErrorString(`cannot dial server: dial tcp [::1]:4242: connect: connection refused`), wantCacheEntries: 0, }, { name: "validateConnection: IPv6 address without port: should call dialer func with correct arguments", syncKey: controllerlib.Key{Name: "test-name"}, - tlsDialerFunc: func(network string, addr string, config *tls.Config) (*tls.Conn, error) { - assert.Equal(t, "tcp", network) - assert.Equal(t, "[0:0:0:0:0:0:0:1]:443", addr, "should add default port when port not provided") - assert.True(t, caForLocalhostAs127001.Pool().Equal(config.RootCAs)) - assert.Equal(t, uint16(tls.VersionTLS12), config.MinVersion) - - return nil, errors.New("IPv6 test fake error to skip real dial in prod code, this is actually success") - }, webhooks: []runtime.Object{ &auth1alpha1.WebhookAuthenticator{ ObjectMeta: metav1.ObjectMeta{ @@ -987,7 +1005,7 @@ func TestController(t *testing.T) { Conditions: conditionstestutil.Replace( allHappyConditionsSuccess("https://[0:0:0:0:0:0:0:1]/some/fake/path", frozenMetav1Now, 0), []metav1.Condition{ - sadWebhookConnectionValidWithMessage(frozenMetav1Now, 0, "cannot dial server: IPv6 test fake error to skip real dial in prod code, this is actually success"), + sadWebhookConnectionValidWithMessage(frozenMetav1Now, 0, "cannot dial server: dial tcp [::1]:443: connect: connection refused"), sadReadyCondition(frozenMetav1Now, 0), unknownAuthenticatorValid(frozenMetav1Now, 0), }, @@ -1002,7 +1020,7 @@ func TestController(t *testing.T) { updateStatusAction, } }, - wantSyncLoopErr: testutil.WantExactErrorString(`cannot dial server: IPv6 test fake error to skip real dial in prod code, this is actually success`), + wantSyncLoopErr: testutil.WantExactErrorString(`cannot dial server: dial tcp [::1]:443: connect: connection refused`), wantCacheEntries: 0, }, { @@ -1091,14 +1109,6 @@ func TestController(t *testing.T) { { name: "validateConnection: IPv6 address without port or brackets: should succeed since IPv6 brackets are optional without port", syncKey: controllerlib.Key{Name: "test-name"}, - tlsDialerFunc: func(network string, addr string, config *tls.Config) (*tls.Conn, error) { - assert.Equal(t, "tcp", network) - assert.Equal(t, "[0:0:0:0:0:0:0:1]:443", addr) - assert.True(t, caForLocalhostAs127001.Pool().Equal(config.RootCAs)) - assert.Equal(t, uint16(tls.VersionTLS12), config.MinVersion) - - return nil, errors.New("IPv6 test fake error to skip real dial in prod code, this is actually success") - }, webhooks: []runtime.Object{ &auth1alpha1.WebhookAuthenticator{ ObjectMeta: metav1.ObjectMeta{ @@ -1127,7 +1137,7 @@ func TestController(t *testing.T) { Conditions: conditionstestutil.Replace( allHappyConditionsSuccess("https://0:0:0:0:0:0:0:1/some/fake/path", frozenMetav1Now, 0), []metav1.Condition{ - sadWebhookConnectionValidWithMessage(frozenMetav1Now, 0, "cannot dial server: IPv6 test fake error to skip real dial in prod code, this is actually success"), + sadWebhookConnectionValidWithMessage(frozenMetav1Now, 0, "cannot dial server: dial tcp [::1]:443: connect: connection refused"), sadReadyCondition(frozenMetav1Now, 0), unknownAuthenticatorValid(frozenMetav1Now, 0), }, @@ -1142,7 +1152,7 @@ func TestController(t *testing.T) { updateStatusAction, } }, - wantSyncLoopErr: testutil.WantExactErrorString(`cannot dial server: IPv6 test fake error to skip real dial in prod code, this is actually success`), + wantSyncLoopErr: testutil.WantExactErrorString(`cannot dial server: dial tcp [::1]:443: connect: connection refused`), wantCacheEntries: 0, }, { @@ -1310,16 +1320,12 @@ func TestController(t *testing.T) { var log bytes.Buffer logger := plog.TestLogger(t, &log) - if tt.tlsDialerFunc == nil { - tt.tlsDialerFunc = tls.Dial - } controller := New( cache, pinnipedAPIClient, informers.Authentication().V1alpha1().WebhookAuthenticators(), frozenClock, - logger, - tt.tlsDialerFunc) + logger) ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -1356,52 +1362,61 @@ func TestController(t *testing.T) { } } - if tt.wantActions != nil { - if !assert.ElementsMatch(t, tt.wantActions(), pinnipedAPIClient.Actions()) { - // cmp.Diff is superior to require.ElementsMatch in terms of readability here. - // require.ElementsMatch will handle pointers better than require.Equal, but - // the timestamps are still incredibly verbose. - require.Fail(t, cmp.Diff(tt.wantActions(), pinnipedAPIClient.Actions()), "actions should be exactly the expected number of actions and also contain the correct resources") - } - } else { - require.Fail(t, "wantActions is required for test "+tt.name) - } - + require.NotEmpty(t, tt.wantActions, "wantActions is required for test %s", tt.name) + require.Equal(t, tt.wantActions(), pinnipedAPIClient.Actions()) require.Equal(t, tt.wantCacheEntries, len(cache.Keys()), fmt.Sprintf("expected cache entries is incorrect. wanted:%d, got: %d, keys: %v", tt.wantCacheEntries, len(cache.Keys()), cache.Keys())) }) } } func TestNewWebhookAuthenticator(t *testing.T) { - goodEndpoint := "https://example.com" + server, serverCA := tlsserver.TestServerIPv4(t, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Webhook clients should always use ptls.Default when making requests to the webhook. Assert that here. + tlsserver.AssertTLS(t, r, ptls.Default) - testServerCABundle, testServerURL := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) { - body, err := io.ReadAll(r.Body) - require.NoError(t, err) - require.Contains(t, string(body), "test-token") - _, err = w.Write([]byte(`{}`)) - require.NoError(t, err) - }) + // Loosely assert on the request body. + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + require.Contains(t, string(body), "test-token") + + // Write a realistic looking fake response for a successfully authenticated user, so we can tell that + // this endpoint was actually called by the test below where it asserts on the fake user and group names. + w.Header().Add("Content-Type", "application/json") + responseBody := authenticationv1beta1.TokenReview{ + TypeMeta: metav1.TypeMeta{ + Kind: "TokenReview", + APIVersion: authenticationv1beta1.SchemeGroupVersion.String(), + }, + Status: authenticationv1beta1.TokenReviewStatus{ + Authenticated: true, + User: authenticationv1beta1.UserInfo{ + Username: "fake-username-from-server", + Groups: []string{"fake-group-from-server-1", "fake-group-from-server-2"}, + }, + }, + } + err = json.NewEncoder(w).Encode(responseBody) + require.NoError(t, err) + }), + tlsserver.RecordTLSHello, + ) tests := []struct { - name string - endpoint string - pemBytes []byte - tempFileFunc func(dir string, pattern string) (*os.File, error) - marshallFunc func(config clientcmdapi.Config, filename string) error - prereqOk bool - wantConditions []*metav1.Condition - wantWebhook bool - wantErr string - testCreatedWebhookWithFakeToken bool + name string + endpoint string + pemBytes []byte + prereqOk bool + wantConditions []*metav1.Condition + wantErr string + wantWebhook bool // When true, we want a webhook client to have been successfully created. + callWebhook bool // When true, really call the webhook endpoint using the created webhook client. }{ { - name: "prerequisites not ready, cannot create webhook authenticator", - endpoint: "", - pemBytes: []byte("irrelevant pem bytes"), - tempFileFunc: os.CreateTemp, - marshallFunc: clientcmd.WriteToFile, - wantErr: "", + name: "prerequisites not ready, cannot create webhook authenticator", + endpoint: "", + pemBytes: []byte("irrelevant pem bytes"), + wantErr: "", wantConditions: []*metav1.Condition{{ Type: "AuthenticatorValid", Status: "Unknown", @@ -1410,58 +1425,22 @@ func TestNewWebhookAuthenticator(t *testing.T) { }}, prereqOk: false, }, { - name: "temp file failure, cannot create webhook authenticator", - endpoint: "", - pemBytes: []byte("irrelevant pem bytes"), - tempFileFunc: func(_ string, _ string) (*os.File, error) { - return nil, fmt.Errorf("some temp file error") - }, - marshallFunc: clientcmd.WriteToFile, - prereqOk: true, - wantConditions: []*metav1.Condition{{ - Type: "AuthenticatorValid", - Status: "False", - Reason: "UnableToCreateTempFile", - Message: "unable to create temporary file: some temp file error", - }}, - wantErr: "unable to create temporary file: some temp file error", - }, { - name: "marshal failure, cannot create webhook authenticator", - endpoint: "", - pemBytes: []byte("irrelevant pem bytes"), - tempFileFunc: os.CreateTemp, - marshallFunc: func(_ clientcmdapi.Config, _ string) error { - return fmt.Errorf("some marshal error") - }, + name: "invalid pem data, unable to parse bytes as PEM block", + endpoint: "https://does-not-matter-will-not-be-used", + pemBytes: []byte("invalid-bas64"), prereqOk: true, wantConditions: []*metav1.Condition{{ Type: "AuthenticatorValid", Status: "False", - Reason: "UnableToMarshallKubeconfig", - Message: "unable to marshal kubeconfig: some marshal error", + Reason: "UnableToCreateClient", + Message: "unable to create client for this webhook: could not create secure client config: unable to load root certificates: unable to parse bytes as PEM block", }}, - wantErr: "unable to marshal kubeconfig: some marshal error", + wantErr: "unable to create client for this webhook: could not create secure client config: unable to load root certificates: unable to parse bytes as PEM block", }, { - name: "invalid pem data, unable to parse bytes as PEM block", - endpoint: goodEndpoint, - pemBytes: []byte("invalid-bas64"), - tempFileFunc: os.CreateTemp, - marshallFunc: clientcmd.WriteToFile, - prereqOk: true, - wantConditions: []*metav1.Condition{{ - Type: "AuthenticatorValid", - Status: "False", - Reason: "UnableToInstantiateWebhook", - Message: "unable to instantiate webhook: unable to load root certificates: unable to parse bytes as PEM block", - }}, - wantErr: "unable to instantiate webhook: unable to load root certificates: unable to parse bytes as PEM block", - }, { - name: "valid config with no TLS spec, webhook authenticator created", - endpoint: goodEndpoint, - pemBytes: nil, - tempFileFunc: os.CreateTemp, - marshallFunc: clientcmd.WriteToFile, - prereqOk: true, + name: "valid config with no PEM bytes, webhook authenticator created", + endpoint: "https://does-not-matter-will-not-be-used", + pemBytes: nil, + prereqOk: true, wantConditions: []*metav1.Condition{{ Type: "AuthenticatorValid", Status: "True", @@ -1470,19 +1449,18 @@ func TestNewWebhookAuthenticator(t *testing.T) { }}, wantWebhook: true, }, { - name: "success, webhook authenticator created", - endpoint: testServerURL, - pemBytes: []byte(testServerCABundle), - tempFileFunc: os.CreateTemp, - marshallFunc: clientcmd.WriteToFile, - prereqOk: true, + name: "valid config, webhook authenticator created, and test calling webhook server", + endpoint: server.URL, + pemBytes: serverCA, + prereqOk: true, wantConditions: []*metav1.Condition{{ Type: "AuthenticatorValid", Status: "True", Reason: "Success", Message: "authenticator initialized", }}, - testCreatedWebhookWithFakeToken: true, + wantWebhook: true, + callWebhook: true, }, } @@ -1491,12 +1469,14 @@ func TestNewWebhookAuthenticator(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() var conditions []*metav1.Condition - webhook, conditions, err := newWebhookAuthenticator(tt.endpoint, tt.pemBytes, tt.tempFileFunc, tt.marshallFunc, conditions, tt.prereqOk) + webhook, conditions, err := newWebhookAuthenticator(tt.endpoint, tt.pemBytes, conditions, tt.prereqOk) require.Equal(t, tt.wantConditions, conditions) if tt.wantWebhook { require.NotNil(t, webhook) + } else { + require.Nil(t, webhook) } if tt.wantErr != "" { @@ -1505,11 +1485,12 @@ func TestNewWebhookAuthenticator(t *testing.T) { require.NoError(t, err) } - if tt.testCreatedWebhookWithFakeToken { + if tt.callWebhook { authResp, isAuthenticated, err := webhook.AuthenticateToken(context.Background(), "test-token") require.NoError(t, err) - require.Nil(t, authResp) - require.False(t, isAuthenticated) + require.True(t, isAuthenticated) + require.Equal(t, "fake-username-from-server", authResp.User.GetName()) + require.Equal(t, []string{"fake-group-from-server-1", "fake-group-from-server-2"}, authResp.User.GetGroups()) } }) } diff --git a/internal/controller/kubecertagent/pod_command_executor_test.go b/internal/controller/kubecertagent/pod_command_executor_test.go index 662b2a011..938fa16b4 100644 --- a/internal/controller/kubecertagent/pod_command_executor_test.go +++ b/internal/controller/kubecertagent/pod_command_executor_test.go @@ -1,4 +1,4 @@ -// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package kubecertagent @@ -19,7 +19,7 @@ import ( func TestSecureTLS(t *testing.T) { var sawRequest bool - server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + server, serverCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { tlsserver.AssertTLS(t, r, ptls.Secure) sawRequest = true }), tlsserver.RecordTLSHello) @@ -27,7 +27,7 @@ func TestSecureTLS(t *testing.T) { config := &rest.Config{ Host: server.URL, TLSClientConfig: rest.TLSClientConfig{ - CAData: tlsserver.TLSTestServerCA(server), + CAData: serverCA, }, } diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go index c6a63698c..6837fa62a 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package oidcupstreamwatcher @@ -35,6 +35,7 @@ import ( "go.pinniped.dev/internal/testutil/oidctestutil" "go.pinniped.dev/internal/testutil/testlogger" "go.pinniped.dev/internal/testutil/tlsassertions" + "go.pinniped.dev/internal/testutil/tlsserver" "go.pinniped.dev/internal/upstreamoidc" ) @@ -113,7 +114,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { earlier := metav1.NewTime(now.Add(-1 * time.Hour).UTC()) // Start another test server that answers discovery successfully. - testIssuerCA, testIssuerURL := newTestIssuer(t) + testIssuerURL, testIssuerCA := newTestIssuer(t) testIssuerCABase64 := base64.StdEncoding.EncodeToString([]byte(testIssuerCA)) testIssuerAuthorizeURL, err := url.Parse("https://example.com/authorize") require.NoError(t, err) @@ -1529,7 +1530,7 @@ func normalizeOIDCUpstreams(upstreams []v1alpha1.OIDCIdentityProvider, now metav func newTestIssuer(t *testing.T) (string, string) { mux := http.NewServeMux() - caBundlePEM, testURL := testutil.TLSTestServer(t, mux.ServeHTTP) + server, serverCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(mux.ServeHTTP), nil) type providerJSON struct { Issuer string `json:"issuer"` @@ -1543,7 +1544,7 @@ func newTestIssuer(t *testing.T) (string, string) { mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&providerJSON{ - Issuer: testURL, + Issuer: server.URL, AuthURL: "https://example.com/authorize", RevocationURL: "https://example.com/revoke", TokenURL: "https://example.com/token", @@ -1554,7 +1555,7 @@ func newTestIssuer(t *testing.T) (string, string) { mux.HandleFunc("/valid-without-revocation/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&providerJSON{ - Issuer: testURL + "/valid-without-revocation", + Issuer: server.URL + "/valid-without-revocation", AuthURL: "https://example.com/authorize", RevocationURL: "", // none TokenURL: "https://example.com/token", @@ -1565,7 +1566,7 @@ func newTestIssuer(t *testing.T) (string, string) { mux.HandleFunc("/invalid/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&providerJSON{ - Issuer: testURL + "/invalid", + Issuer: server.URL + "/invalid", AuthURL: "%", TokenURL: "https://example.com/token", }) @@ -1575,7 +1576,7 @@ func newTestIssuer(t *testing.T) (string, string) { mux.HandleFunc("/invalid-revocation-url/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&providerJSON{ - Issuer: testURL + "/invalid-revocation-url", + Issuer: server.URL + "/invalid-revocation-url", AuthURL: "https://example.com/authorize", RevocationURL: "%", TokenURL: "https://example.com/token", @@ -1586,7 +1587,7 @@ func newTestIssuer(t *testing.T) (string, string) { mux.HandleFunc("/insecure/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&providerJSON{ - Issuer: testURL + "/insecure", + Issuer: server.URL + "/insecure", AuthURL: "http://example.com/authorize", TokenURL: "https://example.com/token", }) @@ -1596,7 +1597,7 @@ func newTestIssuer(t *testing.T) (string, string) { mux.HandleFunc("/insecure-revocation-url/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&providerJSON{ - Issuer: testURL + "/insecure-revocation-url", + Issuer: server.URL + "/insecure-revocation-url", AuthURL: "https://example.com/authorize", RevocationURL: "http://example.com/revoke", TokenURL: "https://example.com/token", @@ -1607,7 +1608,7 @@ func newTestIssuer(t *testing.T) (string, string) { mux.HandleFunc("/insecure-token-url/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&providerJSON{ - Issuer: testURL + "/insecure-token-url", + Issuer: server.URL + "/insecure-token-url", AuthURL: "https://example.com/authorize", RevocationURL: "https://example.com/revoke", TokenURL: "http://example.com/token", @@ -1619,7 +1620,7 @@ func newTestIssuer(t *testing.T) (string, string) { mux.HandleFunc("/missing-token-url/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&providerJSON{ - Issuer: testURL + "/missing-token-url", + Issuer: server.URL + "/missing-token-url", AuthURL: "https://example.com/authorize", RevocationURL: "https://example.com/revoke", }) @@ -1629,7 +1630,7 @@ func newTestIssuer(t *testing.T) (string, string) { mux.HandleFunc("/missing-auth-url/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&providerJSON{ - Issuer: testURL + "/missing-auth-url", + Issuer: server.URL + "/missing-auth-url", RevocationURL: "https://example.com/revoke", TokenURL: "https://example.com/token", }) @@ -1638,13 +1639,13 @@ func newTestIssuer(t *testing.T) (string, string) { // handle the four issuer with trailing slash configs // valid case in= out= - // handled above at the root of testURL + // handled above at the root of server.URL // valid case in=/ out=/ mux.HandleFunc("/ends-with-slash/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&providerJSON{ - Issuer: testURL + "/ends-with-slash/", + Issuer: server.URL + "/ends-with-slash/", AuthURL: "https://example.com/authorize", RevocationURL: "https://example.com/revoke", TokenURL: "https://example.com/token", @@ -1657,5 +1658,5 @@ func newTestIssuer(t *testing.T) (string, string) { // invalid case in=/ out= // can be tested using root endpoint - return caBundlePEM, testURL + return server.URL, string(serverCA) } diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index 3a65c7caf..379db68a4 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -6,7 +6,6 @@ package controllermanager import ( - "crypto/tls" "fmt" "time" @@ -241,7 +240,6 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol informers.pinniped.Authentication().V1alpha1().WebhookAuthenticators(), clock.RealClock{}, plog.New(), - tls.Dial, ), singletonWorker, ). diff --git a/internal/kubeclient/kubeclient.go b/internal/kubeclient/kubeclient.go index 1f5c43220..e1ba1681f 100644 --- a/internal/kubeclient/kubeclient.go +++ b/internal/kubeclient/kubeclient.go @@ -1,4 +1,4 @@ -// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package kubeclient @@ -36,7 +36,9 @@ type Client struct { } func New(opts ...Option) (*Client, error) { - c := &clientConfig{} + c := &clientConfig{ + tlsConfigFunc: ptls.Secure, // Set a default value. Can be overridden by an Option. + } for _, opt := range opts { opt(c) @@ -51,16 +53,16 @@ func New(opts ...Option) (*Client, error) { WithConfig(inClusterConfig)(c) // make sure all writes to clientConfig flow through one code path } - secureKubeConfig, err := createSecureKubeConfig(c.config) + ptlsKubeConfig, err := createPTLSKubeConfig(c.config, c.tlsConfigFunc) if err != nil { return nil, fmt.Errorf("could not create secure client config: %w", err) } // explicitly use json when talking to CRD APIs - jsonKubeConfig := createJSONKubeConfig(secureKubeConfig) + jsonKubeConfig := createJSONKubeConfig(ptlsKubeConfig) // explicitly use protobuf when talking to built-in kube APIs - protoKubeConfig := createProtoKubeConfig(secureKubeConfig) + protoKubeConfig := createProtoKubeConfig(ptlsKubeConfig) // Connect to the core Kubernetes API. k8sClient, err := kubernetes.NewForConfig(configWithWrapper(protoKubeConfig, kubescheme.Scheme, kubescheme.Codecs, c.middlewares, c.transportWrapper)) @@ -120,25 +122,25 @@ func createProtoKubeConfig(kubeConfig *restclient.Config) *restclient.Config { return protoKubeConfig } -// createSecureKubeConfig returns a copy of the input config with the WrapTransport +// createPTLSKubeConfig returns a copy of the input config with the WrapTransport // enhanced to use the secure TLS configuration of the ptls / phttp packages. -func createSecureKubeConfig(kubeConfig *restclient.Config) (*restclient.Config, error) { - secureKubeConfig := restclient.CopyConfig(kubeConfig) +func createPTLSKubeConfig(kubeConfig *restclient.Config, tlsConfigFunc ptls.ConfigFunc) (*restclient.Config, error) { + ptlsKubeConfig := restclient.CopyConfig(kubeConfig) // by setting proxy to always be non-nil, we bust the client-go global TLS config cache. // this is required to make our wrapper function work without data races. the unit tests // associated with this code run in parallel to assert that we are not using the cache. // see k8s.io/client-go/transport.tlsConfigKey - if secureKubeConfig.Proxy == nil { - secureKubeConfig.Proxy = net.NewProxierWithNoProxyCIDR(http.ProxyFromEnvironment) + if ptlsKubeConfig.Proxy == nil { + ptlsKubeConfig.Proxy = net.NewProxierWithNoProxyCIDR(http.ProxyFromEnvironment) } // make sure restclient.TLSConfigFor always returns a non-nil TLS config - if len(secureKubeConfig.NextProtos) == 0 { - secureKubeConfig.NextProtos = ptls.Secure(nil).NextProtos + if len(ptlsKubeConfig.NextProtos) == 0 { + ptlsKubeConfig.NextProtos = tlsConfigFunc(nil).NextProtos } - tlsConfigTest, err := restclient.TLSConfigFor(secureKubeConfig) + tlsConfigTest, err := restclient.TLSConfigFor(ptlsKubeConfig) if err != nil { return nil, err // should never happen because our input config should always be valid } @@ -146,10 +148,10 @@ func createSecureKubeConfig(kubeConfig *restclient.Config) (*restclient.Config, return nil, fmt.Errorf("unexpected empty TLS config") // should never happen because we set NextProtos above } - secureKubeConfig.Wrap(func(rt http.RoundTripper) http.RoundTripper { + ptlsKubeConfig.Wrap(func(rt http.RoundTripper) http.RoundTripper { defer func() { - if err := AssertSecureTransport(rt); err != nil { - panic(err) // not sure what the point of this function would be if it failed to make the config secure + if err := assertTransport(rt, tlsConfigFunc); err != nil { + panic(err) // not sure what the point of this function would be if it failed to make the config use the ptls settings } }() @@ -167,23 +169,23 @@ func createSecureKubeConfig(kubeConfig *restclient.Config) (*restclient.Config, } // mutate the TLS config into our desired state before it is used - ptls.Merge(ptls.Secure, tlsConfig) + ptls.Merge(tlsConfigFunc, tlsConfig) return rt // return the input transport since we mutated it in-place }) - if err := AssertSecureConfig(secureKubeConfig); err != nil { + if err := assertConfig(ptlsKubeConfig, tlsConfigFunc); err != nil { return nil, err // not sure what the point of this function would be if it failed to make the config secure } - return secureKubeConfig, nil + return ptlsKubeConfig, nil } // SecureAnonymousClientConfig has the same properties as restclient.AnonymousClientConfig // while still enforcing the secure TLS configuration of the ptls / phttp packages. func SecureAnonymousClientConfig(kubeConfig *restclient.Config) *restclient.Config { kubeConfig = restclient.AnonymousClientConfig(kubeConfig) - secureKubeConfig, err := createSecureKubeConfig(kubeConfig) + secureKubeConfig, err := createPTLSKubeConfig(kubeConfig, ptls.Secure) if err != nil { panic(err) // should never happen as this would only fail on invalid CA data, which would never work anyway } @@ -194,22 +196,30 @@ func SecureAnonymousClientConfig(kubeConfig *restclient.Config) *restclient.Conf } func AssertSecureConfig(kubeConfig *restclient.Config) error { + return assertConfig(kubeConfig, ptls.Secure) +} + +func assertConfig(kubeConfig *restclient.Config, tlsConfigFunc ptls.ConfigFunc) error { rt, err := restclient.TransportFor(kubeConfig) if err != nil { return fmt.Errorf("failed to build transport: %w", err) } - return AssertSecureTransport(rt) + return assertTransport(rt, tlsConfigFunc) } func AssertSecureTransport(rt http.RoundTripper) error { + return assertTransport(rt, ptls.Secure) +} + +func assertTransport(rt http.RoundTripper, tlsConfigFunc ptls.ConfigFunc) error { tlsConfig, err := net.TLSClientConfig(rt) if err != nil { return fmt.Errorf("failed to get TLS config: %w", err) } tlsConfigCopy := tlsConfig.Clone() - ptls.Merge(ptls.Secure, tlsConfigCopy) // only mutate the copy + ptls.Merge(tlsConfigFunc, tlsConfigCopy) // only mutate the copy //nolint:gosec // the empty TLS config here is not used if diff := cmp.Diff(tlsConfigCopy, tlsConfig, diff --git a/internal/kubeclient/kubeclient_test.go b/internal/kubeclient/kubeclient_test.go index f880118bf..a4e95313f 100644 --- a/internal/kubeclient/kubeclient_test.go +++ b/internal/kubeclient/kubeclient_test.go @@ -1,4 +1,4 @@ -// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package kubeclient @@ -957,7 +957,7 @@ func TestUnwrap(t *testing.T) { regularClient := makeClient(t, restConfig, func(_ *rest.Config) {}) - testUnwrap(t, regularClient, serverSubjects) + testUnwrap(t, regularClient, serverSubjects, ptls.Secure) }) t.Run("exec client", func(t *testing.T) { @@ -972,7 +972,7 @@ func TestUnwrap(t *testing.T) { } }) - testUnwrap(t, execClient, serverSubjects) + testUnwrap(t, execClient, serverSubjects, ptls.Secure) }) t.Run("oidc client", func(t *testing.T) { @@ -988,90 +988,149 @@ func TestUnwrap(t *testing.T) { } }) - testUnwrap(t, oidcClient, serverSubjects) + testUnwrap(t, oidcClient, serverSubjects, ptls.Secure) + }) + + t.Run("regular client with ptls.Default", func(t *testing.T) { + t.Parallel() // make sure to run in parallel to confirm that our client-go TLS cache busting works (i.e. assert no data races) + + regularClient := makeClient(t, restConfig, func(_ *rest.Config) {}, WithTLSConfigFunc(ptls.Default)) + + testUnwrap(t, regularClient, serverSubjects, ptls.Default) + }) + + t.Run("exec client with ptls.Default", func(t *testing.T) { + t.Parallel() // make sure to run in parallel to confirm that our client-go TLS cache busting works (i.e. assert no data races) + + execClient := makeClient(t, restConfig, func(config *rest.Config) { + config.ExecProvider = &clientcmdapi.ExecConfig{ + Command: "echo", + Args: []string{"pandas are awesome"}, + APIVersion: clientauthenticationv1.SchemeGroupVersion.String(), + InteractiveMode: clientcmdapi.NeverExecInteractiveMode, + } + }, WithTLSConfigFunc(ptls.Default)) + + testUnwrap(t, execClient, serverSubjects, ptls.Default) + }) + + t.Run("oidc client with ptls.Default", func(t *testing.T) { + t.Parallel() // make sure to run in parallel to confirm that our client-go TLS cache busting works (i.e. assert no data races) + + oidcClient := makeClient(t, restConfig, func(config *rest.Config) { + config.AuthProvider = &clientcmdapi.AuthProviderConfig{ + Name: "oidc", + Config: map[string]string{ + "idp-issuer-url": "https://pandas.local", + "client-id": "walrus", + }, + } + }, WithTLSConfigFunc(ptls.Default)) + + testUnwrap(t, oidcClient, serverSubjects, ptls.Default) }) } -func testUnwrap(t *testing.T, client *Client, serverSubjects [][]byte) { +func testUnwrap(t *testing.T, client *Client, serverSubjects [][]byte, tlsConfigFuncForExpectedValues ptls.ConfigFunc) { tests := []struct { - name string - rt http.RoundTripper + name string + rt http.RoundTripper + wantConfigFunc ptls.ConfigFunc }{ { - name: "core v1", - rt: extractTransport(client.Kubernetes.CoreV1()), + name: "core v1", + rt: extractTransport(client.Kubernetes.CoreV1()), + wantConfigFunc: tlsConfigFuncForExpectedValues, }, { - name: "coordination v1", - rt: extractTransport(client.Kubernetes.CoordinationV1()), + name: "coordination v1", + rt: extractTransport(client.Kubernetes.CoordinationV1()), + wantConfigFunc: tlsConfigFuncForExpectedValues, }, { - name: "api registration v1", - rt: extractTransport(client.Aggregation.ApiregistrationV1()), + name: "api registration v1", + rt: extractTransport(client.Aggregation.ApiregistrationV1()), + wantConfigFunc: tlsConfigFuncForExpectedValues, }, { - name: "concierge login", - rt: extractTransport(client.PinnipedConcierge.LoginV1alpha1()), + name: "concierge login", + rt: extractTransport(client.PinnipedConcierge.LoginV1alpha1()), + wantConfigFunc: tlsConfigFuncForExpectedValues, }, { - name: "concierge config", - rt: extractTransport(client.PinnipedConcierge.ConfigV1alpha1()), + name: "concierge config", + rt: extractTransport(client.PinnipedConcierge.ConfigV1alpha1()), + wantConfigFunc: tlsConfigFuncForExpectedValues, }, { - name: "supervisor idp", - rt: extractTransport(client.PinnipedSupervisor.IDPV1alpha1()), + name: "supervisor idp", + rt: extractTransport(client.PinnipedSupervisor.IDPV1alpha1()), + wantConfigFunc: tlsConfigFuncForExpectedValues, }, { - name: "supervisor config", - rt: extractTransport(client.PinnipedSupervisor.ConfigV1alpha1()), + name: "supervisor config", + rt: extractTransport(client.PinnipedSupervisor.ConfigV1alpha1()), + wantConfigFunc: tlsConfigFuncForExpectedValues, }, { - name: "json config", - rt: configToTransport(t, client.JSONConfig), + name: "json config", + rt: configToTransport(t, client.JSONConfig), + wantConfigFunc: tlsConfigFuncForExpectedValues, }, { - name: "proto config", - rt: configToTransport(t, client.ProtoConfig), + name: "proto config", + rt: configToTransport(t, client.ProtoConfig), + wantConfigFunc: tlsConfigFuncForExpectedValues, }, { - name: "anonymous json config", - rt: configToTransport(t, SecureAnonymousClientConfig(client.JSONConfig)), + name: "anonymous json config", + rt: configToTransport(t, SecureAnonymousClientConfig(client.JSONConfig)), + wantConfigFunc: ptls.Secure, // SecureAnonymousClientConfig is always ptls.Secure }, { - name: "anonymous proto config", - rt: configToTransport(t, SecureAnonymousClientConfig(client.ProtoConfig)), + name: "anonymous proto config", + rt: configToTransport(t, SecureAnonymousClientConfig(client.ProtoConfig)), + wantConfigFunc: ptls.Secure, // SecureAnonymousClientConfig is always ptls.Secure }, { - name: "json config - no cache", - rt: configToTransport(t, bustTLSCache(client.JSONConfig)), + name: "json config - no cache", + rt: configToTransport(t, bustTLSCache(client.JSONConfig)), + wantConfigFunc: tlsConfigFuncForExpectedValues, }, { - name: "proto config - no cache", - rt: configToTransport(t, bustTLSCache(client.ProtoConfig)), + name: "proto config - no cache", + rt: configToTransport(t, bustTLSCache(client.ProtoConfig)), + wantConfigFunc: tlsConfigFuncForExpectedValues, }, { - name: "anonymous json config - no cache, inner bust", - rt: configToTransport(t, SecureAnonymousClientConfig(bustTLSCache(client.JSONConfig))), + name: "anonymous json config - no cache, inner bust", + rt: configToTransport(t, SecureAnonymousClientConfig(bustTLSCache(client.JSONConfig))), + wantConfigFunc: ptls.Secure, // SecureAnonymousClientConfig is always ptls.Secure }, { - name: "anonymous proto config - no cache, inner bust", - rt: configToTransport(t, SecureAnonymousClientConfig(bustTLSCache(client.ProtoConfig))), + name: "anonymous proto config - no cache, inner bust", + rt: configToTransport(t, SecureAnonymousClientConfig(bustTLSCache(client.ProtoConfig))), + wantConfigFunc: ptls.Secure, // SecureAnonymousClientConfig is always ptls.Secure }, { - name: "anonymous json config - no cache, double bust", - rt: configToTransport(t, bustTLSCache(SecureAnonymousClientConfig(bustTLSCache(client.JSONConfig)))), + name: "anonymous json config - no cache, double bust", + rt: configToTransport(t, bustTLSCache(SecureAnonymousClientConfig(bustTLSCache(client.JSONConfig)))), + wantConfigFunc: ptls.Secure, // SecureAnonymousClientConfig is always ptls.Secure }, { - name: "anonymous proto config - no cache, double bust", - rt: configToTransport(t, bustTLSCache(SecureAnonymousClientConfig(bustTLSCache(client.ProtoConfig)))), + name: "anonymous proto config - no cache, double bust", + rt: configToTransport(t, bustTLSCache(SecureAnonymousClientConfig(bustTLSCache(client.ProtoConfig)))), + wantConfigFunc: ptls.Secure, // SecureAnonymousClientConfig is always ptls.Secure }, { - name: "anonymous json config - no cache, outer bust", - rt: configToTransport(t, bustTLSCache(SecureAnonymousClientConfig(client.JSONConfig))), + name: "anonymous json config - no cache, outer bust", + rt: configToTransport(t, bustTLSCache(SecureAnonymousClientConfig(client.JSONConfig))), + wantConfigFunc: ptls.Secure, // SecureAnonymousClientConfig is always ptls.Secure }, { - name: "anonymous proto config - no cache, outer bust", - rt: configToTransport(t, bustTLSCache(SecureAnonymousClientConfig(client.ProtoConfig))), + name: "anonymous proto config - no cache, outer bust", + rt: configToTransport(t, bustTLSCache(SecureAnonymousClientConfig(client.ProtoConfig))), + wantConfigFunc: ptls.Secure, // SecureAnonymousClientConfig is always ptls.Secure }, } for _, tt := range tests { @@ -1083,11 +1142,11 @@ func testUnwrap(t *testing.T, client *Client, serverSubjects [][]byte) { require.NoError(t, err) require.NotNil(t, tlsConfig) - secureTLSConfig := ptls.Secure(nil) + ptlsConfig := tt.wantConfigFunc(nil) - require.Equal(t, secureTLSConfig.MinVersion, tlsConfig.MinVersion) - require.Equal(t, secureTLSConfig.CipherSuites, tlsConfig.CipherSuites) - require.Equal(t, secureTLSConfig.NextProtos, tlsConfig.NextProtos) + require.Equal(t, ptlsConfig.MinVersion, tlsConfig.MinVersion) + require.Equal(t, ptlsConfig.CipherSuites, tlsConfig.CipherSuites) + require.Equal(t, ptlsConfig.NextProtos, tlsConfig.NextProtos) // x509.CertPool has some embedded functions that make it hard to compare so just look at the subjects //nolint:staticcheck // since we're not using .Subjects() to access the system pool @@ -1120,14 +1179,14 @@ func bustTLSCache(config *rest.Config) *rest.Config { return c } -func makeClient(t *testing.T, restConfig *rest.Config, f func(*rest.Config)) *Client { +func makeClient(t *testing.T, restConfig *rest.Config, f func(*rest.Config), opts ...Option) *Client { t.Helper() restConfig = rest.CopyConfig(restConfig) f(restConfig) - client, err := New(WithConfig(restConfig)) + client, err := New(append([]Option{WithConfig(restConfig)}, opts...)...) require.NoError(t, err) return client diff --git a/internal/kubeclient/option.go b/internal/kubeclient/option.go index 7c10bda98..0099ebd3c 100644 --- a/internal/kubeclient/option.go +++ b/internal/kubeclient/option.go @@ -1,4 +1,4 @@ -// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package kubeclient @@ -6,12 +6,15 @@ package kubeclient import ( restclient "k8s.io/client-go/rest" "k8s.io/client-go/transport" + + "go.pinniped.dev/internal/crypto/ptls" ) type Option func(*clientConfig) type clientConfig struct { config *restclient.Config + tlsConfigFunc ptls.ConfigFunc middlewares []Middleware transportWrapper transport.WrapperFunc } @@ -22,6 +25,15 @@ func WithConfig(config *restclient.Config) Option { } } +// WithTLSConfigFunc will cause the client to use the provided configuration from the ptls package when the +// client makes requests. For example, pass ptls.Default or ptls.Secure as the argument. When this Option +// is not used, the client will default to using ptls.Secure. +func WithTLSConfigFunc(tlsConfigFunc ptls.ConfigFunc) Option { + return func(c *clientConfig) { + c.tlsConfigFunc = tlsConfigFunc + } +} + func WithMiddleware(middleware Middleware) Option { return func(c *clientConfig) { if middleware == nil { diff --git a/internal/net/phttp/phttp_test.go b/internal/net/phttp/phttp_test.go index 83792df1a..3a37983cf 100644 --- a/internal/net/phttp/phttp_test.go +++ b/internal/net/phttp/phttp_test.go @@ -1,4 +1,4 @@ -// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package phttp @@ -83,13 +83,13 @@ func TestClient(t *testing.T) { t.Parallel() var sawRequest bool - server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + server, serverCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { tlsserver.AssertTLS(t, r, tt.configFunc) assertUserAgent(t, r) sawRequest = true }), tlsserver.RecordTLSHello) - rootCAs, err := cert.NewPoolFromBytes(tlsserver.TLSTestServerCA(server)) + rootCAs, err := cert.NewPoolFromBytes(serverCA) require.NoError(t, err) c := tt.clientFunc(rootCAs) diff --git a/internal/testutil/fakekubeapi/fakekubeapi.go b/internal/testutil/fakekubeapi/fakekubeapi.go index 7ac26ccc1..0fb176b34 100644 --- a/internal/testutil/fakekubeapi/fakekubeapi.go +++ b/internal/testutil/fakekubeapi/fakekubeapi.go @@ -1,4 +1,4 @@ -// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 /* @@ -58,7 +58,7 @@ func Start(t *testing.T, resources map[string]runtime.Object) (*httptest.Server, resources = make(map[string]runtime.Object) } - server := tlsserver.TLSTestServer(t, httperr.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { + server, serverCA := tlsserver.TestServerIPv4(t, httperr.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { tlsserver.AssertTLS(t, r, ptls.Secure) obj, err := decodeObj(r) @@ -84,7 +84,7 @@ func Start(t *testing.T, resources map[string]runtime.Object) (*httptest.Server, restConfig := &restclient.Config{ Host: server.URL, TLSClientConfig: restclient.TLSClientConfig{ - CAData: tlsserver.TLSTestServerCA(server), + CAData: serverCA, }, } return server, restConfig diff --git a/internal/testutil/tlsserver.go b/internal/testutil/tlsserver.go deleted file mode 100644 index 4ebe526bd..000000000 --- a/internal/testutil/tlsserver.go +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package testutil - -import ( - "crypto/tls" - "errors" - "net" - "net/http" - "testing" - "time" - - "github.com/stretchr/testify/require" - - "go.pinniped.dev/internal/crypto/ptls" - "go.pinniped.dev/internal/testutil/tlsserver" -) - -// TLSTestServer starts a test server listening on a local port using a test CA. It returns the PEM CA bundle and the -// URL of the listening server. The lifetime of the server is bound to the provided *testing.T. -func TLSTestServer(t *testing.T, handler http.HandlerFunc) (caBundlePEM, url string) { - t.Helper() - - server := tlsserver.TLSTestServer(t, handler, nil) - - return string(tlsserver.TLSTestServerCA(server)), server.URL -} - -func TLSTestServerWithCert(t *testing.T, handler http.HandlerFunc, certificate *tls.Certificate) (url string) { - t.Helper() - - c := ptls.Default(nil) // mimic API server config - c.Certificates = []tls.Certificate{*certificate} - - server := http.Server{ - TLSConfig: c, - Handler: handler, - ReadHeaderTimeout: 10 * time.Second, - } - - l, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - - serverShutdownChan := make(chan error) - go func() { - // Empty certFile and keyFile will use certs from Server.TLSConfig. - serverShutdownChan <- server.ServeTLS(l, "", "") - }() - - t.Cleanup(func() { - _ = server.Close() - serveErr := <-serverShutdownChan - if !errors.Is(serveErr, http.ErrServerClosed) { - t.Log("Got an unexpected error while starting the fake http server!") - require.NoError(t, serveErr) - } - }) - - return l.Addr().String() -} diff --git a/internal/testutil/tlsserver/tlsserver.go b/internal/testutil/tlsserver/tlsserver.go index 9669764d4..a6b3f3457 100644 --- a/internal/testutil/tlsserver/tlsserver.go +++ b/internal/testutil/tlsserver/tlsserver.go @@ -7,6 +7,7 @@ import ( "context" "crypto/tls" "encoding/pem" + "errors" "fmt" "net" "net/http" @@ -14,6 +15,7 @@ import ( "reflect" "sync" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -30,26 +32,80 @@ const ( helloKey ) -func TLSTestServer(t *testing.T, handler http.Handler, f func(*httptest.Server)) *httptest.Server { +// TestServerIPv6 returns a TLS-required server that listens at an IPv6 loopback. +func TestServerIPv6(t *testing.T, handler http.Handler, f func(*httptest.Server)) (*httptest.Server, []byte) { + t.Helper() + + listener, err := net.Listen("tcp6", "[::1]:0") + require.NoError(t, err, "TLSTestIPv6Server: failed to listen on a port") + + server := &httptest.Server{ + Listener: listener, + Config: &http.Server{Handler: handler}, //nolint:gosec //ReadHeaderTimeout is not needed for a localhost listener + } + + return testServer(t, server, f) +} + +// TestServerIPv4 returns a TLS-required server that listens at an IPv4 loopback. +func TestServerIPv4(t *testing.T, handler http.Handler, f func(*httptest.Server)) (*httptest.Server, []byte) { t.Helper() server := httptest.NewUnstartedServer(handler) + return testServer(t, server, f) +} + +func testServer(t *testing.T, server *httptest.Server, f func(*httptest.Server)) (*httptest.Server, []byte) { + t.Helper() + server.TLS = ptls.Default(nil) // mimic API server config if f != nil { f(server) } server.StartTLS() t.Cleanup(server.Close) - return server -} - -func TLSTestServerCA(server *httptest.Server) []byte { - return pem.EncodeToMemory(&pem.Block{ + return server, pem.EncodeToMemory(&pem.Block{ Type: "CERTIFICATE", Bytes: server.Certificate().Raw, }) } +func TLSTestServerWithCert(t *testing.T, handler http.HandlerFunc, certificate *tls.Certificate) (url string) { + t.Helper() + + c := ptls.Default(nil) // mimic API server config + c.Certificates = []tls.Certificate{*certificate} + + server := http.Server{ + TLSConfig: c, + Handler: handler, + ReadHeaderTimeout: 10 * time.Second, + } + + l, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + + serverShutdownChan := make(chan error) + go func() { + // Empty certFile and keyFile will use certs from Server.TLSConfig. + serverShutdownChan <- server.ServeTLS(l, "", "") + }() + + t.Cleanup(func() { + _ = server.Close() + serveErr := <-serverShutdownChan + if !errors.Is(serveErr, http.ErrServerClosed) { + t.Log("Got an unexpected error while starting the fake http server!") + require.NoError(t, serveErr) + } + }) + + return l.Addr().String() +} + +// RecordTLSHello configures the server to record client TLS negotiation info onto each incoming request, +// so that the details of the client's TLS settings can be asserted upon during request handling +// using AssertTLS. func RecordTLSHello(server *httptest.Server) { server.Config.ConnContext = func(ctx context.Context, _ net.Conn) context.Context { return context.WithValue(ctx, mapKey, &sync.Map{}) @@ -67,6 +123,29 @@ func RecordTLSHello(server *httptest.Server) { } } +// AssertEveryTLSHello can be used to make assertions about the client's TLS configuration +// when a test expects the server to be dialed by the client, but does not expect that http +// requests will be performed (and thus assertions cannot be made during request handling). +// For example, a test could use this when the production code is only going to perform a +// tls.Dial to test the connection to the server, without making any actual requests. +func AssertEveryTLSHello(t *testing.T, server *httptest.Server, clientTLSConfigFunc ptls.ConfigFunc) { + server.TLS.GetConfigForClient = func(info *tls.ClientHelloInfo) (*tls.Config, error) { + t.Helper() + + // We don't know yet at this point if the request will be an upgrade request, so as a shortcut, + // we won't assert on that. When that a concern, use AssertTLS in the request handler instead. + assertionsPassed := assertTLSOnHello(t, info, false, clientTLSConfigFunc) + + if !assertionsPassed { + t.Errorf("insecure client TLS hello detected during TLS negotiation") + } + + return nil, nil + } +} + +// AssertTLS makes assertions on a particular incoming http request, assuming that the server +// was configured using RecordTLSHello. func AssertTLS(t *testing.T, r *http.Request, clientTLSConfigFunc ptls.ConfigFunc) { t.Helper() @@ -79,6 +158,16 @@ func AssertTLS(t *testing.T, r *http.Request, clientTLSConfigFunc ptls.ConfigFun actualClientHello, ok := h.(*tls.ClientHelloInfo) require.True(t, ok) + assertionsPassed := assertTLSOnHello(t, actualClientHello, httpstream.IsUpgradeRequest(r), clientTLSConfigFunc) + + if !assertionsPassed { + t.Errorf("insecure TLS detected for %q %q %q upgrade=%v", r.Proto, r.Method, r.URL.String(), httpstream.IsUpgradeRequest(r)) + } +} + +func assertTLSOnHello(t *testing.T, actualClientHello *tls.ClientHelloInfo, isUpgradeRequest bool, clientTLSConfigFunc ptls.ConfigFunc) bool { + t.Helper() + clientTLSConfig := clientTLSConfigFunc(nil) var wantClientSupportedVersions []uint16 @@ -102,7 +191,7 @@ func AssertTLS(t *testing.T, r *http.Request, clientTLSConfigFunc ptls.ConfigFun } wantClientProtos := clientTLSConfig.NextProtos - if httpstream.IsUpgradeRequest(r) { + if isUpgradeRequest { wantClientProtos = clientTLSConfig.NextProtos[1:] } @@ -111,10 +200,7 @@ func AssertTLS(t *testing.T, r *http.Request, clientTLSConfigFunc ptls.ConfigFun ok2 := assert.Equal(t, cipherSuiteIDsToStrings(wantClientSupportedCiphers), cipherSuiteIDsToStrings(actualClientHello.CipherSuites)) ok3 := assert.Equal(t, wantClientProtos, actualClientHello.SupportedProtos) - if all := ok1 && ok2 && ok3; !all { - t.Errorf("insecure TLS detected for %q %q %q upgrade=%v wantClientSupportedVersions=%v wantClientSupportedCiphers=%v wantClientProtos=%v", - r.Proto, r.Method, r.URL.String(), httpstream.IsUpgradeRequest(r), ok1, ok2, ok3) - } + return ok1 && ok2 && ok3 } // appendIfNotAlreadyIncluded only adds the newItems to the list if they are not already included diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index d8f314881..8842292ff 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -2363,7 +2363,7 @@ func TestGetURL(t *testing.T) { // Testing of host parsing, TLS negotiation, and CA bundle, etc. for the production code's dialer. func TestRealTLSDialing(t *testing.T) { - testServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}), + testServer, testServerCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}), func(server *httptest.Server) { tlsserver.RecordTLSHello(server) recordFunc := server.TLS.GetConfigForClient @@ -2378,14 +2378,13 @@ func TestRealTLSDialing(t *testing.T) { parsedURL, err := url.Parse(testServer.URL) require.NoError(t, err) testServerHostAndPort := parsedURL.Host - testServerCABundle := tlsserver.TLSTestServerCA(testServer) caForTestServerWithBadCertName, err := certauthority.New("Test CA", time.Hour) require.NoError(t, err) wrongIP := net.ParseIP("10.2.3.4") cert, err := caForTestServerWithBadCertName.IssueServerCert([]string{"wrong-dns-name"}, []net.IP{wrongIP}, time.Hour) require.NoError(t, err) - testServerWithBadCertNameAddr := testutil.TLSTestServerWithCert(t, func(w http.ResponseWriter, r *http.Request) {}, cert) + testServerWithBadCertNameAddr := tlsserver.TLSTestServerWithCert(t, func(w http.ResponseWriter, r *http.Request) {}, cert) unusedPortGrabbingListener, err := net.Listen("tcp", "127.0.0.1:0") require.NoError(t, err) @@ -2406,7 +2405,7 @@ func TestRealTLSDialing(t *testing.T) { { name: "happy path", host: testServerHostAndPort, - caBundle: testServerCABundle, + caBundle: testServerCA, connProto: TLS, context: context.Background(), }, @@ -2437,7 +2436,7 @@ func TestRealTLSDialing(t *testing.T) { { name: "invalid host with TLS", host: "this:is:not:a:valid:hostname", - caBundle: testServerCABundle, + caBundle: testServerCA, connProto: TLS, context: context.Background(), wantError: testutil.WantExactErrorString(`LDAP Result Code 200 "Network Error": host "this:is:not:a:valid:hostname" is not a valid hostname or IP address`), @@ -2445,7 +2444,7 @@ func TestRealTLSDialing(t *testing.T) { { name: "invalid host with StartTLS", host: "this:is:not:a:valid:hostname", - caBundle: testServerCABundle, + caBundle: testServerCA, connProto: StartTLS, context: context.Background(), wantError: testutil.WantExactErrorString(`LDAP Result Code 200 "Network Error": host "this:is:not:a:valid:hostname" is not a valid hostname or IP address`), @@ -2462,7 +2461,7 @@ func TestRealTLSDialing(t *testing.T) { name: "cannot connect to host", // This is assuming that this port was not reclaimed by another app since the test setup ran. Seems safe enough. host: recentlyClaimedHostAndPort, - caBundle: testServerCABundle, + caBundle: testServerCA, connProto: TLS, context: context.Background(), wantError: testutil.WantSprintfErrorString(`LDAP Result Code 200 "Network Error": dial tcp %s: connect: connection refused`, recentlyClaimedHostAndPort), @@ -2470,7 +2469,7 @@ func TestRealTLSDialing(t *testing.T) { { name: "pays attention to the passed context", host: testServerHostAndPort, - caBundle: testServerCABundle, + caBundle: testServerCA, connProto: TLS, context: alreadyCancelledContext, wantError: testutil.WantSprintfErrorString(`LDAP Result Code 200 "Network Error": dial tcp %s: operation was canceled`, testServerHostAndPort), @@ -2478,7 +2477,7 @@ func TestRealTLSDialing(t *testing.T) { { name: "unsupported connection protocol", host: testServerHostAndPort, - caBundle: testServerCABundle, + caBundle: testServerCA, connProto: "bad usage of this type", context: alreadyCancelledContext, wantError: testutil.WantExactErrorString(`LDAP Result Code 200 "Network Error": did not specify valid ConnectionProtocol`), diff --git a/pkg/conciergeclient/conciergeclient_test.go b/pkg/conciergeclient/conciergeclient_test.go index c0af26f47..d44c9508e 100644 --- a/pkg/conciergeclient/conciergeclient_test.go +++ b/pkg/conciergeclient/conciergeclient_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package conciergeclient @@ -20,7 +20,7 @@ import ( loginv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/login/v1alpha1" "go.pinniped.dev/internal/certauthority" - "go.pinniped.dev/internal/testutil" + "go.pinniped.dev/internal/testutil/tlsserver" ) func TestNew(t *testing.T) { @@ -163,12 +163,12 @@ func TestExchangeToken(t *testing.T) { t.Run("server error", func(t *testing.T) { t.Parallel() // Start a test server that returns only 500 errors. - caBundle, endpoint := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) { + server, serverCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) _, _ = w.Write([]byte("some server error")) - }) + }), nil) - client, err := New(WithEndpoint(endpoint), WithCABundle(caBundle), WithAuthenticator("jwt", "test-authenticator")) + client, err := New(WithEndpoint(server.URL), WithCABundle(string(serverCA)), WithAuthenticator("jwt", "test-authenticator")) require.NoError(t, err) got, err := client.ExchangeToken(ctx, "test-token") @@ -180,15 +180,15 @@ func TestExchangeToken(t *testing.T) { t.Parallel() // Start a test server that returns success but with an error message errorMessage := "some login failure" - caBundle, endpoint := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) { + server, serverCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&loginv1alpha1.TokenCredentialRequest{ TypeMeta: metav1.TypeMeta{APIVersion: "login.concierge.pinniped.dev/v1alpha1", Kind: "TokenCredentialRequest"}, Status: loginv1alpha1.TokenCredentialRequestStatus{Message: &errorMessage}, }) - }) + }), nil) - client, err := New(WithEndpoint(endpoint), WithCABundle(caBundle), WithAuthenticator("jwt", "test-authenticator")) + client, err := New(WithEndpoint(server.URL), WithCABundle(string(serverCA)), WithAuthenticator("jwt", "test-authenticator")) require.NoError(t, err) got, err := client.ExchangeToken(ctx, "test-token") @@ -199,14 +199,14 @@ func TestExchangeToken(t *testing.T) { t.Run("login failure unknown error", func(t *testing.T) { t.Parallel() // Start a test server that returns without any error message but also without valid credentials - caBundle, endpoint := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) { + server, serverCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&loginv1alpha1.TokenCredentialRequest{ TypeMeta: metav1.TypeMeta{APIVersion: "login.concierge.pinniped.dev/v1alpha1", Kind: "TokenCredentialRequest"}, }) - }) + }), nil) - client, err := New(WithEndpoint(endpoint), WithCABundle(caBundle), WithAuthenticator("jwt", "test-authenticator")) + client, err := New(WithEndpoint(server.URL), WithCABundle(string(serverCA)), WithAuthenticator("jwt", "test-authenticator")) require.NoError(t, err) got, err := client.ExchangeToken(ctx, "test-token") @@ -219,7 +219,7 @@ func TestExchangeToken(t *testing.T) { expires := metav1.NewTime(time.Now().Truncate(time.Second)) // Start a test server that returns successfully and asserts various properties of the request. - caBundle, endpoint := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) { + server, serverCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { require.Equal(t, http.MethodPost, r.Method) require.Equal(t, "/apis/login.concierge.pinniped.dev/v1alpha1/tokencredentialrequests", r.URL.Path) require.Equal(t, "application/json", r.Header.Get("content-type")) @@ -257,9 +257,9 @@ func TestExchangeToken(t *testing.T) { }, }, }) - }) + }), nil) - client, err := New(WithEndpoint(endpoint), WithCABundle(caBundle), WithAuthenticator("webhook", "test-webhook")) + client, err := New(WithEndpoint(server.URL), WithCABundle(string(serverCA)), WithAuthenticator("webhook", "test-webhook")) require.NoError(t, err) got, err := client.ExchangeToken(ctx, "test-token") diff --git a/pkg/oidcclient/login_test.go b/pkg/oidcclient/login_test.go index d7669f74d..7d71a8899 100644 --- a/pkg/oidcclient/login_test.go +++ b/pkg/oidcclient/login_test.go @@ -66,10 +66,9 @@ func (m *mockSessionCache) PutToken(key SessionCacheKey, token *oidctypes.Token) m.sawPutTokens = append(m.sawPutTokens, token) } -func newClientForServer(server *httptest.Server) *http.Client { +func buildHTTPClientForPEM(pemData []byte) *http.Client { pool := x509.NewCertPool() - caPEMData := tlsserver.TLSTestServerCA(server) - pool.AppendCertsFromPEM(caPEMData) + pool.AppendCertsFromPEM(pemData) return phttp.Default(pool) } @@ -91,13 +90,13 @@ func TestLogin(t *testing.T) { //nolint:gocyclo } // Start a test server that returns 500 errors. - errorServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + errorServer, errorServerCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Error(w, "some discovery error", http.StatusInternalServerError) }), nil) // Start a test server that returns discovery data with a broken response_modes_supported value. brokenResponseModeMux := http.NewServeMux() - brokenResponseModeServer := tlsserver.TLSTestServer(t, brokenResponseModeMux, nil) + brokenResponseModeServer, brokenResponseModeServerCA := tlsserver.TestServerIPv4(t, brokenResponseModeMux, nil) brokenResponseModeMux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") type providerJSON struct { @@ -116,7 +115,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo // Start a test server that returns discovery data with a broken token URL. brokenTokenURLMux := http.NewServeMux() - brokenTokenURLServer := tlsserver.TLSTestServer(t, brokenTokenURLMux, nil) + brokenTokenURLServer, brokenTokenURLServerCA := tlsserver.TestServerIPv4(t, brokenTokenURLMux, nil) brokenTokenURLMux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") type providerJSON struct { @@ -135,7 +134,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo // Start a test server that returns discovery data with an insecure token URL. insecureTokenURLMux := http.NewServeMux() - insecureTokenURLServer := tlsserver.TLSTestServer(t, insecureTokenURLMux, nil) + insecureTokenURLServer, insecureTokenURLServerCA := tlsserver.TestServerIPv4(t, insecureTokenURLMux, nil) insecureTokenURLMux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") type providerJSON struct { @@ -154,7 +153,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo // Start a test server that returns discovery data with a broken authorize URL. brokenAuthURLMux := http.NewServeMux() - brokenAuthURLServer := tlsserver.TLSTestServer(t, brokenAuthURLMux, nil) + brokenAuthURLServer, brokenAuthURLServerCA := tlsserver.TestServerIPv4(t, brokenAuthURLMux, nil) brokenAuthURLMux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") type providerJSON struct { @@ -173,7 +172,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo // Start a test server that returns discovery data with an insecure authorize URL. insecureAuthURLMux := http.NewServeMux() - insecureAuthURLServer := tlsserver.TLSTestServer(t, insecureAuthURLMux, nil) + insecureAuthURLServer, insecureAuthURLServerCA := tlsserver.TestServerIPv4(t, insecureAuthURLMux, nil) insecureAuthURLMux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") type providerJSON struct { @@ -298,13 +297,13 @@ func TestLogin(t *testing.T) { //nolint:gocyclo // Start a test server that returns a real discovery document and answers refresh requests. providerMux := http.NewServeMux() - successServer := tlsserver.TLSTestServer(t, providerMux, nil) + successServer, successServerCA := tlsserver.TestServerIPv4(t, providerMux, nil) providerMux.HandleFunc("/.well-known/openid-configuration", discoveryHandler(successServer, nil)) providerMux.HandleFunc("/token", tokenHandler) // Start a test server that returns a real discovery document and answers refresh requests, _and_ supports form_mode=post. formPostProviderMux := http.NewServeMux() - formPostSuccessServer := tlsserver.TLSTestServer(t, formPostProviderMux, nil) + formPostSuccessServer, formPostSuccessServerCA := tlsserver.TestServerIPv4(t, formPostProviderMux, nil) formPostProviderMux.HandleFunc("/.well-known/openid-configuration", discoveryHandler(formPostSuccessServer, []string{"query", "form_post"})) formPostProviderMux.HandleFunc("/token", tokenHandler) @@ -339,7 +338,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithCLISendingCredentials()(h)) require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h)) - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) require.NoError(t, WithClient(&http.Client{ Transport: roundtripper.Func(func(req *http.Request) (*http.Response, error) { @@ -436,7 +435,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo clientID: "test-client-id", opt: func(t *testing.T) Option { return func(h *handlerState) error { - require.NoError(t, WithClient(newClientForServer(errorServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(errorServerCA))(h)) cache := &mockSessionCache{t: t, getReturnsToken: &oidctypes.Token{ IDToken: &oidctypes.IDToken{ Token: "test-id-token", @@ -485,7 +484,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo name: "discovery failure due to 500 error", opt: func(t *testing.T) Option { return func(h *handlerState) error { - require.NoError(t, WithClient(newClientForServer(errorServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(errorServerCA))(h)) return nil } }, @@ -497,7 +496,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo name: "discovery failure due to invalid response_modes_supported", opt: func(t *testing.T) Option { return func(h *handlerState) error { - require.NoError(t, WithClient(newClientForServer(brokenResponseModeServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(brokenResponseModeServerCA))(h)) return nil } }, @@ -511,7 +510,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo clientID: "test-client-id", opt: func(t *testing.T) Option { return func(h *handlerState) error { - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) @@ -562,7 +561,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo clientID: "test-client-id", opt: func(t *testing.T) Option { return func(h *handlerState) error { - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) @@ -605,7 +604,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo clientID: "not-the-test-client-id", opt: func(t *testing.T) Option { return func(h *handlerState) error { - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) cache := &mockSessionCache{t: t, getReturnsToken: &oidctypes.Token{ IDToken: &oidctypes.IDToken{ @@ -638,7 +637,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo name: "issuer has invalid token URL", opt: func(t *testing.T) Option { return func(h *handlerState) error { - require.NoError(t, WithClient(newClientForServer(brokenTokenURLServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(brokenTokenURLServerCA))(h)) return nil } }, @@ -650,7 +649,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo name: "issuer has insecure token URL", opt: func(t *testing.T) Option { return func(h *handlerState) error { - require.NoError(t, WithClient(newClientForServer(insecureTokenURLServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(insecureTokenURLServerCA))(h)) return nil } }, @@ -662,7 +661,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo name: "issuer has invalid authorize URL", opt: func(t *testing.T) Option { return func(h *handlerState) error { - require.NoError(t, WithClient(newClientForServer(brokenAuthURLServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(brokenAuthURLServerCA))(h)) return nil } }, @@ -674,7 +673,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo name: "issuer has insecure authorize URL", opt: func(t *testing.T) Option { return func(h *handlerState) error { - require.NoError(t, WithClient(newClientForServer(insecureAuthURLServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(insecureAuthURLServerCA))(h)) return nil } }, @@ -686,7 +685,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo name: "listen failure and non-tty stdin", opt: func(t *testing.T) Option { return func(h *handlerState) error { - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) h.listen = func(net string, addr string) (net.Listener, error) { assert.Equal(t, "tcp", net) assert.Equal(t, "localhost:0", addr) @@ -711,7 +710,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo h.generatePKCE = func() (pkce.Code, error) { return "test-pkce", nil } h.generateNonce = func() (nonce.Nonce, error) { return "test-nonce", nil } h.stdinIsTTY = func() bool { return true } - require.NoError(t, WithClient(newClientForServer(formPostSuccessServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(formPostSuccessServerCA))(h)) require.NoError(t, WithSkipListen()(h)) h.openURL = func(authorizeURL string) error { parsed, err := url.Parse(authorizeURL) @@ -750,7 +749,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo h.generatePKCE = func() (pkce.Code, error) { return "test-pkce", nil } h.generateNonce = func() (nonce.Nonce, error) { return "test-nonce", nil } h.stdinIsTTY = func() bool { return true } - require.NoError(t, WithClient(newClientForServer(formPostSuccessServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(formPostSuccessServerCA))(h)) h.listen = func(string, string) (net.Listener, error) { return nil, fmt.Errorf("some listen error") } h.openURL = func(authorizeURL string) error { parsed, err := url.Parse(authorizeURL) @@ -790,7 +789,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo h.generateNonce = func() (nonce.Nonce, error) { return "test-nonce", nil } h.stdinIsTTY = func() bool { return true } - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) ctx, cancel := context.WithCancel(h.ctx) h.ctx = ctx @@ -824,7 +823,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo h.generatePKCE = func() (pkce.Code, error) { return "test-pkce", nil } h.generateNonce = func() (nonce.Nonce, error) { return "test-nonce", nil } h.stdinIsTTY = func() bool { return true } - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) h.openURL = func(_ string) error { go func() { h.callbacks <- callbackResult{err: fmt.Errorf("some callback error")} @@ -873,7 +872,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }) require.NoError(t, WithSessionCache(cache)(h)) - client := newClientForServer(successServer) + client := buildHTTPClientForPEM(successServerCA) client.Timeout = 10 * time.Second require.NoError(t, WithClient(client)(h)) @@ -947,7 +946,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }) require.NoError(t, WithSessionCache(cache)(h)) - client := newClientForServer(successServer) + client := buildHTTPClientForPEM(successServerCA) client.Timeout = 10 * time.Second require.NoError(t, WithClient(client)(h)) @@ -1013,7 +1012,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }) require.NoError(t, WithSessionCache(cache)(h)) - client := newClientForServer(successServer) + client := buildHTTPClientForPEM(successServerCA) client.Timeout = 10 * time.Second require.NoError(t, WithClient(client)(h)) @@ -1091,7 +1090,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }) require.NoError(t, WithSessionCache(cache)(h)) - client := newClientForServer(successServer) + client := buildHTTPClientForPEM(successServerCA) client.Timeout = 10 * time.Second require.NoError(t, WithClient(client)(h)) @@ -1182,7 +1181,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }) require.NoError(t, WithSessionCache(cache)(h)) - client := newClientForServer(formPostSuccessServer) + client := buildHTTPClientForPEM(formPostSuccessServerCA) client.Timeout = 10 * time.Second require.NoError(t, WithClient(client)(h)) @@ -1258,7 +1257,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "oidc")(h)) - client := newClientForServer(successServer) + client := buildHTTPClientForPEM(successServerCA) client.Timeout = 10 * time.Second require.NoError(t, WithClient(client)(h)) @@ -1349,7 +1348,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo return func(h *handlerState) error { _ = defaultLDAPTestOpts(t, h, nil, nil) - client := newClientForServer(successServer) + client := buildHTTPClientForPEM(successServerCA) client.Transport = roundtripper.Func(func(req *http.Request) (*http.Response, error) { switch req.URL.Scheme + "://" + req.URL.Host + req.URL.Path { case "https://" + successServer.Listener.Addr().String() + "/.well-known/openid-configuration": @@ -1567,7 +1566,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.True(t, authorizeRequestWasMade, "should have made an authorize request") }) - client := newClientForServer(successServer) + client := buildHTTPClientForPEM(successServerCA) client.Transport = roundtripper.Func(func(req *http.Request) (*http.Response, error) { switch req.URL.Scheme + "://" + req.URL.Host + req.URL.Path { case "https://" + successServer.Listener.Addr().String() + "/.well-known/openid-configuration": @@ -1671,7 +1670,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.True(t, authorizeRequestWasMade, "should have made an authorize request") }) - client := newClientForServer(successServer) + client := buildHTTPClientForPEM(successServerCA) client.Transport = roundtripper.Func(func(req *http.Request) (*http.Response, error) { switch req.URL.Scheme + "://" + req.URL.Host + req.URL.Path { case "https://" + successServer.Listener.Addr().String() + "/.well-known/openid-configuration": @@ -1778,7 +1777,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo require.True(t, authorizeRequestWasMade, "should have made an authorize request") }) - client := newClientForServer(successServer) + client := buildHTTPClientForPEM(successServerCA) client.Transport = roundtripper.Func(func(req *http.Request) (*http.Response, error) { switch req.URL.Scheme + "://" + req.URL.Host + req.URL.Path { case "https://" + successServer.Listener.Addr().String() + "/.well-known/openid-configuration": @@ -1842,7 +1841,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }}, cache.sawGetKeys) require.Empty(t, cache.sawPutTokens) }) - require.NoError(t, WithClient(newClientForServer(errorServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(errorServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("cluster-1234")(h)) return nil @@ -1871,7 +1870,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }}, cache.sawGetKeys) require.Empty(t, cache.sawPutTokens) }) - require.NoError(t, WithClient(newClientForServer(insecureTokenURLServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(insecureTokenURLServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("cluster-1234")(h)) return nil @@ -1900,7 +1899,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }}, cache.sawGetKeys) require.Empty(t, cache.sawPutTokens) }) - require.NoError(t, WithClient(newClientForServer(brokenTokenURLServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(brokenTokenURLServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("cluster-1234")(h)) return nil @@ -1929,7 +1928,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }}, cache.sawGetKeys) require.Empty(t, cache.sawPutTokens) }) - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("test-audience-produce-invalid-http-response")(h)) return nil @@ -1958,7 +1957,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }}, cache.sawGetKeys) require.Empty(t, cache.sawPutTokens) }) - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("test-audience-produce-http-400")(h)) return nil @@ -1987,7 +1986,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }}, cache.sawGetKeys) require.Empty(t, cache.sawPutTokens) }) - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("test-audience-produce-invalid-content-type")(h)) return nil @@ -2016,7 +2015,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }}, cache.sawGetKeys) require.Empty(t, cache.sawPutTokens) }) - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("test-audience-produce-wrong-content-type")(h)) return nil @@ -2045,7 +2044,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }}, cache.sawGetKeys) require.Empty(t, cache.sawPutTokens) }) - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("test-audience-produce-invalid-json")(h)) return nil @@ -2074,7 +2073,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }}, cache.sawGetKeys) require.Empty(t, cache.sawPutTokens) }) - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("test-audience-produce-invalid-tokentype")(h)) return nil @@ -2103,7 +2102,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }}, cache.sawGetKeys) require.Empty(t, cache.sawPutTokens) }) - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("test-audience-produce-invalid-issuedtokentype")(h)) return nil @@ -2132,7 +2131,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }}, cache.sawGetKeys) require.Empty(t, cache.sawPutTokens) }) - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("test-audience-produce-invalid-jwt")(h)) return nil @@ -2161,7 +2160,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }}, cache.sawGetKeys) require.Empty(t, cache.sawPutTokens) }) - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("test-audience")(h)) @@ -2204,7 +2203,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }}, cache.sawGetKeys) require.Empty(t, cache.sawPutTokens) }) - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("request-this-test-audience")(h)) @@ -2256,7 +2255,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo Claims: map[string]interface{}{"aud": "test-custom-request-audience"}, }, cache.sawPutTokens[0].IDToken) }) - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("test-custom-request-audience")(h)) @@ -2318,7 +2317,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo }}, cache.sawGetKeys) require.Empty(t, cache.sawPutTokens) }) - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) require.NoError(t, WithSessionCache(cache)(h)) require.NoError(t, WithRequestAudience("request-this-test-audience")(h)) @@ -2343,7 +2342,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo clientID: "test-client-id", opt: func(t *testing.T) Option { return func(h *handlerState) error { - require.NoError(t, WithClient(newClientForServer(successServer))(h)) + require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h)) cache := &mockSessionCache{t: t, getReturnsToken: &oidctypes.Token{ IDToken: &oidctypes.IDToken{ diff --git a/test/integration/concierge_webhookauthenticator_status_test.go b/test/integration/concierge_webhookauthenticator_status_test.go index 1b372c39e..68b019b17 100644 --- a/test/integration/concierge_webhookauthenticator_status_test.go +++ b/test/integration/concierge_webhookauthenticator_status_test.go @@ -276,7 +276,7 @@ func allSuccessfulWebhookAuthenticatorConditions() []metav1.Condition { Type: "EndpointURLValid", Status: "True", Reason: "Success", - Message: "endpoint is a valid URL", + Message: "spec.endpoint is a valid URL", }, { Type: "Ready", @@ -294,7 +294,7 @@ func allSuccessfulWebhookAuthenticatorConditions() []metav1.Condition { Type: "WebhookConnectionValid", Status: "True", Reason: "Success", - Message: "tls verified", + Message: "successfully dialed webhook server", }, } } diff --git a/test/integration/securetls_fips_test.go b/test/integration/securetls_fips_test.go index 5a35ab5ef..63a6d21a3 100644 --- a/test/integration/securetls_fips_test.go +++ b/test/integration/securetls_fips_test.go @@ -1,4 +1,4 @@ -// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 //go:build fips_strict @@ -23,14 +23,13 @@ import ( func TestFIPSCipherSuites_Parallel(t *testing.T) { _ = testlib.IntegrationEnv(t) - server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server, ca := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // use the default fips config which contains a hard coded list of cipher suites // that should be equal to the default list of fips cipher suites. // assert that the client hello response has the same tls config as this test server. tlsserver.AssertTLS(t, r, ptls.Default) }), tlsserver.RecordTLSHello) - ca := tlsserver.TLSTestServerCA(server) pool, err := cert.NewPoolFromBytes(ca) require.NoError(t, err) // create a tls config that does not explicitly set cipher suites, diff --git a/test/integration/securetls_test.go b/test/integration/securetls_test.go index 0d2187bf5..209e1dd89 100644 --- a/test/integration/securetls_test.go +++ b/test/integration/securetls_test.go @@ -23,7 +23,7 @@ import ( func TestSecureTLSPinnipedCLIToKAS_Parallel(t *testing.T) { _ = testlib.IntegrationEnv(t) - server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server, serverCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // pinniped CLI uses ptls.Secure when talking to KAS // in FIPS mode the distinction doesn't matter much because // each of the configs is a wrapper for the same base FIPS config @@ -33,15 +33,13 @@ func TestSecureTLSPinnipedCLIToKAS_Parallel(t *testing.T) { `"status":{"credential":{"token":"some-fancy-token"}}}`) }), tlsserver.RecordTLSHello) - ca := tlsserver.TLSTestServerCA(server) - pinnipedExe := testlib.PinnipedCLIPath(t) stdout, stderr := runPinnipedCLI(t, nil, pinnipedExe, "login", "static", "--token", "does-not-matter", "--concierge-authenticator-type", "webhook", "--concierge-authenticator-name", "does-not-matter", - "--concierge-ca-bundle-data", base64.StdEncoding.EncodeToString(ca), + "--concierge-ca-bundle-data", base64.StdEncoding.EncodeToString(serverCA), "--concierge-endpoint", server.URL, "--enable-concierge", "--credential-cache", "", @@ -57,7 +55,7 @@ func TestSecureTLSPinnipedCLIToKAS_Parallel(t *testing.T) { func TestSecureTLSPinnipedCLIToSupervisor_Parallel(t *testing.T) { _ = testlib.IntegrationEnv(t) - server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server, serverCA := tlsserver.TestServerIPv4(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // pinniped CLI uses ptls.Default when talking to supervisor // in FIPS mode the distinction doesn't matter much because // each of the configs is a wrapper for the same base FIPS config @@ -66,12 +64,10 @@ func TestSecureTLSPinnipedCLIToSupervisor_Parallel(t *testing.T) { fmt.Fprint(w, `{"issuer":"https://not-a-good-issuer"}`) }), tlsserver.RecordTLSHello) - ca := tlsserver.TLSTestServerCA(server) - pinnipedExe := testlib.PinnipedCLIPath(t) stdout, stderr := runPinnipedCLI(&fakeT{T: t}, nil, pinnipedExe, "login", "oidc", - "--ca-bundle-data", base64.StdEncoding.EncodeToString(ca), + "--ca-bundle-data", base64.StdEncoding.EncodeToString(serverCA), "--issuer", server.URL, "--credential-cache", "", "--upstream-identity-provider-flow", "cli_password",