From f012b525cba0ecae37679d4eb81aa626d40df028 Mon Sep 17 00:00:00 2001 From: kagahd Date: Wed, 19 Feb 2025 16:27:02 +0100 Subject: [PATCH 01/13] feat(aws) add check secretsmanager_has_restrictive_resource_policy --- prowler/config/config.yaml | 1 + .../__init__.py | 0 ..._restrictive_resource_policy.metadata.json | 34 +++ ...manager_has_restrictive_resource_policy.py | 217 ++++++++++++++++++ 4 files changed, 252 insertions(+) create mode 100644 prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/__init__.py create mode 100644 prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.metadata.json create mode 100644 prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py diff --git a/prowler/config/config.yaml b/prowler/config/config.yaml index c4bcd40b8e..7d23c45d45 100644 --- a/prowler/config/config.yaml +++ b/prowler/config/config.yaml @@ -127,6 +127,7 @@ aws: # ] organizations_enabled_regions: [] organizations_trusted_delegated_administrators: [] + organizations_trusted_ids: [] # AWS ECR # aws.ecr_repositories_scan_vulnerabilities_in_latest_image diff --git a/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/__init__.py b/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.metadata.json b/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.metadata.json new file mode 100644 index 0000000000..ff7f714244 --- /dev/null +++ b/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.metadata.json @@ -0,0 +1,34 @@ +{ + "Provider": "aws", + "CheckID": "secretsmanager_has_restrictive_resource_policy", + "CheckTitle": "Ensure Secrets Manager secrets have restrictive resource-based policies.", + "CheckType": [ + "Software and Configuration Checks/AWS Security Best Practices" + ], + "ServiceName": "secretsmanager", + "SubServiceName": "", + "ResourceIdTemplate": "arn:aws:secretsmanager:region:account-id:secret:secret-name", + "Severity": "high", + "ResourceType": "AwsSecretsManagerSecret", + "Description": "This check verifies whether Secrets Manager secrets have resource-based policies that restrict access.", + "Risk": "Secrets without restrictive resource-based policies may be accessed by unauthorized entities, leading to potential data breaches.", + "RelatedUrl": "https://docs.aws.amazon.com/secretsmanager/latest/userguide/auth-and-access_resource-policies.html", + "Remediation": { + "Code": { + "CLI": "", + "NativeIaC": "", + "Other": "", + "Terraform": "" + }, + "Recommendation": { + "Text": "Ensure that Secrets Manager policies restrict access to authorized principals only, following the Principle of Least Privilege.", + "Url": "https://docs.aws.amazon.com/secretsmanager/latest/userguide/determine-acccess_examine-iam-policies.html" + } + }, + "Categories": [ + "access-control" + ], + "DependsOn": [], + "RelatedTo": [], + "Notes": "" +} diff --git a/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py b/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py new file mode 100644 index 0000000000..bcf371a32a --- /dev/null +++ b/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py @@ -0,0 +1,217 @@ +from prowler.lib.check.models import Check, Check_Report_AWS +from prowler.providers.aws.services.secretsmanager.secretsmanager_client import ( + secretsmanager_client, +) +import re + + +class secretsmanager_has_restrictive_resource_policy(Check): + def execute(self): + findings = [] + organizations_trusted_ids = secretsmanager_client.audit_config.get( + "organizations_trusted_ids", [] + ) + # Regular expression to match IAM roles or users without wildcard * in their name + arn_pattern = r"arn:aws:iam::\d{12}:(role|user)/([^*]+)$" + # Regular expression to match AWS service names + service_pattern = r"^[a-z0-9-]+\.amazonaws\.com$" + + for secret in secretsmanager_client.secrets.values(): + report = Check_Report_AWS(self.metadata(), resource=secret) + report.region = secret.region + report.resource_id = secret.name + report.resource_arn = secret.arn + report.resource_tags = secret.tags + report.status = "FAIL" + report.status_extended = ( + f"SecretsManager secret '{secret.name}' does not have a resource-based policy " + f"or access to the policy is denied for the role " + f"'{getattr(getattr(secretsmanager_client.provider, '_assumed_role_configuration', None), 'info', None) and getattr(secretsmanager_client.provider._assumed_role_configuration.info, 'role_arn', None) and getattr(secretsmanager_client.provider._assumed_role_configuration.info.role_arn, 'arn', 'N/A')}'" + ) + + if secret.policy: + statements = secret.policy.get("Statement", []) + not_denied_principals = [] + not_denied_services = [] + + # Check for an explicit Deny that applies to all Principals except those defined in the Condition + has_explicit_deny_for_all = any( + "*" in self.extract_field(statement.get("Principal", {})) + and any( + action in self.extract_field(statement.get("Action", [])) + for action in ["*", "secretsmanager:*"] + ) + and self.is_valid_resource( + secret, self.extract_field(statement.get("Resource", "*")) + ) + and "Condition" in statement + # accept only the allowed Condition operators + and all( + operator in ["StringNotEquals", "StringNotEqualsIfExists"] + for operator in statement["Condition"].keys() + ) + # no PrincipalArn nor Service of the Condition must have wildcard * in its name + and all( + self.is_valid_principal( + principal_value=principal_value, + not_denied_list=not_denied_list, + pattern=pattern, + ) + for operator, condition_values in statement["Condition"].items() + for principal_key, principal_value in condition_values.items() + # only these two keys are allowed: + for mapping in [ + { + "aws:PrincipalArn": ( + not_denied_principals, + arn_pattern, + ), + "aws:PrincipalServiceName": ( + not_denied_services, + service_pattern, + ), + }.get(principal_key) + ] + # the principal_key decides which list and pattern is passed to is_valid_principal + for not_denied_list, pattern in [ + mapping if mapping is not None else (None, None) + ] + # direct tuple unpacking of the mapping + ) + for statement in statements + if statement.get("Effect") == "Deny" + ) + + # Check for Deny with "StringNotEquals":"aws:PrincipalOrgID" condition + has_deny_outside_org = ( + True + if not organizations_trusted_ids + else any( + "*" in self.extract_field(statement.get("Principal", {})) + and any( + action in self.extract_field(statement.get("Action", [])) + for action in ["*", "secretsmanager:*"] + ) + and self.is_valid_resource( + secret, self.extract_field(statement.get("Resource", "*")) + ) + and "Condition" in statement + and set(statement["Condition"].keys()) == {"StringNotEquals"} + and set(statement["Condition"]["StringNotEquals"].keys()) + == {"aws:PrincipalOrgID"} + and statement["Condition"]["StringNotEquals"][ + "aws:PrincipalOrgID" + ] + in organizations_trusted_ids + for statement in statements + if statement.get("Effect") == "Deny" + ) + ) + + # Check for "NotActions" without wildcard * for not_denied_principals and not_denied_services + failed_principals = [] + failed_services = [] + + # Validate that NotAction does not contain wildcards for specified principals + for statement in statements: + if statement.get("Effect") == "Deny": + principals = self.extract_field(statement.get("Principal", {})) + + # Check "NotAction" of Deny statements only for not_denied_principals + for principal in principals: + if principal in not_denied_principals: + if "NotAction" not in statement or any( + "*" in action + for action in self.extract_field( + statement.get("NotAction", []) + ) + ): + failed_principals.append(principal) + + # Allow-Statement for not-denied services must not have any wildcards in "Action" + # and "SourceAccount" must be the audited account + for statement in statements: + if statement.get("Effect") == "Allow": + principals = self.extract_field(statement.get("Principal", {})) + for service in principals: + if service in not_denied_services: + condition = statement.get("Condition", {}) + actions = self.extract_field( + statement.get("Action", []) + ) + if ( + "StringEquals" not in condition + or condition.get("StringEquals", {}).get( + "aws:SourceAccount" + ) + != secretsmanager_client.audited_account + or len(condition) + > 1 # only "StringEquals" is allowed + or any("*" in action for action in actions) + ): # wildcard in "Action" is not allowed + failed_services.append(service) + + has_specific_not_actions = len(failed_principals) == 0 + has_valid_service_policies = len(failed_services) == 0 + + # Determine if the policy satisfies all conditions + if ( + has_explicit_deny_for_all + and has_deny_outside_org + and has_specific_not_actions + and has_valid_service_policies + ): + report.status = "PASS" + report.status_extended = f"SecretsManager secret '{secret.name}' has a sufficiently restrictive resource-based policy." + else: + report.status = "FAIL" + report.status_extended = f"SecretsManager secret '{secret.name}' does not meet all required restrictions: " + if not has_explicit_deny_for_all: + report.status_extended += "Missing or incorrect 'Deny' statement for all Principals with wildcard Action. " + if not has_deny_outside_org: + report.status_extended += "Missing or incorrect 'Deny' statement restricting access outside 'PrincipalOrgID'. " + if not has_specific_not_actions: + report.status_extended += f"Missing field 'NotAction' or disallowed wildcard * in the 'NotAction' field of the 'Deny' statement for the specific Principal(s) {failed_principals if failed_principals else ''}. " + if not has_valid_service_policies: + report.status_extended += f"Invalid 'Allow' statements for Service Principals {failed_services if failed_services else ''}. " + + findings.append(report) + + return findings + + # Extract values from a field to return an array containing the field, + # handling single values, arrays and dict with keys "AWS" or "Service". + # If the field is empty or invalid, return the default_value in the array. + def extract_field(self, field, default_value=None): + if isinstance(field, str): + return [field] + elif isinstance(field, list): + return field + elif isinstance(field, dict): + for key in ("AWS", "Service"): + if key in field: + return [field[key]] if isinstance(field[key], str) else field[key] + return [default_value] + + def is_valid_resource(self, secret, resource): + """Check if the Resource field is valid for the given secret.""" + if resource == "*": + return True # Wildcard resource is acceptable in general cases + if isinstance(resource, list): + if "*" in resource: + return True + return all(r == secret.arn for r in resource) + return resource == secret.arn + + def is_valid_principal(self, principal_value, not_denied_list, pattern): + if not_denied_list is None or pattern is None: + return False + + principals = self.extract_field(principal_value) + for principal in principals: + if re.match(pattern, principal): + not_denied_list.append(principal) + else: + return False + + return True From 4a6a8e5e5286ca88594882fbf71385f538042e38 Mon Sep 17 00:00:00 2001 From: kagahd Date: Wed, 19 Feb 2025 16:27:51 +0100 Subject: [PATCH 02/13] feat(aws) add check secretsmanager_has_restrictive_resource_policy --- ...er_has_restrictive_resource_policy_test.py | 829 ++++++++++++++++++ 1 file changed, 829 insertions(+) create mode 100644 tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py diff --git a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py new file mode 100644 index 0000000000..f3c9ba4263 --- /dev/null +++ b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py @@ -0,0 +1,829 @@ +import json +from unittest import mock +from boto3 import client +from moto import mock_aws + +from prowler.providers.aws.services.secretsmanager.secretsmanager_service import ( + SecretsManager, +) +from tests.providers.aws.utils import AWS_REGION_EU_WEST_1, set_mocked_aws_provider + + +class Test_secretsmanager_has_restrictive_resource_policy: + def test_no_secrets(self): + client("secretsmanager", region_name=AWS_REGION_EU_WEST_1) + aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", + new=SecretsManager(aws_provider), + ): + from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( + secretsmanager_has_restrictive_resource_policy, + ) + + check = secretsmanager_has_restrictive_resource_policy() + result = check.execute() + + assert len(result) == 0 + + @mock_aws + def test_secret_with_weak_policy(self): + secretsmanager_client = client( + "secretsmanager", region_name=AWS_REGION_EU_WEST_1 + ) + secret = secretsmanager_client.create_secret(Name="test-secret-weak-policy") + secretsmanager_client.put_resource_policy( + SecretId=secret["ARN"], + ResourcePolicy=json.dumps( + { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": "*", + "Action": "secretsmanager:GetSecretValue", + "Resource": "*", + } + ], + }, + indent=4, + ), + ) + aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", + new=SecretsManager(aws_provider), + ): + from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( + secretsmanager_has_restrictive_resource_policy, + ) + + check = secretsmanager_has_restrictive_resource_policy() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "FAIL" + + @mock_aws + def test_secret_with_restrictive_policy_without_org_id(self): + secretsmanager_client = client( + "secretsmanager", region_name=AWS_REGION_EU_WEST_1 + ) + secret = secretsmanager_client.create_secret(Name="test-secret-modified") + + valid_policy = { + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "DenyUnauthorizedPrincipals", + "Effect": "Deny", + "Principal": "*", + "Action": "*", + "Resource": "*", + "Condition": { + "StringNotEquals": { + "aws:PrincipalArn": [ + "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" + ] + } + }, + }, + { + "Sid": "AllowAuditPolicyRead", + "Effect": "Deny", + "Principal": { + "AWS": "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" + }, + "NotAction": [ + "secretsmanager:DescribeSecret", + "secretsmanager:GetResourcePolicy", + ], + "Resource": "*", + }, + ], + } + + test_cases = [ + ( + "Invalid Policy without DenyOutsideOrganization because OrgID is given", + valid_policy, + "FAIL", + ["o-1234567890"], + ), + ( + "Valid Policy without DenyOutsideOrganization because no OrgID is given", + valid_policy, + "PASS", + [], + ), + ] + + for description, base_policy, expected_status, trusted_org_ids in test_cases: + policy_copy = json.loads(json.dumps(base_policy)) + + secretsmanager_client.put_resource_policy( + SecretId=secret["ARN"], ResourcePolicy=json.dumps(policy_copy) + ) + + aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", + new=SecretsManager(aws_provider), + ), mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client.audit_config", + {"organizations_trusted_ids": trusted_org_ids}, + ): + from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( + secretsmanager_has_restrictive_resource_policy, + ) + + check = secretsmanager_has_restrictive_resource_policy() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == expected_status, f"Test case: {description}" + + @mock_aws + def test_secret_with_modified_restrictive_policies_for_principals(self): + secretsmanager_client = client( + "secretsmanager", region_name=AWS_REGION_EU_WEST_1 + ) + secret = secretsmanager_client.create_secret(Name="test-secret-modified") + + valid_policy = { + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "DenyUnauthorizedPrincipals", + "Effect": "Deny", + "Principal": "*", + "Action": "*", + "Resource": "*", + "Condition": { + "StringNotEquals": { + "aws:PrincipalArn": [ + "arn:aws:iam::123456789012:role/AccountSecurityAuditRole", + "arn:aws:iam::123456789012:role/Role2", + ] + } + }, + }, + { + "Sid": "DenyOutsideOrganization", + "Effect": "Deny", + "Principal": "*", + "Action": "secretsmanager:*", + "Resource": "*", + "Condition": { + "StringNotEquals": {"aws:PrincipalOrgID": "o-1234567890"} + }, + }, + { + "Sid": "AllowAuditPolicyRead", + "Effect": "Deny", + "Principal": { + "AWS": "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" + }, + "NotAction": [ + "secretsmanager:DescribeSecret", + "secretsmanager:GetResourcePolicy", + ], + "Resource": "*", + }, + { + "Sid": "AllowSecretAccessForRole2", + "Effect": "Deny", + "Principal": {"AWS": "arn:aws:iam::123456789012:role/Role2"}, + "NotAction": ["secretsmanager:GetSecretValue"], + "Resource": "*", + }, + ], + } + + test_cases = [ + # test unmodified policy + ("Valid Policy", valid_policy, None, None, "PASS"), + # test modified statement DenyUnauthorizedPrincipals + ( + "Invalid Effect in DenyUnauthorizedPrincipals", + valid_policy, + None, + (0, {"Effect": "Allow"}), + "FAIL", + ), + ( + "Valid Effect in DenyUnauthorizedPrincipals", + valid_policy, + None, + (0, {"Effect": "Deny"}), + "PASS", + ), + ( + "Invalid Action in DenyUnauthorizedPrincipals", + valid_policy, + None, + (0, {"Action": "InvalidAction"}), + "FAIL", + ), + ( + "Valid Action in DenyUnauthorizedPrincipals", + valid_policy, + None, + (0, {"Action": "*"}), + "PASS", + ), + ( + "Invalid Resource in DenyUnauthorizedPrincipals", + valid_policy, + None, + ( + 0, + { + "Resource": "arn:aws:secretsmanager:eu-central-1:123456789012:secret:wrong-secret" + }, + ), + "FAIL", + ), + ( + "Valid Resource in DenyUnauthorizedPrincipals", + valid_policy, + None, + (0, {"Resource": "*"}), + "PASS", + ), + ( + "Invalid Condition Operator in DenyUnauthorizedPrincipals", + valid_policy, + None, + ( + 0, + { + "Condition": { + "WrongOperator": { + "aws:PrincipalArn": "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" + } + } + }, + ), + "FAIL", + ), + ( + "Valid Condition Operator in DenyUnauthorizedPrincipals", + valid_policy, + None, + ( + 0, + { + "Condition": { + "StringNotEquals": { + "aws:PrincipalArn": "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" + } + } + }, + ), + "PASS", + ), + ( + "Invalid Condition Key in DenyUnauthorizedPrincipals", + valid_policy, + None, + ( + 0, + { + "Condition": { + "StringNotEquals": { + "aws:wrongKey": "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" + } + } + }, + ), + "FAIL", + ), + ( + "Valid Condition Key in DenyUnauthorizedPrincipals", + valid_policy, + None, + ( + 0, + { + "Condition": { + "StringNotEquals": { + "aws:PrincipalArn": "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" + } + } + }, + ), + "PASS", + ), + ( + "Invalid Principal with wildcard in Condition in DenyUnauthorizedPrincipals", + valid_policy, + None, + ( + 0, + { + "Condition": { + "StringNotEquals": { + "aws:PrincipalArn": "arn:aws:iam::123456789012:role/*" + } + } + }, + ), + "FAIL", + ), + ( + "Valid Principal w/o wildcard in Condition in DenyUnauthorizedPrincipals", + valid_policy, + None, + ( + 0, + { + "Condition": { + "StringNotEquals": { + "aws:PrincipalArn": "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" + } + } + }, + ), + "PASS", + ), + ( + "Invalid Service Principal in Condition in DenyUnauthorizedPrincipals", + valid_policy, + None, + ( + 0, + { + "Condition": { + "StringNotEquals": { + "aws:PrincipalServiceName": "invalid.service.com" + } + } + }, + ), + "FAIL", + ), + ( + "Valid Service Principal in Condition in DenyUnauthorizedPrincipals", + valid_policy, + None, + ( + 0, + { + "Condition": { + "StringNotEquals": { + "aws:PrincipalServiceName": "valid.amazonaws.com" + } + } + }, + ), + "PASS", + ), + # test modified statement DenyOutsideOrganization + ( + "Invalid Effect in DenyOutsideOrganization", + valid_policy, + None, + (1, {"Effect": "Allow"}), + "FAIL", + ), + ( + "Valid Effect in DenyOutsideOrganization", + valid_policy, + None, + (1, {"Effect": "Deny"}), + "PASS", + ), + ( + "Invalid Action in DenyOutsideOrganization", + valid_policy, + None, + (1, {"Action": "secretsmanager:InvalidAction"}), + "FAIL", + ), + ( + "Valid Action in DenyOutsideOrganization", + valid_policy, + None, + (1, {"Action": "secretsmanager:*"}), + "PASS", + ), + ( + "Invalid Resource in DenyOutsideOrganization", + valid_policy, + None, + ( + 1, + { + "Resource": "arn:aws:secretsmanager:eu-central-1:123456789012:secret:wrong-secret" + }, + ), + "FAIL", + ), + ( + "Invalid Condition Operator in DenyOutsideOrganization", + valid_policy, + None, + ( + 1, + { + "Condition": { + "WrongOperator": {"aws:PrincipalOrgID": "o-1234567890"} + } + }, + ), + "FAIL", + ), + ( + "Valid Condition Operator in DenyOutsideOrganization", + valid_policy, + None, + ( + 1, + { + "Condition": { + "StringNotEquals": {"aws:PrincipalOrgID": "o-1234567890"} + } + }, + ), + "PASS", + ), + ( + "Invalid Condition Key in DenyOutsideOrganization", + valid_policy, + None, + ( + 1, + { + "Condition": { + "StringNotEquals": {"aws:wrongKey": "o-1234567890"} + } + }, + ), + "FAIL", + ), + ( + "Valid Condition Key in DenyOutsideOrganization", + valid_policy, + None, + ( + 1, + { + "Condition": { + "StringNotEquals": {"aws:PrincipalOrgID": "o-1234567890"} + } + }, + ), + "PASS", + ), + ( + "Invalid PrincipalOrgID in DenyOutsideOrganization", + valid_policy, + None, + ( + 1, + { + "Condition": { + "StringNotEquals": {"aws:PrincipalOrgID": "o-invalid"} + } + }, + ), + "FAIL", + ), + ( + "Valid PrincipalOrgID in DenyOutsideOrganization", + valid_policy, + None, + ( + 1, + { + "Condition": { + "StringNotEquals": {"aws:PrincipalOrgID": "o-1234567890"} + } + }, + ), + "PASS", + ), + # test modified statement AllowAuditPolicyRead + ( + "Invalid wildcard in NotAction in AllowAuditPolicyRead", + valid_policy, + None, + (2, {"NotAction": "*"}), + "FAIL", + ), + ( + "No wildcard in NotAction in AllowAuditPolicyRead", + valid_policy, + None, + (2, {"NotAction": "secretsmanager:DescribeSecret"}), + "PASS", + ), + ( + "Invalid wildcard in NotAction in AllowSecretAccessForRole2", + valid_policy, + None, + (3, {"NotAction": "*"}), + "FAIL", + ), + ( + "No wildcard in NotAction in AllowSecretAccessForRole2", + valid_policy, + None, + (3, {"NotAction": "secretsmanager:DescribeSecret"}), + "PASS", + ), + ( + "Invalid wildcard in NotAction in both statements", + valid_policy, + None, + [(2, {"NotAction": "*"}), (3, {"NotAction": "secretsmanager:*"})], + "FAIL", + ), + ( + "No wildcard in NotAction in both statements", + valid_policy, + None, + [ + (2, {"NotAction": "secretsmanager:DescribeSecret"}), + (3, {"NotAction": "secretsmanager:GetSecretValue"}), + ], + "PASS", + ), + # test policy with removed statements + ("Missing DenyUnauthorizedPrincipals", valid_policy, 0, None, "FAIL"), + ("Missing DenyOutsideOrganization", valid_policy, 1, None, "FAIL"), + # the following 2 test cases PASS because these statements are not required to make the Policy secure + # but in practice the AWS Principal will not be able to access the secret + ("Missing AllowAuditPolicyRead", valid_policy, 2, None, "PASS"), + ("Missing AllowSecretAccessForRole2", valid_policy, 3, None, "PASS"), + ] + + for ( + description, + base_policy, + remove_index, + modify_element, + expected_status, + ) in test_cases: + policy_copy = json.loads(json.dumps(base_policy)) + if remove_index is not None: + del policy_copy["Statement"][remove_index] + + if modify_element is not None: + if isinstance(modify_element, list): + for index, value in modify_element: + policy_copy["Statement"][index].update(value) + else: + index, value = modify_element + policy_copy["Statement"][index].update(value) + + secretsmanager_client.put_resource_policy( + SecretId=secret["ARN"], ResourcePolicy=json.dumps(policy_copy) + ) + + aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", + new=SecretsManager(aws_provider), + ), mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client.audit_config", + {"organizations_trusted_ids": "o-1234567890"}, + ): + from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( + secretsmanager_has_restrictive_resource_policy, + ) + + check = secretsmanager_has_restrictive_resource_policy() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == expected_status, f"Test case: {description}" + + @mock_aws + def test_secret_with_modified_restrictive_policies_for_services(self): + secretsmanager_client = client( + "secretsmanager", region_name=AWS_REGION_EU_WEST_1 + ) + secret = secretsmanager_client.create_secret(Name="test-secret-modified") + + valid_policy = { + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "DenyUnauthorizedPrincipals", + "Effect": "Deny", + "Principal": "*", + "Action": "*", + "Resource": "*", + "Condition": { + "StringNotEqualsIfExists": { + "aws:PrincipalArn": [ + "arn:aws:iam::123456789012:role/AccountSecurityAuditRole", + "arn:aws:iam::123456789012:role/Role2", + ], + "aws:PrincipalServiceName": "appflow.amazonaws.com", + } + }, + }, + { + "Sid": "DenyOutsideOrganization", + "Effect": "Deny", + "Principal": "*", + "Action": "secretsmanager:*", + "Resource": "*", + "Condition": { + "StringNotEquals": {"aws:PrincipalOrgID": "o-1234567890"} + }, + }, + { + "Sid": "AllowAuditPolicyRead", + "Effect": "Deny", + "Principal": { + "AWS": "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" + }, + "NotAction": [ + "secretsmanager:DescribeSecret", + "secretsmanager:GetResourcePolicy", + ], + "Resource": "*", + }, + { + "Sid": "AllowSecretAccessForRole2", + "Effect": "Deny", + "Principal": {"AWS": "arn:aws:iam::123456789012:role/Role2"}, + "NotAction": ["secretsmanager:GetSecretValue"], + "Resource": "*", + }, + { + "Sid": "AllowAppFlowAccess", + "Effect": "Allow", + "Principal": {"Service": "appflow.amazonaws.com"}, + "Action": [ + "secretsmanager:GetSecretValue", + "secretsmanager:PutSecretValue", + "secretsmanager:DeleteSecret", + "secretsmanager:DescribeSecret", + "secretsmanager:UpdateSecret", + ], + "Resource": "*", + "Condition": { + "StringEquals": {"aws:SourceAccount": "123456789012"} + }, + }, + ], + } + + test_cases = [ + ( + "Valid unmodified Policy with PrincipalArn and Service", + valid_policy, + None, + "PASS", + ), + ( + "Invalid wildcard '*' in Action in AllowAppFlowAccess", + valid_policy, + (4, {"Action": "*"}), + "FAIL", + ), + ( + "No wildcard '*' in Action in AllowAppFlowAccess", + valid_policy, + (4, {"Action": "secretsmanager:GetSecretValue"}), + "PASS", + ), + ( + "Invalid wildcard 'secretsmanager:*' in Action in AllowAppFlowAccess", + valid_policy, + (4, {"Action": "secretsmanager:*"}), + "FAIL", + ), + ( + "No wildcard 'secretsmanager:*' in Action in AllowAppFlowAccess", + valid_policy, + (4, {"Action": "secretsmanager:ValidAction"}), + "PASS", + ), + ( + "Missing Condition in AllowAppFlowAccess", + valid_policy, + (4, {"Condition": {}}), + "FAIL", + ), + ( + "Valid Condition in AllowAppFlowAccess", + valid_policy, + ( + 4, + { + "Condition": { + "StringEquals": {"aws:SourceAccount": "123456789012"} + } + }, + ), + "PASS", + ), + ( + "Invalid Condition Operator in AllowAppFlowAccess", + valid_policy, + ( + 4, + { + "Condition": { + "WrongOperator": {"aws:SourceAccount": "123456789012"} + } + }, + ), + "FAIL", + ), + ( + "Valid Condition Operator in AllowAppFlowAccess", + valid_policy, + ( + 4, + { + "Condition": { + "StringEquals": {"aws:SourceAccount": "123456789012"} + } + }, + ), + "PASS", + ), + ( + "Invalid Condition Key in AllowAppFlowAccess", + valid_policy, + (4, {"Condition": {"StringEquals": {"aws:WrongKey": "123456789012"}}}), + "FAIL", + ), + ( + "Valid Condition Key in AllowAppFlowAccess", + valid_policy, + ( + 4, + { + "Condition": { + "StringEquals": {"aws:SourceAccount": "123456789012"} + } + }, + ), + "PASS", + ), + ] + + for ( + description, + base_policy, + modify_element, + expected_status, + ) in test_cases: + policy_copy = json.loads(json.dumps(base_policy)) + if modify_element is not None: + if isinstance(modify_element, list): + for index, value in modify_element: + policy_copy["Statement"][index].update(value) + else: + index, value = modify_element + policy_copy["Statement"][index].update(value) + + secretsmanager_client.put_resource_policy( + SecretId=secret["ARN"], ResourcePolicy=json.dumps(policy_copy) + ) + + aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", + new=SecretsManager(aws_provider), + ), mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client.audit_config", + {"organizations_trusted_ids": "o-1234567890"}, + ): + from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( + secretsmanager_has_restrictive_resource_policy, + ) + + check = secretsmanager_has_restrictive_resource_policy() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == expected_status, f"Test case: {description}" From aaf541d072b56f5c81101f2b6405e0ee147c3e17 Mon Sep 17 00:00:00 2001 From: kagahd Date: Wed, 19 Feb 2025 19:52:47 +0100 Subject: [PATCH 03/13] feat(aws) use pytest.mark.parametrize --- ...er_has_restrictive_resource_policy_test.py | 501 +++++++----------- 1 file changed, 191 insertions(+), 310 deletions(-) diff --git a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py index f3c9ba4263..833f30b9bf 100644 --- a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py +++ b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py @@ -1,4 +1,5 @@ import json +import pytest from unittest import mock from boto3 import client from moto import mock_aws @@ -9,35 +10,46 @@ from tests.providers.aws.utils import AWS_REGION_EU_WEST_1, set_mocked_aws_provider -class Test_secretsmanager_has_restrictive_resource_policy: +@pytest.fixture(scope="class") +def secretsmanager_client(): + with mock_aws(): + client_instance = client("secretsmanager", region_name=AWS_REGION_EU_WEST_1) + secret = client_instance.create_secret(Name="test-secret") + yield client_instance, secret["ARN"] + + +class TestSecretsManagerHasRestrictiveResourcePolicy: + def test_no_secrets(self): - client("secretsmanager", region_name=AWS_REGION_EU_WEST_1) - aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) + with mock_aws(): + aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) - with mock.patch( - "prowler.providers.common.provider.Provider.get_global_provider", - return_value=aws_provider, - ), mock.patch( - "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", - new=SecretsManager(aws_provider), - ): - from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( - secretsmanager_has_restrictive_resource_policy, + from prowler.providers.aws.services.secretsmanager.secretsmanager_client import ( + secretsmanager_client, ) - check = secretsmanager_has_restrictive_resource_policy() - result = check.execute() + secretsmanager_client.secrets.clear() + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_client", + new=SecretsManager(aws_provider), + ): + from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( + secretsmanager_has_restrictive_resource_policy, + ) + + check = secretsmanager_has_restrictive_resource_policy() + result = check.execute() - assert len(result) == 0 + assert len(result) == 0 - @mock_aws - def test_secret_with_weak_policy(self): - secretsmanager_client = client( - "secretsmanager", region_name=AWS_REGION_EU_WEST_1 - ) - secret = secretsmanager_client.create_secret(Name="test-secret-weak-policy") - secretsmanager_client.put_resource_policy( - SecretId=secret["ARN"], + def test_secret_with_weak_policy(self, secretsmanager_client): + client_instance, secret_arn = secretsmanager_client + client_instance.put_resource_policy( + SecretId=secret_arn, ResourcePolicy=json.dumps( { "Version": "2012-10-17", @@ -72,181 +84,38 @@ def test_secret_with_weak_policy(self): assert len(result) == 1 assert result[0].status == "FAIL" - @mock_aws - def test_secret_with_restrictive_policy_without_org_id(self): - secretsmanager_client = client( - "secretsmanager", region_name=AWS_REGION_EU_WEST_1 - ) - secret = secretsmanager_client.create_secret(Name="test-secret-modified") - - valid_policy = { - "Version": "2012-10-17", - "Statement": [ - { - "Sid": "DenyUnauthorizedPrincipals", - "Effect": "Deny", - "Principal": "*", - "Action": "*", - "Resource": "*", - "Condition": { - "StringNotEquals": { - "aws:PrincipalArn": [ - "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" - ] - } - }, - }, - { - "Sid": "AllowAuditPolicyRead", - "Effect": "Deny", - "Principal": { - "AWS": "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" - }, - "NotAction": [ - "secretsmanager:DescribeSecret", - "secretsmanager:GetResourcePolicy", - ], - "Resource": "*", - }, - ], - } - - test_cases = [ - ( - "Invalid Policy without DenyOutsideOrganization because OrgID is given", - valid_policy, - "FAIL", - ["o-1234567890"], - ), - ( - "Valid Policy without DenyOutsideOrganization because no OrgID is given", - valid_policy, - "PASS", - [], - ), - ] - - for description, base_policy, expected_status, trusted_org_ids in test_cases: - policy_copy = json.loads(json.dumps(base_policy)) - - secretsmanager_client.put_resource_policy( - SecretId=secret["ARN"], ResourcePolicy=json.dumps(policy_copy) - ) - - aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) - - with mock.patch( - "prowler.providers.common.provider.Provider.get_global_provider", - return_value=aws_provider, - ), mock.patch( - "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", - new=SecretsManager(aws_provider), - ), mock.patch( - "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client.audit_config", - {"organizations_trusted_ids": trusted_org_ids}, - ): - from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( - secretsmanager_has_restrictive_resource_policy, - ) - - check = secretsmanager_has_restrictive_resource_policy() - result = check.execute() - - assert len(result) == 1 - assert result[0].status == expected_status, f"Test case: {description}" - - @mock_aws - def test_secret_with_modified_restrictive_policies_for_principals(self): - secretsmanager_client = client( - "secretsmanager", region_name=AWS_REGION_EU_WEST_1 - ) - secret = secretsmanager_client.create_secret(Name="test-secret-modified") - - valid_policy = { - "Version": "2012-10-17", - "Statement": [ - { - "Sid": "DenyUnauthorizedPrincipals", - "Effect": "Deny", - "Principal": "*", - "Action": "*", - "Resource": "*", - "Condition": { - "StringNotEquals": { - "aws:PrincipalArn": [ - "arn:aws:iam::123456789012:role/AccountSecurityAuditRole", - "arn:aws:iam::123456789012:role/Role2", - ] - } - }, - }, - { - "Sid": "DenyOutsideOrganization", - "Effect": "Deny", - "Principal": "*", - "Action": "secretsmanager:*", - "Resource": "*", - "Condition": { - "StringNotEquals": {"aws:PrincipalOrgID": "o-1234567890"} - }, - }, - { - "Sid": "AllowAuditPolicyRead", - "Effect": "Deny", - "Principal": { - "AWS": "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" - }, - "NotAction": [ - "secretsmanager:DescribeSecret", - "secretsmanager:GetResourcePolicy", - ], - "Resource": "*", - }, - { - "Sid": "AllowSecretAccessForRole2", - "Effect": "Deny", - "Principal": {"AWS": "arn:aws:iam::123456789012:role/Role2"}, - "NotAction": ["secretsmanager:GetSecretValue"], - "Resource": "*", - }, - ], - } - - test_cases = [ + @pytest.mark.parametrize( + "description, remove_index, modify_element, expected_status", + [ # test unmodified policy - ("Valid Policy", valid_policy, None, None, "PASS"), + ("Valid Policy", None, None, "PASS"), # test modified statement DenyUnauthorizedPrincipals ( "Invalid Effect in DenyUnauthorizedPrincipals", - valid_policy, None, (0, {"Effect": "Allow"}), "FAIL", ), ( "Valid Effect in DenyUnauthorizedPrincipals", - valid_policy, None, (0, {"Effect": "Deny"}), "PASS", ), ( "Invalid Action in DenyUnauthorizedPrincipals", - valid_policy, None, (0, {"Action": "InvalidAction"}), "FAIL", ), ( "Valid Action in DenyUnauthorizedPrincipals", - valid_policy, None, (0, {"Action": "*"}), "PASS", ), ( "Invalid Resource in DenyUnauthorizedPrincipals", - valid_policy, None, ( 0, @@ -258,14 +127,12 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): ), ( "Valid Resource in DenyUnauthorizedPrincipals", - valid_policy, None, (0, {"Resource": "*"}), "PASS", ), ( "Invalid Condition Operator in DenyUnauthorizedPrincipals", - valid_policy, None, ( 0, @@ -281,7 +148,6 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): ), ( "Valid Condition Operator in DenyUnauthorizedPrincipals", - valid_policy, None, ( 0, @@ -297,7 +163,6 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): ), ( "Invalid Condition Key in DenyUnauthorizedPrincipals", - valid_policy, None, ( 0, @@ -313,7 +178,6 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): ), ( "Valid Condition Key in DenyUnauthorizedPrincipals", - valid_policy, None, ( 0, @@ -329,7 +193,6 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): ), ( "Invalid Principal with wildcard in Condition in DenyUnauthorizedPrincipals", - valid_policy, None, ( 0, @@ -345,7 +208,6 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): ), ( "Valid Principal w/o wildcard in Condition in DenyUnauthorizedPrincipals", - valid_policy, None, ( 0, @@ -361,7 +223,6 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): ), ( "Invalid Service Principal in Condition in DenyUnauthorizedPrincipals", - valid_policy, None, ( 0, @@ -377,7 +238,6 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): ), ( "Valid Service Principal in Condition in DenyUnauthorizedPrincipals", - valid_policy, None, ( 0, @@ -394,35 +254,30 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): # test modified statement DenyOutsideOrganization ( "Invalid Effect in DenyOutsideOrganization", - valid_policy, None, (1, {"Effect": "Allow"}), "FAIL", ), ( "Valid Effect in DenyOutsideOrganization", - valid_policy, None, (1, {"Effect": "Deny"}), "PASS", ), ( "Invalid Action in DenyOutsideOrganization", - valid_policy, None, (1, {"Action": "secretsmanager:InvalidAction"}), "FAIL", ), ( "Valid Action in DenyOutsideOrganization", - valid_policy, None, (1, {"Action": "secretsmanager:*"}), "PASS", ), ( "Invalid Resource in DenyOutsideOrganization", - valid_policy, None, ( 1, @@ -434,7 +289,6 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): ), ( "Invalid Condition Operator in DenyOutsideOrganization", - valid_policy, None, ( 1, @@ -448,7 +302,6 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): ), ( "Valid Condition Operator in DenyOutsideOrganization", - valid_policy, None, ( 1, @@ -462,7 +315,6 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): ), ( "Invalid Condition Key in DenyOutsideOrganization", - valid_policy, None, ( 1, @@ -476,7 +328,6 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): ), ( "Valid Condition Key in DenyOutsideOrganization", - valid_policy, None, ( 1, @@ -490,7 +341,6 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): ), ( "Invalid PrincipalOrgID in DenyOutsideOrganization", - valid_policy, None, ( 1, @@ -504,7 +354,6 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): ), ( "Valid PrincipalOrgID in DenyOutsideOrganization", - valid_policy, None, ( 1, @@ -519,42 +368,36 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): # test modified statement AllowAuditPolicyRead ( "Invalid wildcard in NotAction in AllowAuditPolicyRead", - valid_policy, None, (2, {"NotAction": "*"}), "FAIL", ), ( "No wildcard in NotAction in AllowAuditPolicyRead", - valid_policy, None, (2, {"NotAction": "secretsmanager:DescribeSecret"}), "PASS", ), ( "Invalid wildcard in NotAction in AllowSecretAccessForRole2", - valid_policy, None, (3, {"NotAction": "*"}), "FAIL", ), ( "No wildcard in NotAction in AllowSecretAccessForRole2", - valid_policy, None, (3, {"NotAction": "secretsmanager:DescribeSecret"}), "PASS", ), ( "Invalid wildcard in NotAction in both statements", - valid_policy, None, [(2, {"NotAction": "*"}), (3, {"NotAction": "secretsmanager:*"})], "FAIL", ), ( "No wildcard in NotAction in both statements", - valid_policy, None, [ (2, {"NotAction": "secretsmanager:DescribeSecret"}), @@ -563,25 +406,78 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): "PASS", ), # test policy with removed statements - ("Missing DenyUnauthorizedPrincipals", valid_policy, 0, None, "FAIL"), - ("Missing DenyOutsideOrganization", valid_policy, 1, None, "FAIL"), + ("Missing DenyUnauthorizedPrincipals", 0, None, "FAIL"), + ("Missing DenyOutsideOrganization", 1, None, "FAIL"), # the following 2 test cases PASS because these statements are not required to make the Policy secure # but in practice the AWS Principal will not be able to access the secret - ("Missing AllowAuditPolicyRead", valid_policy, 2, None, "PASS"), - ("Missing AllowSecretAccessForRole2", valid_policy, 3, None, "PASS"), - ] + ("Missing AllowAuditPolicyRead", 2, None, "PASS"), + ("Missing AllowSecretAccessForRole2", 3, None, "PASS"), + ], + ) + def test_secretsmanager_policies_for_principals( + self, + secretsmanager_client, + description, + remove_index, + modify_element, + expected_status, + ): + with mock_aws(): + client_instance, secret_arn = secretsmanager_client + + valid_policy = { + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "DenyUnauthorizedPrincipals", + "Effect": "Deny", + "Principal": "*", + "Action": "*", + "Resource": "*", + "Condition": { + "StringNotEquals": { + "aws:PrincipalArn": [ + "arn:aws:iam::123456789012:role/AccountSecurityAuditRole", + "arn:aws:iam::123456789012:role/Role2", + ] + } + }, + }, + { + "Sid": "DenyOutsideOrganization", + "Effect": "Deny", + "Principal": "*", + "Action": "secretsmanager:*", + "Resource": "*", + "Condition": { + "StringNotEquals": {"aws:PrincipalOrgID": "o-1234567890"} + }, + }, + { + "Sid": "AllowAuditPolicyRead", + "Effect": "Deny", + "Principal": { + "AWS": "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" + }, + "NotAction": [ + "secretsmanager:DescribeSecret", + "secretsmanager:GetResourcePolicy", + ], + "Resource": "*", + }, + { + "Sid": "AllowSecretAccessForRole2", + "Effect": "Deny", + "Principal": {"AWS": "arn:aws:iam::123456789012:role/Role2"}, + "NotAction": ["secretsmanager:GetSecretValue"], + "Resource": "*", + }, + ], + } - for ( - description, - base_policy, - remove_index, - modify_element, - expected_status, - ) in test_cases: - policy_copy = json.loads(json.dumps(base_policy)) + policy_copy = json.loads(json.dumps(valid_policy)) if remove_index is not None: del policy_copy["Statement"][remove_index] - if modify_element is not None: if isinstance(modify_element, list): for index, value in modify_element: @@ -590,12 +486,11 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): index, value = modify_element policy_copy["Statement"][index].update(value) - secretsmanager_client.put_resource_policy( - SecretId=secret["ARN"], ResourcePolicy=json.dumps(policy_copy) + client_instance.put_resource_policy( + SecretId=secret_arn, ResourcePolicy=json.dumps(policy_copy) ) aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) - with mock.patch( "prowler.providers.common.provider.Provider.get_global_provider", return_value=aws_provider, @@ -616,120 +511,43 @@ def test_secret_with_modified_restrictive_policies_for_principals(self): assert len(result) == 1 assert result[0].status == expected_status, f"Test case: {description}" - @mock_aws - def test_secret_with_modified_restrictive_policies_for_services(self): - secretsmanager_client = client( - "secretsmanager", region_name=AWS_REGION_EU_WEST_1 - ) - secret = secretsmanager_client.create_secret(Name="test-secret-modified") - - valid_policy = { - "Version": "2012-10-17", - "Statement": [ - { - "Sid": "DenyUnauthorizedPrincipals", - "Effect": "Deny", - "Principal": "*", - "Action": "*", - "Resource": "*", - "Condition": { - "StringNotEqualsIfExists": { - "aws:PrincipalArn": [ - "arn:aws:iam::123456789012:role/AccountSecurityAuditRole", - "arn:aws:iam::123456789012:role/Role2", - ], - "aws:PrincipalServiceName": "appflow.amazonaws.com", - } - }, - }, - { - "Sid": "DenyOutsideOrganization", - "Effect": "Deny", - "Principal": "*", - "Action": "secretsmanager:*", - "Resource": "*", - "Condition": { - "StringNotEquals": {"aws:PrincipalOrgID": "o-1234567890"} - }, - }, - { - "Sid": "AllowAuditPolicyRead", - "Effect": "Deny", - "Principal": { - "AWS": "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" - }, - "NotAction": [ - "secretsmanager:DescribeSecret", - "secretsmanager:GetResourcePolicy", - ], - "Resource": "*", - }, - { - "Sid": "AllowSecretAccessForRole2", - "Effect": "Deny", - "Principal": {"AWS": "arn:aws:iam::123456789012:role/Role2"}, - "NotAction": ["secretsmanager:GetSecretValue"], - "Resource": "*", - }, - { - "Sid": "AllowAppFlowAccess", - "Effect": "Allow", - "Principal": {"Service": "appflow.amazonaws.com"}, - "Action": [ - "secretsmanager:GetSecretValue", - "secretsmanager:PutSecretValue", - "secretsmanager:DeleteSecret", - "secretsmanager:DescribeSecret", - "secretsmanager:UpdateSecret", - ], - "Resource": "*", - "Condition": { - "StringEquals": {"aws:SourceAccount": "123456789012"} - }, - }, - ], - } - - test_cases = [ + @pytest.mark.parametrize( + "description, modify_element, expected_status", + [ + # test unmodified policy ( "Valid unmodified Policy with PrincipalArn and Service", - valid_policy, None, "PASS", ), + # test statement AllowAppFlowAccess ( "Invalid wildcard '*' in Action in AllowAppFlowAccess", - valid_policy, (4, {"Action": "*"}), "FAIL", ), ( "No wildcard '*' in Action in AllowAppFlowAccess", - valid_policy, (4, {"Action": "secretsmanager:GetSecretValue"}), "PASS", ), ( "Invalid wildcard 'secretsmanager:*' in Action in AllowAppFlowAccess", - valid_policy, (4, {"Action": "secretsmanager:*"}), "FAIL", ), ( "No wildcard 'secretsmanager:*' in Action in AllowAppFlowAccess", - valid_policy, (4, {"Action": "secretsmanager:ValidAction"}), "PASS", ), ( "Missing Condition in AllowAppFlowAccess", - valid_policy, (4, {"Condition": {}}), "FAIL", ), ( "Valid Condition in AllowAppFlowAccess", - valid_policy, ( 4, { @@ -742,7 +560,6 @@ def test_secret_with_modified_restrictive_policies_for_services(self): ), ( "Invalid Condition Operator in AllowAppFlowAccess", - valid_policy, ( 4, { @@ -755,7 +572,6 @@ def test_secret_with_modified_restrictive_policies_for_services(self): ), ( "Valid Condition Operator in AllowAppFlowAccess", - valid_policy, ( 4, { @@ -768,13 +584,11 @@ def test_secret_with_modified_restrictive_policies_for_services(self): ), ( "Invalid Condition Key in AllowAppFlowAccess", - valid_policy, (4, {"Condition": {"StringEquals": {"aws:WrongKey": "123456789012"}}}), "FAIL", ), ( "Valid Condition Key in AllowAppFlowAccess", - valid_policy, ( 4, { @@ -785,15 +599,83 @@ def test_secret_with_modified_restrictive_policies_for_services(self): ), "PASS", ), - ] + ], + ) + def test_secretsmanager_policies_for_services( + self, secretsmanager_client, description, modify_element, expected_status + ): + with mock_aws(): + client_instance, secret_arn = secretsmanager_client + + valid_policy = { + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "DenyUnauthorizedPrincipals", + "Effect": "Deny", + "Principal": "*", + "Action": "*", + "Resource": "*", + "Condition": { + "StringNotEqualsIfExists": { + "aws:PrincipalArn": [ + "arn:aws:iam::123456789012:role/AccountSecurityAuditRole", + "arn:aws:iam::123456789012:role/Role2", + ], + "aws:PrincipalServiceName": "appflow.amazonaws.com", + } + }, + }, + { + "Sid": "DenyOutsideOrganization", + "Effect": "Deny", + "Principal": "*", + "Action": "secretsmanager:*", + "Resource": "*", + "Condition": { + "StringNotEquals": {"aws:PrincipalOrgID": "o-1234567890"} + }, + }, + { + "Sid": "AllowAuditPolicyRead", + "Effect": "Deny", + "Principal": { + "AWS": "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" + }, + "NotAction": [ + "secretsmanager:DescribeSecret", + "secretsmanager:GetResourcePolicy", + ], + "Resource": "*", + }, + { + "Sid": "AllowSecretAccessForRole2", + "Effect": "Deny", + "Principal": {"AWS": "arn:aws:iam::123456789012:role/Role2"}, + "NotAction": ["secretsmanager:GetSecretValue"], + "Resource": "*", + }, + { + "Sid": "AllowAppFlowAccess", + "Effect": "Allow", + "Principal": {"Service": "appflow.amazonaws.com"}, + "Action": [ + "secretsmanager:GetSecretValue", + "secretsmanager:PutSecretValue", + "secretsmanager:DeleteSecret", + "secretsmanager:DescribeSecret", + "secretsmanager:UpdateSecret", + ], + "Resource": "*", + "Condition": { + "StringEquals": {"aws:SourceAccount": "123456789012"} + }, + }, + ], + } + + policy_copy = json.loads(json.dumps(valid_policy)) - for ( - description, - base_policy, - modify_element, - expected_status, - ) in test_cases: - policy_copy = json.loads(json.dumps(base_policy)) if modify_element is not None: if isinstance(modify_element, list): for index, value in modify_element: @@ -802,12 +684,11 @@ def test_secret_with_modified_restrictive_policies_for_services(self): index, value = modify_element policy_copy["Statement"][index].update(value) - secretsmanager_client.put_resource_policy( - SecretId=secret["ARN"], ResourcePolicy=json.dumps(policy_copy) + client_instance.put_resource_policy( + SecretId=secret_arn, ResourcePolicy=json.dumps(policy_copy) ) aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) - with mock.patch( "prowler.providers.common.provider.Provider.get_global_provider", return_value=aws_provider, From 18bd65178137dd8f7f062b5939aa9ce6f8a77569 Mon Sep 17 00:00:00 2001 From: kagahd Date: Thu, 20 Feb 2025 10:40:14 +0100 Subject: [PATCH 04/13] feat(aws) reset_moto --- ...manager_has_restrictive_resource_policy_test.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py index 833f30b9bf..dc5a3ce577 100644 --- a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py +++ b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py @@ -1,3 +1,4 @@ +import moto import json import pytest from unittest import mock @@ -17,10 +18,16 @@ def secretsmanager_client(): secret = client_instance.create_secret(Name="test-secret") yield client_instance, secret["ARN"] +@pytest.fixture(scope="function", autouse=True) +def reset_moto(): + mock = moto.mock_aws() + mock.start() + yield + mock.stop() class TestSecretsManagerHasRestrictiveResourcePolicy: - def test_no_secrets(self): + def test_no_secrets(self, reset_moto): with mock_aws(): aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) @@ -46,7 +53,7 @@ def test_no_secrets(self): assert len(result) == 0 - def test_secret_with_weak_policy(self, secretsmanager_client): + def test_secret_with_weak_policy(self, secretsmanager_client, reset_moto): client_instance, secret_arn = secretsmanager_client client_instance.put_resource_policy( SecretId=secret_arn, @@ -421,6 +428,7 @@ def test_secretsmanager_policies_for_principals( remove_index, modify_element, expected_status, + reset_moto, ): with mock_aws(): client_instance, secret_arn = secretsmanager_client @@ -602,7 +610,7 @@ def test_secretsmanager_policies_for_principals( ], ) def test_secretsmanager_policies_for_services( - self, secretsmanager_client, description, modify_element, expected_status + self, secretsmanager_client, description, modify_element, expected_status, reset_moto ): with mock_aws(): client_instance, secret_arn = secretsmanager_client From 92c5558042aeda6f527b46ef4ca72371c04eee06 Mon Sep 17 00:00:00 2001 From: kagahd Date: Thu, 20 Feb 2025 10:44:22 +0100 Subject: [PATCH 05/13] make black & flake happy --- ...ecretsmanager_has_restrictive_resource_policy_test.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py index dc5a3ce577..075c66700e 100644 --- a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py +++ b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py @@ -18,6 +18,7 @@ def secretsmanager_client(): secret = client_instance.create_secret(Name="test-secret") yield client_instance, secret["ARN"] + @pytest.fixture(scope="function", autouse=True) def reset_moto(): mock = moto.mock_aws() @@ -25,6 +26,7 @@ def reset_moto(): yield mock.stop() + class TestSecretsManagerHasRestrictiveResourcePolicy: def test_no_secrets(self, reset_moto): @@ -610,7 +612,12 @@ def test_secretsmanager_policies_for_principals( ], ) def test_secretsmanager_policies_for_services( - self, secretsmanager_client, description, modify_element, expected_status, reset_moto + self, + secretsmanager_client, + description, + modify_element, + expected_status, + reset_moto, ): with mock_aws(): client_instance, secret_arn = secretsmanager_client From 14c80a6b3c8b460dc2e095c2c504618f931ffc26 Mon Sep 17 00:00:00 2001 From: kagahd Date: Thu, 20 Feb 2025 10:52:12 +0100 Subject: [PATCH 06/13] implicit reset_moto --- .../secretsmanager_has_restrictive_resource_policy_test.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py index 075c66700e..c56990790d 100644 --- a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py +++ b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py @@ -29,7 +29,7 @@ def reset_moto(): class TestSecretsManagerHasRestrictiveResourcePolicy: - def test_no_secrets(self, reset_moto): + def test_no_secrets(self): with mock_aws(): aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) @@ -55,7 +55,7 @@ def test_no_secrets(self, reset_moto): assert len(result) == 0 - def test_secret_with_weak_policy(self, secretsmanager_client, reset_moto): + def test_secret_with_weak_policy(self, secretsmanager_client): client_instance, secret_arn = secretsmanager_client client_instance.put_resource_policy( SecretId=secret_arn, @@ -430,7 +430,6 @@ def test_secretsmanager_policies_for_principals( remove_index, modify_element, expected_status, - reset_moto, ): with mock_aws(): client_instance, secret_arn = secretsmanager_client @@ -617,7 +616,6 @@ def test_secretsmanager_policies_for_services( description, modify_element, expected_status, - reset_moto, ): with mock_aws(): client_instance, secret_arn = secretsmanager_client From ed2899a06a27dac128f44e9c31dc8f87b05a7a06 Mon Sep 17 00:00:00 2001 From: kagahd Date: Fri, 21 Feb 2025 15:51:50 +0100 Subject: [PATCH 07/13] scope function instead of class --- ...ager_has_restrictive_resource_policy_test.py | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py index c56990790d..ec29d606aa 100644 --- a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py +++ b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py @@ -1,4 +1,3 @@ -import moto import json import pytest from unittest import mock @@ -11,7 +10,7 @@ from tests.providers.aws.utils import AWS_REGION_EU_WEST_1, set_mocked_aws_provider -@pytest.fixture(scope="class") +@pytest.fixture(scope="function") def secretsmanager_client(): with mock_aws(): client_instance = client("secretsmanager", region_name=AWS_REGION_EU_WEST_1) @@ -19,14 +18,6 @@ def secretsmanager_client(): yield client_instance, secret["ARN"] -@pytest.fixture(scope="function", autouse=True) -def reset_moto(): - mock = moto.mock_aws() - mock.start() - yield - mock.stop() - - class TestSecretsManagerHasRestrictiveResourcePolicy: def test_no_secrets(self): @@ -611,11 +602,7 @@ def test_secretsmanager_policies_for_principals( ], ) def test_secretsmanager_policies_for_services( - self, - secretsmanager_client, - description, - modify_element, - expected_status, + self, secretsmanager_client, description, modify_element, expected_status ): with mock_aws(): client_instance, secret_arn = secretsmanager_client From 7a05b5463046943917f804dcf336909d7ebde0d1 Mon Sep 17 00:00:00 2001 From: kagahd Date: Wed, 26 Feb 2025 09:35:03 +0100 Subject: [PATCH 08/13] consider services which do not send PrincipalOrgID --- ...cretsmanager_has_restrictive_resource_policy.py | 14 +++++++++++++- ...manager_has_restrictive_resource_policy_test.py | 12 +++++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py b/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py index bcf371a32a..5da7806a72 100644 --- a/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py +++ b/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py @@ -87,7 +87,19 @@ def execute(self): True if not organizations_trusted_ids else any( - "*" in self.extract_field(statement.get("Principal", {})) + ( + ("*" in self.extract_field(statement.get("Principal", {}))) + or ( + "NotPrincipal" in statement + and any( + allowed_service + in self.extract_field( + statement.get("NotPrincipal", {}) + ) + for allowed_service in not_denied_services + ) + ) + ) and any( action in self.extract_field(statement.get("Action", [])) for action in ["*", "secretsmanager:*"] diff --git a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py index ec29d606aa..2f7e40e331 100644 --- a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py +++ b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py @@ -24,7 +24,7 @@ def test_no_secrets(self): with mock_aws(): aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) - from prowler.providers.aws.services.secretsmanager.secretsmanager_client import ( + from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( secretsmanager_client, ) @@ -34,7 +34,7 @@ def test_no_secrets(self): "prowler.providers.common.provider.Provider.get_global_provider", return_value=aws_provider, ), mock.patch( - "prowler.providers.aws.services.secretsmanager.secretsmanager_client", + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", new=SecretsManager(aws_provider), ): from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( @@ -520,6 +520,12 @@ def test_secretsmanager_policies_for_principals( None, "PASS", ), + # test statement DenyOutsideOrganization + ( + "Invalid DenyOutsideOrganization using NotPrincipal with disallowed service", + (1, {"NotPrincipal": {"Service": "invalid.service.com"}}), + "FAIL", + ), # test statement AllowAppFlowAccess ( "Invalid wildcard '*' in Action in AllowAppFlowAccess", @@ -629,7 +635,7 @@ def test_secretsmanager_policies_for_services( { "Sid": "DenyOutsideOrganization", "Effect": "Deny", - "Principal": "*", + "NotPrincipal": {"Service": "appflow.amazonaws.com"}, "Action": "secretsmanager:*", "Resource": "*", "Condition": { From 8738024e1e3158f51b866f3825fce7ac792ccec2 Mon Sep 17 00:00:00 2001 From: kagahd Date: Fri, 28 Feb 2025 10:32:33 +0100 Subject: [PATCH 09/13] handle assumed role --- ...manager_has_restrictive_resource_policy.py | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py b/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py index 5da7806a72..b5e4ecf155 100644 --- a/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py +++ b/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py @@ -23,10 +23,35 @@ def execute(self): report.resource_arn = secret.arn report.resource_tags = secret.tags report.status = "FAIL" + # Determine the Role ARN to be used + assumed_role_config = getattr( + secretsmanager_client.provider, "_assumed_role_configuration", None + ) + if ( + assumed_role_config + and getattr(assumed_role_config, "info", None) + and getattr(assumed_role_config.info, "role_arn", None) + and getattr(assumed_role_config.info.role_arn, "arn", None) + ): + final_role_arn = assumed_role_config.info.role_arn.arn + else: + identity_arn = secretsmanager_client.provider.identity.identity_arn + if identity_arn: + # If the identity ARN is a sts assumed-role ARN, transform it + match = re.match( + r"arn:aws:sts::(\d+):assumed-role/([^/]+)/", identity_arn + ) + if match: + account_id, role_name = match.groups() + final_role_arn = f"arn:aws:iam::{account_id}:role/{role_name}" + else: + final_role_arn = identity_arn + else: + final_role_arn = "None" + report.status_extended = ( f"SecretsManager secret '{secret.name}' does not have a resource-based policy " - f"or access to the policy is denied for the role " - f"'{getattr(getattr(secretsmanager_client.provider, '_assumed_role_configuration', None), 'info', None) and getattr(secretsmanager_client.provider._assumed_role_configuration.info, 'role_arn', None) and getattr(secretsmanager_client.provider._assumed_role_configuration.info.role_arn, 'arn', 'N/A')}'" + f"or access to the policy is denied for the role '{final_role_arn}'" ) if secret.policy: From 4e35f1278978f5d3b00f1160cfebb323c7387001 Mon Sep 17 00:00:00 2001 From: kagahd Date: Mon, 12 May 2025 14:48:13 +0200 Subject: [PATCH 10/13] adapt check to follow best practices by avoiding "NotPrincipal" in a Deny statement of a resource-based policy --- ...manager_has_restrictive_resource_policy.py | 179 +++++++++++------- ...er_has_restrictive_resource_policy_test.py | 121 ++++++++++-- 2 files changed, 224 insertions(+), 76 deletions(-) diff --git a/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py b/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py index b5e4ecf155..49aafc3246 100644 --- a/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py +++ b/prowler/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py @@ -60,88 +60,137 @@ def execute(self): not_denied_services = [] # Check for an explicit Deny that applies to all Principals except those defined in the Condition - has_explicit_deny_for_all = any( - "*" in self.extract_field(statement.get("Principal", {})) - and any( - action in self.extract_field(statement.get("Action", [])) - for action in ["*", "secretsmanager:*"] - ) - and self.is_valid_resource( + has_explicit_deny_for_all = False + + for statement in statements: + if statement.get("Effect") != "Deny": + continue + principal = self.extract_field(statement.get("Principal", {})) + if "*" not in principal: + continue + actions = self.extract_field(statement.get("Action", [])) + if not any( + action in ["*", "secretsmanager:*"] for action in actions + ): + continue + if not self.is_valid_resource( secret, self.extract_field(statement.get("Resource", "*")) - ) - and "Condition" in statement - # accept only the allowed Condition operators - and all( - operator in ["StringNotEquals", "StringNotEqualsIfExists"] - for operator in statement["Condition"].keys() - ) - # no PrincipalArn nor Service of the Condition must have wildcard * in its name - and all( - self.is_valid_principal( - principal_value=principal_value, - not_denied_list=not_denied_list, - pattern=pattern, + ): + continue + + condition = statement.get("Condition", {}) + + condition_principals = {} + if "StringNotEquals" in condition: + condition_principals = condition.get("StringNotEquals", {}) + elif "StringNotEqualsIfExists" in condition: + condition_principals = condition.get( + "StringNotEqualsIfExists", {} ) - for operator, condition_values in statement["Condition"].items() - for principal_key, principal_value in condition_values.items() - # only these two keys are allowed: - for mapping in [ - { - "aws:PrincipalArn": ( - not_denied_principals, - arn_pattern, - ), - "aws:PrincipalServiceName": ( - not_denied_services, - service_pattern, - ), - }.get(principal_key) - ] - # the principal_key decides which list and pattern is passed to is_valid_principal - for not_denied_list, pattern in [ - mapping if mapping is not None else (None, None) - ] - # direct tuple unpacking of the mapping + + uses_principal_arn = "aws:PrincipalArn" in condition_principals + uses_principal_service = ( + "aws:PrincipalServiceName" in condition_principals ) - for statement in statements - if statement.get("Effect") == "Deny" - ) + + valid_keys = {"aws:PrincipalArn", "aws:PrincipalServiceName"} + if not set(condition_principals.keys()).issubset(valid_keys): + continue + + # check values of principals + all_valid = True + for key, (not_denied_list, pattern) in { + "aws:PrincipalArn": (not_denied_principals, arn_pattern), + "aws:PrincipalServiceName": ( + not_denied_services, + service_pattern, + ), + }.items(): + if key in condition_principals: + if not self.is_valid_principal( + condition_principals[key], not_denied_list, pattern + ): + all_valid = False + break + + if not all_valid: + continue + + # case 1: both keys for Principal and Service exist → require IfExists + Null Condition + if uses_principal_arn and uses_principal_service: + if ( + "StringNotEqualsIfExists" in condition + and "Null" in condition + ): + null_condition = condition.get("Null", {}) + if ( + null_condition.get("aws:PrincipalArn") == "true" + and null_condition.get("aws:PrincipalServiceName") + == "true" + ): + has_explicit_deny_for_all = True + break + + # case 2: only PrincipalArn exists → require StringNotEquals + elif uses_principal_arn and not uses_principal_service: + if "StringNotEquals" in condition: + has_explicit_deny_for_all = True + break # Check for Deny with "StringNotEquals":"aws:PrincipalOrgID" condition has_deny_outside_org = ( True if not organizations_trusted_ids else any( - ( - ("*" in self.extract_field(statement.get("Principal", {}))) - or ( - "NotPrincipal" in statement - and any( - allowed_service - in self.extract_field( - statement.get("NotPrincipal", {}) - ) - for allowed_service in not_denied_services - ) - ) - ) + statement.get("Effect") == "Deny" + and "*" in self.extract_field(statement.get("Principal", {})) and any( - action in self.extract_field(statement.get("Action", [])) - for action in ["*", "secretsmanager:*"] + action in ["*", "secretsmanager:*"] + for action in self.extract_field( + statement.get("Action", []) + ) ) and self.is_valid_resource( secret, self.extract_field(statement.get("Resource", "*")) ) and "Condition" in statement and set(statement["Condition"].keys()) == {"StringNotEquals"} - and set(statement["Condition"]["StringNotEquals"].keys()) - == {"aws:PrincipalOrgID"} - and statement["Condition"]["StringNotEquals"][ - "aws:PrincipalOrgID" - ] - in organizations_trusted_ids + and ( + ( + set(statement["Condition"]["StringNotEquals"].keys()) + == {"aws:PrincipalOrgID"} + and all( + v in organizations_trusted_ids + for v in self.extract_field( + statement["Condition"]["StringNotEquals"][ + "aws:PrincipalOrgID" + ] + ) + ) + ) + if not not_denied_services + else ( + set(statement["Condition"]["StringNotEquals"].keys()) + == {"aws:PrincipalOrgID", "aws:PrincipalServiceName"} + and all( + v in organizations_trusted_ids + for v in self.extract_field( + statement["Condition"]["StringNotEquals"][ + "aws:PrincipalOrgID" + ] + ) + ) + and all( + s in not_denied_services + for s in self.extract_field( + statement["Condition"]["StringNotEquals"][ + "aws:PrincipalServiceName" + ] + ) + ) + ) + ) for statement in statements - if statement.get("Effect") == "Deny" ) ) diff --git a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py index 2f7e40e331..c3f8c9c9ce 100644 --- a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py +++ b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py @@ -229,7 +229,7 @@ def test_secret_with_weak_policy(self, secretsmanager_client): { "Condition": { "StringNotEquals": { - "aws:PrincipalServiceName": "invalid.service.com" + "aws:PrincipalServiceName": "appflow.amazonaws.com" } } }, @@ -237,19 +237,19 @@ def test_secret_with_weak_policy(self, secretsmanager_client): "FAIL", ), ( - "Valid Service Principal in Condition in DenyUnauthorizedPrincipals", + "Invalid Condition Operator for Principal in DenyUnauthorizedPrincipals", None, ( 0, { "Condition": { - "StringNotEquals": { - "aws:PrincipalServiceName": "valid.amazonaws.com" + "StringNotEqualsIfExists": { + "aws:PrincipalArn": "arn:aws:iam::123456789012:role/AccountSecurityAuditRole" } } }, ), - "PASS", + "FAIL", ), # test modified statement DenyOutsideOrganization ( @@ -365,6 +365,38 @@ def test_secret_with_weak_policy(self, secretsmanager_client): ), "PASS", ), + ( + "Invalid multiple Condition Operators in DenyOutsideOrganization", + None, + ( + 1, + { + "Condition": { + "StringNotEquals": {"aws:PrincipalOrgID": "o-1234567890"}, + "StringNotEqualsIfExists": { + "aws:PrincipalOrgID": "o-1234567890" + }, + } + }, + ), + "FAIL", + ), + ( + "Invalid multiple keys in StringNotEquals Condition in DenyOutsideOrganization", + None, + ( + 1, + { + "Condition": { + "StringNotEquals": { + "aws:PrincipalOrgID": "o-1234567890", + "aws:PrincipalServiceName": "appflow.amazonaws.com", + } + } + }, + ), + "FAIL", + ), # test modified statement AllowAuditPolicyRead ( "Invalid wildcard in NotAction in AllowAuditPolicyRead", @@ -520,10 +552,70 @@ def test_secretsmanager_policies_for_principals( None, "PASS", ), + # test statement DenyUnauthorizedPrincipals + ( + "Missing Null Condition", + ( + 0, + { + "Condition": { + "StringNotEqualsIfExists": { + "aws:PrincipalArn": [ + "arn:aws:iam::123456789012:role/AccountSecurityAuditRole", + "arn:aws:iam::123456789012:role/Role2", + ], + "aws:PrincipalServiceName": "appflow.amazonaws.com", + } + } + }, + ), + "FAIL", + ), + ( + "Missing PrincipalArn in Null Condition", + ( + 0, + { + "Condition": { + "StringNotEqualsIfExists": { + "aws:PrincipalArn": [ + "arn:aws:iam::123456789012:role/AccountSecurityAuditRole", + "arn:aws:iam::123456789012:role/Role2", + ], + "aws:PrincipalServiceName": "appflow.amazonaws.com", + }, + "Null": {"aws:PrincipalServiceName": "true"}, + } + }, + ), + "FAIL", + ), + ( + "Invalid value for PrincipalArn in Null Condition", + ( + 0, + { + "Condition": { + "StringNotEqualsIfExists": { + "aws:PrincipalArn": [ + "arn:aws:iam::123456789012:role/AccountSecurityAuditRole", + "arn:aws:iam::123456789012:role/Role2", + ], + "aws:PrincipalServiceName": "appflow.amazonaws.com", + }, + "Null": { + "aws:PrincipalArn": "false", + "aws:PrincipalServiceName": "true", + }, + } + }, + ), + "FAIL", + ), # test statement DenyOutsideOrganization ( - "Invalid DenyOutsideOrganization using NotPrincipal with disallowed service", - (1, {"NotPrincipal": {"Service": "invalid.service.com"}}), + "Invalid DenyOutsideOrganization using Principal with disallowed service", + (1, {"Principal": {"Service": "invalid.service.com"}}), "FAIL", ), # test statement AllowAppFlowAccess @@ -607,7 +699,7 @@ def test_secretsmanager_policies_for_principals( ), ], ) - def test_secretsmanager_policies_for_services( + def test_secretsmanager_policies_for_principals_and_services( self, secretsmanager_client, description, modify_element, expected_status ): with mock_aws(): @@ -629,17 +721,24 @@ def test_secretsmanager_policies_for_services( "arn:aws:iam::123456789012:role/Role2", ], "aws:PrincipalServiceName": "appflow.amazonaws.com", - } + }, + "Null": { + "aws:PrincipalArn": "true", + "aws:PrincipalServiceName": "true", + }, }, }, { "Sid": "DenyOutsideOrganization", "Effect": "Deny", - "NotPrincipal": {"Service": "appflow.amazonaws.com"}, + "Principal": "*", "Action": "secretsmanager:*", "Resource": "*", "Condition": { - "StringNotEquals": {"aws:PrincipalOrgID": "o-1234567890"} + "StringNotEquals": { + "aws:PrincipalOrgID": "o-1234567890", + "aws:PrincipalServiceName": "appflow.amazonaws.com", + } }, }, { From b1e6cd428cc51c4b1c4a71e1c5902f9cd7cdfc21 Mon Sep 17 00:00:00 2001 From: kagahd Date: Tue, 13 May 2025 09:49:53 +0200 Subject: [PATCH 11/13] make black happy --- ...er_has_restrictive_resource_policy_test.py | 76 +++++++++++-------- 1 file changed, 45 insertions(+), 31 deletions(-) diff --git a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py index c3f8c9c9ce..8221591930 100644 --- a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py +++ b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py @@ -30,12 +30,15 @@ def test_no_secrets(self): secretsmanager_client.secrets.clear() - with mock.patch( - "prowler.providers.common.provider.Provider.get_global_provider", - return_value=aws_provider, - ), mock.patch( - "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", - new=SecretsManager(aws_provider), + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), + mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", + new=SecretsManager(aws_provider), + ), ): from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( secretsmanager_has_restrictive_resource_policy, @@ -67,12 +70,15 @@ def test_secret_with_weak_policy(self, secretsmanager_client): ) aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) - with mock.patch( - "prowler.providers.common.provider.Provider.get_global_provider", - return_value=aws_provider, - ), mock.patch( - "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", - new=SecretsManager(aws_provider), + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), + mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", + new=SecretsManager(aws_provider), + ), ): from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( secretsmanager_has_restrictive_resource_policy, @@ -523,15 +529,19 @@ def test_secretsmanager_policies_for_principals( ) aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) - with mock.patch( - "prowler.providers.common.provider.Provider.get_global_provider", - return_value=aws_provider, - ), mock.patch( - "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", - new=SecretsManager(aws_provider), - ), mock.patch( - "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client.audit_config", - {"organizations_trusted_ids": "o-1234567890"}, + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), + mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", + new=SecretsManager(aws_provider), + ), + mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client.audit_config", + {"organizations_trusted_ids": "o-1234567890"}, + ), ): from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( secretsmanager_has_restrictive_resource_policy, @@ -794,15 +804,19 @@ def test_secretsmanager_policies_for_principals_and_services( ) aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) - with mock.patch( - "prowler.providers.common.provider.Provider.get_global_provider", - return_value=aws_provider, - ), mock.patch( - "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", - new=SecretsManager(aws_provider), - ), mock.patch( - "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client.audit_config", - {"organizations_trusted_ids": "o-1234567890"}, + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), + mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", + new=SecretsManager(aws_provider), + ), + mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client.audit_config", + {"organizations_trusted_ids": "o-1234567890"}, + ), ): from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( secretsmanager_has_restrictive_resource_policy, @@ -812,4 +826,4 @@ def test_secretsmanager_policies_for_principals_and_services( result = check.execute() assert len(result) == 1 - assert result[0].status == expected_status, f"Test case: {description}" + assert result[0].status == expected_status, f"Test case: {description}" \ No newline at end of file From 5a725c1776539c576feff23bb65b6bad63ae721c Mon Sep 17 00:00:00 2001 From: kagahd Date: Tue, 13 May 2025 09:52:25 +0200 Subject: [PATCH 12/13] make flake8 happy --- .../secretsmanager_has_restrictive_resource_policy_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py index 8221591930..24a2c8db23 100644 --- a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py +++ b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py @@ -826,4 +826,4 @@ def test_secretsmanager_policies_for_principals_and_services( result = check.execute() assert len(result) == 1 - assert result[0].status == expected_status, f"Test case: {description}" \ No newline at end of file + assert result[0].status == expected_status, f"Test case: {description}" From 0b693eff369456891482b23dcbe233092ad9b5c5 Mon Sep 17 00:00:00 2001 From: kagahd Date: Tue, 13 May 2025 16:24:27 +0200 Subject: [PATCH 13/13] add Unit tests --- ...er_has_restrictive_resource_policy_test.py | 96 ++++++++++++++++++- 1 file changed, 92 insertions(+), 4 deletions(-) diff --git a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py index 24a2c8db23..e60370ae44 100644 --- a/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py +++ b/tests/providers/aws/services/secretsmanager/secretsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py @@ -7,7 +7,10 @@ from prowler.providers.aws.services.secretsmanager.secretsmanager_service import ( SecretsManager, ) -from tests.providers.aws.utils import AWS_REGION_EU_WEST_1, set_mocked_aws_provider +from tests.providers.aws.utils import ( + AWS_REGION_EU_WEST_1, + set_mocked_aws_provider, +) @pytest.fixture(scope="function") @@ -20,6 +23,81 @@ def secretsmanager_client(): class TestSecretsManagerHasRestrictiveResourcePolicy: + def test_assumed_role_identity_arn_triggers_transformation(self): + + with mock_aws(): + boto3_client = client("secretsmanager", region_name=AWS_REGION_EU_WEST_1) + boto3_client.create_secret(Name="mock-secret") + aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) + sm_client = SecretsManager(aws_provider) + sm_client.provider._assumed_role_configuration = None + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), + mock.patch.object( + sm_client.provider.identity, + "identity_arn", + "arn:aws:sts::123456789012:assumed-role/TestRole/SessionName", + ), + mock.patch.object( + sm_client.provider, "_assumed_role_configuration", None + ), + mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", + sm_client, + ), + ): + from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( + secretsmanager_has_restrictive_resource_policy, + ) + + check = secretsmanager_has_restrictive_resource_policy() + result = check.execute() + + assert len(result) == 1 + assert ( + "arn:aws:iam::123456789012:role/TestRole" + in result[0].status_extended + ) + + def test_non_assumed_role_identity_arn_remains_unchanged(self): + with mock_aws(): + boto3_client = client("secretsmanager", region_name=AWS_REGION_EU_WEST_1) + boto3_client.create_secret(Name="mock-secret") + aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) + sm_client = SecretsManager(aws_provider) + sm_client.provider._assumed_role_configuration = None + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), + mock.patch.object( + sm_client.provider.identity, + "identity_arn", + "arn:aws:iam::123456789012:user/MyUser", + ), + mock.patch( + "prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy.secretsmanager_client", + sm_client, + ), + ): + from prowler.providers.aws.services.secretsmanager.secretsmanager_has_restrictive_resource_policy.secretsmanager_has_restrictive_resource_policy import ( + secretsmanager_has_restrictive_resource_policy, + ) + + check = secretsmanager_has_restrictive_resource_policy() + result = check.execute() + + assert len(result) == 1 + assert ( + "arn:aws:iam::123456789012:user/MyUser" in result[0].status_extended + ) + def test_no_secrets(self): with mock_aws(): aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) @@ -431,7 +509,10 @@ def test_secret_with_weak_policy(self, secretsmanager_client): ( "Invalid wildcard in NotAction in both statements", None, - [(2, {"NotAction": "*"}), (3, {"NotAction": "secretsmanager:*"})], + [ + (2, {"NotAction": "*"}), + (3, {"NotAction": "secretsmanager:*"}), + ], "FAIL", ), ( @@ -692,7 +773,10 @@ def test_secretsmanager_policies_for_principals( ), ( "Invalid Condition Key in AllowAppFlowAccess", - (4, {"Condition": {"StringEquals": {"aws:WrongKey": "123456789012"}}}), + ( + 4, + {"Condition": {"StringEquals": {"aws:WrongKey": "123456789012"}}}, + ), "FAIL", ), ( @@ -710,7 +794,11 @@ def test_secretsmanager_policies_for_principals( ], ) def test_secretsmanager_policies_for_principals_and_services( - self, secretsmanager_client, description, modify_element, expected_status + self, + secretsmanager_client, + description, + modify_element, + expected_status, ): with mock_aws(): client_instance, secret_arn = secretsmanager_client