Inherit kubernetes_resources from roles when using access requests to (#22127)

Kube

This PR fixes the inheritance of `kubernetes_resources` from the user's roles when the user has active access requests to a `kube_cluster`.

If the user had an active request to a `kube_cluster`, Teleport ignored the role's `kubernetes_resources` and returned an empty set for the allowed and denied pods. This resulted in the user being unable to see any pods in the cluster.

Fixes #22126
This commit is contained in:
Tiago Silva 2023-02-23 15:35:28 +00:00 committed by GitHub
parent 7f5c185d5b
commit 91eb3c6bbc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 299 additions and 54 deletions

View file

@ -998,8 +998,15 @@ func (f *Forwarder) authorize(ctx context.Context, actx *authContext) error {
// the kubeResource.
// This is required because CheckAccess does not validate the subresource type.
if actx.kubeResource != nil && len(actx.Checker.GetAllowedResourceIDs()) > 0 {
kubeResources := getKubeResourcesFromAllowedRequestIds(ks, actx.Checker.GetAllowedResourceIDs())
if err := matchKubernetesResource(*actx.kubeResource, kubeResources, nil /*denied branch is empty*/); err != nil {
// GetKubeResources returns the allowed and denied Kubernetes resources
// for the user. Since we have active access requests, the allowed
// resources will be the list of pods that the user requested access to if he
// requested access to specific pods or the list of pods that his roles
// allow if the user requested access a kubernetes cluster. If the user
// did not request access to any Kubernetes resource type, the allowed
// list will be empty.
allowed, denied := actx.Checker.GetKubeResources(ks)
if err := matchKubernetesResource(*actx.kubeResource, allowed, denied); err != nil {
return trace.AccessDenied(notFoundMessage)
}
}
@ -1014,27 +1021,6 @@ func (f *Forwarder) authorize(ctx context.Context, actx *authContext) error {
return trace.AccessDenied(notFoundMessage)
}
func getKubeResourcesFromAllowedRequestIds(ks types.KubeCluster, resourceIDs []types.ResourceID) []types.KubernetesResource {
kubeResources := make([]types.KubernetesResource, 0, len(resourceIDs))
for _, resourceID := range resourceIDs {
if !slices.Contains(types.KubernetesResourcesKinds, resourceID.Kind) || resourceID.Name != ks.GetName() {
continue
}
split := strings.SplitN(resourceID.SubResourceName, "/", 3)
if len(split) != 2 {
continue
}
kubeResources = append(kubeResources,
types.KubernetesResource{
Kind: resourceID.Kind,
Namespace: split[0],
Name: split[1],
},
)
}
return kubeResources
}
// matchKubernetesResource checks if the Kubernetes Resource does not match any
// entry from the deny list and matches at least one entry from the allowed list.
func matchKubernetesResource(resource types.KubernetesResource, allowed, denied []types.KubernetesResource) error {

View file

@ -18,6 +18,7 @@ package proxy
import (
"context"
"fmt"
"io"
"mime"
"net/http"
@ -143,6 +144,7 @@ func TestListPodRBAC(t *testing.T) {
type args struct {
user types.User
namespace string
opts []GenTestKubeClientTLSCertOptions
}
type want struct {
listPodsResult []string
@ -232,6 +234,66 @@ func TestListPodRBAC(t *testing.T) {
},
},
},
{
name: "list default namespace pods for user with limited access and a resource access request",
args: args{
user: userWithNamespaceAccess,
namespace: metav1.NamespaceDefault,
opts: []GenTestKubeClientTLSCertOptions{
WithResourceAccessRequests(
types.ResourceID{
ClusterName: testCtx.ClusterName,
Kind: types.KindKubePod,
Name: kubeCluster,
SubResourceName: "default/nginx-1",
},
),
},
},
want: want{
listPodsResult: []string{
// Users roles allow access to all pods in default namespace
// but the access request only allows access to default/nginx-1.
"default/nginx-1",
},
getTestPodResult: &kubeerrors.StatusError{
ErrStatus: metav1.Status{
Status: "Failure",
Message: "pods \"test\" is forbidden: User \"default_user\" cannot get resource \"pods\" in API group \"\" in the namespace \"default\"",
Code: 403,
Reason: metav1.StatusReasonForbidden,
},
},
},
},
{
name: "user with pod access request that no longer fullfils the role requirements",
args: args{
user: userWithLimitedAccess,
namespace: metav1.NamespaceDefault,
opts: []GenTestKubeClientTLSCertOptions{
WithResourceAccessRequests(
types.ResourceID{
ClusterName: testCtx.ClusterName,
Kind: types.KindKubePod,
Name: kubeCluster,
SubResourceName: fmt.Sprintf("%s/%s", metav1.NamespaceDefault, testPodName),
},
),
},
},
want: want{
listPodsResult: []string{},
getTestPodResult: &kubeerrors.StatusError{
ErrStatus: metav1.Status{
Status: "Failure",
Message: "pods \"test\" is forbidden: User \"limited_user\" cannot get resource \"pods\" in API group \"\" in the namespace \"default\"",
Code: 403,
Reason: metav1.StatusReasonForbidden,
},
},
},
},
}
for _, tt := range tests {
@ -243,6 +305,7 @@ func TestListPodRBAC(t *testing.T) {
t,
tt.args.user.GetName(),
kubeCluster,
tt.args.opts...,
)
rsp, err := client.CoreV1().Pods(tt.args.namespace).List(

View file

@ -318,8 +318,19 @@ func newKubeConfigFile(ctx context.Context, t *testing.T, clusters ...KubeCluste
return kubeConfigLocation
}
// GenTestKubeClientTLSCertOptions is a function that can be used to modify the
// identity used to generate the kube client certificate.
type GenTestKubeClientTLSCertOptions func(*tlsca.Identity)
// WithResourceAccessRequests adds resource access requests to the identity.
func WithResourceAccessRequests(r ...types.ResourceID) GenTestKubeClientTLSCertOptions {
return func(identity *tlsca.Identity) {
identity.AllowedResourceIDs = r
}
}
// GenTestKubeClientTLSCert generates a kube client to access kube service
func (c *TestContext) GenTestKubeClientTLSCert(t *testing.T, userName, kubeCluster string) (*kubernetes.Clientset, *rest.Config) {
func (c *TestContext) GenTestKubeClientTLSCert(t *testing.T, userName, kubeCluster string, opts ...GenTestKubeClientTLSCertOptions) (*kubernetes.Clientset, *rest.Config) {
authServer := c.AuthServer
clusterName, err := authServer.GetClusterName()
require.NoError(t, err)
@ -359,6 +370,9 @@ func (c *TestContext) GenTestKubeClientTLSCert(t *testing.T, userName, kubeClust
KubernetesCluster: kubeCluster,
RouteToCluster: c.ClusterName,
}
for _, opt := range opts {
opt(&id)
}
subj, err := id.Subject()
require.NoError(t, err)

View file

@ -23,12 +23,14 @@ import (
"github.com/gravitational/trace"
log "github.com/sirupsen/logrus"
"golang.org/x/crypto/ssh"
"golang.org/x/exp/slices"
"github.com/gravitational/teleport/api/constants"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/api/types/wrappers"
"github.com/gravitational/teleport/api/utils/keys"
"github.com/gravitational/teleport/lib/tlsca"
"github.com/gravitational/teleport/lib/utils"
)
// AccessChecker interface checks access to resources based on roles, traits,
@ -319,30 +321,67 @@ func (a *accessChecker) CheckAccess(r AccessCheckable, state AccessState, matche
// GetKubeResources returns the allowed and denied Kubernetes Resources configured
// for a user.
func (a *accessChecker) GetKubeResources(cluster types.KubeCluster) (allowed, denied []types.KubernetesResource) {
if len(a.info.AllowedResourceIDs) > 0 {
for _, r := range a.info.AllowedResourceIDs {
if r.Kind == types.KindKubePod && r.Name == cluster.GetName() && r.ClusterName == a.localCluster {
splitted := strings.SplitN(r.SubResourceName, "/", 3)
// This condition should never happen since SubResourceName is validated
// but it's better to validate it.
if len(splitted) != 2 {
continue
}
allowed = append(allowed,
types.KubernetesResource{
Kind: r.Kind,
Namespace: splitted[0],
Name: splitted[1],
},
)
}
}
// When the user is granted access through a resource access request,
// he has no denied resources, which results in the denied being an empty slice.
return
if len(a.info.AllowedResourceIDs) == 0 {
return a.RoleSet.GetKubeResources(cluster)
}
return a.RoleSet.GetKubeResources(cluster)
rolesAllowed, rolesDenied := a.RoleSet.GetKubeResources(cluster)
// Allways append the denied resources from the roles. This is because
// the denied resources from the roles take precedence over the allowed
// resources from the certificate.
denied = rolesDenied
for _, r := range a.info.AllowedResourceIDs {
if r.Name != cluster.GetName() || r.ClusterName != a.localCluster {
continue
}
switch {
case slices.Contains(types.KubernetesResourcesKinds, r.Kind):
splitted := strings.SplitN(r.SubResourceName, "/", 3)
// This condition should never happen since SubResourceName is validated
// but it's better to validate it.
if len(splitted) != 2 {
continue
}
r := types.KubernetesResource{
Kind: r.Kind,
Namespace: splitted[0],
Name: splitted[1],
}
if matchKubernetesResource(r, rolesAllowed, rolesDenied) == nil {
allowed = append(allowed, r)
}
case r.Kind == types.KindKubernetesCluster:
// When a user has access to a Kubernetes cluster through Resource Access request,
// he has access to all resources in that cluster that he has access to through his roles.
// In that case, we append the allowed and denied resources from the roles.
return rolesAllowed, rolesDenied
}
}
return
}
// matchKubernetesResource checks if the Kubernetes Resource does not match any
// entry from the deny list and matches at least one entry from the allowed list.
func matchKubernetesResource(resource types.KubernetesResource, allowed, denied []types.KubernetesResource) error {
// utils.KubeResourceMatchesRegex checks if the resource.Kind is strictly equal
// to each entry and validates if the Name and Namespace fields matches the
// regex allowed by each entry.
result, err := utils.KubeResourceMatchesRegex(resource, denied)
if err != nil {
return trace.Wrap(err)
} else if result {
return trace.AccessDenied("access to %s %q denied", resource.Kind, resource.ClusterResource())
}
result, err = utils.KubeResourceMatchesRegex(resource, allowed)
if err != nil {
return trace.Wrap(err)
} else if !result {
return trace.AccessDenied("access to %s %q denied", resource.Kind, resource.ClusterResource())
}
return nil
}
// GetAllowedResourceIDs returns the list of allowed resources the identity for

View file

@ -184,12 +184,8 @@ func TestAccessCheckerKubeResources(t *testing.T) {
Name: "dev",
Namespace: "dev",
},
{
Kind: types.KindKubePod,
Name: "test-3",
Namespace: "test",
},
},
wantDenied: emptySet,
assertAccess: require.NoError,
},
{
@ -197,6 +193,7 @@ func TestAccessCheckerKubeResources(t *testing.T) {
kubeCluster: prodKubeCluster,
fields: fields{
info: &AccessInfo{
Roles: []string{"any", "dev"},
AllowedResourceIDs: []types.ResourceID{
{
Kind: types.KindApp,
@ -229,17 +226,163 @@ func TestAccessCheckerKubeResources(t *testing.T) {
Namespace: "any1",
},
},
wantAllowed: nil,
wantDenied: emptySet,
assertAccess: require.Error,
},
{
name: "dev cluster with kube_cluster resource access request",
kubeCluster: devKubeCluster,
fields: fields{
roleSet: roleSet,
info: &AccessInfo{
Roles: []string{"any", "dev"},
AllowedResourceIDs: []types.ResourceID{
{
Kind: types.KindApp,
ClusterName: localCluster,
Name: "devapp",
},
{
Kind: types.KindKubernetesCluster,
ClusterName: localCluster,
Name: devKubeCluster.GetName(),
},
},
},
resource: types.KubernetesResource{
Kind: types.KindKubePod,
Name: "dev",
Namespace: "dev",
},
},
wantAllowed: []types.KubernetesResource{
{
Kind: types.KindKubePod,
Name: "test-2",
Namespace: "prod",
Name: "any1",
Namespace: "any1",
},
{
Kind: types.KindKubePod,
Name: "any1",
Namespace: "any2",
},
{
Kind: types.KindKubePod,
Name: "dev",
Namespace: "dev",
},
},
wantDenied: emptySet,
assertAccess: require.NoError,
},
{
name: "access dev cluster with kube cluster<prodCluster> and kube pod<devCluster> resource access request",
kubeCluster: devKubeCluster,
fields: fields{
roleSet: roleSet,
info: &AccessInfo{
Roles: []string{"any"},
AllowedResourceIDs: []types.ResourceID{
{
Kind: types.KindKubernetesCluster,
ClusterName: localCluster,
Name: prodKubeCluster.GetName(),
},
{
Kind: types.KindKubePod,
ClusterName: localCluster,
Name: devKubeCluster.GetName(),
SubResourceName: "dev/dev",
},
},
},
resource: types.KubernetesResource{
Kind: types.KindKubePod,
Name: "dev",
Namespace: "dev",
},
},
wantAllowed: []types.KubernetesResource{
{
Kind: types.KindKubePod,
Name: "dev",
Namespace: "dev",
},
},
wantDenied: emptySet,
assertAccess: require.NoError,
},
{
name: "access prod cluster with kube cluster<prodCluster> and kube pod<devCluster> resource access request",
kubeCluster: prodKubeCluster,
fields: fields{
roleSet: roleSet,
info: &AccessInfo{
Roles: []string{"any"},
AllowedResourceIDs: []types.ResourceID{
{
Kind: types.KindKubernetesCluster,
ClusterName: localCluster,
Name: prodKubeCluster.GetName(),
},
{
Kind: types.KindKubePod,
ClusterName: localCluster,
Name: devKubeCluster.GetName(),
SubResourceName: "dev/dev",
},
},
},
resource: types.KubernetesResource{
Kind: types.KindKubePod,
Name: "dev",
Namespace: "dev",
},
},
wantAllowed: []types.KubernetesResource{
{
Kind: types.KindKubePod,
Name: "any1",
Namespace: "any1",
},
{
Kind: types.KindKubePod,
Name: "any1",
Namespace: "any2",
},
},
wantDenied: emptySet,
assertAccess: require.Error,
},
{
name: "access pod outside namespace allowed by roles",
kubeCluster: prodKubeCluster,
fields: fields{
roleSet: roleSet,
info: &AccessInfo{
Roles: []string{"any", "dev"},
AllowedResourceIDs: []types.ResourceID{
{
Kind: types.KindKubePod,
ClusterName: localCluster,
Name: prodKubeCluster.GetName(),
SubResourceName: "wrongNamespace/wrongPodName",
},
},
},
resource: types.KubernetesResource{
Kind: types.KindKubePod,
Name: "wrongPodName",
Namespace: "wrongNamespace",
},
},
wantAllowed: nil,
wantDenied: emptySet,
assertAccess: require.Error,
},
}
for _, tt := range tests[len(tests)-2:] {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
accessChecker := NewAccessCheckerWithRoleSet(tt.fields.info, localCluster, tt.fields.roleSet)
gotAllowed, gotDenied := accessChecker.GetKubeResources(tt.kubeCluster)