From 13bd6fc0bfb821ebfc0f7ee2cc4be5e84c4dfb41 Mon Sep 17 00:00:00 2001 From: Narahari Raghava <70995755+NarahariRaghava@users.noreply.github.com> Date: Thu, 9 Jul 2026 04:19:33 -0500 Subject: [PATCH] =?UTF-8?q?fix(aws):=20check=20statement=20Effect=20instea?= =?UTF-8?q?d=20of=20policy=20Statement=20in=20SCP=20a=E2=80=A6=20(#11727)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniel Barranquero --- .../scp-allow-region-check.fixed.md | 1 + .../organizations_scp_check_deny_regions.py | 2 +- ...ganizations_scp_check_deny_regions_test.py | 138 ++++++++++++++++++ 3 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 prowler/changelog.d/scp-allow-region-check.fixed.md diff --git a/prowler/changelog.d/scp-allow-region-check.fixed.md b/prowler/changelog.d/scp-allow-region-check.fixed.md new file mode 100644 index 0000000000..e8dd521f9f --- /dev/null +++ b/prowler/changelog.d/scp-allow-region-check.fixed.md @@ -0,0 +1 @@ +`organizations_scp_check_deny_regions` no longer reports false `FAIL` for AWS Organizations that restrict regions with Allow-based SCPs; the Allow path now checks the statement `Effect` instead of an always-false comparison that made it unreachable diff --git a/prowler/providers/aws/services/organizations/organizations_scp_check_deny_regions/organizations_scp_check_deny_regions.py b/prowler/providers/aws/services/organizations/organizations_scp_check_deny_regions/organizations_scp_check_deny_regions.py index cced82a762..1b676ff0b5 100644 --- a/prowler/providers/aws/services/organizations/organizations_scp_check_deny_regions/organizations_scp_check_deny_regions.py +++ b/prowler/providers/aws/services/organizations/organizations_scp_check_deny_regions/organizations_scp_check_deny_regions.py @@ -67,7 +67,7 @@ class organizations_scp_check_deny_regions(Check): # Allow if Condition = {"StringEquals": {"aws:RequestedRegion": [region1, region2]}} if ( - policy.content.get("Statement") == "Allow" + statement.get("Effect") == "Allow" and "Condition" in statement and "StringEquals" in statement["Condition"] and "aws:RequestedRegion" diff --git a/tests/providers/aws/services/organizations/organizations_scp_check_deny_regions/organizations_scp_check_deny_regions_test.py b/tests/providers/aws/services/organizations/organizations_scp_check_deny_regions/organizations_scp_check_deny_regions_test.py index d6d9941960..3d5bb16aba 100644 --- a/tests/providers/aws/services/organizations/organizations_scp_check_deny_regions/organizations_scp_check_deny_regions_test.py +++ b/tests/providers/aws/services/organizations/organizations_scp_check_deny_regions/organizations_scp_check_deny_regions_test.py @@ -17,6 +17,10 @@ def scp_restrict_regions_with_deny(): return '{"Version":"2012-10-17","Statement":{"Effect":"Deny","NotAction":"s3:*","Resource":"*","Condition":{"StringNotEquals":{"aws:RequestedRegion":["eu-central-1","eu-west-1"]}}}}' +def scp_restrict_regions_with_allow(): + return '{"Version":"2012-10-17","Statement":{"Effect":"Allow","Action":"*","Resource":"*","Condition":{"StringEquals":{"aws:RequestedRegion":["eu-central-1","eu-west-1"]}}}}' + + def scp_restrict_regions_without_statement(): return '{"Version":"2012-10-17"}' @@ -352,3 +356,137 @@ class Test_organizations_scp_check_deny_regions: == f"AWS Organization {org_id} has SCP policies but don't restrict AWS Regions." ) assert result[0].region == AWS_REGION_EU_WEST_1 + + @mock_aws + def test_organization_with_scp_allow_regions_valid(self): + aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) + + conn = client("organizations", region_name=AWS_REGION_EU_WEST_1) + response = conn.describe_organization() + response_policy = conn.create_policy( + Content=scp_restrict_regions_with_allow(), + Description="Test", + Name="Test", + Type="SERVICE_CONTROL_POLICY", + ) + org_id = response["Organization"]["Id"] + policy_id = response_policy["Policy"]["PolicySummary"]["Id"] + + aws_provider._audit_config = {"organizations_enabled_regions": ["eu-central-1"]} + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ): + with mock.patch( + "prowler.providers.aws.services.organizations.organizations_scp_check_deny_regions.organizations_scp_check_deny_regions.organizations_client", + new=Organizations(aws_provider), + ): + from prowler.providers.aws.services.organizations.organizations_scp_check_deny_regions.organizations_scp_check_deny_regions import ( + organizations_scp_check_deny_regions, + ) + + check = organizations_scp_check_deny_regions() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "PASS" + assert result[0].resource_id == response["Organization"]["Id"] + assert result[0].resource_arn == response["Organization"]["Arn"] + assert ( + result[0].status_extended + == f"AWS Organization {org_id} has SCP policy {policy_id} restricting all configured regions found." + ) + assert result[0].region == AWS_REGION_EU_WEST_1 + + @mock_aws + def test_organization_with_scp_allow_regions_not_valid(self): + aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) + + conn = client("organizations", region_name=AWS_REGION_EU_WEST_1) + response = conn.describe_organization() + response_policy = conn.create_policy( + Content=scp_restrict_regions_with_allow(), + Description="Test", + Name="Test", + Type="SERVICE_CONTROL_POLICY", + ) + org_id = response["Organization"]["Id"] + policy_id = response_policy["Policy"]["PolicySummary"]["Id"] + + aws_provider._audit_config = {"organizations_enabled_regions": ["us-east-1"]} + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ): + with mock.patch( + "prowler.providers.aws.services.organizations.organizations_scp_check_deny_regions.organizations_scp_check_deny_regions.organizations_client", + new=Organizations(aws_provider), + ): + from prowler.providers.aws.services.organizations.organizations_scp_check_deny_regions.organizations_scp_check_deny_regions import ( + organizations_scp_check_deny_regions, + ) + + check = organizations_scp_check_deny_regions() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "FAIL" + assert result[0].resource_id == response["Organization"]["Id"] + assert ( + "arn:aws:organizations::123456789012:organization/o-" + in result[0].resource_arn + ) + assert ( + result[0].status_extended + == f"AWS Organization {org_id} has SCP policies {policy_id} restricting some AWS Regions, but not all the configured ones, please check config." + ) + assert result[0].region == AWS_REGION_EU_WEST_1 + + @mock_aws + def test_organization_with_scp_allow_all_regions_valid(self): + aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) + + conn = client("organizations", region_name=AWS_REGION_EU_WEST_1) + response = conn.describe_organization() + response_policy = conn.create_policy( + Content=scp_restrict_regions_with_allow(), + Description="Test", + Name="Test", + Type="SERVICE_CONTROL_POLICY", + ) + org_id = response["Organization"]["Id"] + policy_id = response_policy["Policy"]["PolicySummary"]["Id"] + + aws_provider._audit_config = { + "organizations_enabled_regions": [ + AWS_REGION_EU_WEST_1, + AWS_REGION_EU_CENTRAL_1, + ] + } + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ): + with mock.patch( + "prowler.providers.aws.services.organizations.organizations_scp_check_deny_regions.organizations_scp_check_deny_regions.organizations_client", + new=Organizations(aws_provider), + ): + from prowler.providers.aws.services.organizations.organizations_scp_check_deny_regions.organizations_scp_check_deny_regions import ( + organizations_scp_check_deny_regions, + ) + + check = organizations_scp_check_deny_regions() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "PASS" + assert result[0].resource_id == response["Organization"]["Id"] + assert result[0].resource_arn == response["Organization"]["Arn"] + assert ( + result[0].status_extended + == f"AWS Organization {org_id} has SCP policy {policy_id} restricting all configured regions found." + ) + assert result[0].region == AWS_REGION_EU_WEST_1