diff --git a/api/types/constants.go b/api/types/constants.go index b737796a0e5..381af237422 100644 --- a/api/types/constants.go +++ b/api/types/constants.go @@ -1450,12 +1450,11 @@ var KubernetesResourcesV7KindGroups = map[string]string{ // to their kubernetes name. // Used to upgrade roles <=v7 as well as to support existing access request // format. -// TODO(@creack): Remove this, find a better way to handle the mapping. +// NOTE: Namespace having a different behavior between versions, it is omitted from this map. var KubernetesResourcesKindsPlurals = map[string]string{ KindKubePod: "pods", KindKubeSecret: "secrets", KindKubeConfigmap: "configmaps", - KindKubeNamespace: "namespaces", KindKubeService: "services", KindKubeServiceAccount: "serviceaccounts", KindKubeNode: "nodes", @@ -1521,7 +1520,7 @@ var KubernetesVerbs = []string{ // KubernetesClusterWideResourceKinds is the list of supported Kubernetes cluster resource kinds // that are not namespaced. -// TODO(@creack): Remove in favor of proper lookup. +// Needed to maintain backward compatibility. var KubernetesClusterWideResourceKinds = []string{ KindKubeNamespace, KindKubeNode, diff --git a/api/types/role.go b/api/types/role.go index a6cc9bea7c1..c31d7d36892 100644 --- a/api/types/role.go +++ b/api/types/role.go @@ -302,13 +302,16 @@ type Role interface { Clone() Role } +// DefaultRoleVersion for NewRole() and test helpers. +// When incrementing the role version, make sure to update the +// role version in the asset file used by the UI. +// See: web/packages/teleport/src/Roles/templates/role.yaml +const DefaultRoleVersion = V8 + // NewRole constructs new standard V8 role. // This creates a V8 role with V4+ RBAC semantics. func NewRole(name string, spec RoleSpecV6) (Role, error) { - // When incrementing the role version, make sure to update the - // role version in the asset file used by the UI. - // See: web/packages/teleport/src/Roles/templates/role.yaml - role, err := NewRoleWithVersion(name, V8, spec) + role, err := NewRoleWithVersion(name, DefaultRoleVersion, spec) return role, trace.Wrap(err) } @@ -463,39 +466,83 @@ func (r *RoleV6) GetKubeResources(rct RoleConditionType) []KubernetesResource { if rct == Allow { return r.convertAllowKubernetesResourcesBetweenRoleVersions(r.Spec.Allow.KubernetesResources) } - return r.convertDenyKubernetesResourcesBetweenRoleVersions(r.Spec.Deny.KubernetesResources) + return r.convertKubernetesResourcesBetweenRoleVersions(r.Spec.Deny.KubernetesResources) } -// convertDenyKubernetesResourcesBetweenRoleVersions converts Kubernetes resources between role versions. +// convertKubernetesResourcesBetweenRoleVersions converts Kubernetes resources between role versions. // This is required to keep compatibility between role versions to avoid breaking changes // when using an older role version. // // For roles v8, it returns the list as it is. // // For roles <=v7, it maps the legacy teleport Kinds to k8s plurals and sets the APIGroup to wildcard. -func (r *RoleV6) convertDenyKubernetesResourcesBetweenRoleVersions(resources []KubernetesResource) []KubernetesResource { +func (r *RoleV6) convertKubernetesResourcesBetweenRoleVersions(resources []KubernetesResource) []KubernetesResource { switch r.Version { case V8: return resources default: v7resources := slices.Clone(resources) + var extraResources []KubernetesResource for i, r := range v7resources { + // "namespace" kind used to mean "namespaces" and all resources in the namespace. + // It is now represented by 'namespaces' for the resource itself and wildcard for + // all resources in the namespace. + if r.Kind == KindKubeNamespace { + r.Kind = Wildcard + if r.Name == Wildcard { + r.Namespace = "^" + Wildcard + "$" + } else { + r.Namespace = r.Name + } + r.Name = Wildcard + r.APIGroup = Wildcard + v7resources[i] = r + extraResources = append(extraResources, KubernetesResource{ + Kind: "namespaces", + Name: r.Namespace, + Verbs: r.Verbs, + }) + continue + } + // The namespace field was ignored in v7 for global resources. + if r.Namespace != "" && slices.Contains(KubernetesClusterWideResourceKinds, r.Kind) { + r.Namespace = "" + } if k, ok := KubernetesResourcesKindsPlurals[r.Kind]; ok { // Can be empty if the kind is a wildcard. r.Kind = k } r.APIGroup = Wildcard v7resources[i] = r + if r.Kind == Wildcard { // If we have a wildcard, inject the clusterwide resources. + for _, elem := range KubernetesClusterWideResourceKinds { + if elem == KindKubeNamespace { // Namespace is handled separately. + continue + } + extraResources = append(extraResources, KubernetesResource{ + Kind: KubernetesResourcesKindsPlurals[elem], + Name: r.Name, + Verbs: r.Verbs, + APIGroup: Wildcard, + }) + } + } } - return v7resources + return append(v7resources, extraResources...) } } -// convertKubeResourcesBetweenRoleVersions converts Kubernetes resources between role versions. +// convertAllowKubeResourcesBetweenRoleVersions converts Kubernetes resources between role versions. // This is required to keep compatibility between role versions to avoid breaking changes // when using an older role version. // // For roles v8, it returns the list as it is. // +// For roles v7, if we have a Wildcard kind, add the v7 cluster-wide resources to maintain +// the existing behavior as in Teleport <=v17, those resources ignored the namespace value +// of the rbac entry. Earlier roles didn't support wildcard so it is not a concern. +// +// For roles v7, if we have a "namespace" kind, map it to a wildcard + namespaces kind. +// // For roles <=v7, it sets the APIGroup to wildcard for all resources and maps the legacy // teleport Kinds to k8s plurals. // @@ -507,18 +554,9 @@ func (r *RoleV6) convertDenyKubernetesResourcesBetweenRoleVersions(resources []K // and append the other supported resources - KubernetesResourcesKinds - for Role v8. func (r *RoleV6) convertAllowKubernetesResourcesBetweenRoleVersions(resources []KubernetesResource) []KubernetesResource { switch r.Version { - case V8: - return resources - case V7: - v7resources := slices.Clone(resources) - for i, r := range v7resources { - if k, ok := KubernetesResourcesKindsPlurals[r.Kind]; ok { // Can be empty if the kind is a wildcard. - r.Kind = k - } - r.APIGroup = Wildcard - v7resources[i] = r - } - return v7resources + case V7, V8: + // V7 and v8 uses the same logic for allow and deny. + return r.convertKubernetesResourcesBetweenRoleVersions(resources) // Teleport does not support role versions < v3. case V6, V5, V4, V3: switch { @@ -1232,9 +1270,10 @@ func (r *RoleV6) CheckAndSetDefaults() error { return trace.Wrap(err) } case V7: - // Kubernetes resources default to {kind:*, name:*, namespace:*} for v7 and v8 roles. + // Kubernetes resources default to {kind:*, name:*, namespace:*, verbs:[*]} for v7 roles. if len(r.Spec.Allow.KubernetesResources) == 0 && r.HasLabelMatchers(Allow, KindKubernetesCluster) { r.Spec.Allow.KubernetesResources = []KubernetesResource{ + // Full access to everything. { Kind: Wildcard, Namespace: Wildcard, @@ -1247,9 +1286,10 @@ func (r *RoleV6) CheckAndSetDefaults() error { return trace.Wrap(err) } case V8: - // Kubernetes resources default to {kind:*, name:*, namespace:*, group:*} for v7 and v8 roles. + // Kubernetes resources default to {kind:*, name:*, namespace:*, api_group:*, verbs:[*]} for v8 roles. if len(r.Spec.Allow.KubernetesResources) == 0 && r.HasLabelMatchers(Allow, KindKubernetesCluster) { r.Spec.Allow.KubernetesResources = []KubernetesResource{ + // Full access to everything. { Kind: Wildcard, Namespace: Wildcard, @@ -1956,17 +1996,17 @@ func validateKubeResources(roleVersion string, kubeResources []KubernetesResourc fallthrough case V7: if kubeResource.APIGroup != "" { - return trace.BadParameter("Group %q is not supported in role version %q. Upgrade the role version to %q", kubeResource.APIGroup, roleVersion, V8) + return trace.BadParameter("API Group %q is not supported in role version %q. Upgrade the role version to %q", kubeResource.APIGroup, roleVersion, V8) } if kubeResource.Kind != Wildcard && !slices.Contains(KubernetesResourcesKinds, kubeResource.Kind) { return trace.BadParameter("KubernetesResource kind %q is invalid or unsupported; Supported: %v", kubeResource.Kind, append([]string{Wildcard}, KubernetesResourcesKinds...)) } if kubeResource.Namespace == "" && !slices.Contains(KubernetesClusterWideResourceKinds, kubeResource.Kind) { - return trace.BadParameter("KubernetesResource must include Namespace") + return trace.BadParameter("KubernetesResource kind %q must include Namespace", kubeResource.Kind) } case V8: if kubeResource.Kind == "" { - return trace.BadParameter("KubernetesResource kind is required in role version %q", roleVersion) + return trace.BadParameter("KubernetesResource kind %q is required in role version %q", kubeResource.Kind, roleVersion) } // If we have a kind that match a role v7 one, check the api group. if slices.Contains(KubernetesResourcesKinds, kubeResource.Kind) { diff --git a/api/types/role_test.go b/api/types/role_test.go index 80a532fb140..b7955781add 100644 --- a/api/types/role_test.go +++ b/api/types/role_test.go @@ -357,6 +357,182 @@ func TestRole_GetKubeResources(t *testing.T) { }, }, }, + { + name: "v7 with allow wildcard kind", + args: args{ + version: V7, + labels: kubeLabels, + resources: []KubernetesResource{ + { + // rolev7 ignored the namespace field for global resources. + Kind: Wildcard, + Namespace: "default", + Name: Wildcard, + Verbs: []string{Wildcard}, + }, + }, + }, + assertErrorCreation: require.NoError, + wantAllow: []KubernetesResource{ + // Expect the main resource to match namespaced resources. + { + Kind: Wildcard, + Namespace: "default", + Name: Wildcard, + Verbs: []string{Wildcard}, + APIGroup: Wildcard, + }, + // Expect injected global resources to maintain v7 behavior. + { + Kind: "nodes", + Name: Wildcard, + Verbs: []string{Wildcard}, + APIGroup: Wildcard, + }, + { + Kind: "persistentvolumes", + Name: Wildcard, + Verbs: []string{Wildcard}, + APIGroup: Wildcard, + }, + { + Kind: "clusterroles", + Name: Wildcard, + Verbs: []string{Wildcard}, + APIGroup: Wildcard, + }, + { + Kind: "clusterrolebindings", + Name: Wildcard, + Verbs: []string{Wildcard}, + APIGroup: Wildcard, + }, + { + Kind: "certificatesigningrequests", + Name: Wildcard, + Verbs: []string{Wildcard}, + APIGroup: Wildcard, + }, + }, + }, + { + name: "v7 with deny wildcard kind", + args: args{ + version: V7, + labels: kubeLabels, + resources: []KubernetesResource{ + { + // rolev7 ignored the namespace field for global resources. + Kind: Wildcard, + Namespace: "default", + Name: Wildcard, + Verbs: []string{Wildcard}, + }, + }, + }, + assertErrorCreation: require.NoError, + wantDeny: []KubernetesResource{ + // Expect the main resource to match namespaced resources. + { + Kind: Wildcard, + Namespace: "default", + Name: Wildcard, + Verbs: []string{Wildcard}, + APIGroup: Wildcard, + }, + // Expect injected global resources to maintain v7 behavior. + { + Kind: "nodes", + Name: Wildcard, + Verbs: []string{Wildcard}, + APIGroup: Wildcard, + }, + { + Kind: "persistentvolumes", + Name: Wildcard, + Verbs: []string{Wildcard}, + APIGroup: Wildcard, + }, + { + Kind: "clusterroles", + Name: Wildcard, + Verbs: []string{Wildcard}, + APIGroup: Wildcard, + }, + { + Kind: "clusterrolebindings", + Name: Wildcard, + Verbs: []string{Wildcard}, + APIGroup: Wildcard, + }, + { + Kind: "certificatesigningrequests", + Name: Wildcard, + Verbs: []string{Wildcard}, + APIGroup: Wildcard, + }, + }, + }, + { + name: "v7 with allow namespace", + args: args{ + version: V7, + labels: kubeLabels, + resources: []KubernetesResource{ + { + Kind: KindKubeNamespace, + Name: "default", + Verbs: []string{Wildcard}, + }, + }, + }, + assertErrorCreation: require.NoError, + wantAllow: []KubernetesResource{ + { + Kind: Wildcard, + Namespace: "default", + Name: Wildcard, + Verbs: []string{Wildcard}, + APIGroup: Wildcard, + }, + { + Kind: "namespaces", + Name: "default", + Verbs: []string{Wildcard}, + }, + }, + }, + + { + name: "v7 with deny namespace", + args: args{ + version: V7, + labels: kubeLabels, + resources: []KubernetesResource{ + { + Kind: KindKubeNamespace, + Name: "default", + Verbs: []string{Wildcard}, + }, + }, + }, + assertErrorCreation: require.NoError, + wantDeny: []KubernetesResource{ + { + Kind: Wildcard, + Namespace: "default", + Name: Wildcard, + Verbs: []string{Wildcard}, + APIGroup: Wildcard, + }, + { + Kind: "namespaces", + Name: "default", + Verbs: []string{Wildcard}, + }, + }, + }, + { name: "v6 without wildcard; labels expression", args: args{ diff --git a/lib/auth/grpcserver.go b/lib/auth/grpcserver.go index 29d5bf96a20..66e06cb70af 100644 --- a/lib/auth/grpcserver.go +++ b/lib/auth/grpcserver.go @@ -2149,12 +2149,8 @@ func maybeDowngradeRoleK8sAPIGroupToV7(role *types.RoleV6) *types.RoleV6 { if elem.APIGroup == types.Wildcard { elem.APIGroup = "" } - // If we have a wildcard kind, keep it. - if elem.Kind == types.Wildcard && elem.APIGroup == "" { - out = append(out, elem) - continue - } - // If Kind is known in v7 and group is known, remove it. + + // If Kind is known in v7 and group is known, remove it the api group and keep the resource. if v, ok := defaultRBACResources[allowedResourcesKey{elem.APIGroup, elem.Kind}]; ok { elem.APIGroup = "" elem.Kind = v @@ -2162,8 +2158,8 @@ func maybeDowngradeRoleK8sAPIGroupToV7(role *types.RoleV6) *types.RoleV6 { continue } - // If we reach this point, we are dealing with a resource we don't know about. - // As <=v17 granted too much access for unknown resource, we deny everything. + // If we reach this point, we are dealing with a resource we don't know about or a wildcard + // As the scope of permissions granted differs, deny everything. role.Spec.Allow.KubernetesResources = nil role.Spec.Deny.KubernetesLabels = types.Labels{ types.Wildcard: {types.Wildcard}, @@ -2200,11 +2196,12 @@ type allowedResourcesKey struct { // TODO(@creack): Delete in v19.0.0. // Only used in the maybeDowngradeRoleVersionToV7 function above. // Must be synced with the defaultRBACResources map in lib/kube/proxy/url.go. +// NOTE: 'namespaces' is not included as the v8 behavior is different from v7. +// A 'namespaces' resource in v8 would result in wildcard deny in older versions. var defaultRBACResources = map[allowedResourcesKey]string{ {apiGroup: "", resourceKind: "pods"}: types.KindKubePod, {apiGroup: "", resourceKind: "secrets"}: types.KindKubeSecret, {apiGroup: "", resourceKind: "configmaps"}: types.KindKubeConfigmap, - {apiGroup: "", resourceKind: "namespaces"}: types.KindKubeNamespace, {apiGroup: "", resourceKind: "services"}: types.KindKubeService, {apiGroup: "", resourceKind: "endpoints"}: types.KindKubeService, {apiGroup: "", resourceKind: "serviceaccounts"}: types.KindKubeServiceAccount, diff --git a/lib/auth/grpcserver_test.go b/lib/auth/grpcserver_test.go index 20deafd1a7b..281145695ad 100644 --- a/lib/auth/grpcserver_test.go +++ b/lib/auth/grpcserver_test.go @@ -5372,13 +5372,6 @@ func TestRoleVersionV8ToV7Downgrade(t *testing.T) { Verbs: []string{types.Wildcard}, APIGroup: types.Wildcard, }, - { - Kind: types.Wildcard, - Name: types.Wildcard, - Namespace: types.Wildcard, - Verbs: []string{types.Wildcard}, - APIGroup: types.Wildcard, - }, { Kind: "ingresses", Name: types.Wildcard, @@ -5404,13 +5397,6 @@ func TestRoleVersionV8ToV7Downgrade(t *testing.T) { Verbs: []string{types.Wildcard}, APIGroup: types.Wildcard, }, - { - Kind: types.Wildcard, - Name: types.Wildcard, - Namespace: types.Wildcard, - Verbs: []string{types.Wildcard}, - APIGroup: types.Wildcard, - }, { Kind: "ingresses", Name: types.Wildcard, @@ -5459,6 +5445,43 @@ func TestRoleVersionV8ToV7Downgrade(t *testing.T) { }, }, }) + downgradev7incompatibleNamespace := newRole("downgrade_v7_incompatible_namespace", types.V8, types.RoleSpecV6{ + Allow: types.RoleConditions{ + KubernetesResources: []types.KubernetesResource{ + { + Kind: "namespaces", + Name: "foo", + Verbs: []string{types.Wildcard}, + }, + }, + }, + }) + downgradev7incompatibleNamespacedWildcard := newRole("downgrade_v7_incompatible_namespaced_wildcard", types.V8, types.RoleSpecV6{ + Allow: types.RoleConditions{ + KubernetesResources: []types.KubernetesResource{ + { + Kind: types.Wildcard, + Name: "foo", + Namespace: "bar", + Verbs: []string{"get"}, + APIGroup: types.Wildcard, + }, + }, + }, + }) + downgradev7incompatibleClusterWideWildcard := newRole("downgrade_v7_incompatible_cluster_wide_wildcard", types.V8, types.RoleSpecV6{ + Allow: types.RoleConditions{ + KubernetesResources: []types.KubernetesResource{ + { + Kind: types.Wildcard, + Name: "bar", + Namespace: "", // Cluster wide. + Verbs: []string{"get"}, + APIGroup: types.Wildcard, + }, + }, + }, + }) downgradev7mixedK8sResourcesRole := newRole("downgrade_v7_mixed_k8s_resources2", types.V8, types.RoleSpecV6{ Allow: types.RoleConditions{ KubernetesResources: []types.KubernetesResource{ @@ -5515,6 +5538,9 @@ func TestRoleVersionV8ToV7Downgrade(t *testing.T) { testRole1, downgradev7comptibleK8sResourcesRole, downgradev7incompatibleK8sResourcesRole, + downgradev7incompatibleNamespace, + downgradev7incompatibleNamespacedWildcard, + downgradev7incompatibleClusterWideWildcard, downgradev7mixedK8sResourcesRole, ) require.NoError(t, err) @@ -5552,12 +5578,6 @@ func TestRoleVersionV8ToV7Downgrade(t *testing.T) { Namespace: types.Wildcard, Verbs: []string{types.Wildcard}, }, - { - Kind: types.Wildcard, - Name: types.Wildcard, - Namespace: types.Wildcard, - Verbs: []string{types.Wildcard}, - }, { Kind: types.KindKubeIngress, Name: types.Wildcard, @@ -5580,12 +5600,6 @@ func TestRoleVersionV8ToV7Downgrade(t *testing.T) { Namespace: types.Wildcard, Verbs: []string{types.Wildcard}, }, - { - Kind: types.Wildcard, - Name: types.Wildcard, - Namespace: types.Wildcard, - Verbs: []string{types.Wildcard}, - }, { Kind: types.KindKubeIngress, Name: types.Wildcard, @@ -5643,6 +5657,105 @@ func TestRoleVersionV8ToV7Downgrade(t *testing.T) { }), expectDowngraded: true, }, + { + desc: "downgrade v7 incompatible k8s namespaces kind", + clientVersions: []string{ + "17.2.7", + }, + inputRole: downgradev7incompatibleNamespace, + expectedRole: newRole(downgradev7incompatibleNamespace.GetName(), types.V7, types.RoleSpecV6{ + Allow: types.RoleConditions{ + KubernetesResources: nil, + }, + Deny: types.RoleConditions{ + KubernetesLabels: types.Labels{ + types.Wildcard: {types.Wildcard}, + }, + KubernetesResources: []types.KubernetesResource{ + { + Kind: types.Wildcard, + Name: types.Wildcard, + Namespace: types.Wildcard, + Verbs: []string{types.Wildcard}, + }, + }, + }, + Options: types.RoleOptions{ + IDP: &types.IdPOptions{ + SAML: &types.IdPSAMLOptions{ + Enabled: types.NewBoolOption(false), + }, + }, + }, + }), + expectDowngraded: true, + }, + { + desc: "downgrade v7 incompatible k8s namespaced wildcard kind", + clientVersions: []string{ + "17.2.7", + }, + inputRole: downgradev7incompatibleNamespacedWildcard, + expectedRole: newRole(downgradev7incompatibleNamespacedWildcard.GetName(), types.V7, types.RoleSpecV6{ + Allow: types.RoleConditions{ + KubernetesResources: nil, + }, + Deny: types.RoleConditions{ + KubernetesLabels: types.Labels{ + types.Wildcard: {types.Wildcard}, + }, + KubernetesResources: []types.KubernetesResource{ + { + Kind: types.Wildcard, + Name: types.Wildcard, + Namespace: types.Wildcard, + Verbs: []string{types.Wildcard}, + }, + }, + }, + Options: types.RoleOptions{ + IDP: &types.IdPOptions{ + SAML: &types.IdPSAMLOptions{ + Enabled: types.NewBoolOption(false), + }, + }, + }, + }), + expectDowngraded: true, + }, + { + desc: "downgrade v7 incompatible k8s cluster wide wildcard kind", + clientVersions: []string{ + "17.2.7", + }, + inputRole: downgradev7incompatibleClusterWideWildcard, + expectedRole: newRole(downgradev7incompatibleClusterWideWildcard.GetName(), types.V7, types.RoleSpecV6{ + Allow: types.RoleConditions{ + KubernetesResources: nil, + }, + Deny: types.RoleConditions{ + KubernetesLabels: types.Labels{ + types.Wildcard: {types.Wildcard}, + }, + KubernetesResources: []types.KubernetesResource{ + { + Kind: types.Wildcard, + Name: types.Wildcard, + Namespace: types.Wildcard, + Verbs: []string{types.Wildcard}, + }, + }, + }, + Options: types.RoleOptions{ + IDP: &types.IdPOptions{ + SAML: &types.IdPSAMLOptions{ + Enabled: types.NewBoolOption(false), + }, + }, + }, + }), + expectDowngraded: true, + }, { desc: "downgrade v7 mixed k8s resources", clientVersions: []string{ diff --git a/lib/auth/helpers.go b/lib/auth/helpers.go index 9e17bc00d4e..6780ef8eac8 100644 --- a/lib/auth/helpers.go +++ b/lib/auth/helpers.go @@ -1395,11 +1395,19 @@ func CreateUser(ctx context.Context, clt clt, username string, roles ...types.Ro type createUserAndRoleOptions struct { mutateUser []func(user types.User) mutateRole []func(role types.Role) + version string } // CreateUserAndRoleOption is a functional option for CreateUserAndRole type CreateUserAndRoleOption func(*createUserAndRoleOptions) +// WithRoleVersion sets the version of the role to be created. +func WithRoleVersion(version string) CreateUserAndRoleOption { + return func(o *createUserAndRoleOptions) { + o.version = version + } +} + // WithUserMutator sets a function that will be called to mutate the user before it is created func WithUserMutator(mutate ...func(user types.User)) CreateUserAndRoleOption { return func(o *createUserAndRoleOptions) { @@ -1419,7 +1427,9 @@ func WithRoleMutator(mutate ...func(role types.Role)) CreateUserAndRoleOption { // If allowRules is not-nil, then the rules associated with the role will be // replaced with those specified. func CreateUserAndRole(clt clt, username string, allowedLogins []string, allowRules []types.Rule, opts ...CreateUserAndRoleOption) (types.User, types.Role, error) { - o := createUserAndRoleOptions{} + o := createUserAndRoleOptions{ + version: types.DefaultRoleVersion, + } for _, opt := range opts { opt(&o) } @@ -1429,7 +1439,7 @@ func CreateUserAndRole(clt clt, username string, allowedLogins []string, allowRu return nil, nil, trace.Wrap(err) } - role := services.RoleForUser(user) + role := services.RoleWithVersionForUser(user, o.version) role.SetLogins(types.Allow, allowedLogins) if allowRules != nil { role.SetRules(types.Allow, allowRules) diff --git a/lib/kube/grpc/grpc_test.go b/lib/kube/grpc/grpc_test.go index af1ff431165..952cecc1322 100644 --- a/lib/kube/grpc/grpc_test.go +++ b/lib/kube/grpc/grpc_test.go @@ -90,7 +90,15 @@ func TestListKubernetesResources(t *testing.T) { // override the role to allow access to all kube resources. r.SetKubeResources( types.Allow, - []types.KubernetesResource{{Kind: types.Wildcard, Name: types.Wildcard, Namespace: types.Wildcard, Verbs: []string{types.Wildcard}, APIGroup: types.Wildcard}}, + []types.KubernetesResource{ + { + Kind: types.Wildcard, + Name: types.Wildcard, + Namespace: types.Wildcard, + Verbs: []string{types.Wildcard}, + APIGroup: types.Wildcard, + }, + }, ) }, }, @@ -555,7 +563,7 @@ func TestListKubernetesResources(t *testing.T) { Kind: types.KindKubeClusterRole, Version: "v1", Metadata: types.Metadata{ - Name: "nginx-1", + Name: "cr-nginx-1", Namespace: "default", }, Spec: types.KubernetesResourceSpecV1{}, @@ -564,7 +572,7 @@ func TestListKubernetesResources(t *testing.T) { Kind: types.KindKubeClusterRole, Version: "v1", Metadata: types.Metadata{ - Name: "nginx-2", + Name: "cr-nginx-2", Namespace: "default", }, Spec: types.KubernetesResourceSpecV1{}, @@ -573,7 +581,7 @@ func TestListKubernetesResources(t *testing.T) { Kind: types.KindKubeClusterRole, Version: "v1", Metadata: types.Metadata{ - Name: "test", + Name: "cr-test", Namespace: "default", }, Spec: types.KubernetesResourceSpecV1{}, diff --git a/lib/kube/proxy/resource_filters.go b/lib/kube/proxy/resource_filters.go index 5a177607a81..887b874da6e 100644 --- a/lib/kube/proxy/resource_filters.go +++ b/lib/kube/proxy/resource_filters.go @@ -75,6 +75,7 @@ func newResourceFilterer(kind, group, verb string, isClusterWideResource bool, c // wildcardFilter is a filter that matches all pods. var wildcardFilter = types.KubernetesResource{ Kind: types.Wildcard, + APIGroup: types.Wildcard, Namespace: types.Wildcard, Name: types.Wildcard, Verbs: []string{types.Wildcard}, @@ -84,6 +85,7 @@ var wildcardFilter = types.KubernetesResource{ func containsWildcard(resources []types.KubernetesResource) bool { for _, r := range resources { if r.Kind == wildcardFilter.Kind && + r.APIGroup == wildcardFilter.APIGroup && r.Name == wildcardFilter.Name && r.Namespace == wildcardFilter.Namespace && len(r.Verbs) == 1 && r.Verbs[0] == wildcardFilter.Verbs[0] { diff --git a/lib/kube/proxy/resource_rbac_test.go b/lib/kube/proxy/resource_rbac_test.go index 5ec6fe78b9b..2e75edb6b14 100644 --- a/lib/kube/proxy/resource_rbac_test.go +++ b/lib/kube/proxy/resource_rbac_test.go @@ -1223,10 +1223,12 @@ func TestDeleteCRDCollectionRBAC(t *testing.T) { } func TestListClusterRoleRBAC(t *testing.T) { + t.Parallel() + const ( usernameWithFullAccess = "full_user" usernameWithLimitedAccess = "limited_user" - testPodName = "test" + testClusterRoleName = "cr-test" ) // kubeMock is a Kubernetes API mock for the session tests. // Once a new session is created, this mock will write to @@ -1286,7 +1288,7 @@ func TestListClusterRoleRBAC(t *testing.T) { []types.KubernetesResource{ { Kind: "clusterroles", - Name: "nginx-*", + Name: "cr-nginx-*", Verbs: []string{types.Wildcard}, APIGroup: types.Wildcard, }, @@ -1317,9 +1319,9 @@ func TestListClusterRoleRBAC(t *testing.T) { }, want: want{ listClusterRolesResult: []string{ - "nginx-1", - "nginx-2", - "test", + "cr-nginx-1", + "cr-nginx-2", + "cr-test", }, }, }, @@ -1330,13 +1332,13 @@ func TestListClusterRoleRBAC(t *testing.T) { }, want: want{ listClusterRolesResult: []string{ - "nginx-1", - "nginx-2", + "cr-nginx-1", + "cr-nginx-2", }, getTestResult: &kubeerrors.StatusError{ ErrStatus: metav1.Status{ Status: "Failure", - Message: "clusterroles \"test\" is forbidden: User \"limited_user\" cannot get resource \"clusterroles\" in API group \"rbac.authorization.k8s.io\"", + Message: "clusterroles \"cr-test\" is forbidden: User \"limited_user\" cannot get resource \"clusterroles\" in API group \"rbac.authorization.k8s.io\"", Code: 403, Reason: metav1.StatusReasonForbidden, }, @@ -1354,7 +1356,7 @@ func TestListClusterRoleRBAC(t *testing.T) { ClusterName: testCtx.ClusterName, Kind: types.KindKubePod, Name: kubeCluster, - SubResourceName: fmt.Sprintf("%s/%s", metav1.NamespaceDefault, testPodName), + SubResourceName: fmt.Sprintf("%s/%s", metav1.NamespaceDefault, testClusterRoleName), }, ), }, @@ -1372,7 +1374,7 @@ func TestListClusterRoleRBAC(t *testing.T) { getTestResult: &kubeerrors.StatusError{ ErrStatus: metav1.Status{ Status: "Failure", - Message: "clusterroles \"test\" is forbidden: User \"limited_user\" cannot get resource \"clusterroles\" in API group \"rbac.authorization.k8s.io\"", + Message: "clusterroles \"cr-test\" is forbidden: User \"limited_user\" cannot get resource \"clusterroles\" in API group \"rbac.authorization.k8s.io\"", Code: 403, Reason: metav1.StatusReasonForbidden, }, @@ -1414,7 +1416,7 @@ func TestListClusterRoleRBAC(t *testing.T) { _, err = client.RbacV1().ClusterRoles().Get( testCtx.Context, - testPodName, + testClusterRoleName, metav1.GetOptions{}, ) @@ -1429,6 +1431,8 @@ func TestListClusterRoleRBAC(t *testing.T) { } func TestGenericCustomResourcesRBAC(t *testing.T) { + t.Parallel() + const ( usernameWithFullAccess = "full_user" usernameWithLimitedAccess = "limited_user" @@ -1453,10 +1457,11 @@ func TestGenericCustomResourcesRBAC(t *testing.T) { SetupRoleFunc: func(r types.Role) { r.SetKubeResources(types.Allow, []types.KubernetesResource{ { - Kind: "namespaces", - Name: types.Wildcard, - Verbs: []string{types.Wildcard}, - APIGroup: types.Wildcard, + Kind: types.Wildcard, + Name: types.Wildcard, + Namespace: types.Wildcard, + Verbs: []string{types.Wildcard}, + APIGroup: types.Wildcard, }, }) }, @@ -1476,10 +1481,16 @@ func TestGenericCustomResourcesRBAC(t *testing.T) { r.SetKubeResources(types.Allow, []types.KubernetesResource{ { - Kind: "namespaces", - Name: "dev", - Verbs: []string{types.Wildcard}, - APIGroup: types.Wildcard, + Kind: types.Wildcard, + Name: types.Wildcard, + Namespace: "dev", + Verbs: []string{types.Wildcard}, + APIGroup: types.Wildcard, + }, + { + Kind: "namespaces", + Name: "dev", + Verbs: []string{types.Wildcard}, }, }, ) @@ -2022,7 +2033,7 @@ func TestSpecificCustomResourcesRBAC(t *testing.T) { { Kind: clusterswagv0.GetKindPlural(), Name: "clusterswag-*", - Namespace: types.Wildcard, + Namespace: "", Verbs: []string{types.Wildcard}, APIGroup: types.Wildcard, }, @@ -2041,6 +2052,24 @@ func TestSpecificCustomResourcesRBAC(t *testing.T) { }, { name: "cluster wide crd no access", + args: args{ + user: newUser("cluster_crd_ko", []types.KubernetesResource{ + { + Kind: telerolev8.GetKindPlural(), + Name: types.Wildcard, + Namespace: "", + Verbs: []string{types.Wildcard}, + APIGroup: types.Wildcard, + }, + }), + crds: []*tkm.CRD{clusterswagv0}, + }, + want: want{ + wantListErr: []bool{true}, + }, + }, + { + name: "cluster wide crd no acces wildcard", args: args{ user: newUser("cluster_crd_ko", []types.KubernetesResource{ { diff --git a/lib/kube/proxy/self_subject_reviews.go b/lib/kube/proxy/self_subject_reviews.go index febfbb2a4e5..ba74c30ac7c 100644 --- a/lib/kube/proxy/self_subject_reviews.go +++ b/lib/kube/proxy/self_subject_reviews.go @@ -144,14 +144,14 @@ func (f *Forwarder) validateSelfSubjectAccessReview(sess *clusterSession, w http // Append a matcher that validates if the Kubernetes resource is allowed // by the roles that satisfy the Kubernetes Cluster. &kubernetesResourceMatcher{ - types.KubernetesResource{ + resource: types.KubernetesResource{ Kind: resource.Name, Name: name, Namespace: namespace, Verbs: []string{accessReview.Spec.ResourceAttributes.Verb}, APIGroup: accessReview.Spec.ResourceAttributes.Group, }, - !resource.Namespaced, + isClusterWideResource: !resource.Namespaced, }, }...); { case errors.Is(err, services.ErrTrustedDeviceRequired): @@ -309,12 +309,20 @@ func (m *kubernetesResourceMatcher) Match(role types.Role, condition types.RoleC if len(m.resource.Verbs) == 1 && !isVerbAllowed(resource.Verbs, m.resource.Verbs[0]) { continue } + + switch ok, err := utils.SliceMatchesRegex(m.resource.APIGroup, []string{resource.APIGroup}); { + case err != nil: + return false, trace.Wrap(err) + case !ok: + continue + } + // If the resource name and namespace are empty, it means that the // user wants to match all resources of the specified kind. // We can return true immediately because the user is allowed to get resources // of the specified kind but might not be able to see any if the matchers do not // match with any resource. - if name == "" && namespace == "" { + if (resource.Namespace == "" || resource.Namespace == types.Wildcard) && name == "" && namespace == "" { return true, nil } // If the resource name isn't empty but the resource kind is a namespace scope @@ -334,11 +342,10 @@ func (m *kubernetesResourceMatcher) Match(role types.Role, condition types.RoleC return ok, trace.Wrap(err) } } else { - if ok, err := utils.SliceMatchesRegex(namespace, []string{resource.Namespace}); err != nil || ok || namespace == "" { - return ok || namespace == "", trace.Wrap(err) + if ok, err := utils.SliceMatchesRegex(namespace, []string{resource.Namespace}); err != nil || ok { + return ok, trace.Wrap(err) } } - } return false, nil diff --git a/lib/kube/proxy/testing/kube_server/clusterrole.go b/lib/kube/proxy/testing/kube_server/clusterrole.go index 007f389cb11..88eb1f2e951 100644 --- a/lib/kube/proxy/testing/kube_server/clusterrole.go +++ b/lib/kube/proxy/testing/kube_server/clusterrole.go @@ -39,9 +39,9 @@ var clusterRoleList = authv1.ClusterRoleList{ ResourceVersion: "1231415", }, Items: []authv1.ClusterRole{ - newClusterRole("nginx-1"), - newClusterRole("nginx-2"), - newClusterRole("test"), + newClusterRole("cr-nginx-1"), + newClusterRole("cr-nginx-2"), + newClusterRole("cr-test"), }, } diff --git a/lib/kube/proxy/testing/kube_server/kube_mock.go b/lib/kube/proxy/testing/kube_server/kube_mock.go index ca0e1e2cec0..7342f791b8e 100644 --- a/lib/kube/proxy/testing/kube_server/kube_mock.go +++ b/lib/kube/proxy/testing/kube_server/kube_mock.go @@ -29,6 +29,7 @@ import ( "log/slog" "net/http" "net/http/httptest" + "regexp" "strings" "sync" "sync/atomic" @@ -168,7 +169,6 @@ type KubeUpgradeRequests struct { } type KubeMockServer struct { - router *httprouter.Router log *slog.Logger server *httptest.Server TLS *tls.Config @@ -197,7 +197,6 @@ type KubeMockServer struct { // TODO(tigrato): add support for other endpoints func NewKubeAPIMock(opts ...Option) (*KubeMockServer, error) { s := &KubeMockServer{ - router: httprouter.New(), log: slog.Default(), deletedResources: make(map[deletedResource][]string), version: &apimachineryversion.Info{ @@ -230,46 +229,53 @@ func NewKubeAPIMock(opts ...Option) (*KubeMockServer, error) { } func (s *KubeMockServer) setup() { - s.router.UseRawPath = true - s.router.POST("/api/:ver/namespaces/:namespace/pods/:name/exec", s.withWriter(s.exec)) - s.router.GET("/api/:ver/namespaces/:namespace/pods/:name/exec", s.withWriter(s.exec)) - s.router.GET("/api/:ver/namespaces/:namespace/pods/:name/portforward", s.withWriter(s.portforward)) - s.router.POST("/api/:ver/namespaces/:namespace/pods/:name/portforward", s.withWriter(s.portforward)) + // NOTE: We use stdlib because the gravitational/httplib package doesn't support k8s patterns, + // it panics due to overlapping routes. + router := http.NewServeMux() - s.router.GET("/apis/rbac.authorization.k8s.io/:ver/clusterroles", s.withWriter(s.listClusterRoles)) - s.router.GET("/apis/rbac.authorization.k8s.io/:ver/clusterroles/:name", s.withWriter(s.getClusterRole)) - s.router.DELETE("/apis/rbac.authorization.k8s.io/:ver/clusterroles/:name", s.withWriter(s.deleteClusterRole)) - s.router.GET("/apis/rbac.authorization.k8s.io/:ver", s.withWriter(s.discoveryEndpoint)) + router.Handle("POST /api/{ver}/namespaces/{namespace}/pods/{name}/exec", s.withWriter(s.exec)) + router.Handle("GET /api/{ver}/namespaces/{namespace}/pods/{name}/exec", s.withWriter(s.exec)) + router.Handle("GET /api/{ver}/namespaces/{namespace}/pods/{name}/portforward", s.withWriter(s.portforward)) + router.Handle("POST /api/{ver}/namespaces/{namespace}/pods/{name}/portforward", s.withWriter(s.portforward)) - s.router.GET("/api/:ver/namespaces/:namespace/pods", s.withWriter(s.listPods)) - s.router.GET("/api/:ver/pods", s.withWriter(s.listPods)) - s.router.GET("/api/:ver/namespaces/:namespace/pods/:name", s.withWriter(s.getPod)) - s.router.DELETE("/api/:ver/namespaces/:namespace/pods/:name", s.withWriter(s.deletePod)) + router.Handle("GET /apis/rbac.authorization.k8s.io/{ver}/clusterroles", s.withWriter(s.listClusterRoles)) + router.Handle("GET /apis/rbac.authorization.k8s.io/{ver}/clusterroles/{name}", s.withWriter(s.getClusterRole)) + router.Handle("DELETE /apis/rbac.authorization.k8s.io/{ver}/clusterroles/{name}", s.withWriter(s.deleteClusterRole)) + router.Handle("GET /apis/rbac.authorization.k8s.io/{ver}", s.withWriter(s.discoveryEndpoint)) - s.router.GET("/api/:ver/namespaces/:namespace/secrets", s.withWriter(s.listSecrets)) - s.router.GET("/api/:ver/secrets", s.withWriter(s.listSecrets)) - s.router.GET("/api/:ver/namespaces/:namespace/secrets/:name", s.withWriter(s.getSecret)) - s.router.DELETE("/api/:ver/namespaces/:namespace/secrets/:name", s.withWriter(s.deleteSecret)) + router.Handle("GET /api/{ver}/namespaces/{namespace}/pods", s.withWriter(s.listPods)) + router.Handle("GET /api/{ver}/pods", s.withWriter(s.listPods)) + router.Handle("GET /api/{ver}/namespaces/{namespace}/pods/{name}", s.withWriter(s.getPod)) + router.Handle("DELETE /api/{ver}/namespaces/{namespace}/pods/{name}", s.withWriter(s.deletePod)) - s.router.POST("/apis/authorization.k8s.io/v1/selfsubjectaccessreviews", s.withWriter(s.selfSubjectAccessReviews)) + router.Handle("GET /api/{ver}/namespaces", s.withWriter(s.listNamespaces)) + router.Handle("GET /api/{ver}/namespaces/{name}", s.withWriter(s.getNamespace)) + router.Handle("DELETE /api/v1/namespaces/{name}", s.withWriter(s.deleteNamespace)) + + router.Handle("GET /api/{ver}/namespaces/{namespace}/secrets", s.withWriter(s.listSecrets)) + router.Handle("GET /api/{ver}/secrets", s.withWriter(s.listSecrets)) + router.Handle("GET /api/{ver}/namespaces/{namespace}/secrets/{name}", s.withWriter(s.getSecret)) + router.Handle("DELETE /api/{ver}/namespaces/{namespace}/secrets/{name}", s.withWriter(s.deleteSecret)) + + router.Handle("POST /apis/authorization.k8s.io/v1/selfsubjectaccessreviews", s.withWriter(s.selfSubjectAccessReviews)) for k, crd := range s.crds { - s.router.GET("/apis/"+k.group+"/"+k.version+"/namespaces/:namespace/"+k.plural, s.withWriter(s.listCRDs(crd))) - s.router.GET("/apis/"+k.group+"/"+k.version+"/"+k.plural, s.withWriter(s.listCRDs(crd))) - s.router.GET("/apis/"+k.group+"/"+k.version+"/namespaces/:namespace/"+k.plural+"/:name", s.withWriter(s.getCRD(crd))) - s.router.DELETE("/apis/"+k.group+"/"+k.version+"/namespaces/:namespace/"+k.plural+"/:name", s.withWriter(s.deleteCRD(crd))) + router.Handle("GET /apis/"+k.group+"/"+k.version+"/namespaces/{namespace}/"+k.plural, s.withWriter(s.listCRDs(crd))) + router.Handle("GET /apis/"+k.group+"/"+k.version+"/"+k.plural, s.withWriter(s.listCRDs(crd))) + router.Handle("GET /apis/"+k.group+"/"+k.version+"/namespaces/{namespace}/"+k.plural+"/{name}", s.withWriter(s.getCRD(crd))) + router.Handle("DELETE /apis/"+k.group+"/"+k.version+"/namespaces/{namespace}/"+k.plural+"/{name}", s.withWriter(s.deleteCRD(crd))) } - s.router.GET("/version", s.withWriter(s.versionEndpoint)) + router.Handle("GET /version", s.withWriter(s.versionEndpoint)) - for _, endpoint := range []string{"/api", "/api/:ver", "/apis"} { - s.router.GET(endpoint, s.withWriter(s.discoveryEndpoint)) + for _, endpoint := range []string{"/api", "/api/{ver}", "/apis"} { + router.Handle("GET "+endpoint, s.withWriter(s.discoveryEndpoint)) } for k, v := range s.crds { - s.router.GET("/apis/"+k.group+"/"+k.version, s.withWriter(crdDiscovery(v))) + router.Handle("GET /apis/"+k.group+"/"+k.version, s.withWriter(crdDiscovery(v))) } - s.server = httptest.NewUnstartedServer(s.router) + s.server = httptest.NewUnstartedServer(router) s.server.EnableHTTP2 = true } @@ -301,8 +307,20 @@ func (s *KubeMockServer) Close() error { return nil } -func (s *KubeMockServer) withWriter(handler httplib.HandlerFunc) httprouter.Handle { - return httplib.MakeHandlerWithErrorWriter(handler, s.formatResponseError) +var routerRe = regexp.MustCompile(`\{([^}]+)\}`) + +// withWriter handles the glue to support stdlib handler. +func (s *KubeMockServer) withWriter(handler httplib.HandlerFunc) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + matches := routerRe.FindAllStringSubmatch(r.Pattern, -1) + + p := httprouter.Params{} + for _, elem := range matches { + p = append(p, httprouter.Param{Key: elem[1], Value: r.PathValue(elem[1])}) + } + + httplib.MakeHandlerWithErrorWriter(handler, s.formatResponseError)(w, r, p) + } } func (s *KubeMockServer) formatResponseError(rw http.ResponseWriter, respErr error) { diff --git a/lib/kube/proxy/testing/kube_server/namespaces.go b/lib/kube/proxy/testing/kube_server/namespaces.go new file mode 100644 index 00000000000..69c00137eb5 --- /dev/null +++ b/lib/kube/proxy/testing/kube_server/namespaces.go @@ -0,0 +1,113 @@ +/* + * Teleport + * Copyright (C) 2025 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package kubeserver + +import ( + "net/http" + "sort" + + "github.com/gravitational/trace" + "github.com/julienschmidt/httprouter" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/gravitational/teleport/api/types" +) + +var namespaceList = corev1.NamespaceList{ + TypeMeta: metav1.TypeMeta{ + Kind: "NamespaceList", + APIVersion: "v1", + }, + ListMeta: metav1.ListMeta{ + ResourceVersion: "1231415", + }, + Items: []corev1.Namespace{ + newNamespace("default"), + newNamespace("test"), + newNamespace("dev"), + newNamespace("prod"), + }, +} + +func newNamespace(name string) corev1.Namespace { + return corev1.Namespace{ + TypeMeta: metav1.TypeMeta{ + Kind: "Namespace", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + } +} + +func (s *KubeMockServer) listNamespaces(w http.ResponseWriter, req *http.Request, p httprouter.Params) (any, error) { + list := namespaceList.DeepCopy() + return list, nil +} + +func (s *KubeMockServer) getNamespace(w http.ResponseWriter, req *http.Request, p httprouter.Params) (any, error) { + name := p.ByName("name") + filter := func(role corev1.Namespace) bool { + return role.Name == name + } + for _, role := range namespaceList.Items { + if filter(role) { + return role, nil + } + } + return nil, trace.NotFound("namespace %q not found", name) +} + +func (s *KubeMockServer) deleteNamespace(w http.ResponseWriter, req *http.Request, p httprouter.Params) (any, error) { + name := p.ByName("name") + + deleteOpts, err := parseDeleteCollectionBody(req) + if err != nil { + return nil, trace.Wrap(err) + } + reqID := "" + if deleteOpts.Preconditions != nil && deleteOpts.Preconditions.UID != nil { + reqID = string(*deleteOpts.Preconditions.UID) + } + filter := func(role corev1.Namespace) bool { + return role.Name == name + } + for _, role := range namespaceList.Items { + if filter(role) { + s.mu.Lock() + key := deletedResource{reqID, types.KindKubeNamespace} + s.deletedResources[key] = append(s.deletedResources[key], name) + s.mu.Unlock() + return role, nil + } + } + return nil, trace.NotFound("namespace %q not found", name) +} + +func (s *KubeMockServer) Deletednamespaces(reqID string) []string { + s.mu.Lock() + key := deletedResource{reqID, types.KindKubeNamespace} + deleted := make([]string, len(s.deletedResources[key])) + copy(deleted, s.deletedResources[key]) + s.mu.Unlock() + sort.Strings(deleted) + return deleted +} diff --git a/lib/kube/proxy/url.go b/lib/kube/proxy/url.go index 7958965c918..75aab4a1ec2 100644 --- a/lib/kube/proxy/url.go +++ b/lib/kube/proxy/url.go @@ -40,13 +40,14 @@ type metaResource struct { resourceDefinition *metav1.APIResource // Resource definition data from the schema. requestedResource apiResource // User input, based on URL. verb string // Verb of the user request. + isClusterWide bool // TODO(@creack): Remove this in favor of resourceDefinition.Namespaced. } func (mr *metaResource) isClusterWideResource() bool { - if mr == nil || mr.resourceDefinition == nil { + if mr == nil { return false } - return !mr.resourceDefinition.Namespaced + return mr.isClusterWide || (mr.resourceDefinition != nil && !mr.resourceDefinition.Namespaced) } func (mr *metaResource) rbacResource() *types.KubernetesResource { @@ -236,6 +237,7 @@ func getResourceFromRequest(req *http.Request, kubeDetails *kubeDetails) (metaRe // If the resource is not supported, return nil. return out, nil } + out.isClusterWide = !resource.Namespaced if apiResource.resourceName == "" && out.verb != types.KubeVerbCreate { // if the resource is supported but the resource name is not present and not a create request, diff --git a/lib/kube/proxy/utils_test.go b/lib/kube/proxy/utils_test.go index 9255517fe88..651b875f29c 100644 --- a/lib/kube/proxy/utils_test.go +++ b/lib/kube/proxy/utils_test.go @@ -451,6 +451,10 @@ type RoleSpec struct { // CreateUserWithTraitsAndRole creates Teleport user and role with specified names func (c *TestContext) CreateUserWithTraitsAndRole(ctx context.Context, t *testing.T, username string, userTraits map[string][]string, roleSpec RoleSpec) (types.User, types.Role) { + return c.CreateUserWithTraitsAndRoleVersion(ctx, t, username, userTraits, types.DefaultRoleVersion, roleSpec) +} + +func (c *TestContext) CreateUserWithTraitsAndRoleVersion(ctx context.Context, t *testing.T, username string, userTraits map[string][]string, roleVersion string, roleSpec RoleSpec) (types.User, types.Role) { user, role, err := auth.CreateUserAndRole( c.TLSServer.Auth(), username, @@ -459,6 +463,7 @@ func (c *TestContext) CreateUserWithTraitsAndRole(ctx context.Context, t *testin auth.WithUserMutator(func(user types.User) { user.SetTraits(userTraits) }), + auth.WithRoleVersion(roleVersion), ) require.NoError(t, err) role.SetKubeUsers(types.Allow, roleSpec.KubeUsers) @@ -478,7 +483,12 @@ func (c *TestContext) CreateUserWithTraitsAndRole(ctx context.Context, t *testin // CreateUserAndRole creates Teleport user and role with specified names func (c *TestContext) CreateUserAndRole(ctx context.Context, t *testing.T, username string, roleSpec RoleSpec) (types.User, types.Role) { - return c.CreateUserWithTraitsAndRole(ctx, t, username, nil, roleSpec) + return c.CreateUserAndRoleVersion(ctx, t, username, types.DefaultRoleVersion, roleSpec) +} + +// CreateUserAndRoleVersion creates Teleport user and role with specified names and role version. +func (c *TestContext) CreateUserAndRoleVersion(ctx context.Context, t *testing.T, username, roleVersion string, roleSpec RoleSpec) (types.User, types.Role) { + return c.CreateUserWithTraitsAndRoleVersion(ctx, t, username, nil, roleVersion, roleSpec) } func newKubeConfigFile(t *testing.T, clusters ...KubeClusterConfig) string { diff --git a/lib/services/access_checker.go b/lib/services/access_checker.go index 1b4cc0e3261..5ed8f1bcea1 100644 --- a/lib/services/access_checker.go +++ b/lib/services/access_checker.go @@ -508,11 +508,26 @@ func (a *accessChecker) GetKubeResources(cluster types.KubeCluster) (allowed, de } var err error rolesAllowed, rolesDenied := a.RoleSet.GetKubeResources(cluster, a.info.Traits) + + // If we have a legacy 'namespace' in the allowedResourceIDs, we need to add the new 'namespaces' one. + // The old one will get mapped to wildcard later. + allowedResourceIDs := slices.Clone(a.info.AllowedResourceIDs) + for _, elem := range a.info.AllowedResourceIDs { + if elem.Kind == types.KindKubeNamespace { + allowedResourceIDs = append(allowedResourceIDs, types.ResourceID{ + ClusterName: elem.ClusterName, + Kind: "namespaces", + SubResourceName: elem.SubResourceName, + Name: elem.Name, + }) + } + } + // 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 { + for _, r := range allowedResourceIDs { if r.Name != cluster.GetName() || r.ClusterName != a.localCluster { continue } @@ -520,6 +535,7 @@ func (a *accessChecker) GetKubeResources(cluster types.KubeCluster) (allowed, de case slices.Contains(types.KubernetesResourcesKinds, r.Kind): namespace := "" name := "" + // TODO(@creack): Make sure this gets handled in the AccessRequest PR. if slices.Contains(types.KubernetesClusterWideResourceKinds, r.Kind) { // Cluster wide resources do not have a namespace. name = r.SubResourceName @@ -540,6 +556,16 @@ func (a *accessChecker) GetKubeResources(cluster types.KubeCluster) (allowed, de if kind == "" { kind = r.Kind } + // NOTE: The 'namespace' behavior changed, to maintain backwards compatibility, + // map the legacy value to wildcard. + if r.Kind == types.KindKubeNamespace { + kind = types.Wildcard + namespace = name + if namespace == types.Wildcard { + namespace = "^" + types.Wildcard + "$" + } + name = types.Wildcard + } r := types.KubernetesResource{ Kind: kind, Namespace: namespace, diff --git a/lib/services/access_request.go b/lib/services/access_request.go index 7196f9d2801..e1700cd95b9 100644 --- a/lib/services/access_request.go +++ b/lib/services/access_request.go @@ -2391,6 +2391,7 @@ func getKubeResourcesFromResourceIDs(resourceIDs []types.ResourceID, clusterName kind = resourceID.Kind } switch { + // TODO(@creack): Make sure this is handled in the AccessRequest PR. case slices.Contains(types.KubernetesClusterWideResourceKinds, resourceID.Kind): kubernetesResources = append(kubernetesResources, types.KubernetesResource{ Kind: kind, diff --git a/lib/services/access_request_test.go b/lib/services/access_request_test.go index 57267e1fcb9..349bf31b793 100644 --- a/lib/services/access_request_test.go +++ b/lib/services/access_request_test.go @@ -2948,7 +2948,7 @@ func TestValidate_WithAllowRequestKubernetesResources(t *testing.T) { "*": {"*"}, }, KubernetesResources: []types.KubernetesResource{ - {Kind: "namespaces", Namespace: "*", Name: "*", Verbs: []string{"*"}, APIGroup: "*"}, + {Kind: "*", Namespace: "*", Name: "*", Verbs: []string{"*"}, APIGroup: "*"}, }, }, }, diff --git a/lib/services/role.go b/lib/services/role.go index 9bf3ffb97f7..130cedc5a7a 100644 --- a/lib/services/role.go +++ b/lib/services/role.go @@ -151,7 +151,14 @@ func NewImplicitRole() types.Role { // // Used in tests only. func RoleForUser(u types.User) types.Role { - role, _ := types.NewRole(RoleNameForUser(u.GetName()), types.RoleSpecV6{ + return RoleWithVersionForUser(u, types.DefaultRoleVersion) +} + +// RoleWithVersionForUser creates an admin role for a services.User. +// +// Used in tests only. +func RoleWithVersionForUser(u types.User, v string) types.Role { + role, _ := types.NewRoleWithVersion(RoleNameForUser(u.GetName()), v, types.RoleSpecV6{ Options: types.RoleOptions{ CertificateFormat: constants.CertificateFormatStandard, MaxSessionTTL: types.NewDuration(defaults.MaxCertDuration), diff --git a/lib/services/role_test.go b/lib/services/role_test.go index e6fca56bfaf..c3c8ca60be9 100644 --- a/lib/services/role_test.go +++ b/lib/services/role_test.go @@ -187,7 +187,7 @@ func TestRoleParse(t *testing.T) { } }`, error: trace.BadParameter(""), - matchMessage: "KubernetesResource must include Namespace", + matchMessage: "KubernetesResource kind \"pod\" must include Namespace", }, { name: "validation error, invalid kubernetes_resources kind", diff --git a/lib/utils/replace.go b/lib/utils/replace.go index 6ad2cda7809..a4bb24659e0 100644 --- a/lib/utils/replace.go +++ b/lib/utils/replace.go @@ -201,19 +201,6 @@ func KubeResourceMatchesRegex(input types.KubernetesResource, isClusterWideResou continue } switch { - // If the user has access to a specific namespace, they should be able to - // access all resources in that namespace. - case resource.Kind == "namespaces" && input.Namespace != "": - // Access to custom resources is determined by the access level of the - // namespace resource where the custom resource is defined. - // This is a special case because custom resources are not defined in the - // user's resources list. - // Access to namspaced resources is determined by the access level of the - // namespace resource where the resource is defined or by the access level - // of the resource if supported. - if ok, err := MatchString(input.Namespace, resource.Name); err != nil || ok { - return ok, trace.Wrap(err) - } case targetsReadOnlyNamespace && cond == types.Allow && resource.Kind != "namespaces" && resource.Namespace != "": // If the user requests a read-only namespace get/list/watch, they should // be able to see the list of namespaces they have resources defined in. @@ -223,6 +210,10 @@ func KubeResourceMatchesRegex(input types.KubernetesResource, isClusterWideResou if ok, err := MatchString(input.Name, resource.Namespace); err != nil || ok { return ok, trace.Wrap(err) } + case targetsReadOnlyNamespace && cond == types.Allow && resource.Kind == "namespaces" && resource.Name != "": + if ok, err := MatchString(input.Name, resource.Name); err != nil || ok { + return ok, trace.Wrap(err) + } default: if input.Kind != resource.Kind && resource.Kind != types.Wildcard { continue @@ -237,16 +228,16 @@ func KubeResourceMatchesRegex(input types.KubernetesResource, isClusterWideResou } else if !ok { continue } - if input.Namespace == "" && isClusterWideResource { - return true, nil - } + if input.Namespace == "" && resource.Namespace != "" && resource.Namespace != types.Wildcard { + continue + } + // At this point everything else matched. If we match the namespace as well, we have a match. if ok, err := MatchString(input.Namespace, resource.Namespace); err != nil || ok { return ok, trace.Wrap(err) } } } - return false, nil } @@ -286,23 +277,6 @@ func KubeResourceCouldMatchRules(input types.KubernetesResource, isClusterWideRe continue } switch { - // If the user has access to a specific namespace, they should be able to - // access all resources in that namespace. - case resource.Kind == "namespaces": - isAllowOrFullDeny := !isDeny || resource.Name == types.Wildcard - if input.Namespace == "" && isAllowOrFullDeny { - return isAllowOrFullDeny, nil - } - // Access to custom resources is determined by the access level of the - // namespace resource where the custom resource is defined. - // This is a special case because custom resources are not defined in the - // user's resources list. - // Access to namespaced resources is determined by the access level of the - // namespace resource where the resource is defined or by the access level - // of the resource if supported. - if ok, err := MatchString(input.Namespace, resource.Name); err != nil || ok && isAllowOrFullDeny { - return isAllowOrFullDeny || isDeny, trace.Wrap(err) - } case targetsReadOnlyNamespace && !isDeny && resource.Kind != "namespaces" && resource.Namespace != "": // If the user requests a read-only namespace get/list/watch, they should // be able to see the list of namespaces they have resources defined in. @@ -336,11 +310,13 @@ func KubeResourceCouldMatchRules(input types.KubernetesResource, isClusterWideRe if input.Namespace == "" && isAllowOrFullDeny { return isAllowOrFullDeny, nil } + if ok, err := MatchString(input.Namespace, resource.Namespace); err != nil { return false, trace.Wrap(err) } else if !ok { continue } + if !isDeny || isDeny && resource.Name == types.Wildcard { return !isDeny || isDeny && resource.Name == types.Wildcard, nil } diff --git a/lib/utils/replace_test.go b/lib/utils/replace_test.go index 6cade964f50..b6ee0261558 100644 --- a/lib/utils/replace_test.go +++ b/lib/utils/replace_test.go @@ -233,10 +233,18 @@ func TestKubeResourceMatchesRegex(t *testing.T) { resources: []types.KubernetesResource{ { Kind: types.Wildcard, + APIGroup: types.Wildcard, Namespace: types.Wildcard, Name: types.Wildcard, Verbs: []string{types.Wildcard}, }, + { + Kind: types.Wildcard, + APIGroup: types.Wildcard, + Namespace: "", + Name: types.Wildcard, + Verbs: []string{types.Wildcard}, + }, }, assert: require.NoError, action: types.Deny, @@ -472,16 +480,18 @@ func TestKubeResourceMatchesRegex(t *testing.T) { { name: "list clusterrole with resource", input: types.KubernetesResource{ - Kind: "clusterroles", - Name: "clusterrole", - Verbs: []string{types.KubeVerbGet}, + Kind: "clusterroles", + APIGroup: "rbac.authorization.k8s.io", + Name: "clusterrole", + Verbs: []string{types.KubeVerbGet}, }, isClusterWide: true, resources: []types.KubernetesResource{ { - Kind: "clusterroles", - Name: "clusterrole", - Verbs: []string{types.Wildcard}, + Kind: "clusterroles", + APIGroup: "rbac.authorization.k8s.io", + Name: "clusterrole", + Verbs: []string{types.Wildcard}, }, }, assert: require.NoError, @@ -491,16 +501,18 @@ func TestKubeResourceMatchesRegex(t *testing.T) { { name: "list clusterrole with wildcard", input: types.KubernetesResource{ - Kind: "clusterroles", - Name: "clusterrole", - Verbs: []string{types.KubeVerbGet}, + Kind: "clusterroles", + APIGroup: "rbac.authorization.k8s.io", + Name: "clusterrole", + Verbs: []string{types.KubeVerbGet}, }, isClusterWide: true, resources: []types.KubernetesResource{ { Kind: types.Wildcard, + APIGroup: types.Wildcard, Name: types.Wildcard, - Namespace: types.Wildcard, + Namespace: "", Verbs: []string{types.Wildcard}, }, }, @@ -571,7 +583,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) { }, { - name: "namespace granting read access to pod", + name: "jailed namespace granting read access to pod", input: types.KubernetesResource{ Kind: "pods", Namespace: "default", @@ -584,13 +596,20 @@ func TestKubeResourceMatchesRegex(t *testing.T) { Name: "default", Verbs: []string{types.KubeVerbGet}, }, + { + Kind: types.Wildcard, + Name: types.Wildcard, + Namespace: "default", + APIGroup: types.Wildcard, + Verbs: []string{types.KubeVerbGet}, + }, }, assert: require.NoError, action: types.Allow, matches: true, }, { - name: "namespace denying update access to pod", + name: "jailed namespace denying update access to pod", input: types.KubernetesResource{ Kind: "pods", Namespace: "default", @@ -603,13 +622,20 @@ func TestKubeResourceMatchesRegex(t *testing.T) { Name: "default", Verbs: []string{types.KubeVerbGet}, }, + { + Kind: types.Wildcard, + Name: types.Wildcard, + Namespace: "default", + APIGroup: types.Wildcard, + Verbs: []string{types.KubeVerbGet}, + }, }, assert: require.NoError, action: types.Allow, matches: false, }, { - name: "namespace granting read access to custom resource", + name: "jailed namespace granting read access to custom resource", input: types.KubernetesResource{ Kind: "mycustomresources", Namespace: "default", @@ -619,10 +645,16 @@ func TestKubeResourceMatchesRegex(t *testing.T) { }, resources: []types.KubernetesResource{ { - Kind: "namespaces", - Name: "default", - Verbs: []string{types.KubeVerbGet}, - APIGroup: "*", + Kind: "namespaces", + Name: "default", + Verbs: []string{types.KubeVerbGet}, + }, + { + Kind: types.Wildcard, + Name: types.Wildcard, + Namespace: "default", + APIGroup: types.Wildcard, + Verbs: []string{types.KubeVerbGet}, }, }, assert: require.NoError, @@ -640,10 +672,9 @@ func TestKubeResourceMatchesRegex(t *testing.T) { }, resources: []types.KubernetesResource{ { - Kind: "namespaces", - Name: "default", - Verbs: []string{types.KubeVerbGet}, - APIGroup: "*", + Kind: "namespaces", + Name: "default", + Verbs: []string{types.KubeVerbGet}, }, }, assert: require.NoError, @@ -665,13 +696,11 @@ func TestKubeResourceMatchesRegex(t *testing.T) { Namespace: "default", Name: "name", Verbs: []string{types.KubeVerbGet}, - APIGroup: "*", }, { - Kind: "namespaces", - Name: "diffnamespace", - Verbs: []string{types.KubeVerbGet}, - APIGroup: "*", + Kind: "namespaces", + Name: "diffnamespace", + Verbs: []string{types.KubeVerbGet}, }, }, assert: require.NoError, @@ -699,7 +728,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) { matches: true, }, { - name: "global custom resource with namespaced wildcard", + name: "global custom resource with jailed namespaced wildcard", input: types.KubernetesResource{ Kind: "mycustomresources", Name: "name", @@ -708,10 +737,16 @@ func TestKubeResourceMatchesRegex(t *testing.T) { }, resources: []types.KubernetesResource{ { - Kind: "namespaces", - Name: "*", - Verbs: []string{types.KubeVerbGet}, - APIGroup: "*", + Kind: "namespaces", + Name: types.Wildcard, + Verbs: []string{types.KubeVerbGet}, + }, + { + Kind: types.Wildcard, + Name: types.Wildcard, + Namespace: "default", + APIGroup: types.Wildcard, + Verbs: []string{types.KubeVerbGet}, }, }, assert: require.NoError, @@ -1269,7 +1304,7 @@ func TestKubeResourceCouldMatchRules(t *testing.T) { }, { - name: "namespace granting read access to pod", + name: "jailed namespace granting read access to pod", input: types.KubernetesResource{ Kind: "pods", Namespace: "default", @@ -1281,31 +1316,20 @@ func TestKubeResourceCouldMatchRules(t *testing.T) { Name: "default", Verbs: []string{types.KubeVerbGet}, }, + { + Kind: types.Wildcard, + Name: types.Wildcard, + Namespace: "default", + APIGroup: types.Wildcard, + Verbs: []string{types.KubeVerbGet}, + }, }, assert: require.NoError, matches: true, action: types.Allow, }, { - name: "namespace denying update access to pod", - input: types.KubernetesResource{ - Kind: "pods", - Namespace: "default", - Verbs: []string{types.KubeVerbList}, - }, - resources: []types.KubernetesResource{ - { - Kind: "namespaces", - Name: "default", - Verbs: []string{types.KubeVerbList}, - }, - }, - assert: require.NoError, - matches: false, - action: types.Deny, - }, - { - name: "namespace denying list access to pod with different namespace", + name: "jailed namespace denying list access to pod with different namespace", input: types.KubernetesResource{ Kind: "pods", Namespace: "default2", @@ -1313,17 +1337,24 @@ func TestKubeResourceCouldMatchRules(t *testing.T) { }, resources: []types.KubernetesResource{ { - Kind: "namespaces", + Kind: "names", Name: "default", Verbs: []string{types.KubeVerbList}, }, + { + Kind: types.Wildcard, + Name: types.Wildcard, + Namespace: "default", + APIGroup: types.Wildcard, + Verbs: []string{types.KubeVerbList}, + }, }, assert: require.NoError, matches: false, action: types.Deny, }, { - name: "namespace denying list access to pod in all namespaces doesnt match deny", + name: "jailed namespace denying list access to pod in all namespaces doesnt match deny", input: types.KubernetesResource{ Kind: "pods", Namespace: "", @@ -1335,13 +1366,20 @@ func TestKubeResourceCouldMatchRules(t *testing.T) { Name: "default", Verbs: []string{types.KubeVerbList}, }, + { + Kind: types.Wildcard, + Name: types.Wildcard, + Namespace: "default", + APIGroup: types.Wildcard, + Verbs: []string{types.KubeVerbList}, + }, }, assert: require.NoError, matches: false, action: types.Deny, }, { - name: "namespace denying update access to pod in all namespaces matches allow", + name: "jailed namespace denying update access to pod in all namespaces matches allow", input: types.KubernetesResource{ Kind: "pods", Namespace: "", @@ -1353,29 +1391,18 @@ func TestKubeResourceCouldMatchRules(t *testing.T) { Name: "default", Verbs: []string{types.KubeVerbList}, }, + { + Kind: types.Wildcard, + Name: types.Wildcard, + Namespace: "default", + APIGroup: types.Wildcard, + Verbs: []string{types.KubeVerbList}, + }, }, assert: require.NoError, matches: true, action: types.Allow, }, - { - name: "namespace denying update access to pod deny matches all namespaces", - input: types.KubernetesResource{ - Kind: "pods", - Namespace: "", - Verbs: []string{types.KubeVerbList}, - }, - resources: []types.KubernetesResource{ - { - Kind: "namespaces", - Name: types.Wildcard, - Verbs: []string{types.KubeVerbList}, - }, - }, - assert: require.NoError, - matches: true, - action: types.Deny, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {