From 87413089f4d144e4b695a116368db16c78db0066 Mon Sep 17 00:00:00 2001 From: Yash Tewari Date: Tue, 26 Jul 2022 06:40:38 +0000 Subject: [PATCH] Fix broken k8s_object_rules tests. --- tests/commonlib/docker_wrapper.py | 1 - tests/commonlib/kubernetes.py | 29 ++++++------ tests/deploy/mock-pod.yml | 2 +- .../tests/data/k8s_object/k8s_object_rules.py | 28 ++++-------- tests/product/tests/test_k8s_objects_rules.py | 44 +++++++++---------- .../test_process_controller_manager_rules.py | 2 +- .../product/tests/test_process_etcd_rules.py | 2 +- 7 files changed, 48 insertions(+), 60 deletions(-) diff --git a/tests/commonlib/docker_wrapper.py b/tests/commonlib/docker_wrapper.py index 2689650834..7d6f105f28 100644 --- a/tests/commonlib/docker_wrapper.py +++ b/tests/commonlib/docker_wrapper.py @@ -29,4 +29,3 @@ def exec_command(self, container_name: str, command: str, param_value: str, reso raise ValueError(f'Failed to execute command: {command_f, output}') return output.decode().strip() - diff --git a/tests/commonlib/kubernetes.py b/tests/commonlib/kubernetes.py index 5c0d593c67..d577ddf85d 100644 --- a/tests/commonlib/kubernetes.py +++ b/tests/commonlib/kubernetes.py @@ -15,6 +15,7 @@ RESOURCE_POD = 'Pod' RESOURCE_SERVICE_ACCOUNT = 'ServiceAccount' + class KubernetesHelper: def __init__(self, is_in_cluster_config: bool = False): @@ -192,15 +193,15 @@ def patch_resources(self, resource_type: str, **kwargs): patch_body = kwargs.pop('body') - pod = self.get_resource(resource_type, **kwargs) + self.get_resource(resource_type, **kwargs) self.delete_resources(resource_type=resource_type, **kwargs) deleted = self.wait_for_resource(resource_type=resource_type, status_list=['DELETED'], **kwargs) if not deleted: - raise ValueError(f'could not delete Pod: {kwargs}') + raise ValueError(f'could not delete {resource_type}: {kwargs}') return self.create_patched_resource(resource_type, patch_body) - + def create_patched_resource(self, patch_resource_type, patch_body): """ """ @@ -225,17 +226,17 @@ def create_patched_resource(self, patch_resource_type, patch_body): done = self.wait_for_resource(resource_type=resource_type, status_list=["RUNNING", "ADDED"], **relevant_metadata) if done: patched_resource = created_resource - + break - + return patched_resource - - def patch_resource_body(self, body: Union[list,dict], patch: Union[list,dict]) -> Union[list,dict]: + + def patch_resource_body(self, body: Union[list, dict], patch: Union[list, dict]) -> Union[list, dict]: """ """ if type(body) != type(patch): raise ValueError(f'Cannot compare {type(body)}: {body} with {type(patch)}: {patch}') - + if isinstance(body, dict): for key, val in patch.items(): if key not in body: @@ -245,23 +246,23 @@ def patch_resource_body(self, body: Union[list,dict], patch: Union[list,dict]) - body[key] = self.patch_resource_body(body[key], val) else: body[key] = val - + elif isinstance(body, list): - for i, val in enumerate(body): - if i >= len(patch): + for i, val in enumerate(patch): + if i >= len(body): break if isinstance(val, list) or isinstance(val, dict): body[i] = self.patch_resource_body(body[i], val) else: body[i] = val - + if len(patch) > len(body): body += val[len(patch):] - + else: raise ValueError(f'Invalid body {body} of type {type(body)}') - + return body def list_resources(self, resource_type: str, **kwargs): diff --git a/tests/deploy/mock-pod.yml b/tests/deploy/mock-pod.yml index 9051496987..b4df07b05a 100644 --- a/tests/deploy/mock-pod.yml +++ b/tests/deploy/mock-pod.yml @@ -16,7 +16,7 @@ spec: - "3600" securityContext: privileged: false - allowPrivilegeEscalation: false + allowPrivilegeEscalation: true capabilities: add: [ "NET_ADMIN", "SYS_TIME" ] --- diff --git a/tests/product/tests/data/k8s_object/k8s_object_rules.py b/tests/product/tests/data/k8s_object/k8s_object_rules.py index b136656cbf..732911f460 100644 --- a/tests/product/tests/data/k8s_object/k8s_object_rules.py +++ b/tests/product/tests/data/k8s_object/k8s_object_rules.py @@ -94,7 +94,7 @@ resource_type='Pod', resource_body={ 'metadata': {'name': TEST_POD_NAME, 'namespace': KUBE_SYSTEM_NAMESPACE}, - 'spec': {'serviceAccount': DEFAULT, 'namespace': DEFAULT}, + 'spec': {'serviceAccount': DEFAULT}, }, expected=RULE_FAIL_STATUS, ) @@ -104,7 +104,7 @@ resource_type='Pod', resource_body={ 'metadata': {'name': TEST_POD_NAME, 'namespace': KUBE_SYSTEM_NAMESPACE}, - 'spec': {'serviceAccountName': DEFAULT, 'namespace': DEFAULT}, + 'spec': {'serviceAccountName': DEFAULT}, }, expected=RULE_FAIL_STATUS, ) @@ -122,9 +122,6 @@ cis_5_1_5 = { "5.1.5 ServiceAccount.Name == default and automountServiceAccountToken == true": cis_5_1_5_service_account, -} - -cis_5_1_5_skip = { '5.1.5 Pod.serviceAccount == default': cis_5_1_5_pod_serviceAccount, '5.1.5 Pod.serviceAccountName == default': cis_5_1_5_pod_serviceAccountName, } @@ -179,7 +176,7 @@ # CIS 5.2.2 cis_5_2_2_pod_fail = KubeTestCase( - rule_tag='CIS 5.2.3', + rule_tag='CIS 5.2.2', resource_type='Pod', resource_body={ 'metadata': {'name': TEST_POD_NAME, 'namespace': KUBE_SYSTEM_NAMESPACE}, @@ -196,7 +193,7 @@ ) cis_5_2_2_pod_pass = KubeTestCase( - rule_tag='CIS 5.2.3', + rule_tag='CIS 5.2.2', resource_type='Pod', resource_body={ 'metadata': {'name': TEST_POD_NAME, 'namespace': KUBE_SYSTEM_NAMESPACE}, @@ -239,9 +236,7 @@ ) cis_5_2_3 = { - '5.2.3 Pod.spec.hostPID == true': cis_5_2_3_pod_fail -} -cis_5_2_3_skip = { + '5.2.3 Pod.spec.hostPID == true': cis_5_2_3_pod_fail, '5.2.3 Pod.spec.hostPID == false': cis_5_2_3_pod_pass, } @@ -267,10 +262,7 @@ ) cis_5_2_4 = { - '5.2.4 Pod.spec.hostIPC == true': cis_5_2_4_pod_fail -} - -cis_5_2_4_skip = { + '5.2.4 Pod.spec.hostIPC == true': cis_5_2_4_pod_fail, '5.2.4 Pod.spec.hostIPC == false': cis_5_2_4_pod_pass } @@ -296,12 +288,8 @@ ) cis_5_2_5 = { - '5.2.5 Pod.spec.hostNetwork == true': cis_5_2_5_pod_fail - -} - -cis_5_2_5_skip = { - '5.2.5 Pod.spec.hostNetwork == false': cis_5_2_5_pod_pass + '5.2.5 Pod.spec.hostNetwork == true': cis_5_2_5_pod_fail, + '5.2.5 Pod.spec.hostNetwork == false': cis_5_2_5_pod_pass, } # CIS 5.2.6 diff --git a/tests/product/tests/test_k8s_objects_rules.py b/tests/product/tests/test_k8s_objects_rules.py index 703be26747..4ffce736a6 100644 --- a/tests/product/tests/test_k8s_objects_rules.py +++ b/tests/product/tests/test_k8s_objects_rules.py @@ -3,12 +3,12 @@ This module verifies correctness of retrieved findings by manipulating audit and remediation actions """ from datetime import datetime +import uuid import pytest -# from product.tests.data.k8s_object.k8s_object_rules import * from product.tests.data.k8s_object import k8s_object_rules as k8s_tc -from commonlib.utils import get_ES_evaluation, get_resource_identifier +from commonlib.utils import get_ES_evaluation from commonlib.framework.reporting import skip_param_case, SkipReportData @@ -22,22 +22,15 @@ *k8s_tc.cis_5_2_3.values(), *k8s_tc.cis_5_2_4.values(), *k8s_tc.cis_5_2_5.values(), - *skip_param_case(skip_list=[*k8s_tc.cis_5_1_5_skip.values(), - *k8s_tc.cis_5_2_2.values(), - *k8s_tc.cis_5_2_3_skip.values(), - *k8s_tc.cis_5_2_4_skip.values(), - *k8s_tc.cis_5_2_5_skip.values(), - *k8s_tc.cis_5_2_6.values(), - *k8s_tc.cis_5_2_7.values(), - *k8s_tc.cis_5_2_8.values() - ], + *k8s_tc.cis_5_2_2.values(), + *k8s_tc.cis_5_2_6.values(), + *k8s_tc.cis_5_2_8.values(), + *skip_param_case(skip_list=[*k8s_tc.cis_5_2_7.values()], data_to_report=SkipReportData( - url_title="security-team: #4312", - url_link="https://github.com/elastic/security-team/issues/4312", - skip_reason="known issue: broken k8s object tests" + url_title="security-team: #4540", + url_link="https://github.com/elastic/security-team/issues/4540", + skip_reason="Known issue: incorrect implementation" )) - # *k8s_tc.cis_5_2_9.values(), - TODO: cases are not implemented - # *k8s_tc.cis_5_2_10.values() - TODO: cases are not implemented ], ids=[ *k8s_tc.cis_5_1_3.keys(), @@ -46,11 +39,7 @@ *k8s_tc.cis_5_2_3.keys(), *k8s_tc.cis_5_2_4.keys(), *k8s_tc.cis_5_2_5.keys(), - *k8s_tc.cis_5_1_5_skip.keys(), *k8s_tc.cis_5_2_2.keys(), - *k8s_tc.cis_5_2_3_skip.keys(), - *k8s_tc.cis_5_2_4_skip.keys(), - *k8s_tc.cis_5_2_5_skip.keys(), *k8s_tc.cis_5_2_6.keys(), *k8s_tc.cis_5_2_7.keys(), *k8s_tc.cis_5_2_8.keys(), @@ -82,6 +71,11 @@ def test_kube_resource_patch(elastic_client, test_env, rule_tag, resource_type, assert resource, f"Resource {resource_type} not found" + test_resource_id = str(uuid.uuid4()) + + labels = metadata.setdefault('labels', {}) + labels['test_resource_id'] = test_resource_id + # patch resource resource = k8s_client.patch_resources( resource_type=resource_type, @@ -92,13 +86,19 @@ def test_kube_resource_patch(elastic_client, test_env, rule_tag, resource_type, raise ValueError( f'Could not patch resource type {resource_type}:' f' {relevant_metadata} with patch {resource_body}') - + + def match_resource(eval_resource): + try: + return eval_resource.metadata.labels.test_resource_id == test_resource_id + except AttributeError: + return False + evaluation = get_ES_evaluation( elastic_client=elastic_client, timeout=agent_config.findings_timeout, rule_tag=rule_tag, exec_timestamp=datetime.utcnow(), - resource_identifier=get_resource_identifier(resource_body), + resource_identifier=match_resource, ) assert evaluation is not None, f"No evaluation for rule {rule_tag} could be found" diff --git a/tests/product/tests/test_process_controller_manager_rules.py b/tests/product/tests/test_process_controller_manager_rules.py index 8187f69350..6d6c8da625 100644 --- a/tests/product/tests/test_process_controller_manager_rules.py +++ b/tests/product/tests/test_process_controller_manager_rules.py @@ -48,7 +48,7 @@ def test_process_controller_manager(elastic_client, # Wait for process reboot # TODO: Implement a more optimal way of waiting time.sleep(60) - + evaluation = get_ES_evaluation( elastic_client=elastic_client, timeout=cloudbeat_agent.findings_timeout, diff --git a/tests/product/tests/test_process_etcd_rules.py b/tests/product/tests/test_process_etcd_rules.py index 70127ec82a..2acfcddc2a 100644 --- a/tests/product/tests/test_process_etcd_rules.py +++ b/tests/product/tests/test_process_etcd_rules.py @@ -44,7 +44,7 @@ def test_process_etcd(elastic_client, api_client.edit_process_file(container_name=node.metadata.name, dictionary=dictionary, resource=resource) - + # Wait for process reboot # TODO: Implement a more optimal way of waiting time.sleep(60)