From 4b7883c464091cf7ad3b1c4cfd47521effda44ad Mon Sep 17 00:00:00 2001 From: Sergio Garcia Date: Wed, 20 Nov 2024 10:06:06 -0400 Subject: [PATCH] fix(kubernetes): filter apiGroup in permission checks (#5829) --- .../services/rbac/lib/role_permissions.py | 5 ++- .../rbac_minimize_csr_approval_access.py | 4 +- ..._minimize_node_proxy_subresource_access.py | 4 +- .../rbac_minimize_pod_creation_access.py | 6 +-- .../rbac_minimize_pv_creation_access.py | 4 +- .../rbac_minimize_secret_access.py | 6 +-- ...minimize_service_account_token_creation.py | 4 +- .../rbac_minimize_webhook_config_access.py | 4 +- .../rbac/lib/role_permissions_test.py | 40 ++++++++++++++----- 9 files changed, 51 insertions(+), 26 deletions(-) diff --git a/prowler/providers/kubernetes/services/rbac/lib/role_permissions.py b/prowler/providers/kubernetes/services/rbac/lib/role_permissions.py index 840bcce1cb..c3db38b2f7 100644 --- a/prowler/providers/kubernetes/services/rbac/lib/role_permissions.py +++ b/prowler/providers/kubernetes/services/rbac/lib/role_permissions.py @@ -1,4 +1,4 @@ -def is_rule_allowing_permisions(rules, resources, verbs): +def is_rule_allowing_permissions(rules, resources, verbs): """ Check Kubernetes role permissions. @@ -17,6 +17,9 @@ def is_rule_allowing_permisions(rules, resources, verbs): if rules: # Iterate through each rule in the list of rules for rule in rules: + # Ensure apiGroups are relevant ("" or "v1" for secrets) + if rule.apiGroups and all(api not in ["", "v1"] for api in rule.apiGroups): + continue # Skip rules with unrelated apiGroups # Check if the rule has resources, verbs, and matches any of the specified resources and verbs if ( rule.resources diff --git a/prowler/providers/kubernetes/services/rbac/rbac_minimize_csr_approval_access/rbac_minimize_csr_approval_access.py b/prowler/providers/kubernetes/services/rbac/rbac_minimize_csr_approval_access/rbac_minimize_csr_approval_access.py index 50b8f774aa..622f4cce26 100644 --- a/prowler/providers/kubernetes/services/rbac/rbac_minimize_csr_approval_access/rbac_minimize_csr_approval_access.py +++ b/prowler/providers/kubernetes/services/rbac/rbac_minimize_csr_approval_access/rbac_minimize_csr_approval_access.py @@ -1,6 +1,6 @@ from prowler.lib.check.models import Check, Check_Report_Kubernetes from prowler.providers.kubernetes.services.rbac.lib.role_permissions import ( - is_rule_allowing_permisions, + is_rule_allowing_permissions, ) from prowler.providers.kubernetes.services.rbac.rbac_client import rbac_client @@ -22,7 +22,7 @@ class rbac_minimize_csr_approval_access(Check): report.status_extended = f"User or group '{subject.name}' does not have access to update the CSR approval sub-resource." for cr in rbac_client.cluster_roles.values(): if cr.metadata.name == crb.roleRef.name: - if is_rule_allowing_permisions( + if is_rule_allowing_permissions( cr.rules, resources, verbs, diff --git a/prowler/providers/kubernetes/services/rbac/rbac_minimize_node_proxy_subresource_access/rbac_minimize_node_proxy_subresource_access.py b/prowler/providers/kubernetes/services/rbac/rbac_minimize_node_proxy_subresource_access/rbac_minimize_node_proxy_subresource_access.py index 3cd2035110..219b478458 100644 --- a/prowler/providers/kubernetes/services/rbac/rbac_minimize_node_proxy_subresource_access/rbac_minimize_node_proxy_subresource_access.py +++ b/prowler/providers/kubernetes/services/rbac/rbac_minimize_node_proxy_subresource_access/rbac_minimize_node_proxy_subresource_access.py @@ -1,6 +1,6 @@ from prowler.lib.check.models import Check, Check_Report_Kubernetes from prowler.providers.kubernetes.services.rbac.lib.role_permissions import ( - is_rule_allowing_permisions, + is_rule_allowing_permissions, ) from prowler.providers.kubernetes.services.rbac.rbac_client import rbac_client @@ -22,7 +22,7 @@ class rbac_minimize_node_proxy_subresource_access(Check): report.status_extended = f"User or group '{subject.name}' does not have access to the node proxy sub-resource." for cr in rbac_client.cluster_roles.values(): if cr.metadata.name == crb.roleRef.name: - if is_rule_allowing_permisions(cr.rules, resources, verbs): + if is_rule_allowing_permissions(cr.rules, resources, verbs): report.status = "FAIL" report.status_extended = f"User or group '{subject.name}' has access to the node proxy sub-resource." break diff --git a/prowler/providers/kubernetes/services/rbac/rbac_minimize_pod_creation_access/rbac_minimize_pod_creation_access.py b/prowler/providers/kubernetes/services/rbac/rbac_minimize_pod_creation_access/rbac_minimize_pod_creation_access.py index d71d9a23da..4b5c0c5661 100644 --- a/prowler/providers/kubernetes/services/rbac/rbac_minimize_pod_creation_access/rbac_minimize_pod_creation_access.py +++ b/prowler/providers/kubernetes/services/rbac/rbac_minimize_pod_creation_access/rbac_minimize_pod_creation_access.py @@ -1,6 +1,6 @@ from prowler.lib.check.models import Check, Check_Report_Kubernetes from prowler.providers.kubernetes.services.rbac.lib.role_permissions import ( - is_rule_allowing_permisions, + is_rule_allowing_permissions, ) from prowler.providers.kubernetes.services.rbac.rbac_client import rbac_client @@ -21,7 +21,7 @@ class rbac_minimize_pod_creation_access(Check): report.status_extended = ( f"ClusterRole {cr.metadata.name} does not have pod create access." ) - if is_rule_allowing_permisions(cr.rules, resources, verbs): + if is_rule_allowing_permissions(cr.rules, resources, verbs): report.status = "FAIL" report.status_extended = ( f"ClusterRole {cr.metadata.name} has pod create access." @@ -39,7 +39,7 @@ class rbac_minimize_pod_creation_access(Check): f"Role {role.metadata.name} does not have pod create access." ) - if is_rule_allowing_permisions(role.rules, resources, verbs): + if is_rule_allowing_permissions(role.rules, resources, verbs): report.status = "FAIL" report.status_extended = ( f"Role {role.metadata.name} has pod create access." diff --git a/prowler/providers/kubernetes/services/rbac/rbac_minimize_pv_creation_access/rbac_minimize_pv_creation_access.py b/prowler/providers/kubernetes/services/rbac/rbac_minimize_pv_creation_access/rbac_minimize_pv_creation_access.py index ff00c0a255..e683aa9337 100644 --- a/prowler/providers/kubernetes/services/rbac/rbac_minimize_pv_creation_access/rbac_minimize_pv_creation_access.py +++ b/prowler/providers/kubernetes/services/rbac/rbac_minimize_pv_creation_access/rbac_minimize_pv_creation_access.py @@ -1,6 +1,6 @@ from prowler.lib.check.models import Check, Check_Report_Kubernetes from prowler.providers.kubernetes.services.rbac.lib.role_permissions import ( - is_rule_allowing_permisions, + is_rule_allowing_permissions, ) from prowler.providers.kubernetes.services.rbac.rbac_client import rbac_client @@ -23,7 +23,7 @@ class rbac_minimize_pv_creation_access(Check): report.status_extended = f"User or group '{subject.name}' does not have access to create PersistentVolumes." for cr in rbac_client.cluster_roles.values(): if cr.metadata.name == crb.roleRef.name: - if is_rule_allowing_permisions(cr.rules, resources, verbs): + if is_rule_allowing_permissions(cr.rules, resources, verbs): report.status = "FAIL" report.status_extended = f"User or group '{subject.name}' has access to create PersistentVolumes." break diff --git a/prowler/providers/kubernetes/services/rbac/rbac_minimize_secret_access/rbac_minimize_secret_access.py b/prowler/providers/kubernetes/services/rbac/rbac_minimize_secret_access/rbac_minimize_secret_access.py index 23078fe438..16d7a90fe1 100644 --- a/prowler/providers/kubernetes/services/rbac/rbac_minimize_secret_access/rbac_minimize_secret_access.py +++ b/prowler/providers/kubernetes/services/rbac/rbac_minimize_secret_access/rbac_minimize_secret_access.py @@ -1,6 +1,6 @@ from prowler.lib.check.models import Check, Check_Report_Kubernetes from prowler.providers.kubernetes.services.rbac.lib.role_permissions import ( - is_rule_allowing_permisions, + is_rule_allowing_permissions, ) from prowler.providers.kubernetes.services.rbac.rbac_client import rbac_client @@ -21,7 +21,7 @@ class rbac_minimize_secret_access(Check): report.status_extended = ( f"ClusterRole {cr.metadata.name} does not have secret access." ) - if is_rule_allowing_permisions(cr.rules, resources, verbs): + if is_rule_allowing_permissions(cr.rules, resources, verbs): report.status = "FAIL" report.status_extended = ( f"ClusterRole {cr.metadata.name} has secret access." @@ -39,7 +39,7 @@ class rbac_minimize_secret_access(Check): f"Role {role.metadata.name} does not have secret access." ) - if is_rule_allowing_permisions(cr.rules, resources, verbs): + if is_rule_allowing_permissions(cr.rules, resources, verbs): report.status = "FAIL" report.status_extended = f"Role {role.metadata.name} has secret access." findings.append(report) diff --git a/prowler/providers/kubernetes/services/rbac/rbac_minimize_service_account_token_creation/rbac_minimize_service_account_token_creation.py b/prowler/providers/kubernetes/services/rbac/rbac_minimize_service_account_token_creation/rbac_minimize_service_account_token_creation.py index 7e3c22c0e0..1b85d3caff 100644 --- a/prowler/providers/kubernetes/services/rbac/rbac_minimize_service_account_token_creation/rbac_minimize_service_account_token_creation.py +++ b/prowler/providers/kubernetes/services/rbac/rbac_minimize_service_account_token_creation/rbac_minimize_service_account_token_creation.py @@ -1,6 +1,6 @@ from prowler.lib.check.models import Check, Check_Report_Kubernetes from prowler.providers.kubernetes.services.rbac.lib.role_permissions import ( - is_rule_allowing_permisions, + is_rule_allowing_permissions, ) from prowler.providers.kubernetes.services.rbac.rbac_client import rbac_client @@ -22,7 +22,7 @@ class rbac_minimize_service_account_token_creation(Check): report.status_extended = f"User or group '{subject.name}' does not have access to create service account tokens." for cr in rbac_client.cluster_roles.values(): if cr.metadata.name == crb.roleRef.name: - if is_rule_allowing_permisions(cr.rules, resources, verbs): + if is_rule_allowing_permissions(cr.rules, resources, verbs): report.status = "FAIL" report.status_extended = f"User or group '{subject.name}' has access to create service account tokens." break diff --git a/prowler/providers/kubernetes/services/rbac/rbac_minimize_webhook_config_access/rbac_minimize_webhook_config_access.py b/prowler/providers/kubernetes/services/rbac/rbac_minimize_webhook_config_access/rbac_minimize_webhook_config_access.py index 489c5a06c7..1ba11fe7c1 100644 --- a/prowler/providers/kubernetes/services/rbac/rbac_minimize_webhook_config_access/rbac_minimize_webhook_config_access.py +++ b/prowler/providers/kubernetes/services/rbac/rbac_minimize_webhook_config_access/rbac_minimize_webhook_config_access.py @@ -1,6 +1,6 @@ from prowler.lib.check.models import Check, Check_Report_Kubernetes from prowler.providers.kubernetes.services.rbac.lib.role_permissions import ( - is_rule_allowing_permisions, + is_rule_allowing_permissions, ) from prowler.providers.kubernetes.services.rbac.rbac_client import rbac_client @@ -25,7 +25,7 @@ class rbac_minimize_webhook_config_access(Check): report.status_extended = f"User or group '{subject.name}' does not have access to create, update, or delete webhook configurations." for cr in rbac_client.cluster_roles.values(): if cr.metadata.name == crb.roleRef.name: - if is_rule_allowing_permisions( + if is_rule_allowing_permissions( cr.rules, resources, verbs, diff --git a/tests/providers/kubernetes/services/rbac/lib/role_permissions_test.py b/tests/providers/kubernetes/services/rbac/lib/role_permissions_test.py index 1ca3ee139b..696550c06c 100644 --- a/tests/providers/kubernetes/services/rbac/lib/role_permissions_test.py +++ b/tests/providers/kubernetes/services/rbac/lib/role_permissions_test.py @@ -1,11 +1,11 @@ from prowler.providers.kubernetes.services.rbac.lib.role_permissions import ( - is_rule_allowing_permisions, + is_rule_allowing_permissions, ) from prowler.providers.kubernetes.services.rbac.rbac_service import Rule class TestCheckRolePermissions: - def test_is_rule_allowing_permisions(self): + def test_is_rule_allowing_permissions(self): # Define some sample rules, resources, and verbs for testing rules = [ # Rule 1: Allows 'get' and 'list' on 'pods' and 'services' @@ -16,7 +16,7 @@ class TestCheckRolePermissions: resources = ["pods", "deployments"] verbs = ["get", "create"] - assert is_rule_allowing_permisions(rules, resources, verbs) + assert is_rule_allowing_permissions(rules, resources, verbs) def test_no_permissions(self): # Test when there are no rules @@ -24,7 +24,7 @@ class TestCheckRolePermissions: resources = ["pods", "deployments"] verbs = ["get", "create"] - assert not is_rule_allowing_permisions(rules, resources, verbs) + assert not is_rule_allowing_permissions(rules, resources, verbs) def test_no_matching_rules(self): # Test when there are rules, but none match the specified resources and verbs @@ -35,7 +35,7 @@ class TestCheckRolePermissions: resources = ["deployments", "configmaps"] verbs = ["get", "create"] - assert not is_rule_allowing_permisions(rules, resources, verbs) + assert not is_rule_allowing_permissions(rules, resources, verbs) def test_empty_rules(self): # Test when the rules list is empty @@ -43,7 +43,7 @@ class TestCheckRolePermissions: resources = ["pods", "deployments"] verbs = ["get", "create"] - assert not is_rule_allowing_permisions(rules, resources, verbs) + assert not is_rule_allowing_permissions(rules, resources, verbs) def test_empty_resources_and_verbs(self): # Test when resources and verbs are empty lists @@ -54,7 +54,7 @@ class TestCheckRolePermissions: resources = [] verbs = [] - assert not is_rule_allowing_permisions(rules, resources, verbs) + assert not is_rule_allowing_permissions(rules, resources, verbs) def test_matching_rule_with_empty_resources_or_verbs(self): # Test when a rule matches, but either resources or verbs are empty @@ -65,9 +65,31 @@ class TestCheckRolePermissions: resources = [] verbs = ["get"] - assert not is_rule_allowing_permisions(rules, resources, verbs) + assert not is_rule_allowing_permissions(rules, resources, verbs) resources = ["pods"] verbs = [] - assert not is_rule_allowing_permisions(rules, resources, verbs) + assert not is_rule_allowing_permissions(rules, resources, verbs) + + def test_rule_with_ignored_api_groups(self): + # Test when a rule has apiGroups that are not relevant + rules = [ + Rule(resources=["pods"], verbs=["get"], apiGroups=["test"]), + Rule(resources=["services"], verbs=["list"], apiGroups=["test2"]), + ] + resources = ["pods"] + verbs = ["get"] + + assert not is_rule_allowing_permissions(rules, resources, verbs) + + def test_rule_with_relevant_api_groups(self): + # Test when a rule has apiGroups that are relevant + rules = [ + Rule(resources=["pods"], verbs=["get"], apiGroups=["", "v1"]), + Rule(resources=["services"], verbs=["list"], apiGroups=["test2"]), + ] + resources = ["pods"] + verbs = ["get"] + + assert is_rule_allowing_permissions(rules, resources, verbs)