From c3c006c2e9685ec23517232d2e4d38e9f6de87e2 Mon Sep 17 00:00:00 2001 From: Sergio Date: Tue, 15 Oct 2024 11:56:09 -0400 Subject: [PATCH] chore: solve comments --- ...lambda_function_not_publicly_accessible.py | 1 + .../dynamodb_table_cross_account_access.py | 1 + ...cr_repositories_not_publicly_accessible.py | 2 +- .../efs_not_publicly_accessible.py | 2 +- .../eventbridge_bus_exposed.py | 2 +- ...ross_service_confused_deputy_prevention.py | 1 + .../providers/aws/services/iam/lib/policy.py | 29 ++---------- .../kms_key_not_publicly_accessible.py | 6 ++- ...service_domains_not_publicly_accessible.py | 4 +- .../redshift_cluster_public_access.py | 13 +++--- .../aws/services/redshift/redshift_service.py | 24 ++++++++++ .../s3_bucket_cross_account_access.py | 2 +- .../s3_bucket_policy_public_write_access.py | 1 + .../s3_bucket_public_access.py | 2 +- .../aws/services/iam/lib/policy_test.py | 46 ++++++++++++------- .../redshift_cluster_public_access_test.py | 12 +++-- .../redshift/redshift_service_test.py | 36 +++++++++++++++ 17 files changed, 124 insertions(+), 60 deletions(-) diff --git a/prowler/providers/aws/services/awslambda/awslambda_function_not_publicly_accessible/awslambda_function_not_publicly_accessible.py b/prowler/providers/aws/services/awslambda/awslambda_function_not_publicly_accessible/awslambda_function_not_publicly_accessible.py index 6f0fde12c3..778c44be93 100644 --- a/prowler/providers/aws/services/awslambda/awslambda_function_not_publicly_accessible/awslambda_function_not_publicly_accessible.py +++ b/prowler/providers/aws/services/awslambda/awslambda_function_not_publicly_accessible/awslambda_function_not_publicly_accessible.py @@ -17,6 +17,7 @@ def execute(self): report.status_extended = f"Lambda function {function.name} has a policy resource-based policy not public." if is_policy_public( function.policy, + awslambda_client.audited_account, is_cross_account_allowed=True, ): report.status = "FAIL" diff --git a/prowler/providers/aws/services/dynamodb/dynamodb_table_cross_account_access/dynamodb_table_cross_account_access.py b/prowler/providers/aws/services/dynamodb/dynamodb_table_cross_account_access/dynamodb_table_cross_account_access.py index 10e84c1c06..15998d5832 100644 --- a/prowler/providers/aws/services/dynamodb/dynamodb_table_cross_account_access/dynamodb_table_cross_account_access.py +++ b/prowler/providers/aws/services/dynamodb/dynamodb_table_cross_account_access/dynamodb_table_cross_account_access.py @@ -20,6 +20,7 @@ def execute(self): report.status_extended = f"DynamoDB table {table.name} has a resource-based policy but is not cross account." if is_policy_public( table.policy, + dynamodb_client.audited_account, ): report.status = "FAIL" report.status_extended = f"DynamoDB table {table.name} has a resource-based policy allowing cross account access." diff --git a/prowler/providers/aws/services/ecr/ecr_repositories_not_publicly_accessible/ecr_repositories_not_publicly_accessible.py b/prowler/providers/aws/services/ecr/ecr_repositories_not_publicly_accessible/ecr_repositories_not_publicly_accessible.py index 17a6742087..146c1cfcd2 100644 --- a/prowler/providers/aws/services/ecr/ecr_repositories_not_publicly_accessible/ecr_repositories_not_publicly_accessible.py +++ b/prowler/providers/aws/services/ecr/ecr_repositories_not_publicly_accessible/ecr_repositories_not_publicly_accessible.py @@ -18,7 +18,7 @@ def execute(self): f"Repository {repository.name} is not publicly accessible." ) if repository.policy: - if is_policy_public(repository.policy): + if is_policy_public(repository.policy, ecr_client.audited_account): report.status = "FAIL" report.status_extended = ( f"Repository {repository.name} is publicly accessible." diff --git a/prowler/providers/aws/services/efs/efs_not_publicly_accessible/efs_not_publicly_accessible.py b/prowler/providers/aws/services/efs/efs_not_publicly_accessible/efs_not_publicly_accessible.py index 2a3f44c578..a6a6f6abec 100644 --- a/prowler/providers/aws/services/efs/efs_not_publicly_accessible/efs_not_publicly_accessible.py +++ b/prowler/providers/aws/services/efs/efs_not_publicly_accessible/efs_not_publicly_accessible.py @@ -17,7 +17,7 @@ def execute(self): if not fs.policy: report.status = "FAIL" report.status_extended = f"EFS {fs.id} doesn't have any policy which means it grants full access to any client within the VPC." - elif is_policy_public(fs.policy) and any( + elif is_policy_public(fs.policy, efs_client.audited_account) and any( statement.get("Condition", {}) .get("Bool", {}) .get("elasticfilesystem:AccessedViaMountTarget", "false") diff --git a/prowler/providers/aws/services/eventbridge/eventbridge_bus_exposed/eventbridge_bus_exposed.py b/prowler/providers/aws/services/eventbridge/eventbridge_bus_exposed/eventbridge_bus_exposed.py index 26e0011eb6..5da8c67b33 100644 --- a/prowler/providers/aws/services/eventbridge/eventbridge_bus_exposed/eventbridge_bus_exposed.py +++ b/prowler/providers/aws/services/eventbridge/eventbridge_bus_exposed/eventbridge_bus_exposed.py @@ -18,7 +18,7 @@ def execute(self): report.resource_arn = bus.arn report.resource_tags = bus.tags report.region = bus.region - if is_policy_public(bus.policy): + if is_policy_public(bus.policy, eventbridge_client.audited_account): report.status = "FAIL" report.status_extended = ( f"EventBridge event bus {bus.name} is exposed to everyone." diff --git a/prowler/providers/aws/services/iam/iam_role_cross_service_confused_deputy_prevention/iam_role_cross_service_confused_deputy_prevention.py b/prowler/providers/aws/services/iam/iam_role_cross_service_confused_deputy_prevention/iam_role_cross_service_confused_deputy_prevention.py index 11d660a03e..266e9a9c60 100644 --- a/prowler/providers/aws/services/iam/iam_role_cross_service_confused_deputy_prevention/iam_role_cross_service_confused_deputy_prevention.py +++ b/prowler/providers/aws/services/iam/iam_role_cross_service_confused_deputy_prevention/iam_role_cross_service_confused_deputy_prevention.py @@ -19,6 +19,7 @@ def execute(self) -> Check_Report_AWS: report.status_extended = f"IAM Service Role {role.name} does not prevent against a cross-service confused deputy attack." if not is_policy_public( role.assume_role_policy, + iam_client.audited_account, not_allowed_actions=["sts:AssumeRole", "sts:*"], ): report.status = "PASS" diff --git a/prowler/providers/aws/services/iam/lib/policy.py b/prowler/providers/aws/services/iam/lib/policy.py index bbddedb3f9..4ca5d10ba5 100644 --- a/prowler/providers/aws/services/iam/lib/policy.py +++ b/prowler/providers/aws/services/iam/lib/policy.py @@ -1,4 +1,3 @@ -import inspect from ipaddress import ip_address, ip_network from prowler.lib.logger import logger @@ -151,6 +150,7 @@ def is_condition_restricting_from_private_ip(condition_statement: dict) -> bool: # TODO: Add logic for deny statements def is_policy_public( policy: dict, + source_account: str = "", is_cross_account_allowed=False, not_allowed_actions: list = [], ) -> bool: @@ -159,20 +159,13 @@ def is_policy_public( If the policy gives access to an AWS service principal is considered public if the policy is not pair with conditions since it can be invoked by AWS services in other accounts. Args: policy (dict): The AWS policy to check + source_account (str): The account to check if the access is restricted to it, default: "" is_cross_account_allowed (bool): If the policy can allow cross-account access, default: False not_allowed_actions (list): List of actions that are not allowed, default: []. If not_allowed_actions is empty, the function will not consider the actions in the policy. Returns: bool: True if the policy allows public access, False otherwise """ is_public = False - source_account = "" - trusted_account_ids = [] - # Get service client from the check to get the trusted account IDs and source account - for var_name, value in inspect.stack()[1].frame.f_globals.items(): - if "_client" in var_name: - trusted_account_ids = value.audit_config.get("trusted_account_ids", []) - source_account = value.audited_account - break for statement in policy.get("Statement", []): # Only check allow statements if statement["Effect"] == "Allow": @@ -240,7 +233,6 @@ def is_policy_public( statement.get("Condition", {}), source_account, is_cross_account_allowed, - trusted_account_ids, ) and not is_condition_block_restrictive_organization( statement.get("Condition", {}) @@ -258,7 +250,6 @@ def is_condition_block_restrictive( condition_statement: dict, source_account: str = "", is_cross_account_allowed=False, - trusted_account_ids: list = [], ): """ is_condition_block_restrictive parses the IAM Condition policy block and, by default, returns True if the source_account passed as argument is within, False if not. @@ -275,8 +266,6 @@ def is_condition_block_restrictive( source_account: str with a 12-digit AWS Account number, e.g.: 111122223333, default: "" - trusted_account_ids: list with a list of trusted AWS Account numbers, e.g.: ["111122223333"], default: [] - is_cross_account_allowed: bool to allow cross-account access, e.g.: True, default: False """ @@ -345,7 +334,7 @@ def is_condition_block_restrictive( # if cross account is not allowed check for each condition block looking for accounts # different than default if not is_cross_account_allowed: - # if there is an arn/account without the source account or trusted accounts -> we do not consider it safe + # if there is an arn/account without the source account -> we do not consider it safe # here by default we assume is true and look for false entries for item in condition_statement[condition_operator][value]: if ( @@ -353,12 +342,8 @@ def is_condition_block_restrictive( and "aws:sourcevpce" != value ): if source_account not in item: - if not any( - trusted_account_id in item - for trusted_account_id in trusted_account_ids - ): - is_condition_key_restrictive = False - break + is_condition_key_restrictive = False + break if is_condition_key_restrictive: is_condition_valid = True @@ -377,10 +362,6 @@ def is_condition_block_restrictive( if ( source_account in condition_statement[condition_operator][value] - ) or any( - trusted_account_id - in condition_statement[condition_operator][value] - for trusted_account_id in trusted_account_ids ): is_condition_valid = True diff --git a/prowler/providers/aws/services/kms/kms_key_not_publicly_accessible/kms_key_not_publicly_accessible.py b/prowler/providers/aws/services/kms/kms_key_not_publicly_accessible/kms_key_not_publicly_accessible.py index e821c6bc13..0ac6f1f0e7 100644 --- a/prowler/providers/aws/services/kms/kms_key_not_publicly_accessible/kms_key_not_publicly_accessible.py +++ b/prowler/providers/aws/services/kms/kms_key_not_publicly_accessible/kms_key_not_publicly_accessible.py @@ -18,7 +18,11 @@ def execute(self): report.resource_tags = key.tags report.region = key.region # If the "Principal" element value is set to { "AWS": "*" } and the policy statement is not using any Condition clauses to filter the access, the selected AWS KMS master key is publicly accessible. - if is_policy_public(key.policy, not_allowed_actions=["kms:*"]): + if is_policy_public( + key.policy, + kms_client.audited_account, + not_allowed_actions=["kms:*"], + ): report.status = "FAIL" report.status_extended = ( f"KMS key {key.id} may be publicly accessible." diff --git a/prowler/providers/aws/services/opensearch/opensearch_service_domains_not_publicly_accessible/opensearch_service_domains_not_publicly_accessible.py b/prowler/providers/aws/services/opensearch/opensearch_service_domains_not_publicly_accessible/opensearch_service_domains_not_publicly_accessible.py index 4a2e4cb989..a367423731 100644 --- a/prowler/providers/aws/services/opensearch/opensearch_service_domains_not_publicly_accessible/opensearch_service_domains_not_publicly_accessible.py +++ b/prowler/providers/aws/services/opensearch/opensearch_service_domains_not_publicly_accessible/opensearch_service_domains_not_publicly_accessible.py @@ -21,7 +21,9 @@ def execute(self): if domain.vpc_id: report.status_extended = f"Opensearch domain {domain.name} is in a VPC, then it is not publicly accessible." - elif domain.access_policy and is_policy_public(domain.access_policy): + elif domain.access_policy and is_policy_public( + domain.access_policy, opensearch_client.audited_account + ): report.status = "FAIL" report.status_extended = f"Opensearch domain {domain.name} is publicly accessible via access policy." diff --git a/prowler/providers/aws/services/redshift/redshift_cluster_public_access/redshift_cluster_public_access.py b/prowler/providers/aws/services/redshift/redshift_cluster_public_access/redshift_cluster_public_access.py index dbd9e8d65e..4dbc46e52e 100644 --- a/prowler/providers/aws/services/redshift/redshift_cluster_public_access/redshift_cluster_public_access.py +++ b/prowler/providers/aws/services/redshift/redshift_cluster_public_access/redshift_cluster_public_access.py @@ -21,13 +21,12 @@ def execute(self): # 1. Check if Redshift Cluster is publicly accessible if cluster.endpoint_address and cluster.public_access: report.status_extended = f"Redshift Cluster {cluster.id} has the endpoint {cluster.endpoint_address} set as publicly accessible but is not publicly exposed." - # 2. Check if Redshift Cluster is in a public VPC - if ( - cluster.vpc_id - and cluster.vpc_id in vpc_client.vpcs - and vpc_client.vpcs[cluster.vpc_id].public + # 2. Check if Redshift Cluster is in a public subnet + if any( + subnet in vpc_client.subnets and vpc_client.subnets[subnet].public + for subnet in cluster.subnets ): - report.status_extended = f"Redshift Cluster {cluster.id} has the endpoint {cluster.endpoint_address} set as publicly accessible in the public VPC {cluster.vpc_id} but is not publicly exposed." + report.status_extended = f"Redshift Cluster {cluster.id} has the endpoint {cluster.endpoint_address} set as publicly accessible in a public subnet but is not publicly exposed." # 3. Check if any Redshift Cluster Security Group is publicly open for sg_id in getattr(cluster, "vpc_security_groups", []): sg_arn = f"arn:{redshift_client.audited_partition}:ec2:{cluster.region}:{redshift_client.audited_account}:security-group/{sg_id}" @@ -39,7 +38,7 @@ def execute(self): ingress_rule, "tcp", any_address=True ): report.status = "FAIL" - report.status_extended = f"Redshift Cluster {cluster.id} has the endpoint {cluster.endpoint_address} set as publicly accessible and it is exposed to the Internet by security group ({sg_id}) in public VPC {cluster.vpc_id}." + report.status_extended = f"Redshift Cluster {cluster.id} has the endpoint {cluster.endpoint_address} set as publicly accessible and it is exposed to the Internet by security group ({sg_id}) in a public subnet." break if report.status == "FAIL": break diff --git a/prowler/providers/aws/services/redshift/redshift_service.py b/prowler/providers/aws/services/redshift/redshift_service.py index d7de8ec4ef..082cf9bafd 100644 --- a/prowler/providers/aws/services/redshift/redshift_service.py +++ b/prowler/providers/aws/services/redshift/redshift_service.py @@ -16,6 +16,7 @@ def __init__(self, provider): self.__threading_call__(self._describe_logging_status, self.clusters) self.__threading_call__(self._describe_cluster_snapshots, self.clusters) self.__threading_call__(self._describe_cluster_parameters, self.clusters) + self.__threading_call__(self._describe_cluster_subnets, self.clusters) def _describe_clusters(self, regional_client): logger.info("Redshift - Describing Clusters...") @@ -54,6 +55,7 @@ def _describe_clusters(self, regional_client): parameter_group_name=cluster.get( "ClusterParameterGroups", [{}] )[0].get("ParameterGroupName", ""), + subnet_group=cluster.get("ClusterSubnetGroupName", ""), ) self.clusters.append(cluster_to_append) except Exception as error: @@ -61,6 +63,26 @@ def _describe_clusters(self, regional_client): f"{regional_client.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" ) + def _describe_cluster_subnets(self, cluster): + logger.info("Redshift - Describing Cluster Subnets...") + try: + regional_client = self.regional_clients[cluster.region] + if cluster.subnet_group: + subnet_group_details = regional_client.describe_cluster_subnet_groups( + ClusterSubnetGroupName=cluster.subnet_group + ) + subnets = [ + subnet["SubnetIdentifier"] + for subnet in subnet_group_details["ClusterSubnetGroups"][0][ + "Subnets" + ] + ] + cluster.subnets = subnets + except Exception as error: + logger.error( + f"{regional_client.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" + ) + def _describe_logging_status(self, cluster): logger.info("Redshift - Describing Logging Status...") try: @@ -133,3 +155,5 @@ class Cluster(BaseModel): enhanced_vpc_routing: bool = False parameter_group_name: str = None require_ssl: bool = False + subnet_group: str = None + subnets: list[str] = [] diff --git a/prowler/providers/aws/services/s3/s3_bucket_cross_account_access/s3_bucket_cross_account_access.py b/prowler/providers/aws/services/s3/s3_bucket_cross_account_access/s3_bucket_cross_account_access.py index 85bb6962ae..0eb74858be 100644 --- a/prowler/providers/aws/services/s3/s3_bucket_cross_account_access/s3_bucket_cross_account_access.py +++ b/prowler/providers/aws/services/s3/s3_bucket_cross_account_access/s3_bucket_cross_account_access.py @@ -20,7 +20,7 @@ def execute(self): report.status_extended = ( f"S3 Bucket {bucket.name} does not have a bucket policy." ) - elif is_policy_public(bucket.policy): + elif is_policy_public(bucket.policy, s3_client.audited_account): report.status = "FAIL" report.status_extended = f"S3 Bucket {bucket.name} has a bucket policy allowing cross account access." diff --git a/prowler/providers/aws/services/s3/s3_bucket_policy_public_write_access/s3_bucket_policy_public_write_access.py b/prowler/providers/aws/services/s3/s3_bucket_policy_public_write_access/s3_bucket_policy_public_write_access.py index 8ad4011ebb..b708317570 100644 --- a/prowler/providers/aws/services/s3/s3_bucket_policy_public_write_access/s3_bucket_policy_public_write_access.py +++ b/prowler/providers/aws/services/s3/s3_bucket_policy_public_write_access/s3_bucket_policy_public_write_access.py @@ -40,6 +40,7 @@ def execute(self): report.status_extended = f"S3 Bucket {bucket.name} does not allow public write access in the bucket policy." if is_policy_public( bucket.policy, + s3_client.audited_account, not_allowed_actions=[ "s3:PutObject", "s3:DeleteObject", diff --git a/prowler/providers/aws/services/s3/s3_bucket_public_access/s3_bucket_public_access.py b/prowler/providers/aws/services/s3/s3_bucket_public_access/s3_bucket_public_access.py index 4278688724..2ac63a622e 100644 --- a/prowler/providers/aws/services/s3/s3_bucket_public_access/s3_bucket_public_access.py +++ b/prowler/providers/aws/services/s3/s3_bucket_public_access/s3_bucket_public_access.py @@ -46,7 +46,7 @@ def execute(self): report.status_extended = f"S3 Bucket {bucket.name} has public access due to bucket ACL." # 4. Check bucket policy - if is_policy_public(bucket.policy): + if is_policy_public(bucket.policy, s3_client.audited_account): report.status = "FAIL" report.status_extended = f"S3 Bucket {bucket.name} has public access due to bucket policy." findings.append(report) diff --git a/tests/providers/aws/services/iam/lib/policy_test.py b/tests/providers/aws/services/iam/lib/policy_test.py index 9528accf77..2b71c8b300 100644 --- a/tests/providers/aws/services/iam/lib/policy_test.py +++ b/tests/providers/aws/services/iam/lib/policy_test.py @@ -1519,7 +1519,9 @@ def test_policy_allows_public_access_with_wildcard_principal(self): ] } assert is_policy_public( - policy_allow_wildcard_principal, not_allowed_actions=["s3:*"] + policy_allow_wildcard_principal, + AWS_ACCOUNT_NUMBER, + not_allowed_actions=["s3:*"], ) def test_policy_allows_public_access_with_aws_wildcard_principal(self): @@ -1534,7 +1536,9 @@ def test_policy_allows_public_access_with_aws_wildcard_principal(self): ] } assert is_policy_public( - policy_allow_aws_wildcard_principal, not_allowed_actions=["s3:*"] + policy_allow_aws_wildcard_principal, + AWS_ACCOUNT_NUMBER, + not_allowed_actions=["s3:*"], ) def test_policy_does_not_allow_public_access_with_specific_aws_principal(self): @@ -1548,7 +1552,9 @@ def test_policy_does_not_allow_public_access_with_specific_aws_principal(self): } ] } - assert not is_policy_public(policy_allow_specific_aws_principal) + assert not is_policy_public( + policy_allow_specific_aws_principal, AWS_ACCOUNT_NUMBER + ) def test_policy_does_not_allow_public_access_with_condition(self): policy_allow_aws_wildcard_principal_with_condition = { @@ -1562,7 +1568,9 @@ def test_policy_does_not_allow_public_access_with_condition(self): } ] } - assert not is_policy_public(policy_allow_aws_wildcard_principal_with_condition) + assert not is_policy_public( + policy_allow_aws_wildcard_principal_with_condition, AWS_ACCOUNT_NUMBER + ) def test_policy_allows_full_service_access_with_wildcard_action_and_resource(self): policy_allow_wildcard_action_and_resource = { @@ -1746,7 +1754,9 @@ def test_is_policy_public_(self): ] } assert is_policy_public( - policy, not_allowed_actions=["elasticfilesystem:ClientMount"] + policy, + AWS_ACCOUNT_NUMBER, + not_allowed_actions=["elasticfilesystem:ClientMount"], ) def test_is_policy_public_with_principal_dict(self): @@ -1761,7 +1771,9 @@ def test_is_policy_public_with_principal_dict(self): ] } assert is_policy_public( - policy, not_allowed_actions=["elasticfilesystem:ClientMount"] + policy, + AWS_ACCOUNT_NUMBER, + not_allowed_actions=["elasticfilesystem:ClientMount"], ) def test_is_policy_public_with_secure_conditions_and_allowed_conditions( @@ -1781,7 +1793,7 @@ def test_is_policy_public_with_secure_conditions_and_allowed_conditions( } ] } - assert not is_policy_public(policy) + assert not is_policy_public(policy, AWS_ACCOUNT_NUMBER) def test_is_policy_public_with_secure_conditions_and_allowed_conditions_nested( self, @@ -1803,7 +1815,7 @@ def test_is_policy_public_with_secure_conditions_and_allowed_conditions_nested( } ] } - assert not is_policy_public(policy) + assert not is_policy_public(policy, AWS_ACCOUNT_NUMBER) def test_is_policy_public_with_secure_conditions_and_allowed_conditions_nested_dict( self, @@ -1827,7 +1839,7 @@ def test_is_policy_public_with_secure_conditions_and_allowed_conditions_nested_d } ] } - assert not is_policy_public(policy) + assert not is_policy_public(policy, AWS_ACCOUNT_NUMBER) def test_is_policy_public_with_secure_conditions_and_allowed_conditions_nested_dict_key( self, @@ -1851,7 +1863,7 @@ def test_is_policy_public_with_secure_conditions_and_allowed_conditions_nested_d } ] } - assert not is_policy_public(policy) + assert not is_policy_public(policy, AWS_ACCOUNT_NUMBER) def test_is_policy_public_with_action_wildcard( self, @@ -1866,7 +1878,7 @@ def test_is_policy_public_with_action_wildcard( } ] } - assert is_policy_public(policy) + assert is_policy_public(policy, AWS_ACCOUNT_NUMBER) def test_is_policy_public_allowing_all_actions( self, @@ -1881,7 +1893,7 @@ def test_is_policy_public_allowing_all_actions( } ] } - assert is_policy_public(policy) + assert is_policy_public(policy, AWS_ACCOUNT_NUMBER) def test_is_policy_public_allowing_other_account(self): policy = { @@ -1894,7 +1906,7 @@ def test_is_policy_public_allowing_other_account(self): } ] } - assert not is_policy_public(policy) + assert not is_policy_public(policy, AWS_ACCOUNT_NUMBER) def test_is_policy_public_secrets_manager( self, @@ -1910,7 +1922,7 @@ def test_is_policy_public_secrets_manager( } ] } - assert not is_policy_public(policy) + assert not is_policy_public(policy, AWS_ACCOUNT_NUMBER) def test_is_policy_public_alexa_condition( self, @@ -1927,7 +1939,7 @@ def test_is_policy_public_alexa_condition( } ] } - assert not is_policy_public(policy) + assert not is_policy_public(policy, AWS_ACCOUNT_NUMBER) def test_is_policy_private_org_s3_bucket( self, @@ -1944,7 +1956,7 @@ def test_is_policy_private_org_s3_bucket( } ] } - assert not is_policy_public(policy) + assert not is_policy_public(policy, AWS_ACCOUNT_NUMBER) def test_is_policy_public_ip( self, @@ -1961,7 +1973,7 @@ def test_is_policy_public_ip( } ], } - assert is_policy_public(policy) + assert is_policy_public(policy, AWS_ACCOUNT_NUMBER) def test_check_admin_access(self): policy = { diff --git a/tests/providers/aws/services/redshift/redshift_cluster_public_access/redshift_cluster_public_access_test.py b/tests/providers/aws/services/redshift/redshift_cluster_public_access/redshift_cluster_public_access_test.py index 1d1476fb58..534d706076 100644 --- a/tests/providers/aws/services/redshift/redshift_cluster_public_access/redshift_cluster_public_access_test.py +++ b/tests/providers/aws/services/redshift/redshift_cluster_public_access/redshift_cluster_public_access_test.py @@ -175,7 +175,7 @@ def test_cluster_is_not_public2(self): assert result[0].resource_id == CLUSTER_ID assert result[0].resource_arn == CLUSTER_ARN - def test_cluster_is_in_public_vpc(self): + def test_cluster_is_in_public_subnet(self): redshift_client = mock.MagicMock redshift_client.clusters = [] redshift_client.clusters.append( @@ -185,11 +185,12 @@ def test_cluster_is_in_public_vpc(self): region=AWS_REGION_EU_WEST_1, public_access=True, vpc_id="vpc-123456", + subnets=["subnet-123456"], endpoint_address="192.192.192.192", ) ) vpc_client = mock.MagicMock - vpc_client.vpcs = {"vpc-123456": mock.MagicMock(public=True)} + vpc_client.subnets = {"subnet-123456": mock.MagicMock(public=True)} ec2_client = mock.MagicMock aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) @@ -216,7 +217,7 @@ def test_cluster_is_in_public_vpc(self): assert result[0].status == "PASS" assert ( result[0].status_extended - == f"Redshift Cluster {CLUSTER_ID} has the endpoint 192.192.192.192 set as publicly accessible in the public VPC vpc-123456 but is not publicly exposed." + == f"Redshift Cluster {CLUSTER_ID} has the endpoint 192.192.192.192 set as publicly accessible in a public subnet but is not publicly exposed." ) assert result[0].resource_id == CLUSTER_ID assert result[0].resource_arn == CLUSTER_ARN @@ -235,10 +236,11 @@ def test_cluster_with_public_vpc_sgs(self): public_access=True, vpc_security_groups=["sg-123456"], endpoint_address="192.192.192.192", + subnets=["subnet-123456"], ) ) vpc_client = mock.MagicMock - vpc_client.vpcs = {"vpc-123456": mock.MagicMock(public=True)} + vpc_client.subnets = {"subnet-123456": mock.MagicMock(public=True)} ec2_client = mock.MagicMock ec2_client.security_groups = { f"arn:aws:ec2:{AWS_REGION_EU_WEST_1}:{AWS_ACCOUNT_NUMBER}:security-group/sg-123456": mock.MagicMock( @@ -279,7 +281,7 @@ def test_cluster_with_public_vpc_sgs(self): assert result[0].status == "FAIL" assert ( result[0].status_extended - == f"Redshift Cluster {CLUSTER_ID} has the endpoint 192.192.192.192 set as publicly accessible and it is exposed to the Internet by security group (sg-123456) in public VPC vpc-123456." + == f"Redshift Cluster {CLUSTER_ID} has the endpoint 192.192.192.192 set as publicly accessible and it is exposed to the Internet by security group (sg-123456) in a public subnet." ) assert result[0].resource_id == CLUSTER_ID assert result[0].resource_arn == CLUSTER_ARN diff --git a/tests/providers/aws/services/redshift/redshift_service_test.py b/tests/providers/aws/services/redshift/redshift_service_test.py index 84dca7033d..4a332860fb 100644 --- a/tests/providers/aws/services/redshift/redshift_service_test.py +++ b/tests/providers/aws/services/redshift/redshift_service_test.py @@ -239,3 +239,39 @@ def test_describe_cluster_parameter_groups(self): ] assert redshift.clusters[0].parameter_group_name == "default.redshift-1.0" assert redshift.clusters[0].require_ssl is True + + @mock_aws + def test_describe_cluster_subnets(self): + ec2_client = client("ec2", region_name=AWS_REGION_EU_WEST_1) + vpc_id = ec2_client.create_vpc(CidrBlock="10.0.0.0/16")["Vpc"]["VpcId"] + + subnet_id = ec2_client.create_subnet( + VpcId=vpc_id, + CidrBlock="10.0.1.0/24", + AvailabilityZone=f"{AWS_REGION_EU_WEST_1}a", + )["Subnet"]["SubnetId"] + redshift_client = client("redshift", region_name=AWS_REGION_EU_WEST_1) + redshift_client.create_cluster_subnet_group( + ClusterSubnetGroupName="test-subnet", + Description="Test Subnet", + SubnetIds=[subnet_id], + ) + _ = redshift_client.create_cluster( + DBName="test", + ClusterIdentifier=cluster_id, + ClusterType="single-node", + NodeType="ds2.xlarge", + MasterUsername="user", + MasterUserPassword="password", + PubliclyAccessible=True, + VpcSecurityGroupIds=["sg-123456"], + ClusterSubnetGroupName="test-subnet", + ) + aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) + redshift = Redshift(aws_provider) + + assert len(redshift.clusters) == 1 + assert redshift.clusters[0].id == cluster_id + assert redshift.clusters[0].region == AWS_REGION_EU_WEST_1 + assert redshift.clusters[0].subnet_group == "test-subnet" + assert redshift.clusters[0].subnets[0] == subnet_id