From 0fff0568fab3b79202de71ff277aa0f9fb18e809 Mon Sep 17 00:00:00 2001 From: Pepe Fagoaga Date: Wed, 27 Dec 2023 11:02:31 +0100 Subject: [PATCH] fix(allowlist): Analyse single and multi account allowlist if present (#3210) Co-authored-by: Sergio Garcia --- .../providers/aws/lib/allowlist/allowlist.py | 42 +- tests/providers/aws/audit_info_utils.py | 3 + .../aws/lib/allowlist/allowlist_test.py | 366 +++++++++++++++++- 3 files changed, 379 insertions(+), 32 deletions(-) diff --git a/prowler/providers/aws/lib/allowlist/allowlist.py b/prowler/providers/aws/lib/allowlist/allowlist.py index 6789ff9e..be9ccfcb 100644 --- a/prowler/providers/aws/lib/allowlist/allowlist.py +++ b/prowler/providers/aws/lib/allowlist/allowlist.py @@ -143,29 +143,23 @@ def is_allowlisted( finding_tags, ): try: - allowlisted_checks = {} # By default is not allowlisted is_finding_allowlisted = False - # First set account key from allowlist dict - if audited_account in allowlist["Accounts"]: - allowlisted_checks = allowlist["Accounts"][audited_account]["Checks"] - # If there is a *, it affects to all accounts - # This cannot be elif since in the case of * and single accounts we - # want to merge allowlisted checks from * to the other accounts check list - if "*" in allowlist["Accounts"]: - checks_multi_account = allowlist["Accounts"]["*"]["Checks"] - allowlisted_checks.update(checks_multi_account) - # Test if it is allowlisted - if is_allowlisted_in_check( - allowlisted_checks, - audited_account, - check, - finding_region, - finding_resource, - finding_tags, - ): - is_finding_allowlisted = True + # We always check all the accounts present in the allowlist + # if one allowlists the finding we set the finding as allowlisted + for account in allowlist["Accounts"]: + if account == audited_account or account == "*": + if is_allowlisted_in_check( + allowlist["Accounts"][account]["Checks"], + audited_account, + check, + finding_region, + finding_resource, + finding_tags, + ): + is_finding_allowlisted = True + break return is_finding_allowlisted except Exception as error: @@ -310,10 +304,10 @@ def is_excepted( is_tag_excepted = __is_item_matched__(excepted_tags, finding_tags) if ( - is_account_excepted - and is_region_excepted - and is_resource_excepted - and is_tag_excepted + (is_account_excepted or not excepted_accounts) + and (is_region_excepted or not excepted_regions) + and (is_resource_excepted or not excepted_resources) + and (is_tag_excepted or not excepted_tags) ): excepted = True return excepted diff --git a/tests/providers/aws/audit_info_utils.py b/tests/providers/aws/audit_info_utils.py index 821ef32c..2faffe99 100644 --- a/tests/providers/aws/audit_info_utils.py +++ b/tests/providers/aws/audit_info_utils.py @@ -18,8 +18,11 @@ AWS_REGION_EU_WEST_2 = "eu-west-2" AWS_REGION_CN_NORTHWEST_1 = "cn-northwest-1" AWS_REGION_CN_NORTH_1 = "cn-north-1" AWS_REGION_EU_SOUTH_2 = "eu-south-2" +AWS_REGION_EU_SOUTH_3 = "eu-south-3" AWS_REGION_US_WEST_2 = "us-west-2" AWS_REGION_US_EAST_2 = "us-east-2" +AWS_REGION_EU_CENTRAL_1 = "eu-central-1" + # China Regions AWS_REGION_CHINA_NORHT_1 = "cn-north-1" diff --git a/tests/providers/aws/lib/allowlist/allowlist_test.py b/tests/providers/aws/lib/allowlist/allowlist_test.py index ff881fc0..1e7118f8 100644 --- a/tests/providers/aws/lib/allowlist/allowlist_test.py +++ b/tests/providers/aws/lib/allowlist/allowlist_test.py @@ -15,6 +15,8 @@ from prowler.providers.aws.lib.allowlist.allowlist import ( ) from tests.providers.aws.audit_info_utils import ( AWS_ACCOUNT_NUMBER, + AWS_REGION_EU_CENTRAL_1, + AWS_REGION_EU_SOUTH_3, AWS_REGION_EU_WEST_1, AWS_REGION_US_EAST_1, set_mocked_aws_audit_info, @@ -132,8 +134,7 @@ class Test_Allowlist: ) # Allowlist tests - - def test_allowlist_findings(self): + def test_allowlist_findings_only_wildcard(self): # Allowlist example allowlist = { "Accounts": { @@ -205,12 +206,6 @@ class Test_Allowlist: "Tags": ["*"], "Regions": ["*"], "Resources": ["*"], - "Exceptions": { - "Tags": [], - "Regions": [], - "Accounts": [], - "Resources": [], - }, } } } @@ -444,6 +439,155 @@ class Test_Allowlist: ) ) + def test_is_allowlisted_all_and_single_account_with_different_resources(self): + # Allowlist example + allowlist = { + "Accounts": { + "*": { + "Checks": { + "check_test_1": { + "Regions": ["*"], + "Resources": ["resource_1", "resource_2"], + }, + } + }, + AWS_ACCOUNT_NUMBER: { + "Checks": { + "check_test_1": { + "Regions": ["*"], + "Resources": ["resource_3"], + } + } + }, + } + } + + assert is_allowlisted( + allowlist, + "111122223333", + "check_test_1", + AWS_REGION_US_EAST_1, + "resource_1", + "", + ) + + assert is_allowlisted( + allowlist, + "111122223333", + "check_test_1", + AWS_REGION_US_EAST_1, + "resource_2", + "", + ) + + assert not is_allowlisted( + allowlist, + "111122223333", + "check_test_1", + AWS_REGION_US_EAST_1, + "resource_3", + "", + ) + + assert is_allowlisted( + allowlist, + AWS_ACCOUNT_NUMBER, + "check_test_1", + AWS_REGION_US_EAST_1, + "resource_3", + "", + ) + + assert is_allowlisted( + allowlist, + AWS_ACCOUNT_NUMBER, + "check_test_1", + AWS_REGION_US_EAST_1, + "resource_2", + "", + ) + + def test_is_allowlisted_all_and_single_account_with_different_resources_and_exceptions( + self, + ): + # Allowlist example + allowlist = { + "Accounts": { + "*": { + "Checks": { + "check_test_1": { + "Regions": ["*"], + "Resources": ["resource_1", "resource_2"], + "Exceptions": {"Regions": [AWS_REGION_US_EAST_1]}, + }, + } + }, + AWS_ACCOUNT_NUMBER: { + "Checks": { + "check_test_1": { + "Regions": ["*"], + "Resources": ["resource_3"], + "Exceptions": {"Regions": [AWS_REGION_EU_WEST_1]}, + } + } + }, + } + } + + assert not is_allowlisted( + allowlist, + AWS_ACCOUNT_NUMBER, + "check_test_1", + AWS_REGION_US_EAST_1, + "resource_2", + "", + ) + + assert not is_allowlisted( + allowlist, + "111122223333", + "check_test_1", + AWS_REGION_US_EAST_1, + "resource_1", + "", + ) + + assert is_allowlisted( + allowlist, + "111122223333", + "check_test_1", + AWS_REGION_EU_WEST_1, + "resource_2", + "", + ) + + assert not is_allowlisted( + allowlist, + "111122223333", + "check_test_1", + AWS_REGION_US_EAST_1, + "resource_3", + "", + ) + + assert is_allowlisted( + allowlist, + AWS_ACCOUNT_NUMBER, + "check_test_1", + AWS_REGION_US_EAST_1, + "resource_3", + "", + ) + + assert not is_allowlisted( + allowlist, + AWS_ACCOUNT_NUMBER, + "check_test_1", + AWS_REGION_EU_WEST_1, + "resource_3", + "", + ) + def test_is_allowlisted_single_account(self): allowlist = { "Accounts": { @@ -717,6 +861,111 @@ class Test_Allowlist: ) ) + def test_is_allowlisted_specific_account_with_other_account_excepted(self): + # Allowlist example + allowlist = { + "Accounts": { + AWS_ACCOUNT_NUMBER: { + "Checks": { + "check_test": { + "Regions": [AWS_REGION_EU_WEST_1], + "Resources": ["*"], + "Tags": [], + "Exceptions": {"Accounts": ["111122223333"]}, + } + } + } + } + } + + assert is_allowlisted( + allowlist, + AWS_ACCOUNT_NUMBER, + "check_test", + AWS_REGION_EU_WEST_1, + "prowler", + "environment=dev", + ) + + assert not is_allowlisted( + allowlist, + "111122223333", + "check_test", + AWS_REGION_EU_WEST_1, + "prowler", + "environment=dev", + ) + + def test_is_allowlisted_complex_allowlist(self): + # Allowlist example + allowlist = { + "Accounts": { + "*": { + "Checks": { + "s3_bucket_object_versioning": { + "Regions": [AWS_REGION_EU_WEST_1, AWS_REGION_US_EAST_1], + "Resources": ["ci-logs", "logs", ".+-logs"], + }, + "ecs_task_definitions_no_environment_secrets": { + "Regions": ["*"], + "Resources": ["*"], + "Exceptions": { + "Accounts": [AWS_ACCOUNT_NUMBER], + "Regions": [ + AWS_REGION_EU_WEST_1, + AWS_REGION_EU_SOUTH_3, + ], + }, + }, + "*": { + "Regions": ["*"], + "Resources": ["*"], + "Tags": ["environment=dev"], + }, + } + }, + AWS_ACCOUNT_NUMBER: { + "Checks": { + "*": { + "Regions": ["*"], + "Resources": ["*"], + "Exceptions": { + "Resources": ["test"], + "Tags": ["environment=prod"], + }, + } + } + }, + } + } + + assert is_allowlisted( + allowlist, + AWS_ACCOUNT_NUMBER, + "test_check", + AWS_REGION_EU_WEST_1, + "prowler-logs", + "environment=dev", + ) + + assert is_allowlisted( + allowlist, + AWS_ACCOUNT_NUMBER, + "ecs_task_definitions_no_environment_secrets", + AWS_REGION_EU_WEST_1, + "prowler", + "environment=dev", + ) + + assert is_allowlisted( + allowlist, + AWS_ACCOUNT_NUMBER, + "s3_bucket_object_versioning", + AWS_REGION_EU_WEST_1, + "prowler-logs", + "environment=dev", + ) + def test_is_allowlisted_in_tags(self): allowlist_tags = ["environment=dev", "project=prowler"] @@ -791,6 +1040,107 @@ class Test_Allowlist: "environment=test", ) + def test_is_excepted_only_in_account(self): + # Allowlist example + exceptions = { + "Accounts": [AWS_ACCOUNT_NUMBER], + "Regions": [], + "Resources": [], + "Tags": [], + } + + assert is_excepted( + exceptions, + AWS_ACCOUNT_NUMBER, + "eu-central-1", + "test", + "environment=test", + ) + + def test_is_excepted_only_in_region(self): + # Allowlist example + exceptions = { + "Accounts": [], + "Regions": [AWS_REGION_EU_CENTRAL_1, AWS_REGION_EU_SOUTH_3], + "Resources": [], + "Tags": [], + } + + assert is_excepted( + exceptions, + AWS_ACCOUNT_NUMBER, + AWS_REGION_EU_CENTRAL_1, + "test", + "environment=test", + ) + + def test_is_excepted_only_in_resources(self): + # Allowlist example + exceptions = { + "Accounts": [], + "Regions": [], + "Resources": ["resource_1"], + "Tags": [], + } + + assert is_excepted( + exceptions, + AWS_ACCOUNT_NUMBER, + AWS_REGION_EU_CENTRAL_1, + "resource_1", + "environment=test", + ) + + def test_is_excepted_only_in_tags(self): + # Allowlist example + exceptions = { + "Accounts": [], + "Regions": [], + "Resources": [], + "Tags": ["environment=test"], + } + + assert is_excepted( + exceptions, + AWS_ACCOUNT_NUMBER, + AWS_REGION_EU_CENTRAL_1, + "resource_1", + "environment=test", + ) + + def test_is_excepted_in_account_and_tags(self): + # Allowlist example + exceptions = { + "Accounts": [AWS_ACCOUNT_NUMBER], + "Regions": [], + "Resources": [], + "Tags": ["environment=test"], + } + + assert is_excepted( + exceptions, + AWS_ACCOUNT_NUMBER, + AWS_REGION_EU_CENTRAL_1, + "resource_1", + "environment=test", + ) + + assert not is_excepted( + exceptions, + "111122223333", + AWS_REGION_EU_CENTRAL_1, + "resource_1", + "environment=test", + ) + + assert not is_excepted( + exceptions, + "111122223333", + AWS_REGION_EU_CENTRAL_1, + "resource_1", + "environment=dev", + ) + def test_is_excepted_all_wildcard(self): exceptions = { "Accounts": ["*"],