diff --git a/internal/concierge/impersonator/doc.go b/internal/concierge/impersonator/doc.go new file mode 100644 index 00000000..d8504603 --- /dev/null +++ b/internal/concierge/impersonator/doc.go @@ -0,0 +1,42 @@ +// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +/* +Package impersonator implements an HTTP server that reverse proxies all requests +to the Kubernetes API server with impersonation headers set to match the calling +user. Since impersonation cannot be disabled, this allows us to dynamically +configure authentication on any cluster, even the cloud hosted ones. + +The specifics of how it is implemented are of interest. The most novel detail +about the implementation is that we use the "front-end" of the aggregated API +server logic, mainly the DefaultBuildHandlerChain func, to handle how incoming +requests are authenticated, authorized, etc. The "back-end" of the proxy is a +reverse proxy that impersonates the user (instead of serving REST APIs). + +In terms of authentication, we aim to handle every type of authentication that +the Kubernetes API server supports by delegating most of the checks to it. We +also honor client certs from a CA that is specific to the impersonation proxy. +This approach allows clients to use the Token Credential Request API even when +we do not have the cluster's signing key. + +In terms of authorization, we rely mostly on the Kubernetes API server. Since we +impersonate the user, the proxied request will be authorized against that user. +Thus for all regular REST verbs, we perform no authorization checks. + +Nested impersonation is handled by performing the same authorization checks the +Kubernetes API server would (we get this mostly for free by using the aggregated +API server code). We preserve the original user in the reserved extra key +original-user-info.impersonation-proxy.concierge.pinniped.dev as a JSON blob of +the authenticationv1.UserInfo struct. This is necessary to make sure that the +Kubernetes audit log contains all three identities (original user, impersonated +user and the impersonation proxy's service account). Capturing the original +user information requires that we enable the auditing stack (WithImpersonation +only shares this information with the audit stack). To keep things simple, +we use the fake audit backend at the Metadata level for all requests. This +guarantees that we always have an audit event on every request. + +For all normal requests, we only use http/2.0 when proxying to the API server. +For upgrade requests, we only use http/1.1 since these always go from http/1.1 +to either websockets or SPDY. +*/ +package impersonator diff --git a/internal/concierge/impersonator/impersonator.go b/internal/concierge/impersonator/impersonator.go index f3a7372d..087329dd 100644 --- a/internal/concierge/impersonator/impersonator.go +++ b/internal/concierge/impersonator/impersonator.go @@ -4,11 +4,14 @@ package impersonator import ( + "context" + "encoding/json" "fmt" "net" "net/http" "net/http/httputil" "net/url" + "regexp" "strings" "time" @@ -21,6 +24,8 @@ import ( "k8s.io/apimachinery/pkg/util/httpstream" utilnet "k8s.io/apimachinery/pkg/util/net" "k8s.io/apimachinery/pkg/util/sets" + auditinternal "k8s.io/apiserver/pkg/apis/audit" + "k8s.io/apiserver/pkg/audit/policy" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/endpoints/filterlatency" @@ -31,6 +36,7 @@ import ( "k8s.io/apiserver/pkg/server/dynamiccertificates" "k8s.io/apiserver/pkg/server/filters" genericoptions "k8s.io/apiserver/pkg/server/options" + auditfake "k8s.io/apiserver/plugin/pkg/audit/fake" "k8s.io/client-go/rest" "k8s.io/client-go/transport" @@ -100,7 +106,6 @@ func newInternal( //nolint:funlen // yeah, it's kind of long. if err != nil { return nil, err } - recommendedOptions.Authentication.ClientCert.ClientCA = "---irrelevant-but-needs-to-be-non-empty---" // drop when we pick up https://github.com/kubernetes/kubernetes/pull/100055 recommendedOptions.Authentication.ClientCert.CAContentProvider = dynamiccertificates.NewUnionCAContentProvider( impersonationProxySignerCA, kubeClientCA, ) @@ -163,35 +168,55 @@ func newInternal( //nolint:funlen // yeah, it's kind of long. })) handler = filterlatency.TrackStarted(handler, "impersonationproxy") + handler = filterlatency.TrackCompleted(handler) + handler = deleteKnownImpersonationHeaders(handler) + handler = filterlatency.TrackStarted(handler, "deleteimpersonationheaders") + // The standard Kube handler chain (authn, authz, impersonation, audit, etc). // See the genericapiserver.DefaultBuildHandlerChain func for details. handler = defaultBuildHandlerChainFunc(handler, c) // Always set security headers so browsers do the right thing. + handler = filterlatency.TrackCompleted(handler) handler = securityheader.Wrap(handler) + handler = filterlatency.TrackStarted(handler, "securityheaders") return handler } - // Overwrite the delegating authorizer with one that only cares about impersonation. - // Empty string is disallowed because request info has had bugs in the past where it would leave it empty. - disallowedVerbs := sets.NewString("", "impersonate") - noImpersonationAuthorizer := &comparableAuthorizer{ - AuthorizerFunc: func(a authorizer.Attributes) (authorizer.Decision, string, error) { - // Supporting impersonation is not hard, it would just require a bunch of testing - // and configuring the audit layer (to preserve the caller) which we can do later. - // We would also want to delete the incoming impersonation headers - // instead of overwriting the delegating authorizer, we would - // actually use it to make the impersonation authorization checks. - if disallowedVerbs.Has(a.GetVerb()) { - return authorizer.DecisionDeny, "impersonation is not allowed or invalid verb", nil - } + // wire up a fake audit backend at the metadata level so we can preserve the original user during nested impersonation + // TODO: wire up the real std out logging audit backend based on plog log level + serverConfig.AuditPolicyChecker = policy.FakeChecker(auditinternal.LevelMetadata, nil) + serverConfig.AuditBackend = &auditfake.Backend{} - return authorizer.DecisionAllow, "deferring authorization to kube API server", nil + delegatingAuthorizer := serverConfig.Authorization.Authorizer + nestedImpersonationAuthorizer := &comparableAuthorizer{ + authorizerFunc: func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) { + switch a.GetVerb() { + case "": + // Empty string is disallowed because request info has had bugs in the past where it would leave it empty. + return authorizer.DecisionDeny, "invalid verb", nil + case "create", + "update", + "delete", + "deletecollection", + "get", + "list", + "watch", + "patch", + "proxy": + // we know these verbs are from the request info parsing which is safe to delegate to KAS + return authorizer.DecisionAllow, "deferring standard verb authorization to kube API server", nil + default: + // assume everything else is internal SAR checks that we need to run against the requesting user + // because when KAS does the check, it may run the check against our service account and not the + // requesting user. This also handles the impersonate verb to allow for nested impersonation. + return delegatingAuthorizer.Authorize(ctx, a) + } }, } // Set our custom authorizer before calling Compete(), which will use it. - serverConfig.Authorization.Authorizer = noImpersonationAuthorizer + serverConfig.Authorization.Authorizer = nestedImpersonationAuthorizer impersonationProxyServer, err := serverConfig.Complete().New("impersonation-proxy", genericapiserver.NewEmptyDelegate()) if err != nil { @@ -201,7 +226,7 @@ func newInternal( //nolint:funlen // yeah, it's kind of long. preparedRun := impersonationProxyServer.PrepareRun() // Sanity check. Make sure that our custom authorizer is still in place and did not get changed or wrapped. - if preparedRun.Authorizer != noImpersonationAuthorizer { + if preparedRun.Authorizer != nestedImpersonationAuthorizer { return nil, constable.Error("invalid mutation of impersonation authorizer detected") } @@ -225,9 +250,44 @@ func newInternal( //nolint:funlen // yeah, it's kind of long. return result, nil } +func deleteKnownImpersonationHeaders(delegate http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // remove known impersonation headers while avoiding mutation of input request + // unknown future impersonation headers will still get caught by our later checks + if ensureNoImpersonationHeaders(r) != nil { + r = r.Clone(r.Context()) + + impersonationHeaders := []string{ + transport.ImpersonateUserHeader, + transport.ImpersonateGroupHeader, + } + + for k := range r.Header { + if !strings.HasPrefix(k, transport.ImpersonateUserExtraHeaderPrefix) { + continue + } + impersonationHeaders = append(impersonationHeaders, k) + } + + for _, header := range impersonationHeaders { + r.Header.Del(header) // delay mutation until the end when we are done iterating over the map + } + } + + delegate.ServeHTTP(w, r) + }) +} + // No-op wrapping around AuthorizerFunc to allow for comparisons. type comparableAuthorizer struct { - authorizer.AuthorizerFunc + authorizerFunc +} + +// TODO: delete when we pick up https://github.com/kubernetes/kubernetes/pull/100963 +type authorizerFunc func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) + +func (f authorizerFunc) Authorize(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) { + return f(ctx, a) } func newImpersonationReverseProxyFunc(restConfig *rest.Config) (func(*genericapiserver.Config) http.Handler, error) { @@ -258,7 +318,7 @@ func newImpersonationReverseProxyFunc(restConfig *rest.Config) (func(*genericapi } if err := ensureNoImpersonationHeaders(r); err != nil { - plog.Error("noImpersonationAuthorizer logic did not prevent nested impersonation but it is always supposed to do so", + plog.Error("unknown impersonation header seen", err, "url", r.URL.String(), "method", r.Method, @@ -277,6 +337,16 @@ func newImpersonationReverseProxyFunc(restConfig *rest.Config) (func(*genericapi return } + ae := request.AuditEventFrom(r.Context()) + if ae == nil { + plog.Warning("aggregated API server logic did not set audit event but it is always supposed to do so", + "url", r.URL.String(), + "method", r.Method, + ) + newInternalErrResponse(w, r, c.Serializer, "invalid audit event") + return + } + // KAS only supports upgrades via http/1.1 to websockets/SPDY (upgrades never use http/2.0) // Thus we default to using http/2.0 when the request is not an upgrade, otherwise we use http/1.1 baseRT := http2RoundTripper @@ -285,7 +355,7 @@ func newImpersonationReverseProxyFunc(restConfig *rest.Config) (func(*genericapi baseRT = http1RoundTripper } - rt, err := getTransportForUser(userInfo, baseRT) + rt, err := getTransportForUser(userInfo, baseRT, ae) if err != nil { plog.WarningErr("rejecting request as we cannot act as the current user", err, "url", r.URL.String(), @@ -332,6 +402,9 @@ func newImpersonationReverseProxyFunc(restConfig *rest.Config) (func(*genericapi func ensureNoImpersonationHeaders(r *http.Request) error { for key := range r.Header { + // even though we have unit tests that try to cover this case, it is hard to tell if Go does + // client side canonicalization on encode, server side canonicalization on decode, or both + key := http.CanonicalHeaderKey(key) if strings.HasPrefix(key, "Impersonate") { return fmt.Errorf("%q header already exists", key) } @@ -340,12 +413,17 @@ func ensureNoImpersonationHeaders(r *http.Request) error { return nil } -func getTransportForUser(userInfo user.Info, delegate http.RoundTripper) (http.RoundTripper, error) { +func getTransportForUser(userInfo user.Info, delegate http.RoundTripper, ae *auditinternal.Event) (http.RoundTripper, error) { if len(userInfo.GetUID()) == 0 { + extra, err := buildExtra(userInfo.GetExtra(), ae) + if err != nil { + return nil, err + } + impersonateConfig := transport.ImpersonationConfig{ UserName: userInfo.GetName(), Groups: userInfo.GetGroups(), - Extra: userInfo.GetExtra(), + Extra: extra, } // transport.NewImpersonatingRoundTripper clones the request before setting headers // thus it will not accidentally mutate the input request (see http.Handler docs) @@ -365,6 +443,44 @@ func getTransportForUser(userInfo user.Info, delegate http.RoundTripper) (http.R return nil, constable.Error("unexpected uid") } +func buildExtra(extra map[string][]string, ae *auditinternal.Event) (map[string][]string, error) { + const reservedImpersonationProxySuffix = ".impersonation-proxy.concierge.pinniped.dev" + + // always validate that the extra is something we support irregardless of nested impersonation + for k := range extra { + if !extraKeyRegexp.MatchString(k) { + return nil, fmt.Errorf("disallowed extra key seen: %s", k) + } + + if strings.HasSuffix(k, reservedImpersonationProxySuffix) { + return nil, fmt.Errorf("disallowed extra key with reserved prefix seen: %s", k) + } + } + + if ae.ImpersonatedUser == nil { + return extra, nil // just return the given extra since nested impersonation is not being used + } + + // avoid mutating input map, preallocate new map to store original user info + out := make(map[string][]string, len(extra)+1) + + for k, v := range extra { + out[k] = v // shallow copy of slice since we are not going to mutate it + } + + origUserInfoJSON, err := json.Marshal(ae.User) + if err != nil { + return nil, err + } + + out["original-user-info"+reservedImpersonationProxySuffix] = []string{string(origUserInfoJSON)} + + return out, nil +} + +// extraKeyRegexp is a very conservative regex to handle impersonation's extra key fidelity limitations such as casing and escaping. +var extraKeyRegexp = regexp.MustCompile(`^[a-z0-9/\-._]+$`) + func newInternalErrResponse(w http.ResponseWriter, r *http.Request, s runtime.NegotiatedSerializer, msg string) { newStatusErrResponse(w, r, s, apierrors.NewInternalError(constable.Error(msg))) } diff --git a/internal/concierge/impersonator/impersonator_test.go b/internal/concierge/impersonator/impersonator_test.go index 7633c9df..dfd765c3 100644 --- a/internal/concierge/impersonator/impersonator_test.go +++ b/internal/concierge/impersonator/impersonator_test.go @@ -15,11 +15,13 @@ import ( "time" "github.com/stretchr/testify/require" + authenticationv1 "k8s.io/api/authentication/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/util/httpstream" + auditinternal "k8s.io/apiserver/pkg/apis/audit" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/features" @@ -39,8 +41,6 @@ import ( ) func TestImpersonator(t *testing.T) { - const port = 9444 - ca, err := certauthority.New("ca", time.Hour) require.NoError(t, err) caKey, err := ca.PrivateKeyToPEM() @@ -58,13 +58,7 @@ func TestImpersonator(t *testing.T) { unrelatedCA, err := certauthority.New("ca", time.Hour) require.NoError(t, err) - // Punch out just enough stuff to make New actually run without error. - recOpts := func(options *genericoptions.RecommendedOptions) { - options.Authentication.RemoteKubeConfigFileOptional = true - options.Authorization.RemoteKubeConfigFileOptional = true - options.CoreAPI = nil - options.Admission = nil - } + // turn off this code path for all tests because it does not handle the config we remove correctly defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.APIPriorityAndFairness, false)() tests := []struct { @@ -140,7 +134,7 @@ func TestImpersonator(t *testing.T) { clientCert: newClientCert(t, ca, "test-username2", []string{"test-group3", "test-group4"}), kubeAPIServerClientBearerTokenFile: "required-to-be-set", clientMutateHeaders: func(header http.Header) { - header.Add("x-FORWARDED-for", "example.com") + header["x-FORWARDED-for"] = append(header["x-FORWARDED-for"], "example.com") }, wantKubeAPIServerRequestHeaders: http.Header{ "Impersonate-User": {"test-username2"}, @@ -189,20 +183,128 @@ func TestImpersonator(t *testing.T) { wantError: "Unauthorized", }, { - name: "double impersonation is not allowed by regular users", + name: "nested impersonation by regular users calls delegating authorizer", clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}), clientImpersonateUser: rest.ImpersonationConfig{UserName: "some-other-username"}, kubeAPIServerClientBearerTokenFile: "required-to-be-set", + // this fails because the delegating authorizer in this test only allows system:masters and fails everything else wantError: `users "some-other-username" is forbidden: User "test-username" ` + - `cannot impersonate resource "users" in API group "" at the cluster scope: impersonation is not allowed or invalid verb`, + `cannot impersonate resource "users" in API group "" at the cluster scope`, }, { - name: "double impersonation is not allowed by admin users", - clientCert: newClientCert(t, ca, "test-admin", []string{"system:masters", "test-group2"}), - clientImpersonateUser: rest.ImpersonationConfig{UserName: "some-other-username"}, + name: "nested impersonation by admin users calls delegating authorizer", + clientCert: newClientCert(t, ca, "test-admin", []string{"system:masters", "test-group2"}), + clientImpersonateUser: rest.ImpersonationConfig{ + UserName: "fire", + Groups: []string{"elements"}, + Extra: map[string][]string{ + "colors": {"red", "orange", "blue"}, + + // gke + "iam.gke.io/user-assertion": {"good", "stuff"}, + "user-assertion.cloud.google.com": {"smaller", "things"}, + + // openshift + "scopes.authorization.openshift.io": {"user:info", "user:full", "user:check-access"}, + + // openstack + "alpha.kubernetes.io/identity/roles": {"a-role1", "a-role2"}, + "alpha.kubernetes.io/identity/project/id": {"a-project-id"}, + "alpha.kubernetes.io/identity/project/name": {"a-project-name"}, + "alpha.kubernetes.io/identity/user/domain/id": {"a-domain-id"}, + "alpha.kubernetes.io/identity/user/domain/name": {"a-domain-name"}, + }, + }, kubeAPIServerClientBearerTokenFile: "required-to-be-set", - wantError: `users "some-other-username" is forbidden: User "test-admin" ` + - `cannot impersonate resource "users" in API group "" at the cluster scope: impersonation is not allowed or invalid verb`, + wantKubeAPIServerRequestHeaders: http.Header{ + "Impersonate-User": {"fire"}, + "Impersonate-Group": {"elements", "system:authenticated"}, + "Impersonate-Extra-Colors": {"red", "orange", "blue"}, + "Impersonate-Extra-Iam.gke.io%2fuser-Assertion": {"good", "stuff"}, + "Impersonate-Extra-User-Assertion.cloud.google.com": {"smaller", "things"}, + "Impersonate-Extra-Scopes.authorization.openshift.io": {"user:info", "user:full", "user:check-access"}, + "Impersonate-Extra-Alpha.kubernetes.io%2fidentity%2froles": {"a-role1", "a-role2"}, + "Impersonate-Extra-Alpha.kubernetes.io%2fidentity%2fproject%2fid": {"a-project-id"}, + "Impersonate-Extra-Alpha.kubernetes.io%2fidentity%2fproject%2fname": {"a-project-name"}, + "Impersonate-Extra-Alpha.kubernetes.io%2fidentity%2fuser%2fdomain%2fid": {"a-domain-id"}, + "Impersonate-Extra-Alpha.kubernetes.io%2fidentity%2fuser%2fdomain%2fname": {"a-domain-name"}, + "Impersonate-Extra-Original-User-Info.impersonation-Proxy.concierge.pinniped.dev": {`{"username":"test-admin","groups":["test-group2","system:masters","system:authenticated"]}`}, + "Authorization": {"Bearer some-service-account-token"}, + "User-Agent": {"test-agent"}, + "Accept": {"application/vnd.kubernetes.protobuf,application/json"}, + "Accept-Encoding": {"gzip"}, + "X-Forwarded-For": {"127.0.0.1"}, + }, + }, + { + name: "nested impersonation by admin users cannot impersonate UID", + clientCert: newClientCert(t, ca, "test-admin", []string{"system:masters", "test-group2"}), + clientImpersonateUser: rest.ImpersonationConfig{UserName: "some-other-username"}, + clientMutateHeaders: func(header http.Header) { + header["Impersonate-Uid"] = []string{"root"} + }, + kubeAPIServerClientBearerTokenFile: "required-to-be-set", + wantError: "Internal error occurred: invalid impersonation", + }, + { + name: "nested impersonation by admin users cannot impersonate UID header canonicalization", + clientCert: newClientCert(t, ca, "test-admin", []string{"system:masters", "test-group2"}), + clientImpersonateUser: rest.ImpersonationConfig{UserName: "some-other-username"}, + clientMutateHeaders: func(header http.Header) { + header["imPerSoNaTE-uid"] = []string{"magic"} + }, + kubeAPIServerClientBearerTokenFile: "required-to-be-set", + wantError: "Internal error occurred: invalid impersonation", + }, + { + name: "nested impersonation by admin users cannot use reserved key", + clientCert: newClientCert(t, ca, "test-admin", []string{"system:masters", "test-group2"}), + clientImpersonateUser: rest.ImpersonationConfig{ + UserName: "other-user-to-impersonate", + Groups: []string{"other-peeps"}, + Extra: map[string][]string{ + "key": {"good"}, + "something.impersonation-proxy.concierge.pinniped.dev": {"bad data"}, + }, + }, + kubeAPIServerClientBearerTokenFile: "required-to-be-set", + wantError: "Internal error occurred: unimplemented functionality - unable to act as current user", + }, + { + name: "nested impersonation by admin users cannot use invalid key", + clientCert: newClientCert(t, ca, "test-admin", []string{"system:masters", "test-group2"}), + clientImpersonateUser: rest.ImpersonationConfig{ + UserName: "panda", + Groups: []string{"other-peeps"}, + Extra: map[string][]string{ + "party~~time": {"danger"}, + }, + }, + kubeAPIServerClientBearerTokenFile: "required-to-be-set", + wantError: "Internal error occurred: unimplemented functionality - unable to act as current user", + }, + { + name: "nested impersonation by admin users can use uppercase key because impersonation is lossy", + clientCert: newClientCert(t, ca, "test-admin", []string{"system:masters", "test-group2"}), + clientImpersonateUser: rest.ImpersonationConfig{ + UserName: "panda", + Groups: []string{"other-peeps"}, + Extra: map[string][]string{ + "ROAR": {"tiger"}, // by the time our code sees this key, it is lowercased to "roar" + }, + }, + kubeAPIServerClientBearerTokenFile: "required-to-be-set", + wantKubeAPIServerRequestHeaders: http.Header{ + "Impersonate-User": {"panda"}, + "Impersonate-Group": {"other-peeps", "system:authenticated"}, + "Impersonate-Extra-Roar": {"tiger"}, + "Impersonate-Extra-Original-User-Info.impersonation-Proxy.concierge.pinniped.dev": {`{"username":"test-admin","groups":["test-group2","system:masters","system:authenticated"]}`}, + "Authorization": {"Bearer some-service-account-token"}, + "User-Agent": {"test-agent"}, + "Accept": {"application/vnd.kubernetes.protobuf,application/json"}, + "Accept-Encoding": {"gzip"}, + "X-Forwarded-For": {"127.0.0.1"}, + }, }, { name: "no bearer token file in Kube API server client config", @@ -212,17 +314,17 @@ func TestImpersonator(t *testing.T) { name: "header canonicalization user header", clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}), clientMutateHeaders: func(header http.Header) { - header.Set("imPerSonaTE-USer", "PANDA") + header["imPerSonaTE-USer"] = []string{"PANDA"} }, kubeAPIServerClientBearerTokenFile: "required-to-be-set", wantError: `users "PANDA" is forbidden: User "test-username" ` + - `cannot impersonate resource "users" in API group "" at the cluster scope: impersonation is not allowed or invalid verb`, + `cannot impersonate resource "users" in API group "" at the cluster scope`, }, { name: "header canonicalization future UID header", clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}), clientMutateHeaders: func(header http.Header) { - header.Set("imPerSonaTE-uid", "007") + header["imPerSonaTE-uid"] = []string{"007"} }, kubeAPIServerClientBearerTokenFile: "required-to-be-set", wantError: "Internal error occurred: invalid impersonation", @@ -231,7 +333,7 @@ func TestImpersonator(t *testing.T) { name: "future UID header", clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}), clientMutateHeaders: func(header http.Header) { - header.Set("Impersonate-Uid", "008") + header["Impersonate-Uid"] = []string{"008"} }, kubeAPIServerClientBearerTokenFile: "required-to-be-set", wantError: "Internal error occurred: invalid impersonation", @@ -239,8 +341,14 @@ func TestImpersonator(t *testing.T) { } for _, tt := range tests { tt := tt - // This is a serial test because the production code binds to the port. t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // we need to create this listener ourselves because the API server + // code treats (port == 0 && listener == nil) to mean "do nothing" + listener, port, err := genericoptions.CreateListener("", "127.0.0.1:0", net.ListenConfig{}) + require.NoError(t, err) + // After failing to start and after shutdown, the impersonator port should be available again. defer requireCanBindToPort(t, port) @@ -293,8 +401,17 @@ func TestImpersonator(t *testing.T) { } clientOpts := []kubeclient.Option{kubeclient.WithConfig(&testKubeAPIServerKubeconfig)} - // Create an impersonator. - runner, constructionErr := newInternal(port, certKeyContent, caContent, clientOpts, recOpts) + // Punch out just enough stuff to make New actually run without error. + recOpts := func(options *genericoptions.RecommendedOptions) { + options.Authentication.RemoteKubeConfigFileOptional = true + options.Authorization.RemoteKubeConfigFileOptional = true + options.CoreAPI = nil + options.Admission = nil + options.SecureServing.Listener = listener // use our listener with the dynamic port + } + + // Create an impersonator. Use an invalid port number to make sure our listener override works. + runner, constructionErr := newInternal(-1000, certKeyContent, caContent, clientOpts, recOpts) if len(tt.wantConstructionError) > 0 { require.EqualError(t, constructionErr, tt.wantConstructionError) require.Nil(t, runner) @@ -383,20 +500,30 @@ func TestImpersonatorHTTPHandler(t *testing.T) { } validURL, _ := url.Parse("http://pinniped.dev/blah") - newRequest := func(h http.Header, userInfo user.Info) *http.Request { + newRequest := func(h http.Header, userInfo user.Info, event *auditinternal.Event) *http.Request { ctx := context.Background() + if userInfo != nil { ctx = request.WithUser(ctx, userInfo) } - r, err := http.NewRequestWithContext(ctx, http.MethodGet, validURL.String(), nil) - require.NoError(t, err) - r.Header = h + + ae := &auditinternal.Event{Level: auditinternal.LevelMetadata} + if event != nil { + ae = event + } + ctx = request.WithAuditEvent(ctx, ae) + reqInfo := &request.RequestInfo{ IsResourceRequest: false, Path: validURL.Path, Verb: "get", } - r = r.WithContext(request.WithRequestInfo(ctx, reqInfo)) + ctx = request.WithRequestInfo(ctx, reqInfo) + + r, err := http.NewRequestWithContext(ctx, http.MethodGet, validURL.String(), nil) + require.NoError(t, err) + r.Header = h + return r } @@ -436,43 +563,123 @@ func TestImpersonatorHTTPHandler(t *testing.T) { }, { name: "Impersonate-User header already in request", - request: newRequest(map[string][]string{"Impersonate-User": {"some-user"}}, nil), + request: newRequest(map[string][]string{"Impersonate-User": {"some-user"}}, nil, nil), wantHTTPBody: `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Internal error occurred: invalid impersonation","reason":"InternalError","details":{"causes":[{"message":"invalid impersonation"}]},"code":500}` + "\n", wantHTTPStatus: http.StatusInternalServerError, }, { name: "Impersonate-Group header already in request", - request: newRequest(map[string][]string{"Impersonate-Group": {"some-group"}}, nil), + request: newRequest(map[string][]string{"Impersonate-Group": {"some-group"}}, nil, nil), wantHTTPBody: `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Internal error occurred: invalid impersonation","reason":"InternalError","details":{"causes":[{"message":"invalid impersonation"}]},"code":500}` + "\n", wantHTTPStatus: http.StatusInternalServerError, }, { name: "Impersonate-Extra header already in request", - request: newRequest(map[string][]string{"Impersonate-Extra-something": {"something"}}, nil), + request: newRequest(map[string][]string{"Impersonate-Extra-something": {"something"}}, nil, nil), wantHTTPBody: `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Internal error occurred: invalid impersonation","reason":"InternalError","details":{"causes":[{"message":"invalid impersonation"}]},"code":500}` + "\n", wantHTTPStatus: http.StatusInternalServerError, }, { name: "Impersonate-* header already in request", - request: newRequest(map[string][]string{"Impersonate-Something": {"some-newfangled-impersonate-header"}}, nil), + request: newRequest(map[string][]string{"Impersonate-Something": {"some-newfangled-impersonate-header"}}, nil, nil), wantHTTPBody: `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Internal error occurred: invalid impersonation","reason":"InternalError","details":{"causes":[{"message":"invalid impersonation"}]},"code":500}` + "\n", wantHTTPStatus: http.StatusInternalServerError, }, { name: "unexpected authorization header", - request: newRequest(map[string][]string{"Authorization": {"panda"}}, nil), + request: newRequest(map[string][]string{"Authorization": {"panda"}}, nil, nil), wantHTTPBody: `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Internal error occurred: invalid authorization header","reason":"InternalError","details":{"causes":[{"message":"invalid authorization header"}]},"code":500}` + "\n", wantHTTPStatus: http.StatusInternalServerError, }, { name: "missing user", - request: newRequest(map[string][]string{}, nil), + request: newRequest(map[string][]string{}, nil, nil), wantHTTPBody: `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Internal error occurred: invalid user","reason":"InternalError","details":{"causes":[{"message":"invalid user"}]},"code":500}` + "\n", wantHTTPStatus: http.StatusInternalServerError, }, { name: "unexpected UID", - request: newRequest(map[string][]string{}, &user.DefaultInfo{UID: "007"}), + request: newRequest(map[string][]string{}, &user.DefaultInfo{UID: "007"}, nil), + wantHTTPBody: `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Internal error occurred: unimplemented functionality - unable to act as current user","reason":"InternalError","details":{"causes":[{"message":"unimplemented functionality - unable to act as current user"}]},"code":500}` + "\n", + wantHTTPStatus: http.StatusInternalServerError, + }, + { + name: "authenticated user but missing audit event", + request: func() *http.Request { + req := newRequest(map[string][]string{ + "User-Agent": {"test-user-agent"}, + "Connection": {"Upgrade"}, + "Upgrade": {"some-upgrade"}, + "Other-Header": {"test-header-value-1"}, + }, &user.DefaultInfo{ + Name: testUser, + Groups: testGroups, + Extra: testExtra, + }, nil) + ctx := request.WithAuditEvent(req.Context(), nil) + req = req.WithContext(ctx) + return req + }(), + wantHTTPBody: `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Internal error occurred: invalid audit event","reason":"InternalError","details":{"causes":[{"message":"invalid audit event"}]},"code":500}` + "\n", + wantHTTPStatus: http.StatusInternalServerError, + }, + { + name: "authenticated user with upper case extra", + request: newRequest(map[string][]string{ + "User-Agent": {"test-user-agent"}, + "Connection": {"Upgrade"}, + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Content-Length": {"some-length"}, + "Other-Header": {"test-header-value-1"}, + }, &user.DefaultInfo{ + Name: testUser, + Groups: testGroups, + Extra: map[string][]string{ + "valid-key": {"valid-value"}, + "Invalid-key": {"still-valid-value"}, + }, + }, nil), + wantHTTPBody: `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Internal error occurred: unimplemented functionality - unable to act as current user","reason":"InternalError","details":{"causes":[{"message":"unimplemented functionality - unable to act as current user"}]},"code":500}` + "\n", + wantHTTPStatus: http.StatusInternalServerError, + }, + { + name: "authenticated user with upper case extra across multiple lines", + request: newRequest(map[string][]string{ + "User-Agent": {"test-user-agent"}, + "Connection": {"Upgrade"}, + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Content-Length": {"some-length"}, + "Other-Header": {"test-header-value-1"}, + }, &user.DefaultInfo{ + Name: testUser, + Groups: testGroups, + Extra: map[string][]string{ + "valid-key": {"valid-value"}, + "valid-data\nInvalid-key": {"still-valid-value"}, + }, + }, nil), + wantHTTPBody: `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Internal error occurred: unimplemented functionality - unable to act as current user","reason":"InternalError","details":{"causes":[{"message":"unimplemented functionality - unable to act as current user"}]},"code":500}` + "\n", + wantHTTPStatus: http.StatusInternalServerError, + }, + { + name: "authenticated user with reserved extra key", + request: newRequest(map[string][]string{ + "User-Agent": {"test-user-agent"}, + "Connection": {"Upgrade"}, + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Content-Length": {"some-length"}, + "Other-Header": {"test-header-value-1"}, + }, &user.DefaultInfo{ + Name: testUser, + Groups: testGroups, + Extra: map[string][]string{ + "valid-key": {"valid-value"}, + "foo.impersonation-proxy.concierge.pinniped.dev": {"still-valid-value"}, + }, + }, nil), wantHTTPBody: `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Internal error occurred: unimplemented functionality - unable to act as current user","reason":"InternalError","details":{"causes":[{"message":"unimplemented functionality - unable to act as current user"}]},"code":500}` + "\n", wantHTTPStatus: http.StatusInternalServerError, }, @@ -492,7 +699,7 @@ func TestImpersonatorHTTPHandler(t *testing.T) { Name: testUser, Groups: testGroups, Extra: testExtra, - }), + }, nil), wantKubeAPIServerRequestHeaders: map[string][]string{ "Authorization": {"Bearer some-service-account-token"}, "Impersonate-Extra-Extra-1": {"some", "extra", "stuff"}, @@ -510,6 +717,318 @@ func TestImpersonatorHTTPHandler(t *testing.T) { wantHTTPBody: "successful proxied response", wantHTTPStatus: http.StatusOK, }, + { + name: "authenticated gke user", + request: newRequest(map[string][]string{ + "User-Agent": {"test-user-agent"}, + "Accept": {"some-accepted-format"}, + "Accept-Encoding": {"some-accepted-encoding"}, + "Connection": {"Upgrade"}, // the value "Upgrade" is handled in a special way by `httputil.NewSingleHostReverseProxy` + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Content-Length": {"some-length"}, + "Other-Header": {"test-header-value-1"}, // this header will be passed through + }, &user.DefaultInfo{ + Name: "username@company.com", + Groups: []string{"system:authenticated"}, + Extra: map[string][]string{ + // make sure we can handle these keys + "iam.gke.io/user-assertion": {"ABC"}, + "user-assertion.cloud.google.com": {"XYZ"}, + }, + }, nil), + wantKubeAPIServerRequestHeaders: map[string][]string{ + "Authorization": {"Bearer some-service-account-token"}, + "Impersonate-Extra-Iam.gke.io%2fuser-Assertion": {"ABC"}, + "Impersonate-Extra-User-Assertion.cloud.google.com": {"XYZ"}, + "Impersonate-Group": {"system:authenticated"}, + "Impersonate-User": {"username@company.com"}, + "User-Agent": {"test-user-agent"}, + "Accept": {"some-accepted-format"}, + "Accept-Encoding": {"some-accepted-encoding"}, + "Connection": {"Upgrade"}, + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Other-Header": {"test-header-value-1"}, + }, + wantHTTPBody: "successful proxied response", + wantHTTPStatus: http.StatusOK, + }, + { + name: "authenticated openshift/openstack user", + request: newRequest(map[string][]string{ + "User-Agent": {"test-user-agent"}, + "Accept": {"some-accepted-format"}, + "Accept-Encoding": {"some-accepted-encoding"}, + "Connection": {"Upgrade"}, // the value "Upgrade" is handled in a special way by `httputil.NewSingleHostReverseProxy` + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Content-Length": {"some-length"}, + "Other-Header": {"test-header-value-1"}, // this header will be passed through + }, &user.DefaultInfo{ + Name: "kube:admin", + // both of these auth stacks set UID but we cannot handle it today + // UID: "user-id", + Groups: []string{"system:cluster-admins", "system:authenticated"}, + Extra: map[string][]string{ + // openshift + "scopes.authorization.openshift.io": {"user:info", "user:full"}, + + // openstack + "alpha.kubernetes.io/identity/roles": {"role1", "role2"}, + "alpha.kubernetes.io/identity/project/id": {"project-id"}, + "alpha.kubernetes.io/identity/project/name": {"project-name"}, + "alpha.kubernetes.io/identity/user/domain/id": {"domain-id"}, + "alpha.kubernetes.io/identity/user/domain/name": {"domain-name"}, + }, + }, nil), + wantKubeAPIServerRequestHeaders: map[string][]string{ + "Authorization": {"Bearer some-service-account-token"}, + "Impersonate-Extra-Scopes.authorization.openshift.io": {"user:info", "user:full"}, + "Impersonate-Extra-Alpha.kubernetes.io%2fidentity%2froles": {"role1", "role2"}, + "Impersonate-Extra-Alpha.kubernetes.io%2fidentity%2fproject%2fid": {"project-id"}, + "Impersonate-Extra-Alpha.kubernetes.io%2fidentity%2fproject%2fname": {"project-name"}, + "Impersonate-Extra-Alpha.kubernetes.io%2fidentity%2fuser%2fdomain%2fid": {"domain-id"}, + "Impersonate-Extra-Alpha.kubernetes.io%2fidentity%2fuser%2fdomain%2fname": {"domain-name"}, + "Impersonate-Group": {"system:cluster-admins", "system:authenticated"}, + "Impersonate-User": {"kube:admin"}, + "User-Agent": {"test-user-agent"}, + "Accept": {"some-accepted-format"}, + "Accept-Encoding": {"some-accepted-encoding"}, + "Connection": {"Upgrade"}, + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Other-Header": {"test-header-value-1"}, + }, + wantHTTPBody: "successful proxied response", + wantHTTPStatus: http.StatusOK, + }, + { + name: "authenticated user with almost reserved key", + request: newRequest(map[string][]string{ + "User-Agent": {"test-user-agent"}, + "Accept": {"some-accepted-format"}, + "Accept-Encoding": {"some-accepted-encoding"}, + "Connection": {"Upgrade"}, // the value "Upgrade" is handled in a special way by `httputil.NewSingleHostReverseProxy` + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Content-Length": {"some-length"}, + "Other-Header": {"test-header-value-1"}, // this header will be passed through + }, &user.DefaultInfo{ + Name: "username@company.com", + Groups: []string{"system:authenticated"}, + Extra: map[string][]string{ + "foo.iimpersonation-proxy.concierge.pinniped.dev": {"still-valid-value"}, + }, + }, nil), + wantKubeAPIServerRequestHeaders: map[string][]string{ + "Authorization": {"Bearer some-service-account-token"}, + "Impersonate-Extra-Foo.iimpersonation-Proxy.concierge.pinniped.dev": {"still-valid-value"}, + "Impersonate-Group": {"system:authenticated"}, + "Impersonate-User": {"username@company.com"}, + "User-Agent": {"test-user-agent"}, + "Accept": {"some-accepted-format"}, + "Accept-Encoding": {"some-accepted-encoding"}, + "Connection": {"Upgrade"}, + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Other-Header": {"test-header-value-1"}, + }, + wantHTTPBody: "successful proxied response", + wantHTTPStatus: http.StatusOK, + }, + { + name: "authenticated user with almost reserved key and nested impersonation", + request: newRequest(map[string][]string{ + "User-Agent": {"test-user-agent"}, + "Accept": {"some-accepted-format"}, + "Accept-Encoding": {"some-accepted-encoding"}, + "Connection": {"Upgrade"}, // the value "Upgrade" is handled in a special way by `httputil.NewSingleHostReverseProxy` + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Content-Length": {"some-length"}, + "Other-Header": {"test-header-value-1"}, // this header will be passed through + }, &user.DefaultInfo{ + Name: "username@company.com", + Groups: []string{"system:authenticated"}, + Extra: map[string][]string{ + "original-user-info.impersonation-proxyy.concierge.pinniped.dev": {"log confusion stuff here"}, + }, + }, + &auditinternal.Event{ + User: authenticationv1.UserInfo{ + Username: "panda", + UID: "0x001", + Groups: []string{"bears", "friends"}, + Extra: map[string]authenticationv1.ExtraValue{ + "original-user-info.impersonation-proxy.concierge.pinniped.dev": {"this is allowed"}, + }, + }, + ImpersonatedUser: &authenticationv1.UserInfo{}, + }, + ), + wantKubeAPIServerRequestHeaders: map[string][]string{ + "Authorization": {"Bearer some-service-account-token"}, + "Impersonate-Extra-Original-User-Info.impersonation-Proxyy.concierge.pinniped.dev": {"log confusion stuff here"}, + "Impersonate-Extra-Original-User-Info.impersonation-Proxy.concierge.pinniped.dev": {`{"username":"panda","uid":"0x001","groups":["bears","friends"],"extra":{"original-user-info.impersonation-proxy.concierge.pinniped.dev":["this is allowed"]}}`}, + "Impersonate-Group": {"system:authenticated"}, + "Impersonate-User": {"username@company.com"}, + "User-Agent": {"test-user-agent"}, + "Accept": {"some-accepted-format"}, + "Accept-Encoding": {"some-accepted-encoding"}, + "Connection": {"Upgrade"}, + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Other-Header": {"test-header-value-1"}, + }, + wantHTTPBody: "successful proxied response", + wantHTTPStatus: http.StatusOK, + }, + { + name: "authenticated user with nested impersonation", + request: newRequest(map[string][]string{ + "User-Agent": {"test-user-agent"}, + "Accept": {"some-accepted-format"}, + "Accept-Encoding": {"some-accepted-encoding"}, + "Connection": {"Upgrade"}, // the value "Upgrade" is handled in a special way by `httputil.NewSingleHostReverseProxy` + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Content-Length": {"some-length"}, + "Other-Header": {"test-header-value-1"}, // this header will be passed through + }, &user.DefaultInfo{ + Name: testUser, + Groups: testGroups, + Extra: testExtra, + }, + &auditinternal.Event{ + User: authenticationv1.UserInfo{ + Username: "panda", + UID: "0x001", + Groups: []string{"bears", "friends"}, + Extra: map[string]authenticationv1.ExtraValue{ + "assertion": {"sha", "md5"}, + "req-id": {"0123"}, + }, + }, + ImpersonatedUser: &authenticationv1.UserInfo{}, + }, + ), + wantKubeAPIServerRequestHeaders: map[string][]string{ + "Authorization": {"Bearer some-service-account-token"}, + "Impersonate-Extra-Extra-1": {"some", "extra", "stuff"}, + "Impersonate-Extra-Extra-2": {"some", "more", "extra", "stuff"}, + "Impersonate-Group": {"test-group-1", "test-group-2"}, + "Impersonate-User": {"test-user"}, + "User-Agent": {"test-user-agent"}, + "Accept": {"some-accepted-format"}, + "Accept-Encoding": {"some-accepted-encoding"}, + "Connection": {"Upgrade"}, + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Other-Header": {"test-header-value-1"}, + "Impersonate-Extra-Original-User-Info.impersonation-Proxy.concierge.pinniped.dev": {`{"username":"panda","uid":"0x001","groups":["bears","friends"],"extra":{"assertion":["sha","md5"],"req-id":["0123"]}}`}, + }, + wantHTTPBody: "successful proxied response", + wantHTTPStatus: http.StatusOK, + }, + { + name: "authenticated gke user with nested impersonation", + request: newRequest(map[string][]string{ + "User-Agent": {"test-user-agent"}, + "Accept": {"some-accepted-format"}, + "Accept-Encoding": {"some-accepted-encoding"}, + "Connection": {"Upgrade"}, // the value "Upgrade" is handled in a special way by `httputil.NewSingleHostReverseProxy` + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Content-Length": {"some-length"}, + "Other-Header": {"test-header-value-1"}, // this header will be passed through + }, &user.DefaultInfo{ + Name: testUser, + Groups: testGroups, + Extra: testExtra, + }, + &auditinternal.Event{ + User: authenticationv1.UserInfo{ + Username: "username@company.com", + Groups: []string{"system:authenticated"}, + Extra: map[string]authenticationv1.ExtraValue{ + // make sure we can handle these keys + "iam.gke.io/user-assertion": {"ABC"}, + "user-assertion.cloud.google.com": {"999"}, + }, + }, + ImpersonatedUser: &authenticationv1.UserInfo{}, + }, + ), + wantKubeAPIServerRequestHeaders: map[string][]string{ + "Authorization": {"Bearer some-service-account-token"}, + "Impersonate-Extra-Extra-1": {"some", "extra", "stuff"}, + "Impersonate-Extra-Extra-2": {"some", "more", "extra", "stuff"}, + "Impersonate-Group": {"test-group-1", "test-group-2"}, + "Impersonate-User": {"test-user"}, + "User-Agent": {"test-user-agent"}, + "Accept": {"some-accepted-format"}, + "Accept-Encoding": {"some-accepted-encoding"}, + "Connection": {"Upgrade"}, + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Other-Header": {"test-header-value-1"}, + "Impersonate-Extra-Original-User-Info.impersonation-Proxy.concierge.pinniped.dev": {`{"username":"username@company.com","groups":["system:authenticated"],"extra":{"iam.gke.io/user-assertion":["ABC"],"user-assertion.cloud.google.com":["999"]}}`}, + }, + wantHTTPBody: "successful proxied response", + wantHTTPStatus: http.StatusOK, + }, + { + name: "authenticated user with nested impersonation of gke user", + request: newRequest(map[string][]string{ + "User-Agent": {"test-user-agent"}, + "Accept": {"some-accepted-format"}, + "Accept-Encoding": {"some-accepted-encoding"}, + "Connection": {"Upgrade"}, // the value "Upgrade" is handled in a special way by `httputil.NewSingleHostReverseProxy` + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Content-Length": {"some-length"}, + "Other-Header": {"test-header-value-1"}, // this header will be passed through + }, &user.DefaultInfo{ + Name: "username@company.com", + Groups: []string{"system:authenticated"}, + Extra: map[string][]string{ + // make sure we can handle these keys + "iam.gke.io/user-assertion": {"DEF"}, + "user-assertion.cloud.google.com": {"XYZ"}, + }, + }, + &auditinternal.Event{ + User: authenticationv1.UserInfo{ + Username: "panda", + UID: "0x001", + Groups: []string{"bears", "friends"}, + Extra: map[string]authenticationv1.ExtraValue{ + "assertion": {"sha", "md5"}, + "req-id": {"0123"}, + }, + }, + ImpersonatedUser: &authenticationv1.UserInfo{}, + }, + ), + wantKubeAPIServerRequestHeaders: map[string][]string{ + "Authorization": {"Bearer some-service-account-token"}, + "Impersonate-Extra-Iam.gke.io%2fuser-Assertion": {"DEF"}, + "Impersonate-Extra-User-Assertion.cloud.google.com": {"XYZ"}, + "Impersonate-Group": {"system:authenticated"}, + "Impersonate-User": {"username@company.com"}, + "User-Agent": {"test-user-agent"}, + "Accept": {"some-accepted-format"}, + "Accept-Encoding": {"some-accepted-encoding"}, + "Connection": {"Upgrade"}, + "Upgrade": {"some-upgrade"}, + "Content-Type": {"some-type"}, + "Other-Header": {"test-header-value-1"}, + "Impersonate-Extra-Original-User-Info.impersonation-Proxy.concierge.pinniped.dev": {`{"username":"panda","uid":"0x001","groups":["bears","friends"],"extra":{"assertion":["sha","md5"],"req-id":["0123"]}}`}, + }, + wantHTTPBody: "successful proxied response", + wantHTTPStatus: http.StatusOK, + }, { name: "user is authenticated but the kube API request returns an error", request: newRequest(map[string][]string{ @@ -518,7 +1037,7 @@ func TestImpersonatorHTTPHandler(t *testing.T) { Name: testUser, Groups: testGroups, Extra: testExtra, - }), + }, nil), kubeAPIServerStatusCode: http.StatusNotFound, wantKubeAPIServerRequestHeaders: map[string][]string{ "Accept-Encoding": {"gzip"}, // because the rest client used in this test does not disable compression @@ -623,6 +1142,7 @@ type clientCert struct { } func newClientCert(t *testing.T, ca *certauthority.CA, username string, groups []string) *clientCert { + t.Helper() certPEM, keyPEM, err := ca.IssueClientCertPEM(username, groups, time.Hour) require.NoError(t, err) return &clientCert{ @@ -632,7 +1152,113 @@ func newClientCert(t *testing.T, ca *certauthority.CA, username string, groups [ } func requireCanBindToPort(t *testing.T, port int) { + t.Helper() ln, _, listenErr := genericoptions.CreateListener("", "0.0.0.0:"+strconv.Itoa(port), net.ListenConfig{}) require.NoError(t, listenErr) require.NoError(t, ln.Close()) } + +func Test_deleteKnownImpersonationHeaders(t *testing.T) { + tests := []struct { + name string + headers, want http.Header + }{ + { + name: "no impersonation", + headers: map[string][]string{ + "a": {"b"}, + "Accept-Encoding": {"gzip"}, + "User-Agent": {"test-user-agent"}, + }, + want: map[string][]string{ + "a": {"b"}, + "Accept-Encoding": {"gzip"}, + "User-Agent": {"test-user-agent"}, + }, + }, + { + name: "impersonate user header is dropped", + headers: map[string][]string{ + "a": {"b"}, + "Impersonate-User": {"panda"}, + "Accept-Encoding": {"gzip"}, + "User-Agent": {"test-user-agent"}, + }, + want: map[string][]string{ + "a": {"b"}, + "Accept-Encoding": {"gzip"}, + "User-Agent": {"test-user-agent"}, + }, + }, + { + name: "all known impersonate headers are dropped", + headers: map[string][]string{ + "Accept-Encoding": {"gzip"}, + "Authorization": {"Bearer some-service-account-token"}, + "Impersonate-Extra-Extra-1": {"some", "extra", "stuff"}, + "Impersonate-Extra-Extra-2": {"some", "more", "extra", "stuff"}, + "Impersonate-Group": {"test-group-1", "test-group-2"}, + "Impersonate-User": {"test-user"}, + "User-Agent": {"test-user-agent"}, + }, + want: map[string][]string{ + "Accept-Encoding": {"gzip"}, + "Authorization": {"Bearer some-service-account-token"}, + "User-Agent": {"test-user-agent"}, + }, + }, + { + name: "future UID header is not dropped", + headers: map[string][]string{ + "Accept-Encoding": {"gzip"}, + "Authorization": {"Bearer some-service-account-token"}, + "Impersonate-Extra-Extra-1": {"some", "extra", "stuff"}, + "Impersonate-Extra-Extra-2": {"some", "more", "extra", "stuff"}, + "Impersonate-Group": {"test-group-1", "test-group-2"}, + "Impersonate-User": {"test-user"}, + "Impersonate-Uid": {"008"}, + "User-Agent": {"test-user-agent"}, + }, + want: map[string][]string{ + "Accept-Encoding": {"gzip"}, + "Authorization": {"Bearer some-service-account-token"}, + "User-Agent": {"test-user-agent"}, + "Impersonate-Uid": {"008"}, + }, + }, + { + name: "future UID header is not dropped, no other headers", + headers: map[string][]string{ + "Impersonate-Uid": {"009"}, + }, + want: map[string][]string{ + "Impersonate-Uid": {"009"}, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + inputReq := (&http.Request{Header: tt.headers}).WithContext(context.Background()) + inputReqCopy := inputReq.Clone(inputReq.Context()) + + delegate := http.HandlerFunc(func(w http.ResponseWriter, outputReq *http.Request) { + require.Nil(t, w) + + // assert only headers mutated + outputReqCopy := outputReq.Clone(outputReq.Context()) + outputReqCopy.Header = tt.headers + require.Equal(t, inputReqCopy, outputReqCopy) + + require.Equal(t, tt.want, outputReq.Header) + + if ensureNoImpersonationHeaders(inputReq) == nil { + require.True(t, inputReq == outputReq, "expect req to passed through when no modification needed") + } + }) + + deleteKnownImpersonationHeaders(delegate).ServeHTTP(nil, inputReq) + require.Equal(t, inputReqCopy, inputReq) // assert no mutation occurred + }) + } +} diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index 5a43751e..19eb1731 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go @@ -16,6 +16,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -25,6 +26,7 @@ import ( "gopkg.in/square/go-jose.v2/jwt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/user" @@ -345,15 +347,6 @@ func TestController(t *testing.T) { return // end of test unless we wanted to run tests on the resulting authenticator from the cache } - // The implementation of AuthenticateToken() that we use waits 10 seconds after creation to - // perform OIDC discovery. Therefore, the JWTAuthenticator is not functional for the first 10 - // seconds. We sleep for 13 seconds in this unit test to give a little bit of cushion to that 10 - // second delay. - // - // We should get rid of this 10 second delay. See - // https://github.com/vmware-tanzu/pinniped/issues/260. - time.Sleep(time.Second * 13) - // We expected the cache to have an entry, so pull that entry from the cache and test it. expectedCacheKey := authncache.Key{ APIGroup: auth1alpha1.GroupName, @@ -428,7 +421,17 @@ func TestController(t *testing.T) { tt.wantUsernameClaim, username, ) - rsp, authenticated, err := cachedAuthenticator.AuthenticateToken(context.Background(), jwt) + + // Loop for a while here to allow the underlying OIDC authenticator to initialize itself asynchronously. + var ( + rsp *authenticator.Response + authenticated bool + err error + ) + _ = wait.PollImmediate(10*time.Millisecond, 5*time.Second, func() (bool, error) { + rsp, authenticated, err = cachedAuthenticator.AuthenticateToken(context.Background(), jwt) + return !isNotInitialized(err), nil + }) if test.wantErrorRegexp != "" { require.Error(t, err) require.Regexp(t, test.wantErrorRegexp, err.Error()) @@ -443,6 +446,12 @@ func TestController(t *testing.T) { } } +// isNotInitialized checks if the error is the internally-defined "oidc: authenticator not initialized" error from +// the underlying OIDC authenticator, which is initialized asynchronously. +func isNotInitialized(err error) bool { + return err != nil && strings.Contains(err.Error(), "authenticator not initialized") +} + func testTableForAuthenticateTokenTests( t *testing.T, goodRSASigningKey *rsa.PrivateKey, diff --git a/internal/kubeclient/copied.go b/internal/kubeclient/copied.go index a822deaa..3b4efd9b 100644 --- a/internal/kubeclient/copied.go +++ b/internal/kubeclient/copied.go @@ -6,7 +6,6 @@ package kubeclient import ( "bytes" "encoding/hex" - "fmt" "net/url" "k8s.io/apimachinery/pkg/runtime/schema" @@ -32,39 +31,17 @@ func defaultServerUrlFor(config *restclient.Config) (*url.URL, string, error) { return restclient.DefaultServerURL(host, config.APIPath, schema.GroupVersion{}, defaultTLS) } -// truncateBody was copied from k8s.io/client-go/rest/request.go -// ...except i changed klog invocations to analogous plog invocations -// -// truncateBody decides if the body should be truncated, based on the glog Verbosity. -func truncateBody(body string) string { - max := 0 - switch { - case plog.Enabled(plog.LevelAll): - return body - case plog.Enabled(plog.LevelTrace): - max = 10240 - case plog.Enabled(plog.LevelDebug): - max = 1024 - } - - if len(body) <= max { - return body - } - - return body[:max] + fmt.Sprintf(" [truncated %d chars]", len(body)-max) -} - // glogBody logs a body output that could be either JSON or protobuf. It explicitly guards against // allocating a new string for the body output unless necessary. Uses a simple heuristic to determine // whether the body is printable. func glogBody(prefix string, body []byte) { - if plog.Enabled(plog.LevelDebug) { + if plog.Enabled(plog.LevelAll) { if bytes.IndexFunc(body, func(r rune) bool { return r < 0x0a }) != -1 { - plog.Debug(prefix, "body", truncateBody(hex.Dump(body))) + plog.Debug(prefix, "body", hex.Dump(body)) } else { - plog.Debug(prefix, "body", truncateBody(string(body))) + plog.Debug(prefix, "body", string(body)) } } } diff --git a/test/integration/category_test.go b/test/integration/category_test.go index 592ccb24..253d0d62 100644 --- a/test/integration/category_test.go +++ b/test/integration/category_test.go @@ -65,7 +65,10 @@ func requireCleanKubectlStderr(t *testing.T, stderr string) { if strings.Contains(line, "Throttling request took") { continue } - require.Failf(t, "unexpected kubectl stderr", "kubectl produced unexpected stderr output:\n%s\n\n", stderr) + if strings.Contains(line, "due to client-side throttling, not priority and fairness") { + continue + } + require.Failf(t, "unexpected kubectl stderr", "kubectl produced unexpected stderr:\n%s\n\n", stderr) return } } diff --git a/test/integration/concierge_impersonation_proxy_test.go b/test/integration/concierge_impersonation_proxy_test.go index 372f7ca4..97a328ac 100644 --- a/test/integration/concierge_impersonation_proxy_test.go +++ b/test/integration/concierge_impersonation_proxy_test.go @@ -26,7 +26,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/net/http2" - v1 "k8s.io/api/authorization/v1" + authenticationv1 "k8s.io/api/authentication/v1" + authorizationv1 "k8s.io/api/authorization/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -34,9 +35,14 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/watch" + "k8s.io/apiserver/pkg/authentication/authenticator" + "k8s.io/apiserver/pkg/authentication/request/bearertoken" + "k8s.io/apiserver/pkg/authentication/serviceaccount" k8sinformers "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + "k8s.io/client-go/transport" + "k8s.io/client-go/util/keyutil" "sigs.k8s.io/yaml" conciergev1alpha "go.pinniped.dev/generated/latest/apis/concierge/config/v1alpha1" @@ -44,6 +50,7 @@ import ( loginv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/login/v1alpha1" pinnipedconciergeclientset "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned" "go.pinniped.dev/internal/concierge/impersonator" + "go.pinniped.dev/internal/httputil/roundtripper" "go.pinniped.dev/internal/kubeclient" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/test/library" @@ -102,23 +109,13 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl // The address of the ClusterIP service that points at the impersonation proxy's port (used when there is no load balancer). proxyServiceEndpoint := fmt.Sprintf("%s-proxy.%s.svc.cluster.local", env.ConciergeAppName, env.ConciergeNamespace) - newImpersonationProxyClientWithCredentials := func(credentials *loginv1alpha1.ClusterCredential, impersonationProxyURL string, impersonationProxyCACertPEM []byte, doubleImpersonateUser string) *kubeclient.Client { - kubeconfig := impersonationProxyRestConfig(credentials, impersonationProxyURL, impersonationProxyCACertPEM, doubleImpersonateUser) - if !clusterSupportsLoadBalancers { - // Only if there is no possibility to send traffic through a load balancer, then send the traffic through the Squid proxy. - // Prefer to go through a load balancer because that's how the impersonator is intended to be used in the real world. - kubeconfig.Proxy = kubeconfigProxyFunc(t, env.Proxy) - } - return library.NewKubeclient(t, kubeconfig) - } - - newAnonymousImpersonationProxyClient := func(impersonationProxyURL string, impersonationProxyCACertPEM []byte, doubleImpersonateUser string) *kubeclient.Client { - emptyCredentials := &loginv1alpha1.ClusterCredential{} - return newImpersonationProxyClientWithCredentials(emptyCredentials, impersonationProxyURL, impersonationProxyCACertPEM, doubleImpersonateUser) - } - - var mostRecentTokenCredentialRequestResponse *loginv1alpha1.TokenCredentialRequest + var ( + mostRecentTokenCredentialRequestResponse *loginv1alpha1.TokenCredentialRequest + mostRecentTokenCredentialRequestResponseLock sync.Mutex + ) refreshCredential := func(t *testing.T, impersonationProxyURL string, impersonationProxyCACertPEM []byte) *loginv1alpha1.ClusterCredential { + mostRecentTokenCredentialRequestResponseLock.Lock() + defer mostRecentTokenCredentialRequestResponseLock.Unlock() if mostRecentTokenCredentialRequestResponse == nil || credentialAlmostExpired(t, mostRecentTokenCredentialRequestResponse) { var err error // Make a TokenCredentialRequest. This can either return a cert signed by the Kube API server's CA (e.g. on kind) @@ -132,7 +129,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl // what would normally happen when a user is using a kubeconfig where the server is the impersonation proxy, // so it more closely simulates the normal use case, and also because we want this to work on AKS clusters // which do not allow anonymous requests. - client := newAnonymousImpersonationProxyClient(impersonationProxyURL, impersonationProxyCACertPEM, "").PinnipedConcierge + client := newAnonymousImpersonationProxyClient(t, impersonationProxyURL, impersonationProxyCACertPEM, nil).PinnipedConcierge require.Eventually(t, func() bool { mostRecentTokenCredentialRequestResponse, err = createTokenCredentialRequest(credentialRequestSpecWithWorkingCredentials, client) if err != nil { @@ -154,19 +151,6 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl return mostRecentTokenCredentialRequestResponse.Status.Credential } - impersonationProxyViaSquidKubeClientWithoutCredential := func() kubernetes.Interface { - proxyURL := "https://" + proxyServiceEndpoint - kubeconfig := impersonationProxyRestConfig(&loginv1alpha1.ClusterCredential{}, proxyURL, nil, "") - kubeconfig.Proxy = kubeconfigProxyFunc(t, env.Proxy) - return library.NewKubeclient(t, kubeconfig).Kubernetes - } - - newImpersonationProxyClient := func(t *testing.T, impersonationProxyURL string, impersonationProxyCACertPEM []byte, doubleImpersonateUser string) *kubeclient.Client { - refreshedCredentials := refreshCredential(t, impersonationProxyURL, impersonationProxyCACertPEM).DeepCopy() - refreshedCredentials.Token = "not a valid token" // demonstrates that client certs take precedence over tokens by setting both on the requests - return newImpersonationProxyClientWithCredentials(refreshedCredentials, impersonationProxyURL, impersonationProxyCACertPEM, doubleImpersonateUser) - } - oldConfigMap, err := adminClient.CoreV1().ConfigMaps(env.ConciergeNamespace).Get(ctx, impersonationProxyConfigMapName(env), metav1.GetOptions{}) if !k8serrors.IsNotFound(err) { require.NoError(t, err) // other errors aside from NotFound are unexpected @@ -175,7 +159,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl } // At the end of the test, clean up the ConfigMap. t.Cleanup(func() { - ctx, cancel = context.WithTimeout(context.Background(), 2*time.Minute) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() // Delete any version that was created by this test. @@ -249,7 +233,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl }, 10*time.Second, 500*time.Millisecond) // Check that we can't use the impersonation proxy to execute kubectl commands yet. - _, err = impersonationProxyViaSquidKubeClientWithoutCredential().CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) + _, err = impersonationProxyViaSquidKubeClientWithoutCredential(t, proxyServiceEndpoint).CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) isErr, message := isServiceUnavailableViaSquidError(err, proxyServiceEndpoint) require.Truef(t, isErr, "wanted error %q to be service unavailable via squid error, but: %s", err, message) @@ -279,7 +263,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl // so we don't have to keep repeating them. // This client performs TLS checks, so it also provides test coverage that the impersonation proxy server is generating TLS certs correctly. impersonationProxyKubeClient := func(t *testing.T) kubernetes.Interface { - return newImpersonationProxyClient(t, impersonationProxyURL, impersonationProxyCACertPEM, "").Kubernetes + return newImpersonationProxyClient(t, impersonationProxyURL, impersonationProxyCACertPEM, nil, refreshCredential).Kubernetes } t.Run("positive tests", func(t *testing.T) { @@ -289,7 +273,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl rbacv1.RoleRef{Kind: "ClusterRole", APIGroup: rbacv1.GroupName, Name: "edit"}, ) // Wait for the above RBAC rule to take effect. - library.WaitForUserToHaveAccess(t, env.TestUser.ExpectedUsername, []string{}, &v1.ResourceAttributes{ + library.WaitForUserToHaveAccess(t, env.TestUser.ExpectedUsername, []string{}, &authorizationv1.ResourceAttributes{ Verb: "get", Group: "", Version: "v1", Resource: "namespaces", }) @@ -323,12 +307,13 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl } t.Run("kubectl port-forward and keeping the connection open for over a minute (non-idle)", func(t *testing.T) { + t.Parallel() kubeconfigPath, envVarsWithProxy, _ := getImpersonationKubeconfig(t, env, impersonationProxyURL, impersonationProxyCACertPEM, credentialRequestSpecWithWorkingCredentials.Authenticator) // Run the kubectl port-forward command. timeout, cancelFunc := context.WithTimeout(ctx, 2*time.Minute) defer cancelFunc() - portForwardCmd, _, portForwardStderr := kubectlCommand(timeout, t, kubeconfigPath, envVarsWithProxy, "port-forward", "--namespace", env.ConciergeNamespace, conciergePod.Name, "8443:8443") + portForwardCmd, _, portForwardStderr := kubectlCommand(timeout, t, kubeconfigPath, envVarsWithProxy, "port-forward", "--namespace", env.ConciergeNamespace, conciergePod.Name, "10443:8443") portForwardCmd.Env = envVarsWithProxy // Start, but don't wait for the command to finish. @@ -348,7 +333,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl defer cancelFunc() startTime := time.Now() for time.Now().Before(startTime.Add(70 * time.Second)) { - curlCmd := exec.CommandContext(timeout, "curl", "-k", "-sS", "https://127.0.0.1:8443") // -sS turns off the progressbar but still prints errors + curlCmd := exec.CommandContext(timeout, "curl", "-k", "-sS", "https://127.0.0.1:10443") // -sS turns off the progressbar but still prints errors curlCmd.Stdout = &curlStdOut curlCmd.Stderr = &curlStdErr curlErr := curlCmd.Run() @@ -364,7 +349,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl // curl the endpoint once more, once 70 seconds has elapsed, to make sure the connection is still open. timeout, cancelFunc = context.WithTimeout(ctx, 30*time.Second) defer cancelFunc() - curlCmd := exec.CommandContext(timeout, "curl", "-k", "-sS", "https://127.0.0.1:8443") // -sS turns off the progressbar but still prints errors + curlCmd := exec.CommandContext(timeout, "curl", "-k", "-sS", "https://127.0.0.1:10443") // -sS turns off the progressbar but still prints errors curlCmd.Stdout = &curlStdOut curlCmd.Stderr = &curlStdErr curlErr := curlCmd.Run() @@ -376,16 +361,17 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl } // We expect this to 403, but all we care is that it gets through. require.NoError(t, curlErr) - require.Contains(t, curlStdOut.String(), "\"forbidden: User \\\"system:anonymous\\\" cannot get path \\\"/\\\"\"") + require.Contains(t, curlStdOut.String(), `"forbidden: User \"system:anonymous\" cannot get path \"/\""`) }) t.Run("kubectl port-forward and keeping the connection open for over a minute (idle)", func(t *testing.T) { + t.Parallel() kubeconfigPath, envVarsWithProxy, _ := getImpersonationKubeconfig(t, env, impersonationProxyURL, impersonationProxyCACertPEM, credentialRequestSpecWithWorkingCredentials.Authenticator) // Run the kubectl port-forward command. timeout, cancelFunc := context.WithTimeout(ctx, 2*time.Minute) defer cancelFunc() - portForwardCmd, _, portForwardStderr := kubectlCommand(timeout, t, kubeconfigPath, envVarsWithProxy, "port-forward", "--namespace", env.ConciergeNamespace, conciergePod.Name, "8443:8443") + portForwardCmd, _, portForwardStderr := kubectlCommand(timeout, t, kubeconfigPath, envVarsWithProxy, "port-forward", "--namespace", env.ConciergeNamespace, conciergePod.Name, "10444:8443") portForwardCmd.Env = envVarsWithProxy // Start, but don't wait for the command to finish. @@ -401,7 +387,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl timeout, cancelFunc = context.WithTimeout(ctx, 2*time.Minute) defer cancelFunc() - curlCmd := exec.CommandContext(timeout, "curl", "-k", "-sS", "https://127.0.0.1:8443") // -sS turns off the progressbar but still prints errors + curlCmd := exec.CommandContext(timeout, "curl", "-k", "-sS", "https://127.0.0.1:10444") // -sS turns off the progressbar but still prints errors var curlStdOut, curlStdErr bytes.Buffer curlCmd.Stdout = &curlStdOut curlCmd.Stderr = &curlStdErr @@ -413,10 +399,11 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl } // We expect this to 403, but all we care is that it gets through. require.NoError(t, err) - require.Contains(t, curlStdOut.String(), "\"forbidden: User \\\"system:anonymous\\\" cannot get path \\\"/\\\"\"") + require.Contains(t, curlStdOut.String(), `"forbidden: User \"system:anonymous\" cannot get path \"/\""`) }) t.Run("using and watching all the basic verbs", func(t *testing.T) { + t.Parallel() // Create a namespace, because it will be easier to exercise "deletecollection" if we have a namespace. namespaceName := createTestNamespace(t, adminClient) @@ -536,10 +523,12 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl require.Len(t, listResult.Items, 0) }) - t.Run("double impersonation as a regular user is blocked", func(t *testing.T) { + t.Run("nested impersonation as a regular user is allowed if they have enough RBAC permissions", func(t *testing.T) { + t.Parallel() // Make a client which will send requests through the impersonation proxy and will also add // impersonate headers to the request. - doubleImpersonationKubeClient := newImpersonationProxyClient(t, impersonationProxyURL, impersonationProxyCACertPEM, "other-user-to-impersonate").Kubernetes + nestedImpersonationClient := newImpersonationProxyClient(t, impersonationProxyURL, impersonationProxyCACertPEM, + &rest.ImpersonationConfig{UserName: "other-user-to-impersonate"}, refreshCredential) // Check that we can get some resource through the impersonation proxy without any impersonation headers on the request. // We could use any resource for this, but we happen to know that this one should exist. @@ -548,68 +537,236 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl // Now we'll see what happens when we add an impersonation header to the request. This should generate a // request similar to the one above, except that it will also have an impersonation header. - _, err = doubleImpersonationKubeClient.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, impersonationProxyTLSSecretName(env), metav1.GetOptions{}) - // Double impersonation is not supported yet, so we should get an error. + _, err = nestedImpersonationClient.Kubernetes.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, impersonationProxyTLSSecretName(env), metav1.GetOptions{}) + // this user is not allowed to impersonate other users + require.True(t, k8serrors.IsForbidden(err), err) require.EqualError(t, err, fmt.Sprintf( `users "other-user-to-impersonate" is forbidden: `+ - `User "%s" cannot impersonate resource "users" in API group "" at the cluster scope: `+ - `impersonation is not allowed or invalid verb`, + `User "%s" cannot impersonate resource "users" in API group "" at the cluster scope`, env.TestUser.ExpectedUsername)) - }) - // This is a separate test from the above double impersonation test because the cluster admin user gets special - // authorization treatment from the Kube API server code that we are using, and we want to ensure that we are blocking - // double impersonation even for the cluster admin. - t.Run("double impersonation as a cluster admin user is blocked", func(t *testing.T) { - // Copy the admin credentials from the admin kubeconfig. - adminClientRestConfig := library.NewClientConfig(t) + // impersonate the GC service account instead which can read anything (the binding to edit allows this) + nestedImpersonationClientAsSA := newImpersonationProxyClient(t, impersonationProxyURL, impersonationProxyCACertPEM, + &rest.ImpersonationConfig{UserName: "system:serviceaccount:kube-system:generic-garbage-collector"}, refreshCredential) - if adminClientRestConfig.BearerToken == "" && adminClientRestConfig.CertData == nil && adminClientRestConfig.KeyData == nil { - t.Skip("The admin kubeconfig does not include credentials, so skipping this test.") + _, err = nestedImpersonationClientAsSA.Kubernetes.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, impersonationProxyTLSSecretName(env), metav1.GetOptions{}) + require.NoError(t, err) + + expectedGroups := make([]string, 0, len(env.TestUser.ExpectedGroups)+1) // make sure we do not mutate env.TestUser.ExpectedGroups + expectedGroups = append(expectedGroups, env.TestUser.ExpectedGroups...) + expectedGroups = append(expectedGroups, "system:authenticated") + expectedOriginalUserInfo := authenticationv1.UserInfo{ + Username: env.TestUser.ExpectedUsername, + Groups: expectedGroups, } + expectedOriginalUserInfoJSON, err := json.Marshal(expectedOriginalUserInfo) + require.NoError(t, err) - clusterAdminCredentials := &loginv1alpha1.ClusterCredential{ - Token: adminClientRestConfig.BearerToken, - ClientCertificateData: string(adminClientRestConfig.CertData), - ClientKeyData: string(adminClientRestConfig.KeyData), - } - - // Make a client using the admin credentials which will send requests through the impersonation proxy - // and will also add impersonate headers to the request. - doubleImpersonationKubeClient := newImpersonationProxyClientWithCredentials( - clusterAdminCredentials, impersonationProxyURL, impersonationProxyCACertPEM, "other-user-to-impersonate", - ).Kubernetes - - _, err := doubleImpersonationKubeClient.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, impersonationProxyTLSSecretName(env), metav1.GetOptions{}) - // Double impersonation is not supported yet, so we should get an error. - require.Error(t, err) - require.Regexp(t, - `users "other-user-to-impersonate" is forbidden: `+ - `User ".*" cannot impersonate resource "users" in API group "" at the cluster scope: `+ - `impersonation is not allowed or invalid verb`, - err.Error(), - ) - }) - - t.Run("WhoAmIRequests and different kinds of authentication through the impersonation proxy", func(t *testing.T) { - // Test using the TokenCredentialRequest for authentication. - impersonationProxyPinnipedConciergeClient := newImpersonationProxyClient(t, - impersonationProxyURL, impersonationProxyCACertPEM, "", - ).PinnipedConcierge - whoAmI, err := impersonationProxyPinnipedConciergeClient.IdentityV1alpha1().WhoAmIRequests(). + // check that we impersonated the correct user and that the original user is retained in the extra + whoAmI, err := nestedImpersonationClientAsSA.PinnipedConcierge.IdentityV1alpha1().WhoAmIRequests(). Create(ctx, &identityv1alpha1.WhoAmIRequest{}, metav1.CreateOptions{}) require.NoError(t, err) + require.Equal(t, + expectedWhoAmIRequestResponse( + "system:serviceaccount:kube-system:generic-garbage-collector", + []string{"system:serviceaccounts", "system:serviceaccounts:kube-system", "system:authenticated"}, + map[string]identityv1alpha1.ExtraValue{ + "original-user-info.impersonation-proxy.concierge.pinniped.dev": {string(expectedOriginalUserInfoJSON)}, + }, + ), + whoAmI, + ) + + _, err = newImpersonationProxyClient(t, impersonationProxyURL, impersonationProxyCACertPEM, + &rest.ImpersonationConfig{ + UserName: "system:serviceaccount:kube-system:generic-garbage-collector", + Extra: map[string][]string{ + "some-fancy-key": {"with a dangerous value"}, + }, + }, + refreshCredential).PinnipedConcierge.IdentityV1alpha1().WhoAmIRequests(). + Create(ctx, &identityv1alpha1.WhoAmIRequest{}, metav1.CreateOptions{}) + // this user should not be able to impersonate extra + require.True(t, k8serrors.IsForbidden(err), err) + require.EqualError(t, err, fmt.Sprintf( + `userextras.authentication.k8s.io "with a dangerous value" is forbidden: `+ + `User "%s" cannot impersonate resource "userextras/some-fancy-key" in API group "authentication.k8s.io" at the cluster scope`, + env.TestUser.ExpectedUsername)) + }) + + t.Run("nested impersonation as a cluster admin user is allowed", func(t *testing.T) { + t.Parallel() + // Copy the admin credentials from the admin kubeconfig. + adminClientRestConfig := library.NewClientConfig(t) + clusterAdminCredentials := getCredForConfig(t, adminClientRestConfig) + + // figure out who the admin user is + whoAmIAdmin, err := newImpersonationProxyClientWithCredentials(t, + clusterAdminCredentials, impersonationProxyURL, impersonationProxyCACertPEM, nil). + PinnipedConcierge.IdentityV1alpha1().WhoAmIRequests(). + Create(ctx, &identityv1alpha1.WhoAmIRequest{}, metav1.CreateOptions{}) + require.NoError(t, err) + + expectedExtra := make(map[string]authenticationv1.ExtraValue, len(whoAmIAdmin.Status.KubernetesUserInfo.User.Extra)) + for k, v := range whoAmIAdmin.Status.KubernetesUserInfo.User.Extra { + expectedExtra[k] = authenticationv1.ExtraValue(v) + } + expectedOriginalUserInfo := authenticationv1.UserInfo{ + Username: whoAmIAdmin.Status.KubernetesUserInfo.User.Username, + // The WhoAmI API is lossy so this will fail when the admin user actually does have a UID + UID: whoAmIAdmin.Status.KubernetesUserInfo.User.UID, + Groups: whoAmIAdmin.Status.KubernetesUserInfo.User.Groups, + Extra: expectedExtra, + } + expectedOriginalUserInfoJSON, err := json.Marshal(expectedOriginalUserInfo) + require.NoError(t, err) + + // Make a client using the admin credentials which will send requests through the impersonation proxy + // and will also add impersonate headers to the request. + nestedImpersonationClient := newImpersonationProxyClientWithCredentials(t, + clusterAdminCredentials, impersonationProxyURL, impersonationProxyCACertPEM, + &rest.ImpersonationConfig{ + UserName: "other-user-to-impersonate", + Groups: []string{"other-group-1", "other-group-2"}, + Extra: map[string][]string{ + "this-key": {"to this value"}, + }, + }, + ) + + _, err = nestedImpersonationClient.Kubernetes.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, impersonationProxyTLSSecretName(env), metav1.GetOptions{}) + // the impersonated user lacks the RBAC to perform this call + require.True(t, k8serrors.IsForbidden(err), err) + require.EqualError(t, err, fmt.Sprintf( + `secrets "%s" is forbidden: User "other-user-to-impersonate" cannot get resource "secrets" in API group "" in the namespace "%s"`, + impersonationProxyTLSSecretName(env), env.ConciergeNamespace, + )) + + // check that we impersonated the correct user and that the original user is retained in the extra + whoAmI, err := nestedImpersonationClient.PinnipedConcierge.IdentityV1alpha1().WhoAmIRequests(). + Create(ctx, &identityv1alpha1.WhoAmIRequest{}, metav1.CreateOptions{}) + require.NoError(t, err) + require.Equal(t, + expectedWhoAmIRequestResponse( + "other-user-to-impersonate", + []string{"other-group-1", "other-group-2", "system:authenticated"}, + map[string]identityv1alpha1.ExtraValue{ + "this-key": {"to this value"}, + "original-user-info.impersonation-proxy.concierge.pinniped.dev": {string(expectedOriginalUserInfoJSON)}, + }, + ), + whoAmI, + ) + }) + + t.Run("nested impersonation as a cluster admin fails on reserved key", func(t *testing.T) { + t.Parallel() + adminClientRestConfig := library.NewClientConfig(t) + clusterAdminCredentials := getCredForConfig(t, adminClientRestConfig) + + nestedImpersonationClient := newImpersonationProxyClientWithCredentials(t, + clusterAdminCredentials, impersonationProxyURL, impersonationProxyCACertPEM, + &rest.ImpersonationConfig{ + UserName: "other-user-to-impersonate", + Groups: []string{"other-group-1", "other-group-2"}, + Extra: map[string][]string{ + "this-good-key": {"to this good value"}, + "something.impersonation-proxy.concierge.pinniped.dev": {"super sneaky value"}, + }, + }, + ) + + _, err := nestedImpersonationClient.Kubernetes.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, impersonationProxyTLSSecretName(env), metav1.GetOptions{}) + require.EqualError(t, err, "Internal error occurred: unimplemented functionality - unable to act as current user") + require.True(t, k8serrors.IsInternalError(err), err) + require.Equal(t, &k8serrors.StatusError{ + ErrStatus: metav1.Status{ + Status: metav1.StatusFailure, + Code: http.StatusInternalServerError, + Reason: metav1.StatusReasonInternalError, + Details: &metav1.StatusDetails{ + Causes: []metav1.StatusCause{ + { + Message: "unimplemented functionality - unable to act as current user", + }, + }, + }, + Message: "Internal error occurred: unimplemented functionality - unable to act as current user", + }, + }, err) + }) + + // this works because impersonation cannot set UID and thus the final user info the proxy sees has no UID + t.Run("nested impersonation as a service account is allowed if it has enough RBAC permissions", func(t *testing.T) { + t.Parallel() + namespaceName := createTestNamespace(t, adminClient) + saName, saToken, saUID := createServiceAccountToken(ctx, t, adminClient, namespaceName) + nestedImpersonationClient := newImpersonationProxyClientWithCredentials(t, + &loginv1alpha1.ClusterCredential{Token: saToken}, impersonationProxyURL, impersonationProxyCACertPEM, + &rest.ImpersonationConfig{UserName: "system:serviceaccount:kube-system:root-ca-cert-publisher"}).PinnipedConcierge + _, err := nestedImpersonationClient.IdentityV1alpha1().WhoAmIRequests(). + Create(ctx, &identityv1alpha1.WhoAmIRequest{}, metav1.CreateOptions{}) + // this SA is not yet allowed to impersonate SAs + require.True(t, k8serrors.IsForbidden(err), err) + require.EqualError(t, err, fmt.Sprintf( + `serviceaccounts "root-ca-cert-publisher" is forbidden: `+ + `User "%s" cannot impersonate resource "serviceaccounts" in API group "" in the namespace "kube-system"`, + serviceaccount.MakeUsername(namespaceName, saName))) + + // webhook authorizer deny cache TTL is 10 seconds so we need to wait long enough for it to drain + time.Sleep(15 * time.Second) + + // allow the test SA to impersonate any SA + library.CreateTestClusterRoleBinding(t, + rbacv1.Subject{Kind: rbacv1.ServiceAccountKind, Name: saName, Namespace: namespaceName}, + rbacv1.RoleRef{Kind: "ClusterRole", APIGroup: rbacv1.GroupName, Name: "edit"}, + ) + library.WaitForUserToHaveAccess(t, serviceaccount.MakeUsername(namespaceName, saName), []string{}, &authorizationv1.ResourceAttributes{ + Verb: "impersonate", Group: "", Version: "v1", Resource: "serviceaccounts", + }) + + whoAmI, err := nestedImpersonationClient.IdentityV1alpha1().WhoAmIRequests(). + Create(ctx, &identityv1alpha1.WhoAmIRequest{}, metav1.CreateOptions{}) + require.NoError(t, err) + require.Equal(t, + expectedWhoAmIRequestResponse( + "system:serviceaccount:kube-system:root-ca-cert-publisher", + []string{"system:serviceaccounts", "system:serviceaccounts:kube-system", "system:authenticated"}, + map[string]identityv1alpha1.ExtraValue{ + "original-user-info.impersonation-proxy.concierge.pinniped.dev": { + fmt.Sprintf(`{"username":"%s","uid":"%s","groups":["system:serviceaccounts","system:serviceaccounts:%s","system:authenticated"]}`, + serviceaccount.MakeUsername(namespaceName, saName), saUID, namespaceName), + }, + }, + ), + whoAmI, + ) + }) + + t.Run("WhoAmIRequests and different kinds of authentication through the impersonation proxy", func(t *testing.T) { + t.Parallel() + // Test using the TokenCredentialRequest for authentication. + impersonationProxyPinnipedConciergeClient := newImpersonationProxyClient(t, + impersonationProxyURL, impersonationProxyCACertPEM, nil, refreshCredential, + ).PinnipedConcierge + whoAmI, err := impersonationProxyPinnipedConciergeClient.IdentityV1alpha1().WhoAmIRequests(). + Create(ctx, &identityv1alpha1.WhoAmIRequest{}, metav1.CreateOptions{}) + require.NoError(t, err) + expectedGroups := make([]string, 0, len(env.TestUser.ExpectedGroups)+1) // make sure we do not mutate env.TestUser.ExpectedGroups + expectedGroups = append(expectedGroups, env.TestUser.ExpectedGroups...) + expectedGroups = append(expectedGroups, "system:authenticated") require.Equal(t, expectedWhoAmIRequestResponse( env.TestUser.ExpectedUsername, - append(env.TestUser.ExpectedGroups, "system:authenticated"), + expectedGroups, + nil, ), whoAmI, ) // Test an unauthenticated request which does not include any credentials. impersonationProxyAnonymousPinnipedConciergeClient := newAnonymousImpersonationProxyClient( - impersonationProxyURL, impersonationProxyCACertPEM, "", + t, impersonationProxyURL, impersonationProxyCACertPEM, nil, ).PinnipedConcierge whoAmI, err = impersonationProxyAnonymousPinnipedConciergeClient.IdentityV1alpha1().WhoAmIRequests(). Create(ctx, &identityv1alpha1.WhoAmIRequest{}, metav1.CreateOptions{}) @@ -618,6 +775,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl expectedWhoAmIRequestResponse( "system:anonymous", []string{"system:unauthenticated"}, + nil, ), whoAmI, ) @@ -625,9 +783,10 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl // Test using a service account token. Authenticating as Service Accounts through the impersonation // proxy is not supported, so it should fail. namespaceName := createTestNamespace(t, adminClient) - impersonationProxyServiceAccountPinnipedConciergeClient := newImpersonationProxyClientWithCredentials( - &loginv1alpha1.ClusterCredential{Token: createServiceAccountToken(ctx, t, adminClient, namespaceName)}, - impersonationProxyURL, impersonationProxyCACertPEM, "").PinnipedConcierge + _, saToken, _ := createServiceAccountToken(ctx, t, adminClient, namespaceName) + impersonationProxyServiceAccountPinnipedConciergeClient := newImpersonationProxyClientWithCredentials(t, + &loginv1alpha1.ClusterCredential{Token: saToken}, + impersonationProxyURL, impersonationProxyCACertPEM, nil).PinnipedConcierge _, err = impersonationProxyServiceAccountPinnipedConciergeClient.IdentityV1alpha1().WhoAmIRequests(). Create(ctx, &identityv1alpha1.WhoAmIRequest{}, metav1.CreateOptions{}) require.EqualError(t, err, "Internal error occurred: unimplemented functionality - unable to act as current user") @@ -650,6 +809,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl }) t.Run("kubectl as a client", func(t *testing.T) { + t.Parallel() kubeconfigPath, envVarsWithProxy, tempDir := getImpersonationKubeconfig(t, env, impersonationProxyURL, impersonationProxyCACertPEM, credentialRequestSpecWithWorkingCredentials.Authenticator) // Try "kubectl exec" through the impersonation proxy. @@ -720,11 +880,12 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl }) t.Run("websocket client", func(t *testing.T) { + t.Parallel() namespaceName := createTestNamespace(t, adminClient) impersonationRestConfig := impersonationProxyRestConfig( refreshCredential(t, impersonationProxyURL, impersonationProxyCACertPEM), - impersonationProxyURL, impersonationProxyCACertPEM, "", + impersonationProxyURL, impersonationProxyCACertPEM, nil, ) tlsConfig, err := rest.TLSConfigFor(impersonationRestConfig) require.NoError(t, err) @@ -793,6 +954,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl }) t.Run("http2 client", func(t *testing.T) { + t.Parallel() namespaceName := createTestNamespace(t, adminClient) wantConfigMapLabelKey, wantConfigMapLabelValue := "some-label-key", "some-label-value" @@ -811,7 +973,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl // create rest client restConfig := impersonationProxyRestConfig( refreshCredential(t, impersonationProxyURL, impersonationProxyCACertPEM), - impersonationProxyURL, impersonationProxyCACertPEM, "", + impersonationProxyURL, impersonationProxyCACertPEM, nil, ) tlsConfig, err := rest.TLSConfigFor(restConfig) @@ -916,7 +1078,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl require.Eventually(t, func() bool { // It's okay if this returns RBAC errors because this user has no role bindings. // What we want to see is that the proxy eventually shuts down entirely. - _, err := impersonationProxyViaSquidKubeClientWithoutCredential().CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) + _, err := impersonationProxyViaSquidKubeClientWithoutCredential(t, proxyServiceEndpoint).CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) isErr, _ := isServiceUnavailableViaSquidError(err, proxyServiceEndpoint) return isErr }, 20*time.Second, 500*time.Millisecond) @@ -977,7 +1139,7 @@ func createTestNamespace(t *testing.T, adminClient kubernetes.Interface) string return namespace.Name } -func createServiceAccountToken(ctx context.Context, t *testing.T, adminClient kubernetes.Interface, namespaceName string) string { +func createServiceAccountToken(ctx context.Context, t *testing.T, adminClient kubernetes.Interface, namespaceName string) (name, token string, uid types.UID) { t.Helper() serviceAccount, err := adminClient.CoreV1().ServiceAccounts(namespaceName).Create(ctx, @@ -1011,10 +1173,10 @@ func createServiceAccountToken(ctx context.Context, t *testing.T, adminClient ku return len(secret.Data[corev1.ServiceAccountTokenKey]) > 0, nil }, time.Minute, time.Second) - return string(secret.Data[corev1.ServiceAccountTokenKey]) + return serviceAccount.Name, string(secret.Data[corev1.ServiceAccountTokenKey]), serviceAccount.UID } -func expectedWhoAmIRequestResponse(username string, groups []string) *identityv1alpha1.WhoAmIRequest { +func expectedWhoAmIRequestResponse(username string, groups []string, extra map[string]identityv1alpha1.ExtraValue) *identityv1alpha1.WhoAmIRequest { return &identityv1alpha1.WhoAmIRequest{ Status: identityv1alpha1.WhoAmIRequestStatus{ KubernetesUserInfo: identityv1alpha1.KubernetesUserInfo{ @@ -1022,7 +1184,7 @@ func expectedWhoAmIRequestResponse(username string, groups []string) *identityv1 Username: username, UID: "", // no way to impersonate UID: https://github.com/kubernetes/kubernetes/issues/93699 Groups: groups, - Extra: nil, + Extra: extra, }, }, }, @@ -1031,6 +1193,7 @@ func expectedWhoAmIRequestResponse(username string, groups []string) *identityv1 func performImpersonatorDiscovery(ctx context.Context, t *testing.T, env *library.TestEnv, adminConciergeClient pinnipedconciergeclientset.Interface) (string, []byte) { t.Helper() + var impersonationProxyURL string var impersonationProxyCACertPEM []byte @@ -1105,6 +1268,7 @@ func requireDisabledStrategy(ctx context.Context, t *testing.T, env *library.Tes func credentialAlmostExpired(t *testing.T, credential *loginv1alpha1.TokenCredentialRequest) bool { t.Helper() + pemBlock, _ := pem.Decode([]byte(credential.Status.Credential.ClientCertificateData)) parsedCredential, err := x509.ParseCertificate(pemBlock.Bytes) require.NoError(t, err) @@ -1117,7 +1281,7 @@ func credentialAlmostExpired(t *testing.T, credential *loginv1alpha1.TokenCreden return false } -func impersonationProxyRestConfig(credential *loginv1alpha1.ClusterCredential, host string, caData []byte, doubleImpersonateUser string) *rest.Config { +func impersonationProxyRestConfig(credential *loginv1alpha1.ClusterCredential, host string, caData []byte, nestedImpersonationConfig *rest.ImpersonationConfig) *rest.Config { config := rest.Config{ Host: host, TLSClientConfig: rest.TLSClientConfig{ @@ -1135,8 +1299,8 @@ func impersonationProxyRestConfig(credential *loginv1alpha1.ClusterCredential, h // We would like the impersonation proxy to imitate that behavior, so we test it here. BearerToken: credential.Token, } - if doubleImpersonateUser != "" { - config.Impersonate = rest.ImpersonationConfig{UserName: doubleImpersonateUser} + if nestedImpersonationConfig != nil { + config.Impersonate = *nestedImpersonationConfig } return &config } @@ -1144,6 +1308,7 @@ func impersonationProxyRestConfig(credential *loginv1alpha1.ClusterCredential, h func kubeconfigProxyFunc(t *testing.T, squidProxyURL string) func(req *http.Request) (*url.URL, error) { return func(req *http.Request) (*url.URL, error) { t.Helper() + parsedSquidProxyURL, err := url.Parse(squidProxyURL) require.NoError(t, err) t.Logf("passing request for %s through proxy %s", req.URL, parsedSquidProxyURL.String()) @@ -1153,6 +1318,7 @@ func kubeconfigProxyFunc(t *testing.T, squidProxyURL string) func(req *http.Requ func impersonationProxyConfigMapForConfig(t *testing.T, env *library.TestEnv, config impersonator.Config) corev1.ConfigMap { t.Helper() + configString, err := yaml.Marshal(config) require.NoError(t, err) configMap := corev1.ConfigMap{ @@ -1244,6 +1410,8 @@ func getImpersonationKubeconfig(t *testing.T, env *library.TestEnv, impersonatio // func to create kubectl commands with a kubeconfig. func kubectlCommand(timeout context.Context, t *testing.T, kubeconfigPath string, envVarsWithProxy []string, args ...string) (*exec.Cmd, *syncBuffer, *syncBuffer) { + t.Helper() + allArgs := append([]string{"--kubeconfig", kubeconfigPath}, args...) //nolint:gosec // we are not performing malicious argument injection against ourselves kubectlCmd := exec.CommandContext(timeout, "kubectl", allArgs...) @@ -1296,6 +1464,7 @@ func isServiceUnavailableViaSquidError(err error, proxyServiceEndpoint string) ( func requireClose(t *testing.T, c chan struct{}, timeout time.Duration) { t.Helper() + timer := time.NewTimer(timeout) select { case <-c: @@ -1317,3 +1486,117 @@ func createTokenCredentialRequest( &loginv1alpha1.TokenCredentialRequest{Spec: spec}, metav1.CreateOptions{}, ) } + +func newImpersonationProxyClientWithCredentials(t *testing.T, credentials *loginv1alpha1.ClusterCredential, impersonationProxyURL string, impersonationProxyCACertPEM []byte, nestedImpersonationConfig *rest.ImpersonationConfig) *kubeclient.Client { + t.Helper() + + env := library.IntegrationEnv(t) + clusterSupportsLoadBalancers := env.HasCapability(library.HasExternalLoadBalancerProvider) + + kubeconfig := impersonationProxyRestConfig(credentials, impersonationProxyURL, impersonationProxyCACertPEM, nestedImpersonationConfig) + if !clusterSupportsLoadBalancers { + // Only if there is no possibility to send traffic through a load balancer, then send the traffic through the Squid proxy. + // Prefer to go through a load balancer because that's how the impersonator is intended to be used in the real world. + kubeconfig.Proxy = kubeconfigProxyFunc(t, env.Proxy) + } + return library.NewKubeclient(t, kubeconfig) +} + +func newAnonymousImpersonationProxyClient(t *testing.T, impersonationProxyURL string, impersonationProxyCACertPEM []byte, nestedImpersonationConfig *rest.ImpersonationConfig) *kubeclient.Client { + t.Helper() + + emptyCredentials := &loginv1alpha1.ClusterCredential{} + return newImpersonationProxyClientWithCredentials(t, emptyCredentials, impersonationProxyURL, impersonationProxyCACertPEM, nestedImpersonationConfig) +} + +func impersonationProxyViaSquidKubeClientWithoutCredential(t *testing.T, proxyServiceEndpoint string) kubernetes.Interface { + t.Helper() + + env := library.IntegrationEnv(t) + proxyURL := "https://" + proxyServiceEndpoint + kubeconfig := impersonationProxyRestConfig(&loginv1alpha1.ClusterCredential{}, proxyURL, nil, nil) + kubeconfig.Proxy = kubeconfigProxyFunc(t, env.Proxy) + return library.NewKubeclient(t, kubeconfig).Kubernetes +} + +func newImpersonationProxyClient( + t *testing.T, + impersonationProxyURL string, + impersonationProxyCACertPEM []byte, + nestedImpersonationConfig *rest.ImpersonationConfig, + refreshCredentialFunc func(t *testing.T, impersonationProxyURL string, impersonationProxyCACertPEM []byte) *loginv1alpha1.ClusterCredential, +) *kubeclient.Client { + t.Helper() + + refreshedCredentials := refreshCredentialFunc(t, impersonationProxyURL, impersonationProxyCACertPEM).DeepCopy() + refreshedCredentials.Token = "not a valid token" // demonstrates that client certs take precedence over tokens by setting both on the requests + return newImpersonationProxyClientWithCredentials(t, refreshedCredentials, impersonationProxyURL, impersonationProxyCACertPEM, nestedImpersonationConfig) +} + +// getCredForConfig is mostly just a hacky workaround for impersonationProxyRestConfig needing creds directly. +func getCredForConfig(t *testing.T, config *rest.Config) *loginv1alpha1.ClusterCredential { + t.Helper() + + out := &loginv1alpha1.ClusterCredential{} + + config = rest.CopyConfig(config) + + config.Wrap(func(rt http.RoundTripper) http.RoundTripper { + return roundtripper.Func(func(req *http.Request) (*http.Response, error) { + resp, err := rt.RoundTrip(req) + + r := req + if resp != nil && resp.Request != nil { + r = resp.Request + } + + _, _, _ = bearertoken.New(authenticator.TokenFunc(func(_ context.Context, token string) (*authenticator.Response, bool, error) { + out.Token = token + return nil, false, nil + })).AuthenticateRequest(r) + + return resp, err + }) + }) + + transportConfig, err := config.TransportConfig() + require.NoError(t, err) + + rt, err := transport.New(transportConfig) + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, "GET", "https://localhost", nil) + require.NoError(t, err) + resp, _ := rt.RoundTrip(req) + if resp != nil && resp.Body != nil { + _ = resp.Body.Close() + } + + tlsConfig, err := transport.TLSConfigFor(transportConfig) + require.NoError(t, err) + + if tlsConfig != nil && tlsConfig.GetClientCertificate != nil { + cert, err := tlsConfig.GetClientCertificate(nil) + require.NoError(t, err) + require.Len(t, cert.Certificate, 1) + + publicKey := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: cert.Certificate[0], + }) + out.ClientCertificateData = string(publicKey) + + privateKey, err := keyutil.MarshalPrivateKeyToPEM(cert.PrivateKey) + require.NoError(t, err) + out.ClientKeyData = string(privateKey) + } + + if *out == (loginv1alpha1.ClusterCredential{}) { + t.Fatal("failed to get creds for config") + } + + return out +} diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index 2cb207e7..b70a5053 100644 --- a/test/integration/e2e_test.go +++ b/test/integration/e2e_test.go @@ -151,11 +151,6 @@ func TestE2EFullIntegration(t *testing.T) { kubeconfigPath := filepath.Join(tempDir, "kubeconfig.yaml") require.NoError(t, ioutil.WriteFile(kubeconfigPath, []byte(kubeconfigYAML), 0600)) - // Wait 10 seconds for the JWTAuthenticator to become initialized. - // TODO: remove this sleep once we have fixed the initialization problem. - t.Log("sleeping 10s to wait for JWTAuthenticator to become initialized") - time.Sleep(10 * time.Second) - // Run "kubectl get namespaces" which should trigger a browser login via the plugin. start := time.Now() kubectlCmd := exec.CommandContext(ctx, "kubectl", "get", "namespace", "--kubeconfig", kubeconfigPath)