From ae607714be7ea3deb6ed9c3e3e19f92ff410373e Mon Sep 17 00:00:00 2001 From: Tiago Silva Date: Mon, 16 Oct 2023 18:23:46 +0100 Subject: [PATCH 1/2] Prevent remote proxies from impersonating users from different clusters This PR prevents root proxies from impersonating users from different clusters when accessing a leaf cluster. During authentication, the proxy presents its certificate and sends the impersonation header. A malicious attacker in possession of the root cluster proxy cert-key pair could bypass the root-leaf cluster permissions boundary by impersonating local users. This PR prevents that and remote proxies can only impersonate users belonging to their cluster. KubeCSR Flow: ```mermaid sequenceDiagram ROOT PROXY->>+LEAF PROXY: Forward the request identity cert LEAF PROXY ->> LEAF AUTH SRV: Sign identity via KubeCSR LEAF AUTH SRV -->> LEAF PROXY: Identity cert LEAF PROXY ->> LEAF KUBE SERVICE: Forward the request using cert LEAF KUBE SERVICE -->> LEAF PROXY: Return response LEAF PROXY -->> ROOT PROXY: Return response ``` Impersonation Flow: ```mermaid sequenceDiagram ROOT PROXY->>+LEAF PROXY: Forward the request identity by Impersonating LEAF PROXY ->> LEAF KUBE SERVICE: Forward the request identity by Impersonating LEAF KUBE SERVICE -->> LEAF PROXY: Return response LEAF PROXY -->> ROOT PROXY: Return response ``` Fixes gravitational/teleport-private#968 Signed-off-by: Tiago Silva --- lib/auth/auth_with_roles.go | 31 ++++++++++++++++++++++++++ lib/auth/middleware.go | 10 +++++++-- lib/auth/middleware_test.go | 43 ++++++++++++++++++++++++++++++++----- 3 files changed, 77 insertions(+), 7 deletions(-) diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index fdda573b0bd50..03cac6577bf9b 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -70,6 +70,7 @@ import ( "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/services/local" "github.com/gravitational/teleport/lib/session" + "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/lib/utils" ) @@ -4613,9 +4614,39 @@ func (a *ServerWithRoles) ProcessKubeCSR(req KubeCSR) (*KubeCSRResponse, error) if !a.hasBuiltinRole(types.RoleProxy) { return nil, trace.AccessDenied("this request can be only executed by a proxy") } + clusterName, err := a.GetClusterName() + if err != nil { + return nil, trace.Wrap(err) + } + proxyClusterName := a.context.Identity.GetIdentity().TeleportCluster + identityClusterName, err := extractOriginalClusterNameFromCSR(req) + if err != nil { + return nil, trace.Wrap(err) + } + if proxyClusterName != "" && + proxyClusterName != clusterName.GetClusterName() && + proxyClusterName != identityClusterName { + log.Warnf("received Kube CSR for %v", identityClusterName) + return nil, trace.AccessDenied("can not sign certs for users via a different cluster proxy") + } return a.authServer.ProcessKubeCSR(req) } +func extractOriginalClusterNameFromCSR(req KubeCSR) (string, error) { + csr, err := tlsca.ParseCertificateRequestPEM(req.CSR) + if err != nil { + return "", trace.Wrap(err) + } + + // Extract identity from the CSR. Pass zero time for id.Expiry, it won't be + // used here. + id, err := tlsca.FromSubject(csr.Subject, time.Time{}) + if err != nil { + return "", trace.Wrap(err) + } + return id.TeleportCluster, nil +} + // GetDatabaseServers returns all registered database servers. func (a *ServerWithRoles) GetDatabaseServers(ctx context.Context, namespace string, opts ...services.MarshalOption) ([]types.DatabaseServer, error) { if err := a.action(namespace, types.KindDatabaseServer, types.VerbList, types.VerbRead); err != nil { diff --git a/lib/auth/middleware.go b/lib/auth/middleware.go index 013574e8ef0e6..a94f6513e1b70 100644 --- a/lib/auth/middleware.go +++ b/lib/auth/middleware.go @@ -663,7 +663,8 @@ func (a *Middleware) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - if user, err = a.extractIdentityFromImpersonationHeader(impersonateUser); err != nil { + proxyClusterName := user.GetIdentity().TeleportCluster + if user, err = a.extractIdentityFromImpersonationHeader(proxyClusterName, impersonateUser); err != nil { trace.WriteError(w, err) return } @@ -789,7 +790,7 @@ func isProxyRole(identity authz.IdentityGetter) bool { // extractIdentityFromImpersonationHeader extracts the identity from the impersonation // header and returns it. If the impersonation header holds an identity of a // system role, an error is returned. -func (a *Middleware) extractIdentityFromImpersonationHeader(impersonate string) (authz.IdentityGetter, error) { +func (a *Middleware) extractIdentityFromImpersonationHeader(proxyCluster string, impersonate string) (authz.IdentityGetter, error) { // Unmarshal the impersonated user from the header. var impersonatedIdentity tlsca.Identity if err := json.Unmarshal([]byte(impersonate), &impersonatedIdentity); err != nil { @@ -801,6 +802,11 @@ func (a *Middleware) extractIdentityFromImpersonationHeader(impersonate string) // make sure that this user does not have system role // since system roles are not allowed to be impersonated. return nil, trace.AccessDenied("can not impersonate a system role") + case proxyCluster != "" && proxyCluster != a.ClusterName && proxyCluster != impersonatedIdentity.TeleportCluster: + // If a remote proxy is impersonating a user from a different cluster, we + // must reject the request. This is because the proxy is not allowed to + // impersonate a user from a different cluster. + return nil, trace.AccessDenied("can not impersonate users via a different cluster proxy") case impersonatedIdentity.TeleportCluster != a.ClusterName: // if the impersonated user is from a different cluster, we need to // use him as remote user. diff --git a/lib/auth/middleware_test.go b/lib/auth/middleware_test.go index 19e0688317d40..193e1be7a303e 100644 --- a/lib/auth/middleware_test.go +++ b/lib/auth/middleware_test.go @@ -361,6 +361,15 @@ func TestMiddleware_ServeHTTP(t *testing.T) { Principals: []string{}, } + remoteProxyIdentity := tlsca.Identity{ + Username: "proxy...", + Groups: []string{string(types.RoleProxy)}, + TeleportCluster: remoteClusterName, + Expires: now, + Usage: []string{}, + Principals: []string{}, + } + dbIdentity := tlsca.Identity{ Username: "db...", Groups: []string{string(types.RoleDatabase)}, @@ -381,11 +390,12 @@ func TestMiddleware_ServeHTTP(t *testing.T) { userIPAddr string } tests := []struct { - name string - args args - want want - credentialsForwardingDennied bool - enableCredentialsForwarding bool + name string + args args + want want + credentialsForwardingDennied bool + enableCredentialsForwarding bool + impersonateLocalUserViaRemoteProxyErr bool }{ { name: "local user without impersonation", @@ -549,6 +559,21 @@ func TestMiddleware_ServeHTTP(t *testing.T) { credentialsForwardingDennied: false, enableCredentialsForwarding: false, }, + { + name: "remote proxy with local user impersonation", + args: args{ + peers: []*x509.Certificate{{ + Subject: subject(t, remoteProxyIdentity), + NotAfter: now, + Issuer: pkix.Name{Organization: []string{remoteClusterName}}, + }}, + impersonateIdentity: &localUserIdentity, + sourceIPAddr: "127.0.0.1:6514", + impersonatedIPAddr: "127.0.0.2:6514", + }, + enableCredentialsForwarding: true, + impersonateLocalUserViaRemoteProxyErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -593,6 +618,14 @@ func TestMiddleware_ServeHTTP(t *testing.T) { ), ) } + if tt.impersonateLocalUserViaRemoteProxyErr { + require.True(t, + bytes.Contains( + rsp.Body.Bytes(), + []byte("can not impersonate users via a different cluster proxy"), + ), + ) + } }) } } From b418ff4a7ca7e1a09c7cf302d07a2c0b27d8660c Mon Sep 17 00:00:00 2001 From: Tiago Silva Date: Mon, 16 Oct 2023 18:41:23 +0100 Subject: [PATCH 2/2] fix message --- lib/auth/auth_with_roles.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 03cac6577bf9b..7af20a9ce4b13 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -4626,7 +4626,12 @@ func (a *ServerWithRoles) ProcessKubeCSR(req KubeCSR) (*KubeCSRResponse, error) if proxyClusterName != "" && proxyClusterName != clusterName.GetClusterName() && proxyClusterName != identityClusterName { - log.Warnf("received Kube CSR for %v", identityClusterName) + log.WithFields( + logrus.Fields{ + "proxy_cluster_name": proxyClusterName, + "identity_cluster_name": identityClusterName, + }, + ).Warn("KubeCSR request denied because the proxy and identity clusters didn't match") return nil, trace.AccessDenied("can not sign certs for users via a different cluster proxy") } return a.authServer.ProcessKubeCSR(req)