From b2d3e99be43796e202e8cf55d48fd2e14f4e64f5 Mon Sep 17 00:00:00 2001 From: Tiago Silva Date: Thu, 5 Oct 2023 14:13:17 +0100 Subject: [PATCH 1/3] Fix `tsh kube credentials` when root cluster roles don't allow Kube access This PR fixes an edge case where an error message is printed to the users without proper knowledge of the role mappings between root and leaf clusters. The user certificates include the `kubernetes_users` and `kubernetes_groups` allowed in the root cluster but nothing prevents the access to be sucessfull if the leaf cluster roles after the mapping introduce the kubernetes principals. This PR prevents tsh from failing when generating certificates for leaf Kubernetes clusters. Signed-off-by: Tiago Silva --- tool/tsh/common/kube.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tool/tsh/common/kube.go b/tool/tsh/common/kube.go index 0141046c9b6fb..35cc845903f64 100644 --- a/tool/tsh/common/kube.go +++ b/tool/tsh/common/kube.go @@ -788,7 +788,15 @@ func (c *kubeCredentialsCommand) issueCert(cf *CLIConf) error { // via the RBAC rules, but we also need to make sure that the user has // access to the cluster with at least one kubernetes_user or kubernetes_group // defined. - if err := checkIfCertsAreAllowedToAccessCluster(k, c.kubeCluster); err != nil { + // This is a safety check in order to print a better message to the user even + // before hitting Teleport Kubernetes Proxy. + // We only enforce this check for root clusters, since we don't have knowledge + // of the RBAC role mappings for remote clusters. + rootClusterName, err := tc.RootClusterName(cf.Context) + if err != nil { + return trace.Wrap(err) + } + if err := checkIfCertsAreAllowedToAccessCluster(k, c.kubeCluster); err != nil && rootClusterName == c.teleportCluster { return trace.Wrap(err) } // Cache the new cert on disk for reuse. From 07dc921d7f162ebe17e9442ea6a0ce817c655d00 Mon Sep 17 00:00:00 2001 From: Tiago Silva Date: Fri, 6 Oct 2023 12:46:06 +0100 Subject: [PATCH 2/3] Update tool/tsh/common/kube.go Co-authored-by: Edoardo Spadolini --- tool/tsh/common/kube.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tool/tsh/common/kube.go b/tool/tsh/common/kube.go index 35cc845903f64..4f3f491d723d2 100644 --- a/tool/tsh/common/kube.go +++ b/tool/tsh/common/kube.go @@ -796,8 +796,10 @@ func (c *kubeCredentialsCommand) issueCert(cf *CLIConf) error { if err != nil { return trace.Wrap(err) } - if err := checkIfCertsAreAllowedToAccessCluster(k, c.kubeCluster); err != nil && rootClusterName == c.teleportCluster { - return trace.Wrap(err) + if rootClusterName == c.teleportCluster { + if err := checkIfCertsAreAllowedToAccessCluster(k, c.kubeCluster); err != nil { + return trace.Wrap(err) + } } // Cache the new cert on disk for reuse. if err := tc.LocalAgent().AddKubeKey(k); err != nil { From 756ba42bf05a3eba0d1057d2613c9677d42d3f0b Mon Sep 17 00:00:00 2001 From: Tiago Silva Date: Mon, 9 Oct 2023 18:05:14 +0100 Subject: [PATCH 3/3] add check to tsh proxy --- tool/tsh/common/kube.go | 18 +++++++++++++----- tool/tsh/common/kube_proxy.go | 15 ++++++++++++++- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/tool/tsh/common/kube.go b/tool/tsh/common/kube.go index 4f3f491d723d2..1eef564c847b6 100644 --- a/tool/tsh/common/kube.go +++ b/tool/tsh/common/kube.go @@ -796,10 +796,11 @@ func (c *kubeCredentialsCommand) issueCert(cf *CLIConf) error { if err != nil { return trace.Wrap(err) } - if rootClusterName == c.teleportCluster { - if err := checkIfCertsAreAllowedToAccessCluster(k, c.kubeCluster); err != nil { - return trace.Wrap(err) - } + if err := checkIfCertsAreAllowedToAccessCluster(k, + rootClusterName, + c.teleportCluster, + c.kubeCluster); err != nil { + return trace.Wrap(err) } // Cache the new cert on disk for reuse. if err := tc.LocalAgent().AddKubeKey(k); err != nil { @@ -829,7 +830,14 @@ func (c *kubeCredentialsCommand) checkLocalProxyRequirement(profile *profile.Pro // defined. If not, it returns an error. // This is a safety check in order to print a better message to the user even // before hitting Teleport Kubernetes Proxy. -func checkIfCertsAreAllowedToAccessCluster(k *client.Key, kubeCluster string) error { +func checkIfCertsAreAllowedToAccessCluster(k *client.Key, rootCluster, teleportCluster, kubeCluster string) error { + // This is a safety check in order to print a better message to the user even + // before hitting Teleport Kubernetes Proxy. + // We only enforce this check for root clusters, since we don't have knowledge + // of the RBAC role mappings for remote clusters. + if rootCluster != teleportCluster { + return nil + } for k8sCluster, cert := range k.KubeTLSCerts { if k8sCluster != kubeCluster { continue diff --git a/tool/tsh/common/kube_proxy.go b/tool/tsh/common/kube_proxy.go index 8eed002742e2c..1fb6d34430a27 100644 --- a/tool/tsh/common/kube_proxy.go +++ b/tool/tsh/common/kube_proxy.go @@ -491,7 +491,20 @@ func issueKubeCert(ctx context.Context, tc *client.TeleportClient, proxy *client return tls.Certificate{}, trace.Wrap(err) } - if err := checkIfCertsAreAllowedToAccessCluster(key, kubeCluster); err != nil { + // Make sure the cert is allowed to access the cluster. + // At this point we already know that the user has access to the cluster + // via the RBAC rules, but we also need to make sure that the user has + // access to the cluster with at least one kubernetes_user or kubernetes_group + // defined. + rootClusterName, err := tc.RootClusterName(ctx) + if err != nil { + return tls.Certificate{}, trace.Wrap(err) + } + if err := checkIfCertsAreAllowedToAccessCluster( + key, + rootClusterName, + teleportCluster, + kubeCluster); err != nil { return tls.Certificate{}, trace.Wrap(err) }