Skip to content

Commit

Permalink
[v14] Prevent remote proxies from impersonating users from different …
Browse files Browse the repository at this point in the history
…clusters (#33539)

* 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 <tiago.silva@goteleport.com>

* fix message
  • Loading branch information
tigrato committed Oct 16, 2023
1 parent caec073 commit 97a516f
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 7 deletions.
36 changes: 36 additions & 0 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,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"
)

Expand Down Expand Up @@ -4778,9 +4779,44 @@ 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.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)
}

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 {
Expand Down
10 changes: 8 additions & 2 deletions lib/auth/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,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
}
Expand Down Expand Up @@ -791,7 +792,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 {
Expand All @@ -803,6 +804,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.
Expand Down
43 changes: 38 additions & 5 deletions lib/auth/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)},
Expand All @@ -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",
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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"),
),
)
}
})
}
}
Expand Down

0 comments on commit 97a516f

Please sign in to comment.