From 66fe101ccd3cc8dcdb57ef6599fd9c6c1b2e0c05 Mon Sep 17 00:00:00 2001 From: Nacho Rivera Date: Wed, 12 Jul 2023 14:22:42 +0200 Subject: [PATCH] fix(allowlist): handle wildcard in account field (#2577) --- .../providers/aws/lib/allowlist/allowlist.py | 19 ++++-- .../aws/lib/allowlist/allowlist_test.py | 65 ++++++++++++++++--- 2 files changed, 69 insertions(+), 15 deletions(-) diff --git a/prowler/providers/aws/lib/allowlist/allowlist.py b/prowler/providers/aws/lib/allowlist/allowlist.py index 142b338e..d6efc055 100644 --- a/prowler/providers/aws/lib/allowlist/allowlist.py +++ b/prowler/providers/aws/lib/allowlist/allowlist.py @@ -115,18 +115,27 @@ def parse_allowlist_file(audit_info, allowlist_file): def is_allowlisted(allowlist, audited_account, check, region, resource, 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"]: - account = audited_account + allowlisted_checks = allowlist["Accounts"][audited_account]["Checks"] # If there is a *, it affects to all accounts - elif "*" in allowlist["Accounts"]: - account = "*" + # 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"] # Test if it is allowlisted - allowlisted_checks = allowlist["Accounts"][account]["Checks"] + allowlisted_checks.update(checks_multi_account) if is_allowlisted_in_check( - allowlisted_checks, audited_account, account, check, region, resource, tags + allowlisted_checks, + audited_account, + audited_account, + check, + region, + resource, + tags, ): is_finding_allowlisted = True diff --git a/tests/providers/aws/lib/allowlist/allowlist_test.py b/tests/providers/aws/lib/allowlist/allowlist_test.py index c02713d2..573692a6 100644 --- a/tests/providers/aws/lib/allowlist/allowlist_test.py +++ b/tests/providers/aws/lib/allowlist/allowlist_test.py @@ -88,7 +88,7 @@ class Test_Allowlist: Item={ "Accounts": "*", "Checks": "iam_user_hardware_mfa_enabled", - "Regions": ["eu-west-1", "us-east-1"], + "Regions": ["eu-west-1", AWS_REGION], "Resources": ["keyword"], } ) @@ -159,7 +159,7 @@ class Test_Allowlist: "*": { "Checks": { "check_test": { - "Regions": ["us-east-1", "eu-west-1"], + "Regions": [AWS_REGION, "eu-west-1"], "Resources": ["prowler", "^test", "prowler-pro"], } } @@ -201,7 +201,7 @@ class Test_Allowlist: "*": { "Checks": { "check_test": { - "Regions": ["us-east-1", "eu-west-1"], + "Regions": [AWS_REGION, "eu-west-1"], "Resources": [".*"], } } @@ -234,7 +234,7 @@ class Test_Allowlist: "*": { "Checks": { "check_test": { - "Regions": ["us-east-1", "eu-west-1"], + "Regions": [AWS_REGION, "eu-west-1"], "Resources": ["*"], } } @@ -260,9 +260,54 @@ class Test_Allowlist: ) ) + def test_is_allowlisted_all_and_single_account(self): + # Allowlist example + allowlist = { + "Accounts": { + "*": { + "Checks": { + "check_test_2": { + "Regions": [AWS_REGION, "eu-west-1"], + "Resources": ["*"], + } + } + }, + AWS_ACCOUNT_NUMBER: { + "Checks": { + "check_test": { + "Regions": [AWS_REGION], + "Resources": ["*"], + } + } + }, + } + } + + assert is_allowlisted( + allowlist, AWS_ACCOUNT_NUMBER, "check_test_2", AWS_REGION, "prowler", "" + ) + + assert is_allowlisted( + allowlist, AWS_ACCOUNT_NUMBER, "check_test", AWS_REGION, "prowler", "" + ) + + assert is_allowlisted( + allowlist, AWS_ACCOUNT_NUMBER, "check_test", AWS_REGION, "prowler-test", "" + ) + + assert is_allowlisted( + allowlist, AWS_ACCOUNT_NUMBER, "check_test", AWS_REGION, "test-prowler", "" + ) + + assert not ( + is_allowlisted( + allowlist, AWS_ACCOUNT_NUMBER, "check_test", "us-east-2", "test", "" + ) + ) + def test_is_allowlisted_in_region(self): # Allowlist example - allowlisted_regions = ["us-east-1", "eu-west-1"] + allowlisted_regions = [AWS_REGION, "eu-west-1"] allowlisted_resources = ["*"] assert is_allowlisted_in_region( @@ -301,7 +346,7 @@ class Test_Allowlist: def test_is_allowlisted_in_check(self): allowlisted_checks = { "check_test": { - "Regions": ["us-east-1", "eu-west-1"], + "Regions": [AWS_REGION, "eu-west-1"], "Resources": ["*"], } } @@ -352,7 +397,7 @@ class Test_Allowlist: # Allowlist example allowlisted_checks = { "s3_*": { - "Regions": ["us-east-1", "eu-west-1"], + "Regions": [AWS_REGION, "eu-west-1"], "Resources": ["*"], } } @@ -402,7 +447,7 @@ class Test_Allowlist: def test_is_allowlisted_lambda_generic_check(self): allowlisted_checks = { "lambda_*": { - "Regions": ["us-east-1", "eu-west-1"], + "Regions": [AWS_REGION, "eu-west-1"], "Resources": ["*"], } } @@ -480,7 +525,7 @@ class Test_Allowlist: def test_is_allowlisted_lambda_concrete_check(self): allowlisted_checks = { "lambda_function_no_secrets_in_variables": { - "Regions": ["us-east-1", "eu-west-1"], + "Regions": [AWS_REGION, "eu-west-1"], "Resources": ["*"], } } @@ -502,7 +547,7 @@ class Test_Allowlist: "*": { "Checks": { "check_test": { - "Regions": ["us-east-1", "eu-west-1"], + "Regions": [AWS_REGION, "eu-west-1"], "Resources": ["*"], "Tags": ["environment=dev", "project=.*"], }