Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v13] Always collect deny arm of kubernetes_resources #28285

Merged
merged 1 commit into from Jun 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 54 additions & 0 deletions lib/kube/proxy/pod_rbac_test.go
Expand Up @@ -51,6 +51,7 @@ func TestListPodRBAC(t *testing.T) {
usernameWithFullAccess = "full_user"
usernameWithNamespaceAccess = "default_user"
usernameWithLimitedAccess = "limited_user"
usernameWithDenyRule = "denied_user"
testPodName = "test"
)
// kubeMock is a Kubernetes API mock for the session tests.
Expand Down Expand Up @@ -141,6 +142,39 @@ func TestListPodRBAC(t *testing.T) {
},
)

// create a user allowed to access all namespaces except the default namespace.
// (kubernetes_user and kubernetes_groups specified)
userWithDenyRule, _ := testCtx.CreateUserAndRole(
testCtx.Context,
t,
usernameWithDenyRule,
RoleSpec{
Name: usernameWithDenyRule,
KubeUsers: roleKubeUsers,
KubeGroups: roleKubeGroups,
SetupRoleFunc: func(r types.Role) {
r.SetKubeResources(types.Allow,
[]types.KubernetesResource{
{
Kind: types.KindKubePod,
Name: types.Wildcard,
Namespace: types.Wildcard,
},
},
)
r.SetKubeResources(types.Deny,
[]types.KubernetesResource{
{
Kind: types.KindKubePod,
Name: types.Wildcard,
Namespace: metav1.NamespaceDefault,
},
},
)
},
},
)

type args struct {
user types.User
namespace string
Expand Down Expand Up @@ -234,6 +268,26 @@ func TestListPodRBAC(t *testing.T) {
},
},
},
{
name: "list pods in every namespace for user with default namespace deny rule",
args: args{
user: userWithDenyRule,
},
want: want{
listPodsResult: []string{
"dev/nginx-1",
"dev/nginx-2",
},
getTestPodResult: &kubeerrors.StatusError{
ErrStatus: metav1.Status{
Status: "Failure",
Message: "pods \"test\" is forbidden: User \"denied_user\" cannot get resource \"pods\" in API group \"\" in the namespace \"default\"",
Code: 403,
Reason: metav1.StatusReasonForbidden,
},
},
},
},
{
name: "list default namespace pods for user with limited access and a resource access request",
args: args{
Expand Down
11 changes: 6 additions & 5 deletions lib/services/role.go
Expand Up @@ -2824,11 +2824,12 @@ func (set RoleSet) GetKubeResources(cluster types.KubeCluster, userTraits wrappe
}

for _, role := range set {
matchLabels, _, err := checkRoleLabelsMatch(types.Deny, role, userTraits, cluster, false)
if err != nil || !matchLabels {
continue
}

// deny rules are not checked for labels because they are greedy. It means that
// if there is a deny rule for a cluster, it will deny access to all resources
// in that cluster, regardless of kubernetes_resources (i.e. making them irrelevant).
// If the goal is to deny access to a specific resource, it should be done by collecting
// all kube resources in deny rules and ignoring if the role matches or not
// the cluster (i.e. no labels check).
denied = append(denied, role.GetKubeResources(types.Deny)...)
}

Expand Down
1 change: 1 addition & 0 deletions lib/services/role_test.go
Expand Up @@ -6660,6 +6660,7 @@ func TestGetKubeResources(t *testing.T) {
},
clusterLabels: map[string]string{"env": "staging"},
expectAllowed: []types.KubernetesResource{podA, podB},
expectDenied: []types.KubernetesResource{podA},
},
} {
t.Run(tc.desc, func(t *testing.T) {
Expand Down