From 3951295c0c9c8ddce33fbfe89a99f91f2bd64806 Mon Sep 17 00:00:00 2001 From: Sergio Garcia <38561120+sergargar@users.noreply.github.com> Date: Tue, 24 Sep 2024 08:28:21 -0400 Subject: [PATCH] chore(organizations): improve AWS Organizations service (#5151) --- .../organizations_scp_check_deny_regions.py | 154 ++++++++---------- .../organizations/organizations_service.py | 28 ++-- ...ions_tags_policies_enabled_and_attached.py | 23 ++- .../organizations_service_test.py | 23 ++- ...tags_policies_enabled_and_attached_test.py | 47 +++--- 5 files changed, 134 insertions(+), 141 deletions(-) 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 7c74be208c..049ca9f665 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 @@ -16,91 +16,81 @@ class organizations_scp_check_deny_regions(Check): report.resource_id = org.id report.resource_arn = org.arn report.region = organizations_client.region + report.status = "FAIL" + report.status_extended = ( + "AWS Organizations is not in-use for this AWS Account." + ) + if org.status == "ACTIVE": - if org.policies is None: - # Access Denied to list_policies - continue - if not org.policies: - report.status = "FAIL" - report.status_extended = ( - f"AWS Organization {org.id} has no SCP policies." - ) - else: - # We use this flag if we find a statement that is restricting regions but not all the configured ones: - is_region_restricted_statement = False - - for policy in org.policies: - # We only check SCP policies here - if policy.type != "SERVICE_CONTROL_POLICY": - continue - - # Statements are not always list - statements = policy.content.get("Statement") - if type(policy.content["Statement"]) is not list: - statements = [policy.content.get("Statement")] - - for statement in statements: - # Deny if Condition = {"StringNotEquals": {"aws:RequestedRegion": [region1, region2]}} - if ( - statement.get("Effect") == "Deny" - and "Condition" in statement - and "StringNotEquals" in statement["Condition"] - and "aws:RequestedRegion" - in statement["Condition"]["StringNotEquals"] - ): - if all( - region - in statement["Condition"]["StringNotEquals"][ - "aws:RequestedRegion" - ] - for region in organizations_enabled_regions - ): - # All defined regions are restricted, we exit here, no need to continue. - report.status = "PASS" - report.status_extended = f"AWS Organization {org.id} has SCP policy {policy.id} restricting all configured regions found." - findings.append(report) - return findings - else: - # Regions are restricted, but not the ones defined, we keep this finding, but we continue analyzing: - is_region_restricted_statement = True - report.status = "FAIL" - report.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." - - # Allow if Condition = {"StringEquals": {"aws:RequestedRegion": [region1, region2]}} - if ( - policy.content.get("Statement") == "Allow" - and "Condition" in statement - and "StringEquals" in statement["Condition"] - and "aws:RequestedRegion" - in statement["Condition"]["StringEquals"] - ): - if all( - region - in statement["Condition"]["StringEquals"][ - "aws:RequestedRegion" - ] - for region in organizations_enabled_regions - ): - # All defined regions are restricted, we exit here, no need to continue. - report.status = "PASS" - report.status_extended = f"AWS Organization {org.id} has SCP policy {policy.id} restricting all configured regions found." - findings.append(report) - return findings - else: - # Regions are restricted, but not the ones defined, we keep this finding, but we continue analyzing: - is_region_restricted_statement = True - report.status = "FAIL" - report.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." - - if not is_region_restricted_statement: - report.status = "FAIL" - report.status_extended = f"AWS Organization {org.id} has SCP policies but don't restrict AWS Regions." - - else: - report.status = "FAIL" report.status_extended = ( - "AWS Organizations is not in-use for this AWS Account." + f"AWS Organizations {org.id} does not have SCP policies." ) + # We use this flag if we find a statement that is restricting regions but not all the configured ones: + is_region_restricted_statement = False + + for policy in org.policies.get("SERVICE_CONTROL_POLICY", []): + + # Statements are not always list + statements = policy.content.get("Statement") + if type(policy.content["Statement"]) is not list: + statements = [policy.content.get("Statement")] + + for statement in statements: + # Deny if Condition = {"StringNotEquals": {"aws:RequestedRegion": [region1, region2]}} + if ( + statement.get("Effect") == "Deny" + and "Condition" in statement + and "StringNotEquals" in statement["Condition"] + and "aws:RequestedRegion" + in statement["Condition"]["StringNotEquals"] + ): + if all( + region + in statement["Condition"]["StringNotEquals"][ + "aws:RequestedRegion" + ] + for region in organizations_enabled_regions + ): + # All defined regions are restricted, we exit here, no need to continue. + report.status = "PASS" + report.status_extended = f"AWS Organization {org.id} has SCP policy {policy.id} restricting all configured regions found." + findings.append(report) + return findings + else: + # Regions are restricted, but not the ones defined, we keep this finding, but we continue analyzing: + is_region_restricted_statement = True + report.status = "FAIL" + report.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." + + # Allow if Condition = {"StringEquals": {"aws:RequestedRegion": [region1, region2]}} + if ( + policy.content.get("Statement") == "Allow" + and "Condition" in statement + and "StringEquals" in statement["Condition"] + and "aws:RequestedRegion" + in statement["Condition"]["StringEquals"] + ): + if all( + region + in statement["Condition"]["StringEquals"][ + "aws:RequestedRegion" + ] + for region in organizations_enabled_regions + ): + # All defined regions are restricted, we exit here, no need to continue. + report.status = "PASS" + report.status_extended = f"AWS Organization {org.id} has SCP policy {policy.id} restricting all configured regions found." + findings.append(report) + return findings + else: + # Regions are restricted, but not the ones defined, we keep this finding, but we continue analyzing: + is_region_restricted_statement = True + report.status = "FAIL" + report.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." + + if not is_region_restricted_statement: + report.status = "FAIL" + report.status_extended = f"AWS Organization {org.id} has SCP policies but don't restrict AWS Regions." findings.append(report) diff --git a/prowler/providers/aws/services/organizations/organizations_service.py b/prowler/providers/aws/services/organizations/organizations_service.py index 9f749280c4..68ae41bec4 100644 --- a/prowler/providers/aws/services/organizations/organizations_service.py +++ b/prowler/providers/aws/services/organizations/organizations_service.py @@ -8,7 +8,7 @@ from prowler.lib.logger import logger from prowler.lib.scan_filters.scan_filters import is_resource_filtered from prowler.providers.aws.lib.service.service import AWSService -available_organizations_policies = [ +AVAILABLE_ORGANIZATIONS_POLICIES = [ "SERVICE_CONTROL_POLICY", "TAG_POLICY", "BACKUP_POLICY", @@ -16,13 +16,12 @@ available_organizations_policies = [ ] -################## Organizations class Organizations(AWSService): def __init__(self, provider): # Call AWSService's __init__ super().__init__(__class__.__name__, provider) self.organizations = [] - self.policies = [] + self.policies = {} self.delegated_administrators = [] self._describe_organization() @@ -30,15 +29,12 @@ class Organizations(AWSService): logger.info("Organizations - Describe Organization...") try: - # Check if Organizations is in-use try: organization_desc = self.client.describe_organization()["Organization"] organization_arn = organization_desc.get("Arn") organization_id = organization_desc.get("Id") organization_master_id = organization_desc.get("MasterAccountId") - # Fetch policies for organization: organization_policies = self._list_policies() - # Fetch delegated administrators for organization: organization_delegated_administrator = ( self._list_delegated_administrators() ) @@ -89,23 +85,24 @@ class Organizations(AWSService): f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" ) - # I'm using list_policies instead of list_policies_for_target, because the last one only returns "Attached directly" policies but not "Inherited from..." policies. def _list_policies(self): logger.info("Organizations - List policies...") try: list_policies_paginator = self.client.get_paginator("list_policies") - for policy_type in available_organizations_policies: + policies = {} + for policy_type in AVAILABLE_ORGANIZATIONS_POLICIES: logger.info( "Organizations - List policies... - Type: %s", policy_type, ) + policies[policy_type] = [] for page in list_policies_paginator.paginate(Filter=policy_type): for policy in page["Policies"]: policy_id = policy.get("Id") policy_content = self._describe_policy(policy_id) policy_targets = self._list_targets_for_policy(policy_id) - self.policies.append( + policies[policy_type].append( Policy( arn=policy.get("Arn"), id=policy_id, @@ -118,7 +115,7 @@ class Organizations(AWSService): except ClientError as error: if error.response["Error"]["Code"] == "AccessDeniedException": - self.policies = None + policies = None except Exception as error: logger.error( @@ -126,12 +123,10 @@ class Organizations(AWSService): ) finally: - return self.policies + return policies def _describe_policy(self, policy_id) -> dict: logger.info("Organizations - Describe policy: %s ...", policy_id) - - # This operation can be called only from the organization’s management account or by a member account that is a delegated administrator for an Amazon Web Services service. try: policy_content = {} if policy_id: @@ -143,8 +138,7 @@ class Organizations(AWSService): if isinstance(policy_content, str): policy_content = json.loads(policy_content) - return policy_content # This could be not be a dict, because json.loads could return a list or a string depending on the content of policy_content object. - + return policy_content except Exception as error: logger.error( f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" @@ -170,7 +164,7 @@ class Organizations(AWSService): return [] def _list_delegated_administrators(self): - logger.info("Organizations - List Delegated Administrators") + logger.info("Organizations - List Delegated Administrators...") try: list_delegated_administrators_paginator = self.client.get_paginator( @@ -225,5 +219,5 @@ class Organization(BaseModel): id: str status: str master_id: str - policies: list[Policy] = None + policies: dict[str, list[Policy]] = {} delegated_administrators: list[DelegatedAdministrator] = None diff --git a/prowler/providers/aws/services/organizations/organizations_tags_policies_enabled_and_attached/organizations_tags_policies_enabled_and_attached.py b/prowler/providers/aws/services/organizations/organizations_tags_policies_enabled_and_attached/organizations_tags_policies_enabled_and_attached.py index 7dcc4d3e95..f3cb419ed3 100644 --- a/prowler/providers/aws/services/organizations/organizations_tags_policies_enabled_and_attached/organizations_tags_policies_enabled_and_attached.py +++ b/prowler/providers/aws/services/organizations/organizations_tags_policies_enabled_and_attached/organizations_tags_policies_enabled_and_attached.py @@ -17,20 +17,17 @@ class organizations_tags_policies_enabled_and_attached(Check): report.status_extended = ( "AWS Organizations is not in-use for this AWS Account." ) + if org.status == "ACTIVE": - if org.policies is None: - # Access Denied to list_policies - continue - for policy in org.policies: - # We only check SCP policies here - if policy.type != "TAG_POLICY": - continue - - report.status_extended = f"AWS Organization {org.id} has tag policies enabled but not attached." - - if policy.targets: - report.status = "PASS" - report.status_extended = f"AWS Organization {org.id} has tag policies enabled and attached to an AWS account." + report.status_extended = ( + f"AWS Organizations {org.id} does not have tag policies." + ) + if org.policies is not None: # Access Denied to list_policies + for policy in org.policies.get("TAG_POLICY", []): + report.status_extended = f"AWS Organization {org.id} has tag policies enabled but not attached." + if policy.targets: + report.status = "PASS" + report.status_extended = f"AWS Organization {org.id} has tag policies enabled and attached to an AWS account." findings.append(report) diff --git a/tests/providers/aws/services/organizations/organizations_service_test.py b/tests/providers/aws/services/organizations/organizations_service_test.py index 724f00146c..dfae12d402 100644 --- a/tests/providers/aws/services/organizations/organizations_service_test.py +++ b/tests/providers/aws/services/organizations/organizations_service_test.py @@ -22,15 +22,12 @@ class Test_Organizations_Service: @mock_aws def test_describe_organization(self): - # Create Organization conn = client("organizations", region_name=AWS_REGION_EU_WEST_1) response = conn.create_organization() - # Mock aws_provider = set_mocked_aws_provider( [AWS_REGION_EU_WEST_1], create_default_organization=False ) organizations = Organizations(aws_provider) - # Tests assert len(organizations.organizations) == 1 assert organizations.organizations[0].arn == response["Organization"]["Arn"] assert organizations.organizations[0].id == response["Organization"]["Id"] @@ -43,7 +40,6 @@ class Test_Organizations_Service: @mock_aws def test_list_policies(self): - # Create Policy conn = client("organizations", region_name=AWS_REGION_EU_WEST_1) conn.create_organization() response = conn.create_policy( @@ -52,13 +48,28 @@ class Test_Organizations_Service: Name="Test", Type="SERVICE_CONTROL_POLICY", ) - # Mock aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) organizations = Organizations(aws_provider) - # Tests for policy in organizations.policies: if policy.arn == response["Policy"]["PolicySummary"]["Arn"]: assert policy.type == "SERVICE_CONTROL_POLICY" assert policy.aws_managed is False assert policy.content == json.loads(response["Policy"]["Content"]) assert policy.targets == [] + + @mock_aws + def test_describe_policy(self): + conn = client("organizations", region_name=AWS_REGION_EU_WEST_1) + conn.create_organization() + response = conn.create_policy( + Content=scp_restrict_regions_with_deny(), + Description="Test", + Name="Test", + Type="SERVICE_CONTROL_POLICY", + ) + aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) + organizations = Organizations(aws_provider) + policy = organizations._describe_policy( + response["Policy"]["PolicySummary"]["Id"] + ) + assert policy == json.loads(response["Policy"]["Content"]) diff --git a/tests/providers/aws/services/organizations/organizations_tags_policies_enabled_and_attached/organizations_tags_policies_enabled_and_attached_test.py b/tests/providers/aws/services/organizations/organizations_tags_policies_enabled_and_attached/organizations_tags_policies_enabled_and_attached_test.py index 966e5fd14d..9a90c9f61c 100644 --- a/tests/providers/aws/services/organizations/organizations_tags_policies_enabled_and_attached/organizations_tags_policies_enabled_and_attached_test.py +++ b/tests/providers/aws/services/organizations/organizations_tags_policies_enabled_and_attached/organizations_tags_policies_enabled_and_attached_test.py @@ -10,9 +10,6 @@ from tests.providers.aws.utils import ( set_mocked_aws_provider, ) -# Moto: NotImplementedError: The TAG_POLICY policy type has not been implemented -# Needs to Mock manually - class Test_organizations_tags_policies_enabled_and_attached: def test_organization_no_organization(self): @@ -64,16 +61,18 @@ class Test_organizations_tags_policies_enabled_and_attached: arn="arn:aws:organizations::1234567890:organization/o-1234567890", status="ACTIVE", master_id="1234567890", - policies=[ - Policy( - id="p-1234567890", - arn="arn:aws:organizations::1234567890:policy/o-1234567890/p-1234567890", - type="TAG_POLICY", - aws_managed=False, - content={"tags": {"Owner": {}}}, - targets=[], - ) - ], + policies={ + "TAG_POLICY": [ + Policy( + id="p-1234567890", + arn="arn:aws:organizations::1234567890:policy/o-1234567890/p-1234567890", + type="TAG_POLICY", + aws_managed=False, + content={"tags": {"Owner": {}}}, + targets=[], + ) + ] + }, delegated_administrators=None, ) ] @@ -118,16 +117,18 @@ class Test_organizations_tags_policies_enabled_and_attached: arn="arn:aws:organizations::1234567890:organization/o-1234567890", status="ACTIVE", master_id="1234567890", - policies=[ - Policy( - id="p-1234567890", - arn="arn:aws:organizations::1234567890:policy/o-1234567890/p-1234567890", - type="TAG_POLICY", - aws_managed=False, - content={"tags": {"Owner": {}}}, - targets=["1234567890"], - ) - ], + policies={ + "TAG_POLICY": [ + Policy( + id="p-1234567890", + arn="arn:aws:organizations::1234567890:policy/o-1234567890/p-1234567890", + type="TAG_POLICY", + aws_managed=False, + content={"tags": {"Owner": {}}}, + targets=["1234567890"], + ) + ] + }, delegated_administrators=None, ) ]