From 44877db71e69c9f9880748429e951128aaf6ffea Mon Sep 17 00:00:00 2001 From: Brian Henderson Date: Thu, 28 Aug 2025 09:13:28 +0200 Subject: [PATCH 1/9] Add AssumableByComputeService findings to role detail output Surface compute service assumability findings at the role level to provide better visibility into roles that can be assumed by EC2, ECS, EKS, or Lambda services, helping security teams identify potential privilege escalation paths through compute services. --- .../scan/assume_role_policy_document.py | 4 ++++ cloudsplaining/scan/role_details.py | 15 +++++++++++++++ test/files/scanning/test_role_detail_results.json | 9 +++++++-- test/scanning/test_trust_policies.py | 12 +++++++++++- 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/cloudsplaining/scan/assume_role_policy_document.py b/cloudsplaining/scan/assume_role_policy_document.py index 2a2b08ec..9102d5f0 100644 --- a/cloudsplaining/scan/assume_role_policy_document.py +++ b/cloudsplaining/scan/assume_role_policy_document.py @@ -82,6 +82,10 @@ def role_assumable_by_compute_services(self) -> list[str]: if "sts:AssumeRole".lower() not in lowercase_actions: return [] + # Effect must be Allow + if self.effect.lower() != "allow": + return [] + assumable_by_compute_services = [] for principal in self.principals: if principal.endswith(".amazonaws.com"): diff --git a/cloudsplaining/scan/role_details.py b/cloudsplaining/scan/role_details.py index dc8179d5..99ce25ea 100644 --- a/cloudsplaining/scan/role_details.py +++ b/cloudsplaining/scan/role_details.py @@ -9,6 +9,7 @@ from cloudsplaining.scan.assume_role_policy_document import AssumeRolePolicyDocument from cloudsplaining.scan.inline_policy import InlinePolicy from cloudsplaining.shared import utils +from cloudsplaining.shared.constants import ISSUE_SEVERITY, RISK_DEFINITION from cloudsplaining.shared.exceptions import NotFoundException from cloudsplaining.shared.exclusions import ( DEFAULT_EXCLUSIONS, @@ -147,6 +148,7 @@ def __init__( :param policy_details: The ManagedPolicyDetails object - i.e., details about all managed policies in the account so the role can inherit those attributes """ + self.severity = [] if severity is None else severity # Metadata self.path = role_detail["Path"] self.role_name = role_detail["RoleName"] @@ -330,5 +332,18 @@ def json(self) -> dict[str, Any]: customer_managed_policies=self.attached_customer_managed_policies_pointer_json, aws_managed_policies=self.attached_aws_managed_policies_pointer_json, is_excluded=self.is_excluded, + AssumableByComputeServices={ + "severity": ISSUE_SEVERITY["AssumableByComputeService"], + "description": RISK_DEFINITION["AssumableByComputeService"], + "findings": ( + self.assume_role_policy_document.role_assumable_by_compute_services + if self.assume_role_policy_document + and ( + ISSUE_SEVERITY["AssumableByComputeService"] in [x.lower() for x in self.severity] + or not self.severity + ) + else [] + ), + }, ) return this_role_detail diff --git a/test/files/scanning/test_role_detail_results.json b/test/files/scanning/test_role_detail_results.json index 3809ee2a..964b1171 100644 --- a/test/files/scanning/test_role_detail_results.json +++ b/test/files/scanning/test_role_detail_results.json @@ -16,7 +16,7 @@ }, "aws_managed_policies": {}, "create_date": "2018-08-20 18:48:00+00:00", - "customer_managed_policies": {"InsecurePolicy": "InsecurePolicy"}, + "customer_managed_policies": { "InsecurePolicy": "InsecurePolicy" }, "id": "OverprivilegedEC2", "inline_policies": { "d09fe3603cd65058b6e2d9817cf37093e83e98318a56ce1e29c8491ac989e57e": "SupYo" @@ -55,5 +55,10 @@ "is_excluded": false, "name": "OverprivilegedEC2", "path": "/", - "role_last_used": "2019-01-20 18:48:00+00:00" + "role_last_used": "2019-01-20 18:48:00+00:00", + "AssumableByComputeServices": { + "description": "

IAM Roles can be assumed by AWS Compute Services (such as EC2, ECS, EKS, or Lambda) can present greater risk than user-defined roles, especially if the AWS Compute service is on an instance that is directly or indirectly exposed to the internet. Flagging these roles is particularly useful to penetration testers (or attackers) under certain scenarios.
For example, if an attacker obtains privileges to execute ssm:SendCommand and there are privileged EC2 instances with the SSM agent installed, they can effectively have the privileges of those EC2 instances.

", + "findings": ["ec2"], + "severity": "low" + } } diff --git a/test/scanning/test_trust_policies.py b/test/scanning/test_trust_policies.py index c19bfe0f..1281237a 100644 --- a/test/scanning/test_trust_policies.py +++ b/test/scanning/test_trust_policies.py @@ -1,6 +1,7 @@ -from cloudsplaining.scan.assume_role_policy_document import AssumeRoleStatement import unittest +from cloudsplaining.scan.assume_role_policy_document import AssumeRoleStatement + class TestAssumeRole(unittest.TestCase): """ @@ -131,3 +132,12 @@ def test_assume_role_assumable_by_compute_services(self): ) assume_role_empty_actions_statement = AssumeRoleStatement(empty_actions_statement) self.assertListEqual(assume_role_empty_actions_statement.actions, []) + + # Case: Deny statement, for a compute service + deny_statement = dict( + Effect="Deny", + Principal={"Service": ["lambda.amazonaws.com"]}, + Action=["sts:AssumeRole"], + ) + assume_role_deny_statement = AssumeRoleStatement(deny_statement) + self.assertListEqual(assume_role_deny_statement.role_assumable_by_compute_services, []) From 52a6cc7b281e0f24bc71ea991d79b541cd4482d5 Mon Sep 17 00:00:00 2001 From: Brian Henderson Date: Thu, 4 Sep 2025 18:55:08 +0200 Subject: [PATCH 2/9] Add AssumableByCrossAccountPrincipal findings to role detail output Surface cross-account assumability findings at the role level to provide better visibility into roles that can be assumed by principals from other AWS accounts, helping security teams identify potential attack surface expansion beyond organizational boundaries. --- .../scan/assume_role_policy_document.py | 43 +++++++++++- cloudsplaining/scan/role_details.py | 25 ++++++- cloudsplaining/shared/constants.py | 2 + .../scanning/test_role_detail_results.json | 5 ++ test/scanning/test_trust_policies.py | 68 +++++++++++++++++++ 5 files changed, 139 insertions(+), 4 deletions(-) diff --git a/cloudsplaining/scan/assume_role_policy_document.py b/cloudsplaining/scan/assume_role_policy_document.py index 9102d5f0..41ac428c 100644 --- a/cloudsplaining/scan/assume_role_policy_document.py +++ b/cloudsplaining/scan/assume_role_policy_document.py @@ -7,9 +7,12 @@ # or https://opensource.org/licenses/BSD-3-Clause from __future__ import annotations +import contextlib import logging from typing import Any +from policy_sentry.util.arns import get_account_from_arn + from cloudsplaining.scan.resource_policy_document import ( ResourcePolicyDocument, ResourceStatement, @@ -25,9 +28,10 @@ class AssumeRolePolicyDocument(ResourcePolicyDocument): It is a specialized version of a Resource-based policy """ - def __init__(self, policy: dict[str, Any]) -> None: + def __init__(self, policy: dict[str, Any], current_account_id: str | None = None) -> None: statement_structure = policy.get("Statement", []) self.policy = policy + self.current_account_id = current_account_id # We would actually need to define a proper base class with a generic type for statements self.statements: list[AssumeRoleStatement] = [] # type:ignore[assignment] # leaving here but excluding from tests because IAM Policy grammar dictates that it must be a list @@ -35,7 +39,7 @@ def __init__(self, policy: dict[str, Any]) -> None: statement_structure = [statement_structure] for statement in statement_structure: - self.statements.append(AssumeRoleStatement(statement)) + self.statements.append(AssumeRoleStatement(statement, current_account_id)) @property def role_assumable_by_compute_services(self) -> list[str]: @@ -46,14 +50,24 @@ def role_assumable_by_compute_services(self) -> list[str]: assumable_by_compute_services.extend(statement.role_assumable_by_compute_services) return assumable_by_compute_services + @property + def role_assumable_by_cross_account_principals(self) -> list[str]: + """Determines whether or not the role can be assumed from principals in other accounts, and if so which ones.""" + assumable_from_other_accounts = [] + for statement in self.statements: + if statement.role_assumable_by_cross_account_principals: + assumable_from_other_accounts.extend(statement.role_assumable_by_cross_account_principals) + return assumable_from_other_accounts + class AssumeRoleStatement(ResourceStatement): """ Statements in an AssumeRole/Trust Policy document """ - def __init__(self, statement: dict[str, Any]) -> None: + def __init__(self, statement: dict[str, Any], current_account_id: str | None = None) -> None: super().__init__(statement=statement) + self.current_account_id = current_account_id # self.not_principal = statement.get("NotPrincipal") if statement.get("NotPrincipal"): @@ -93,3 +107,26 @@ def role_assumable_by_compute_services(self) -> list[str]: if service_prefix_to_evaluate in SERVICE_PREFIXES_WITH_COMPUTE_ROLES: assumable_by_compute_services.append(service_prefix_to_evaluate) return assumable_by_compute_services + + @property + def role_assumable_by_cross_account_principals(self) -> list[str]: + """Determines whether or not the role can be assumed from principals in other accounts, and if so which ones.""" + # sts:AssumeRole must be there + lowercase_actions = [x.lower() for x in self.actions] + if "sts:AssumeRole".lower() not in lowercase_actions: + return [] + + # Effect must be Allow + if self.effect.lower() != "allow": + return [] + + other_account_principals = [] + for principal in self.principals: + # Check if this is an AWS IAM principal from another account + if principal.startswith("arn:aws:iam::"): + with contextlib.suppress(Exception): + principal_account_id = get_account_from_arn(principal) + # Only include if it's from a different account (or if we don't know our current account) + if self.current_account_id is None or principal_account_id != self.current_account_id: + other_account_principals.append(principal) + return other_account_principals diff --git a/cloudsplaining/scan/role_details.py b/cloudsplaining/scan/role_details.py index 99ce25ea..8374ea89 100644 --- a/cloudsplaining/scan/role_details.py +++ b/cloudsplaining/scan/role_details.py @@ -2,10 +2,13 @@ from __future__ import annotations +import contextlib import json import logging from typing import TYPE_CHECKING, Any +from policy_sentry.util.arns import get_account_from_arn + from cloudsplaining.scan.assume_role_policy_document import AssumeRolePolicyDocument from cloudsplaining.scan.inline_policy import InlinePolicy from cloudsplaining.shared import utils @@ -178,7 +181,14 @@ def __init__( self.assume_role_policy_document = None assume_role_policy = role_detail.get("AssumeRolePolicyDocument") if assume_role_policy: - self.assume_role_policy_document = AssumeRolePolicyDocument(assume_role_policy) + # Extract current account ID from role ARN + current_account_id = None + if self.arn: + with contextlib.suppress(Exception): + # If we can't parse the account ID, continue without it + current_account_id = get_account_from_arn(self.arn) + + self.assume_role_policy_document = AssumeRolePolicyDocument(assume_role_policy, current_account_id) # TODO: Create a class for InstanceProfileList self.instance_profile_list = role_detail.get("InstanceProfileList", []) @@ -345,5 +355,18 @@ def json(self) -> dict[str, Any]: else [] ), }, + AssumableByCrossAccountPrincipal={ + "severity": ISSUE_SEVERITY["AssumableByCrossAccountPrincipal"], + "description": RISK_DEFINITION["AssumableByCrossAccountPrincipal"], + "findings": ( + self.assume_role_policy_document.role_assumable_by_cross_account_principals + if self.assume_role_policy_document + and ( + ISSUE_SEVERITY["AssumableByCrossAccountPrincipal"] in [x.lower() for x in self.severity] + or not self.severity + ) + else [] + ), + }, ) return this_role_detail diff --git a/cloudsplaining/shared/constants.py b/cloudsplaining/shared/constants.py index 0fe52dbe..c79d8ecc 100644 --- a/cloudsplaining/shared/constants.py +++ b/cloudsplaining/shared/constants.py @@ -83,6 +83,7 @@ "CredentialsExposure": "high", "InfrastructureModification": "low", "AssumableByComputeService": "low", + "AssumableByCrossAccountPrincipal": "low", } RISK_DEFINITION = { @@ -93,6 +94,7 @@ "CredentialsExposure": "

Credentials Exposure actions return credentials as part of the API response , such as ecr:GetAuthorizationToken, iam:UpdateAccessKey, and others. The full list is maintained here: https://gist.github.com/kmcquade/33860a617e651104d243c324ddf7992a

", "InfrastructureModification": "", "AssumableByComputeService": "

IAM Roles can be assumed by AWS Compute Services (such as EC2, ECS, EKS, or Lambda) can present greater risk than user-defined roles, especially if the AWS Compute service is on an instance that is directly or indirectly exposed to the internet. Flagging these roles is particularly useful to penetration testers (or attackers) under certain scenarios.
For example, if an attacker obtains privileges to execute ssm:SendCommand and there are privileged EC2 instances with the SSM agent installed, they can effectively have the privileges of those EC2 instances.

", + "AssumableByCrossAccountPrincipal": "

Roles that can be assumed from other AWS accounts can present a greater risk than roles that can only be assumed within the same AWS account. This is especially true if the trusting account is not owned by your organization.

", } PRIVILEGE_ESCALATION_METHODS = { diff --git a/test/files/scanning/test_role_detail_results.json b/test/files/scanning/test_role_detail_results.json index 964b1171..597d4f7c 100644 --- a/test/files/scanning/test_role_detail_results.json +++ b/test/files/scanning/test_role_detail_results.json @@ -60,5 +60,10 @@ "description": "

IAM Roles can be assumed by AWS Compute Services (such as EC2, ECS, EKS, or Lambda) can present greater risk than user-defined roles, especially if the AWS Compute service is on an instance that is directly or indirectly exposed to the internet. Flagging these roles is particularly useful to penetration testers (or attackers) under certain scenarios.
For example, if an attacker obtains privileges to execute ssm:SendCommand and there are privileged EC2 instances with the SSM agent installed, they can effectively have the privileges of those EC2 instances.

", "findings": ["ec2"], "severity": "low" + }, + "AssumableByCrossAccountPrincipal": { + "description": "

Roles that can be assumed from other AWS accounts can present a greater risk than roles that can only be assumed within the same AWS account. This is especially true if the trusting account is not owned by your organization.

", + "findings": [], + "severity": "low" } } diff --git a/test/scanning/test_trust_policies.py b/test/scanning/test_trust_policies.py index 1281237a..7dceffd7 100644 --- a/test/scanning/test_trust_policies.py +++ b/test/scanning/test_trust_policies.py @@ -141,3 +141,71 @@ def test_assume_role_assumable_by_compute_services(self): ) assume_role_deny_statement = AssumeRoleStatement(deny_statement) self.assertListEqual(assume_role_deny_statement.role_assumable_by_compute_services, []) + + def test_assume_role_assumable_by_cross_account_principals(self): + """scan.assume_role_policy_document.AssumeRoleStatement.role_assumable_by_cross_account_principals""" + from cloudsplaining.scan.assume_role_policy_document import AssumeRolePolicyDocument + + # Test basic cross-account detection with different principal types + statement_cross_account = dict( + Effect="Allow", + Principal={ + "AWS": [ + "arn:aws:iam::123456789012:root", + "arn:aws:iam::123456789012:user/testuser", + "arn:aws:iam::098765432109:role/testrole", + ] + }, + Action=["sts:AssumeRole"], + ) + assume_role_cross_account = AssumeRoleStatement(statement_cross_account) + expected = [ + "arn:aws:iam::123456789012:root", + "arn:aws:iam::123456789012:user/testuser", + "arn:aws:iam::098765432109:role/testrole", + ] + self.assertListEqual(assume_role_cross_account.role_assumable_by_cross_account_principals, expected) + + # Test current account filtering + assume_role_with_filtering = AssumeRoleStatement(statement_cross_account, current_account_id="123456789012") + self.assertListEqual( + assume_role_with_filtering.role_assumable_by_cross_account_principals, + ["arn:aws:iam::098765432109:role/testrole"], + ) + + # Test conditions that should return empty results + # No sts:AssumeRole action + statement_no_assume_role = dict( + Effect="Allow", Principal={"AWS": "arn:aws:iam::123456789012:root"}, Action=["s3:GetObject"] + ) + self.assertListEqual( + AssumeRoleStatement(statement_no_assume_role).role_assumable_by_cross_account_principals, [] + ) + + # Deny effect + statement_deny = dict( + Effect="Deny", Principal={"AWS": "arn:aws:iam::123456789012:root"}, Action=["sts:AssumeRole"] + ) + self.assertListEqual(AssumeRoleStatement(statement_deny).role_assumable_by_cross_account_principals, []) + + # Service principals only + statement_service = dict( + Effect="Allow", Principal={"Service": "lambda.amazonaws.com"}, Action=["sts:AssumeRole"] + ) + self.assertListEqual(AssumeRoleStatement(statement_service).role_assumable_by_cross_account_principals, []) + + # Test AssumeRolePolicyDocument aggregation + policy = { + "Version": "2012-10-17", + "Statement": [ + {"Effect": "Allow", "Principal": {"AWS": "arn:aws:iam::123456789012:root"}, "Action": "sts:AssumeRole"}, + { + "Effect": "Allow", + "Principal": {"AWS": "arn:aws:iam::098765432109:user/testuser"}, + "Action": "sts:AssumeRole", + }, + ], + } + policy_doc = AssumeRolePolicyDocument(policy) + expected_policy = ["arn:aws:iam::123456789012:root", "arn:aws:iam::098765432109:user/testuser"] + self.assertListEqual(policy_doc.role_assumable_by_cross_account_principals, expected_policy) From 5052df17775712c85f120ef7a82cc2f2456058bd Mon Sep 17 00:00:00 2001 From: Brian Henderson Date: Thu, 4 Sep 2025 19:22:35 +0200 Subject: [PATCH 3/9] Add --flag-trust-policies CLI option to toggle trust policy findings Add a new command line flag --flag-trust-policies that allows users to control whether trust policy findings are included in role detail output, providing more granular control over which security findings are reported. --- cloudsplaining/command/scan.py | 14 +++++ cloudsplaining/command/scan_multi_account.py | 14 +++++ cloudsplaining/scan/authorization_details.py | 3 + cloudsplaining/scan/role_details.py | 63 ++++++++++++-------- test/scanning/test_role_detail_list.py | 7 ++- 5 files changed, 72 insertions(+), 29 deletions(-) diff --git a/cloudsplaining/command/scan.py b/cloudsplaining/command/scan.py index e11f17f2..cb6689b5 100644 --- a/cloudsplaining/command/scan.py +++ b/cloudsplaining/command/scan.py @@ -96,6 +96,14 @@ multiple=True, type=click.Choice(["CRITICAL", "HIGH", "MEDIUM", "LOW", "NONE"], case_sensitive=False), ) +@click.option( + "-t", + "--flag-trust-policies", + required=False, + default=False, + is_flag=True, + help="Flag risky trust policies in roles.", +) def scan( input_file: str, exclusions_file: str, @@ -105,6 +113,7 @@ def scan( flag_all_risky_actions: bool, verbosity: int, severity: list[str], + flag_trust_policies: bool, ) -> None: # pragma: no cover """ Given the path to account authorization details files and the exclusions config file, scan all inline and @@ -142,6 +151,7 @@ def scan( minimize=minimize, flag_conditional_statements=flag_conditional_statements, flag_resource_arn_statements=flag_resource_arn_statements, + flag_trust_policies=flag_trust_policies, severity=severity, ) html_output_file = os.path.join(output, f"iam-report-{account_name}.html") @@ -207,6 +217,7 @@ def scan_account_authorization_details( return_json_results: Literal[True], flag_conditional_statements: bool = ..., flag_resource_arn_statements: bool = ..., + flag_trust_policies: bool = ..., severity: list[str] | None = ..., ) -> dict[str, Any]: ... @@ -222,6 +233,7 @@ def scan_account_authorization_details( return_json_results: Literal[False] = ..., flag_conditional_statements: bool = ..., flag_resource_arn_statements: bool = ..., + flag_trust_policies: bool = ..., severity: list[str] | None = ..., ) -> str: ... @@ -236,6 +248,7 @@ def scan_account_authorization_details( return_json_results: bool = False, flag_conditional_statements: bool = False, flag_resource_arn_statements: bool = False, + flag_trust_policies: bool = False, severity: list[str] | None = None, ) -> str | dict[str, Any]: # pragma: no cover """ @@ -250,6 +263,7 @@ def scan_account_authorization_details( exclusions=exclusions, flag_conditional_statements=flag_conditional_statements, flag_resource_arn_statements=flag_resource_arn_statements, + flag_trust_policies=flag_trust_policies, severity=severity, ) results = authorization_details.results diff --git a/cloudsplaining/command/scan_multi_account.py b/cloudsplaining/command/scan_multi_account.py index 0d7ebe7d..09c6d00c 100644 --- a/cloudsplaining/command/scan_multi_account.py +++ b/cloudsplaining/command/scan_multi_account.py @@ -120,6 +120,14 @@ def _accounts(self) -> dict[str, str]: multiple=True, type=click.Choice(["CRITICAL", "HIGH", "MEDIUM", "LOW", "NONE"], case_sensitive=False), ) +@click.option( + "-t", + "--flag-trust-policies", + required=False, + default=False, + is_flag=True, + help="Flag risky trust policies in roles.", +) def scan_multi_account( config_file: str, profile: str, @@ -131,6 +139,7 @@ def scan_multi_account( flag_all_risky_actions: bool, verbosity: int, severity: list[str], + flag_trust_policies: bool, ) -> None: """Scan multiple accounts via AssumeRole""" set_log_level(verbosity) @@ -160,6 +169,7 @@ def scan_multi_account( severity=severity, flag_conditional_statements=flag_conditional_statements, flag_resource_arn_statements=flag_resource_arn_statements, + flag_trust_policies=flag_trust_policies, ) @@ -174,6 +184,7 @@ def scan_accounts( severity: list[str] | None = None, flag_conditional_statements: bool = False, flag_resource_arn_statements: bool = False, + flag_trust_policies: bool = False, ) -> None: """Use this method as a library to scan multiple accounts""" # TODO: Speed improvements? Multithreading? This currently runs sequentially. @@ -187,6 +198,7 @@ def scan_accounts( severity=severity, flag_conditional_statements=flag_conditional_statements, flag_resource_arn_statements=flag_resource_arn_statements, + flag_trust_policies=flag_trust_policies, ) html_report = HTMLReport( account_id=target_account_id, @@ -233,6 +245,7 @@ def scan_account( severity: list[str] | None = None, flag_conditional_statements: bool = False, flag_resource_arn_statements: bool = False, + flag_trust_policies: bool = False, ) -> dict[str, dict[str, Any]]: """Scan a target account in one shot""" account_authorization_details = download_account_authorization_details( @@ -247,6 +260,7 @@ def scan_account( severity=severity, flag_conditional_statements=flag_conditional_statements, flag_resource_arn_statements=flag_resource_arn_statements, + flag_trust_policies=flag_trust_policies, ) results = authorization_details.results return results diff --git a/cloudsplaining/scan/authorization_details.py b/cloudsplaining/scan/authorization_details.py index 663f988c..498f8f59 100644 --- a/cloudsplaining/scan/authorization_details.py +++ b/cloudsplaining/scan/authorization_details.py @@ -35,6 +35,7 @@ def __init__( exclusions: Exclusions = DEFAULT_EXCLUSIONS, flag_conditional_statements: bool = False, flag_resource_arn_statements: bool = False, + flag_trust_policies: bool = False, severity: list[str] | None = None, ) -> None: """ @@ -53,6 +54,7 @@ def __init__( self.exclusions = exclusions self.flag_conditional_statements = flag_conditional_statements self.flag_resource_arn_statements = flag_resource_arn_statements + self.flag_trust_policies = flag_trust_policies self.policies = ManagedPolicyDetails( auth_json.get("Policies", []), @@ -86,6 +88,7 @@ def __init__( exclusions, flag_conditional_statements=flag_conditional_statements, flag_resource_arn_statements=flag_resource_arn_statements, + flag_trust_policies=flag_trust_policies, severity=severity, ) diff --git a/cloudsplaining/scan/role_details.py b/cloudsplaining/scan/role_details.py index 8374ea89..54e7b6ba 100644 --- a/cloudsplaining/scan/role_details.py +++ b/cloudsplaining/scan/role_details.py @@ -43,6 +43,7 @@ def __init__( exclusions: Exclusions = DEFAULT_EXCLUSIONS, flag_conditional_statements: bool = False, flag_resource_arn_statements: bool = False, + flag_trust_policies: bool = False, severity: list[str] | None = None, ) -> None: self.severity = [] if severity is None else severity @@ -54,6 +55,7 @@ def __init__( # Fix Issue #254 - Allow flagging risky actions even when there are resource constraints self.flag_conditional_statements = flag_conditional_statements self.flag_resource_arn_statements = flag_resource_arn_statements + self.flag_trust_policies = flag_trust_policies self.iam_data: dict[str, dict[Any, Any]] = { "groups": {}, "users": {}, @@ -77,6 +79,7 @@ def __init__( exclusions=exclusions, flag_conditional_statements=self.flag_conditional_statements, flag_resource_arn_statements=self.flag_resource_arn_statements, + flag_trust_policies=flag_trust_policies, severity=self.severity, ) ) @@ -142,6 +145,7 @@ def __init__( exclusions: Exclusions = DEFAULT_EXCLUSIONS, flag_conditional_statements: bool = False, flag_resource_arn_statements: bool = False, + flag_trust_policies: bool = False, severity: list[str] | None = None, ) -> None: """ @@ -170,6 +174,7 @@ def __init__( # Fix Issue #254 - Allow flagging risky actions even when there are resource constraints self.flag_conditional_statements = flag_conditional_statements self.flag_resource_arn_statements = flag_resource_arn_statements + self.flag_trust_policies = flag_trust_policies self.iam_data: dict[str, dict[Any, Any]] = { "groups": {}, @@ -342,31 +347,37 @@ def json(self) -> dict[str, Any]: customer_managed_policies=self.attached_customer_managed_policies_pointer_json, aws_managed_policies=self.attached_aws_managed_policies_pointer_json, is_excluded=self.is_excluded, - AssumableByComputeServices={ - "severity": ISSUE_SEVERITY["AssumableByComputeService"], - "description": RISK_DEFINITION["AssumableByComputeService"], - "findings": ( - self.assume_role_policy_document.role_assumable_by_compute_services - if self.assume_role_policy_document - and ( - ISSUE_SEVERITY["AssumableByComputeService"] in [x.lower() for x in self.severity] - or not self.severity - ) - else [] - ), - }, - AssumableByCrossAccountPrincipal={ - "severity": ISSUE_SEVERITY["AssumableByCrossAccountPrincipal"], - "description": RISK_DEFINITION["AssumableByCrossAccountPrincipal"], - "findings": ( - self.assume_role_policy_document.role_assumable_by_cross_account_principals - if self.assume_role_policy_document - and ( - ISSUE_SEVERITY["AssumableByCrossAccountPrincipal"] in [x.lower() for x in self.severity] - or not self.severity - ) - else [] - ), - }, ) + + if self.flag_trust_policies: + this_role_detail.update( + { + "AssumableByComputeServices": { + "severity": ISSUE_SEVERITY["AssumableByComputeService"], + "description": RISK_DEFINITION["AssumableByComputeService"], + "findings": ( + self.assume_role_policy_document.role_assumable_by_compute_services + if self.assume_role_policy_document + and ( + ISSUE_SEVERITY["AssumableByComputeService"] in [x.lower() for x in self.severity] + or not self.severity + ) + else [] + ), + }, + "AssumableByCrossAccountPrincipal": { + "severity": ISSUE_SEVERITY["AssumableByCrossAccountPrincipal"], + "description": RISK_DEFINITION["AssumableByCrossAccountPrincipal"], + "findings": ( + self.assume_role_policy_document.role_assumable_by_cross_account_principals + if self.assume_role_policy_document + and ( + ISSUE_SEVERITY["AssumableByCrossAccountPrincipal"] in [x.lower() for x in self.severity] + or not self.severity + ) + else [] + ), + }, + } + ) return this_role_detail diff --git a/test/scanning/test_role_detail_list.py b/test/scanning/test_role_detail_list.py index 88df2daf..580e1c0c 100644 --- a/test/scanning/test_role_detail_list.py +++ b/test/scanning/test_role_detail_list.py @@ -1,8 +1,9 @@ +import json import os import unittest -import json -from cloudsplaining.scan.role_details import RoleDetail + from cloudsplaining.scan.managed_policy_detail import ManagedPolicyDetails +from cloudsplaining.scan.role_details import RoleDetail example_authz_details_file = os.path.abspath( os.path.join( @@ -22,7 +23,7 @@ def test_role_detail_attached_managed_policies(self): role_detail_json_input = auth_details_json["RoleDetailList"][2] policy_details = ManagedPolicyDetails(auth_details_json.get("Policies")) - role_detail = RoleDetail(role_detail_json_input, policy_details) + role_detail = RoleDetail(role_detail_json_input, policy_details, flag_trust_policies=True) expected_detail_policy_results_file = os.path.abspath( os.path.join( os.path.dirname(__file__), From 12d6bac22e27e961cbf5dc9db8cd52fc6ed51fa9 Mon Sep 17 00:00:00 2001 From: Brian Henderson Date: Fri, 5 Sep 2025 10:08:27 +0200 Subject: [PATCH 4/9] Add AssumableByAnyPrincipal findings to role detail output Surface roles that can be assumed by any principal (*) or any AWS account root to provide critical visibility into the most dangerous trust policy configurations, helping security teams immediately identify roles that present the highest risk of unauthorized access from any AWS account. --- .../scan/assume_role_policy_document.py | 27 ++++++++ cloudsplaining/scan/role_details.py | 13 ++++ cloudsplaining/shared/constants.py | 4 +- .../scanning/test_role_detail_results.json | 7 ++- test/scanning/test_trust_policies.py | 62 +++++++++++++++++++ 5 files changed, 111 insertions(+), 2 deletions(-) diff --git a/cloudsplaining/scan/assume_role_policy_document.py b/cloudsplaining/scan/assume_role_policy_document.py index 41ac428c..04e783bb 100644 --- a/cloudsplaining/scan/assume_role_policy_document.py +++ b/cloudsplaining/scan/assume_role_policy_document.py @@ -59,6 +59,15 @@ def role_assumable_by_cross_account_principals(self) -> list[str]: assumable_from_other_accounts.extend(statement.role_assumable_by_cross_account_principals) return assumable_from_other_accounts + @property + def role_assumable_by_any_principal(self) -> list[str]: + """Determines whether or not the role can be assumed by any principal (*) or any AWS account root.""" + any_principals = [] + for statement in self.statements: + if statement.role_assumable_by_any_principal: + any_principals.extend(statement.role_assumable_by_any_principal) + return any_principals + class AssumeRoleStatement(ResourceStatement): """ @@ -130,3 +139,21 @@ def role_assumable_by_cross_account_principals(self) -> list[str]: if self.current_account_id is None or principal_account_id != self.current_account_id: other_account_principals.append(principal) return other_account_principals + + @property + def role_assumable_by_any_principal(self) -> list[str]: + """Determines whether or not the role can be assumed by any principal (*) or any AWS account root.""" + # sts:AssumeRole must be there + lowercase_actions = [x.lower() for x in self.actions] + if "sts:AssumeRole".lower() not in lowercase_actions: + return [] + + # Effect must be Allow + if self.effect.lower() != "allow": + return [] + + # Check if any principal is "*" or "arn:aws:iam::*:root" + any_principals = [ + principal for principal in self.principals if principal == "*" or principal == "arn:aws:iam::*:root" + ] + return any_principals diff --git a/cloudsplaining/scan/role_details.py b/cloudsplaining/scan/role_details.py index 54e7b6ba..61e85d42 100644 --- a/cloudsplaining/scan/role_details.py +++ b/cloudsplaining/scan/role_details.py @@ -378,6 +378,19 @@ def json(self) -> dict[str, Any]: else [] ), }, + "AssumableByAnyPrincipal": { + "severity": ISSUE_SEVERITY["AssumableByAnyPrincipal"], + "description": RISK_DEFINITION["AssumableByAnyPrincipal"], + "findings": ( + self.assume_role_policy_document.role_assumable_by_any_principal + if self.assume_role_policy_document + and ( + ISSUE_SEVERITY["AssumableByAnyPrincipal"] in [x.lower() for x in self.severity] + or not self.severity + ) + else [] + ), + }, } ) return this_role_detail diff --git a/cloudsplaining/shared/constants.py b/cloudsplaining/shared/constants.py index c79d8ecc..dacd0e17 100644 --- a/cloudsplaining/shared/constants.py +++ b/cloudsplaining/shared/constants.py @@ -84,6 +84,7 @@ "InfrastructureModification": "low", "AssumableByComputeService": "low", "AssumableByCrossAccountPrincipal": "low", + "AssumableByAnyPrincipal": "high", } RISK_DEFINITION = { @@ -94,7 +95,8 @@ "CredentialsExposure": "

Credentials Exposure actions return credentials as part of the API response , such as ecr:GetAuthorizationToken, iam:UpdateAccessKey, and others. The full list is maintained here: https://gist.github.com/kmcquade/33860a617e651104d243c324ddf7992a

", "InfrastructureModification": "", "AssumableByComputeService": "

IAM Roles can be assumed by AWS Compute Services (such as EC2, ECS, EKS, or Lambda) can present greater risk than user-defined roles, especially if the AWS Compute service is on an instance that is directly or indirectly exposed to the internet. Flagging these roles is particularly useful to penetration testers (or attackers) under certain scenarios.
For example, if an attacker obtains privileges to execute ssm:SendCommand and there are privileged EC2 instances with the SSM agent installed, they can effectively have the privileges of those EC2 instances.

", - "AssumableByCrossAccountPrincipal": "

Roles that can be assumed from other AWS accounts can present a greater risk than roles that can only be assumed within the same AWS account. This is especially true if the trusting account is not owned by your organization.

", + "AssumableByCrossAccountPrincipal": "

IAM Roles that can be assumed from other AWS accounts can present a greater risk than roles that can only be assumed within the same AWS account. This is especially true if the trusting account is not owned by your organization.

", + "AssumableByAnyPrincipal": "

IAM Roles that can be assumed by any principal (i.e. Principal: '*') present a very high risk and should be remediated immediately.

", } PRIVILEGE_ESCALATION_METHODS = { diff --git a/test/files/scanning/test_role_detail_results.json b/test/files/scanning/test_role_detail_results.json index 597d4f7c..f2b5cdf6 100644 --- a/test/files/scanning/test_role_detail_results.json +++ b/test/files/scanning/test_role_detail_results.json @@ -62,8 +62,13 @@ "severity": "low" }, "AssumableByCrossAccountPrincipal": { - "description": "

Roles that can be assumed from other AWS accounts can present a greater risk than roles that can only be assumed within the same AWS account. This is especially true if the trusting account is not owned by your organization.

", + "description": "

IAM Roles that can be assumed from other AWS accounts can present a greater risk than roles that can only be assumed within the same AWS account. This is especially true if the trusting account is not owned by your organization.

", "findings": [], "severity": "low" + }, + "AssumableByAnyPrincipal": { + "description": "

IAM Roles that can be assumed by any principal (i.e. Principal: '*') present a very high risk and should be remediated immediately.

", + "findings": [], + "severity": "high" } } diff --git a/test/scanning/test_trust_policies.py b/test/scanning/test_trust_policies.py index 7dceffd7..0d662914 100644 --- a/test/scanning/test_trust_policies.py +++ b/test/scanning/test_trust_policies.py @@ -209,3 +209,65 @@ def test_assume_role_assumable_by_cross_account_principals(self): policy_doc = AssumeRolePolicyDocument(policy) expected_policy = ["arn:aws:iam::123456789012:root", "arn:aws:iam::098765432109:user/testuser"] self.assertListEqual(policy_doc.role_assumable_by_cross_account_principals, expected_policy) + + def test_assume_role_assumable_by_any_principal(self): + """scan.assume_role_policy_document.AssumeRoleStatement.role_assumable_by_any_principal""" + from cloudsplaining.scan.assume_role_policy_document import AssumeRolePolicyDocument + + # Test wildcard principal "*" + statement_wildcard = dict( + Effect="Allow", + Principal="*", + Action=["sts:AssumeRole"], + ) + assume_role_wildcard = AssumeRoleStatement(statement_wildcard) + self.assertListEqual(assume_role_wildcard.role_assumable_by_any_principal, ["*"]) + + # Test "arn:aws:iam::*:root" principal + statement_any_root = dict( + Effect="Allow", + Principal="arn:aws:iam::*:root", + Action=["sts:AssumeRole"], + ) + assume_role_any_root = AssumeRoleStatement(statement_any_root) + self.assertListEqual(assume_role_any_root.role_assumable_by_any_principal, ["arn:aws:iam::*:root"]) + + # Test both wildcard and any root in the same statement + statement_both = dict( + Effect="Allow", + Principal={"AWS": ["*", "arn:aws:iam::*:root"]}, + Action=["sts:AssumeRole"], + ) + assume_role_both = AssumeRoleStatement(statement_both) + self.assertListEqual(assume_role_both.role_assumable_by_any_principal, ["*", "arn:aws:iam::*:root"]) + + # Test conditions that should return empty results + # No sts:AssumeRole action + statement_no_assume_role = dict(Effect="Allow", Principal="*", Action=["s3:GetObject"]) + self.assertListEqual(AssumeRoleStatement(statement_no_assume_role).role_assumable_by_any_principal, []) + + # Deny effect + statement_deny = dict(Effect="Deny", Principal="*", Action=["sts:AssumeRole"]) + self.assertListEqual(AssumeRoleStatement(statement_deny).role_assumable_by_any_principal, []) + + # Specific principals only (not wildcard) + statement_specific = dict( + Effect="Allow", Principal={"AWS": "arn:aws:iam::123456789012:root"}, Action=["sts:AssumeRole"] + ) + self.assertListEqual(AssumeRoleStatement(statement_specific).role_assumable_by_any_principal, []) + + # Test AssumeRolePolicyDocument aggregation + policy = { + "Version": "2012-10-17", + "Statement": [ + {"Effect": "Allow", "Principal": "*", "Action": "sts:AssumeRole"}, + { + "Effect": "Allow", + "Principal": {"AWS": "arn:aws:iam::*:root"}, + "Action": "sts:AssumeRole", + }, + ], + } + policy_doc = AssumeRolePolicyDocument(policy) + expected_policy = ["*", "arn:aws:iam::*:root"] + self.assertListEqual(policy_doc.role_assumable_by_any_principal, expected_policy) From 05997fff6c7547767b56a9270ed522c846d45a18 Mon Sep 17 00:00:00 2001 From: Brian Henderson Date: Fri, 5 Sep 2025 14:19:29 +0200 Subject: [PATCH 5/9] Add AssumableByAnyPrincipalWithConditions findings to role detail output Surface roles that can be assumed by any principal (*) or any AWS account root when conditions are present to provide enhanced visibility into potentially dangerous trust policy configurations. While conditions may appear to provide security controls, they can be overly permissive or contain logical flaws, helping security teams identify roles that require careful review to ensure conditions adequately restrict access and prevent unintended privilege escalation. --- .../scan/assume_role_policy_document.py | 35 ++++++ cloudsplaining/scan/role_details.py | 14 +++ cloudsplaining/shared/constants.py | 2 + .../scanning/test_role_detail_results.json | 5 + test/scanning/test_trust_policies.py | 111 ++++++++++++++++++ 5 files changed, 167 insertions(+) diff --git a/cloudsplaining/scan/assume_role_policy_document.py b/cloudsplaining/scan/assume_role_policy_document.py index 04e783bb..6c60af27 100644 --- a/cloudsplaining/scan/assume_role_policy_document.py +++ b/cloudsplaining/scan/assume_role_policy_document.py @@ -68,6 +68,15 @@ def role_assumable_by_any_principal(self) -> list[str]: any_principals.extend(statement.role_assumable_by_any_principal) return any_principals + @property + def role_assumable_by_any_principal_with_conditions(self) -> list[str]: + """Determines whether or not the role can be assumed by any principal (*) or any AWS account root with conditions.""" + any_principals_with_conditions = [] + for statement in self.statements: + if statement.role_assumable_by_any_principal_with_conditions: + any_principals_with_conditions.extend(statement.role_assumable_by_any_principal_with_conditions) + return any_principals_with_conditions + class AssumeRoleStatement(ResourceStatement): """ @@ -152,6 +161,32 @@ def role_assumable_by_any_principal(self) -> list[str]: if self.effect.lower() != "allow": return [] + # Must have no conditions + if self.statement.get("Condition"): + return [] + + # Check if any principal is "*" or "arn:aws:iam::*:root" + any_principals = [ + principal for principal in self.principals if principal == "*" or principal == "arn:aws:iam::*:root" + ] + return any_principals + + @property + def role_assumable_by_any_principal_with_conditions(self) -> list[str]: + """Determines whether or not the role can be assumed by any principal (*) or any AWS account root with conditions.""" + # sts:AssumeRole must be there + lowercase_actions = [x.lower() for x in self.actions] + if "sts:AssumeRole".lower() not in lowercase_actions: + return [] + + # Effect must be Allow + if self.effect.lower() != "allow": + return [] + + # Must have conditions (opposite of role_assumable_by_any_principal) + if not self.statement.get("Condition"): + return [] + # Check if any principal is "*" or "arn:aws:iam::*:root" any_principals = [ principal for principal in self.principals if principal == "*" or principal == "arn:aws:iam::*:root" diff --git a/cloudsplaining/scan/role_details.py b/cloudsplaining/scan/role_details.py index 61e85d42..747b8940 100644 --- a/cloudsplaining/scan/role_details.py +++ b/cloudsplaining/scan/role_details.py @@ -391,6 +391,20 @@ def json(self) -> dict[str, Any]: else [] ), }, + "AssumableByAnyPrincipalWithConditions": { + "severity": ISSUE_SEVERITY["AssumableByAnyPrincipalWithConditions"], + "description": RISK_DEFINITION["AssumableByAnyPrincipalWithConditions"], + "findings": ( + self.assume_role_policy_document.role_assumable_by_any_principal_with_conditions + if self.assume_role_policy_document + and ( + ISSUE_SEVERITY["AssumableByAnyPrincipalWithConditions"] + in [x.lower() for x in self.severity] + or not self.severity + ) + else [] + ), + }, } ) return this_role_detail diff --git a/cloudsplaining/shared/constants.py b/cloudsplaining/shared/constants.py index dacd0e17..7372e21d 100644 --- a/cloudsplaining/shared/constants.py +++ b/cloudsplaining/shared/constants.py @@ -85,6 +85,7 @@ "AssumableByComputeService": "low", "AssumableByCrossAccountPrincipal": "low", "AssumableByAnyPrincipal": "high", + "AssumableByAnyPrincipalWithConditions": "medium", } RISK_DEFINITION = { @@ -97,6 +98,7 @@ "AssumableByComputeService": "

IAM Roles can be assumed by AWS Compute Services (such as EC2, ECS, EKS, or Lambda) can present greater risk than user-defined roles, especially if the AWS Compute service is on an instance that is directly or indirectly exposed to the internet. Flagging these roles is particularly useful to penetration testers (or attackers) under certain scenarios.
For example, if an attacker obtains privileges to execute ssm:SendCommand and there are privileged EC2 instances with the SSM agent installed, they can effectively have the privileges of those EC2 instances.

", "AssumableByCrossAccountPrincipal": "

IAM Roles that can be assumed from other AWS accounts can present a greater risk than roles that can only be assumed within the same AWS account. This is especially true if the trusting account is not owned by your organization.

", "AssumableByAnyPrincipal": "

IAM Roles that can be assumed by any principal (i.e. Principal: '*') present a very high risk and should be remediated immediately.

", + "AssumableByAnyPrincipalWithConditions": "

IAM Roles that can be assumed by any principal (i.e. Principal: '*') but have conditions present can lead to unexpected outcomes. The conditions should be carefully reviewed to ensure they are not overly permissive.

", } PRIVILEGE_ESCALATION_METHODS = { diff --git a/test/files/scanning/test_role_detail_results.json b/test/files/scanning/test_role_detail_results.json index f2b5cdf6..92bc5078 100644 --- a/test/files/scanning/test_role_detail_results.json +++ b/test/files/scanning/test_role_detail_results.json @@ -70,5 +70,10 @@ "description": "

IAM Roles that can be assumed by any principal (i.e. Principal: '*') present a very high risk and should be remediated immediately.

", "findings": [], "severity": "high" + }, + "AssumableByAnyPrincipalWithConditions": { + "description": "

IAM Roles that can be assumed by any principal (i.e. Principal: '*') but have conditions present can lead to unexpected outcomes. The conditions should be carefully reviewed to ensure they are not overly permissive.

", + "findings": [], + "severity": "medium" } } diff --git a/test/scanning/test_trust_policies.py b/test/scanning/test_trust_policies.py index 0d662914..ffe730ee 100644 --- a/test/scanning/test_trust_policies.py +++ b/test/scanning/test_trust_policies.py @@ -256,6 +256,15 @@ def test_assume_role_assumable_by_any_principal(self): ) self.assertListEqual(AssumeRoleStatement(statement_specific).role_assumable_by_any_principal, []) + # Wildcard principal with conditions (should return empty) + statement_with_conditions = dict( + Effect="Allow", + Principal="*", + Action=["sts:AssumeRole"], + Condition={"StringEquals": {"aws:sourceip": "203.0.113.0/24"}}, + ) + self.assertListEqual(AssumeRoleStatement(statement_with_conditions).role_assumable_by_any_principal, []) + # Test AssumeRolePolicyDocument aggregation policy = { "Version": "2012-10-17", @@ -271,3 +280,105 @@ def test_assume_role_assumable_by_any_principal(self): policy_doc = AssumeRolePolicyDocument(policy) expected_policy = ["*", "arn:aws:iam::*:root"] self.assertListEqual(policy_doc.role_assumable_by_any_principal, expected_policy) + + def test_assume_role_assumable_by_any_principal_with_conditions(self): + """scan.assume_role_policy_document.AssumeRoleStatement.role_assumable_by_any_principal_with_conditions""" + from cloudsplaining.scan.assume_role_policy_document import AssumeRolePolicyDocument + + # Test wildcard principal "*" with conditions + statement_wildcard_with_conditions = dict( + Effect="Allow", + Principal="*", + Action=["sts:AssumeRole"], + Condition={"StringEquals": {"aws:sourceip": "203.0.113.0/24"}}, + ) + assume_role_wildcard_conditions = AssumeRoleStatement(statement_wildcard_with_conditions) + self.assertListEqual(assume_role_wildcard_conditions.role_assumable_by_any_principal_with_conditions, ["*"]) + + # Test "arn:aws:iam::*:root" principal with conditions + statement_any_root_with_conditions = dict( + Effect="Allow", + Principal="arn:aws:iam::*:root", + Action=["sts:AssumeRole"], + Condition={"StringEquals": {"sts:ExternalId": "unique-id"}}, + ) + assume_role_any_root_conditions = AssumeRoleStatement(statement_any_root_with_conditions) + self.assertListEqual( + assume_role_any_root_conditions.role_assumable_by_any_principal_with_conditions, ["arn:aws:iam::*:root"] + ) + + # Test both wildcard and any root with conditions + statement_both_with_conditions = dict( + Effect="Allow", + Principal={"AWS": ["*", "arn:aws:iam::*:root"]}, + Action=["sts:AssumeRole"], + Condition={"StringEquals": {"aws:RequestedRegion": "us-east-1"}}, + ) + assume_role_both_conditions = AssumeRoleStatement(statement_both_with_conditions) + self.assertListEqual( + assume_role_both_conditions.role_assumable_by_any_principal_with_conditions, ["*", "arn:aws:iam::*:root"] + ) + + # Test conditions that should return empty results + # No sts:AssumeRole action + statement_no_assume_role = dict( + Effect="Allow", + Principal="*", + Action=["s3:GetObject"], + Condition={"StringEquals": {"aws:sourceip": "203.0.113.0/24"}}, + ) + self.assertListEqual( + AssumeRoleStatement(statement_no_assume_role).role_assumable_by_any_principal_with_conditions, [] + ) + + # Deny effect + statement_deny = dict( + Effect="Deny", + Principal="*", + Action=["sts:AssumeRole"], + Condition={"StringEquals": {"aws:sourceip": "203.0.113.0/24"}}, + ) + self.assertListEqual(AssumeRoleStatement(statement_deny).role_assumable_by_any_principal_with_conditions, []) + + # Wildcard principal without conditions (should return empty) + statement_no_conditions = dict( + Effect="Allow", + Principal="*", + Action=["sts:AssumeRole"], + ) + self.assertListEqual( + AssumeRoleStatement(statement_no_conditions).role_assumable_by_any_principal_with_conditions, [] + ) + + # Specific principals only (not wildcard) with conditions + statement_specific_with_conditions = dict( + Effect="Allow", + Principal={"AWS": "arn:aws:iam::123456789012:root"}, + Action=["sts:AssumeRole"], + Condition={"StringEquals": {"aws:sourceip": "203.0.113.0/24"}}, + ) + self.assertListEqual( + AssumeRoleStatement(statement_specific_with_conditions).role_assumable_by_any_principal_with_conditions, [] + ) + + # Test AssumeRolePolicyDocument aggregation + policy = { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": "*", + "Action": "sts:AssumeRole", + "Condition": {"StringEquals": {"aws:sourceip": "203.0.113.0/24"}}, + }, + { + "Effect": "Allow", + "Principal": {"AWS": "arn:aws:iam::*:root"}, + "Action": "sts:AssumeRole", + "Condition": {"StringEquals": {"sts:ExternalId": "unique-id"}}, + }, + ], + } + policy_doc = AssumeRolePolicyDocument(policy) + expected_policy = ["*", "arn:aws:iam::*:root"] + self.assertListEqual(policy_doc.role_assumable_by_any_principal_with_conditions, expected_policy) From aab032d6c112c78e1555627bf23afca3ee528913 Mon Sep 17 00:00:00 2001 From: Brian Henderson Date: Fri, 5 Sep 2025 15:15:20 +0200 Subject: [PATCH 6/9] Add known-accounts exclusions to reduce cross-account principal false positives Allow users to specify known AWS account IDs in the exclusions configuration to filter out trusted accounts from cross-account assumability findings, reducing noise from legitimate organizational accounts and third-party vendor accounts while maintaining security visibility for unknown external principals. --- .../scan/assume_role_policy_document.py | 27 ++++++++++-- cloudsplaining/scan/role_details.py | 4 +- cloudsplaining/shared/constants.py | 8 +++- cloudsplaining/shared/default-exclusions.yml | 7 ++- cloudsplaining/shared/exclusions.py | 7 +++ cloudsplaining/shared/validation.py | 1 + test/files/test-exclusions.yml | 7 ++- test/scanning/test_trust_policies.py | 43 +++++++++++++++++-- 8 files changed, 92 insertions(+), 12 deletions(-) diff --git a/cloudsplaining/scan/assume_role_policy_document.py b/cloudsplaining/scan/assume_role_policy_document.py index 6c60af27..f4cd1e28 100644 --- a/cloudsplaining/scan/assume_role_policy_document.py +++ b/cloudsplaining/scan/assume_role_policy_document.py @@ -18,6 +18,10 @@ ResourceStatement, ) from cloudsplaining.shared.constants import SERVICE_PREFIXES_WITH_COMPUTE_ROLES +from cloudsplaining.shared.exclusions import ( + DEFAULT_EXCLUSIONS, + Exclusions, +) logger = logging.getLogger(__name__) @@ -28,18 +32,24 @@ class AssumeRolePolicyDocument(ResourcePolicyDocument): It is a specialized version of a Resource-based policy """ - def __init__(self, policy: dict[str, Any], current_account_id: str | None = None) -> None: + def __init__( + self, + policy: dict[str, Any], + current_account_id: str | None = None, + exclusions: Exclusions = DEFAULT_EXCLUSIONS, + ) -> None: statement_structure = policy.get("Statement", []) self.policy = policy self.current_account_id = current_account_id # We would actually need to define a proper base class with a generic type for statements self.statements: list[AssumeRoleStatement] = [] # type:ignore[assignment] + self.exclusions = exclusions # leaving here but excluding from tests because IAM Policy grammar dictates that it must be a list if not isinstance(statement_structure, list): # pragma: no cover statement_structure = [statement_structure] for statement in statement_structure: - self.statements.append(AssumeRoleStatement(statement, current_account_id)) + self.statements.append(AssumeRoleStatement(statement, current_account_id, exclusions)) @property def role_assumable_by_compute_services(self) -> list[str]: @@ -83,9 +93,15 @@ class AssumeRoleStatement(ResourceStatement): Statements in an AssumeRole/Trust Policy document """ - def __init__(self, statement: dict[str, Any], current_account_id: str | None = None) -> None: + def __init__( + self, + statement: dict[str, Any], + current_account_id: str | None = None, + exclusions: Exclusions = DEFAULT_EXCLUSIONS, + ) -> None: super().__init__(statement=statement) self.current_account_id = current_account_id + self.exclusions = exclusions # self.not_principal = statement.get("NotPrincipal") if statement.get("NotPrincipal"): @@ -145,7 +161,10 @@ def role_assumable_by_cross_account_principals(self) -> list[str]: with contextlib.suppress(Exception): principal_account_id = get_account_from_arn(principal) # Only include if it's from a different account (or if we don't know our current account) - if self.current_account_id is None or principal_account_id != self.current_account_id: + # and the account is not in the known-accounts exclusion list + if ( + self.current_account_id is None or principal_account_id != self.current_account_id + ) and principal_account_id not in self.exclusions.known_accounts: other_account_principals.append(principal) return other_account_principals diff --git a/cloudsplaining/scan/role_details.py b/cloudsplaining/scan/role_details.py index 747b8940..c85da48c 100644 --- a/cloudsplaining/scan/role_details.py +++ b/cloudsplaining/scan/role_details.py @@ -193,7 +193,9 @@ def __init__( # If we can't parse the account ID, continue without it current_account_id = get_account_from_arn(self.arn) - self.assume_role_policy_document = AssumeRolePolicyDocument(assume_role_policy, current_account_id) + self.assume_role_policy_document = AssumeRolePolicyDocument( + assume_role_policy, current_account_id, exclusions + ) # TODO: Create a class for InstanceProfileList self.instance_profile_list = role_detail.get("InstanceProfileList", []) diff --git a/cloudsplaining/shared/constants.py b/cloudsplaining/shared/constants.py index 7372e21d..9bc06257 100644 --- a/cloudsplaining/shared/constants.py +++ b/cloudsplaining/shared/constants.py @@ -54,6 +54,12 @@ # Write actions to include from the results, such as kms:Decrypt exclude-actions: - "" +# Known AWS Account IDs to exclude from assume role analysis. +# Add your organization's account IDs and any thrid party vendor's +# account IDs here to avoid false positives. +# https://github.com/fwdcloudsec/known_aws_accounts/blob/main/accounts.yaml +known-accounts: + - "" """ MULTI_ACCOUNT_CONFIG_TEMPLATE = """accounts: @@ -83,7 +89,7 @@ "CredentialsExposure": "high", "InfrastructureModification": "low", "AssumableByComputeService": "low", - "AssumableByCrossAccountPrincipal": "low", + "AssumableByCrossAccountPrincipal": "medium", "AssumableByAnyPrincipal": "high", "AssumableByAnyPrincipalWithConditions": "medium", } diff --git a/cloudsplaining/shared/default-exclusions.yml b/cloudsplaining/shared/default-exclusions.yml index bdd5b190..0479bd04 100644 --- a/cloudsplaining/shared/default-exclusions.yml +++ b/cloudsplaining/shared/default-exclusions.yml @@ -31,4 +31,9 @@ include-actions: # Write actions to include from the results, such as kms:Decrypt exclude-actions: - "" - +# Known AWS Account IDs to exclude from assume role analysis. +# Add your organization's account IDs and any thrid party vendor's +# account IDs here to avoid false positives. +# https://github.com/fwdcloudsec/known_aws_accounts/blob/main/accounts.yaml +known-accounts: + - "" diff --git a/cloudsplaining/shared/exclusions.py b/cloudsplaining/shared/exclusions.py index 529f0411..006a89b4 100644 --- a/cloudsplaining/shared/exclusions.py +++ b/cloudsplaining/shared/exclusions.py @@ -28,6 +28,7 @@ def __init__(self, exclusions_config: dict[str, list[str]] = DEFAULT_EXCLUSIONS_ self.users = self._users() self.groups = self._groups() self.policies = self._policies() + self.known_accounts = self._known_accounts() def _roles(self) -> list[str]: provided_roles = self.config.get("roles", []) @@ -65,6 +66,12 @@ def _exclude_actions(self) -> list[str]: always_exclude_actions = [x.lower() for x in exclude_actions] return always_exclude_actions + def _known_accounts(self) -> list[str]: + provided_known_accounts = self.config.get("known-accounts", []) + # Normalize for comparisons - remove empty strings + known_accounts = [account for account in provided_known_accounts if account.strip()] + return known_accounts + def is_action_always_included(self, action_in_question: str) -> bool | str: """ Supply an IAM action, and get a decision about whether or not it is excluded. diff --git a/cloudsplaining/shared/validation.py b/cloudsplaining/shared/validation.py index 678c6c57..23c673de 100644 --- a/cloudsplaining/shared/validation.py +++ b/cloudsplaining/shared/validation.py @@ -22,6 +22,7 @@ Optional("groups"): [str], Optional("exclude-actions"): [str], Optional("include-actions"): [str], + Optional("known-accounts"): [str], } ) diff --git a/test/files/test-exclusions.yml b/test/files/test-exclusions.yml index 7aa229a6..9ba8b7c2 100644 --- a/test/files/test-exclusions.yml +++ b/test/files/test-exclusions.yml @@ -29,4 +29,9 @@ include-actions: # Write actions to include from the results, such as kms:Decrypt exclude-actions: - "" - +# Known AWS Account IDs to exclude from assume role analysis. +# Add your organization's account IDs and any thrid party vendor's +# account IDs here to avoid false positives. +# https://github.com/fwdcloudsec/known_aws_accounts/blob/main/accounts.yaml +known-accounts: + - "" diff --git a/test/scanning/test_trust_policies.py b/test/scanning/test_trust_policies.py index ffe730ee..2943caad 100644 --- a/test/scanning/test_trust_policies.py +++ b/test/scanning/test_trust_policies.py @@ -1,6 +1,7 @@ import unittest -from cloudsplaining.scan.assume_role_policy_document import AssumeRoleStatement +from cloudsplaining.scan.assume_role_policy_document import AssumeRolePolicyDocument, AssumeRoleStatement +from cloudsplaining.shared.exclusions import Exclusions class TestAssumeRole(unittest.TestCase): @@ -144,7 +145,6 @@ def test_assume_role_assumable_by_compute_services(self): def test_assume_role_assumable_by_cross_account_principals(self): """scan.assume_role_policy_document.AssumeRoleStatement.role_assumable_by_cross_account_principals""" - from cloudsplaining.scan.assume_role_policy_document import AssumeRolePolicyDocument # Test basic cross-account detection with different principal types statement_cross_account = dict( @@ -173,6 +173,34 @@ def test_assume_role_assumable_by_cross_account_principals(self): ["arn:aws:iam::098765432109:role/testrole"], ) + # Test known accounts exclusions + exclusions_config = {"known-accounts": ["123456789012", "098765432109"]} + exclusions = Exclusions(exclusions_config) + assume_role_with_exclusions = AssumeRoleStatement(statement_cross_account, exclusions=exclusions) + # All accounts should be excluded since they're in the known-accounts list + self.assertListEqual(assume_role_with_exclusions.role_assumable_by_cross_account_principals, []) + + # Test partial exclusions - only exclude one account + partial_exclusions_config = {"known-accounts": ["123456789012"]} + partial_exclusions = Exclusions(partial_exclusions_config) + assume_role_partial_exclusions = AssumeRoleStatement(statement_cross_account, exclusions=partial_exclusions) + # Only the principals from non-excluded accounts should remain + self.assertListEqual( + assume_role_partial_exclusions.role_assumable_by_cross_account_principals, + ["arn:aws:iam::098765432109:role/testrole"], + ) + + # Test exclusions with current account filtering combined + assume_role_combined_filtering = AssumeRoleStatement( + statement_cross_account, current_account_id="123456789012", exclusions=partial_exclusions + ) + # Current account filtering should happen first, then exclusions applied + # Since 123456789012 is the current account AND in exclusions, 098765432109 should remain + self.assertListEqual( + assume_role_combined_filtering.role_assumable_by_cross_account_principals, + ["arn:aws:iam::098765432109:role/testrole"], + ) + # Test conditions that should return empty results # No sts:AssumeRole action statement_no_assume_role = dict( @@ -210,9 +238,17 @@ def test_assume_role_assumable_by_cross_account_principals(self): expected_policy = ["arn:aws:iam::123456789012:root", "arn:aws:iam::098765432109:user/testuser"] self.assertListEqual(policy_doc.role_assumable_by_cross_account_principals, expected_policy) + # Test AssumeRolePolicyDocument with exclusions + partial_exclusions_config_doc = {"known-accounts": ["123456789012"]} + partial_exclusions_doc = Exclusions(partial_exclusions_config_doc) + policy_doc_with_exclusions = AssumeRolePolicyDocument(policy, exclusions=partial_exclusions_doc) + expected_policy_with_exclusions = ["arn:aws:iam::098765432109:user/testuser"] + self.assertListEqual( + policy_doc_with_exclusions.role_assumable_by_cross_account_principals, expected_policy_with_exclusions + ) + def test_assume_role_assumable_by_any_principal(self): """scan.assume_role_policy_document.AssumeRoleStatement.role_assumable_by_any_principal""" - from cloudsplaining.scan.assume_role_policy_document import AssumeRolePolicyDocument # Test wildcard principal "*" statement_wildcard = dict( @@ -283,7 +319,6 @@ def test_assume_role_assumable_by_any_principal(self): def test_assume_role_assumable_by_any_principal_with_conditions(self): """scan.assume_role_policy_document.AssumeRoleStatement.role_assumable_by_any_principal_with_conditions""" - from cloudsplaining.scan.assume_role_policy_document import AssumeRolePolicyDocument # Test wildcard principal "*" with conditions statement_wildcard_with_conditions = dict( From 47dcc00a70f0d3ed7d43edb6f3337eaeac6cd382 Mon Sep 17 00:00:00 2001 From: Brian Henderson Date: Mon, 8 Sep 2025 09:03:36 +0200 Subject: [PATCH 7/9] Add documentation for new trust policy findings Provide documentation for the three new trust policy findings (AssumableByCrossAccountPrincipal, AssumableByAnyPrincipal, and AssumableByAnyPrincipalWithConditions) to help security teams understand the risks associated with each type of role assumption configuration and provide actionable guidance for remediation and review. --- cloudsplaining/shared/constants.py | 2 +- ...umable-by-any-principal-with-conditions.md | 45 +++++++++++ .../roles-assumable-by-any-principal.md | 26 ++++++ .../roles-assumable-by-compute-service.md | 3 - ...es-assumable-by-cross-account-principal.md | 15 ++++ mkdocs.yml | 79 ++++++++++--------- .../scanning/test_role_detail_results.json | 4 +- 7 files changed, 130 insertions(+), 44 deletions(-) create mode 100644 docs/glossary/roles-assumable-by-any-principal-with-conditions.md create mode 100644 docs/glossary/roles-assumable-by-any-principal.md create mode 100644 docs/glossary/roles-assumable-by-cross-account-principal.md diff --git a/cloudsplaining/shared/constants.py b/cloudsplaining/shared/constants.py index 9bc06257..49491fe1 100644 --- a/cloudsplaining/shared/constants.py +++ b/cloudsplaining/shared/constants.py @@ -90,7 +90,7 @@ "InfrastructureModification": "low", "AssumableByComputeService": "low", "AssumableByCrossAccountPrincipal": "medium", - "AssumableByAnyPrincipal": "high", + "AssumableByAnyPrincipal": "critical", "AssumableByAnyPrincipalWithConditions": "medium", } diff --git a/docs/glossary/roles-assumable-by-any-principal-with-conditions.md b/docs/glossary/roles-assumable-by-any-principal-with-conditions.md new file mode 100644 index 00000000..b1ad38dc --- /dev/null +++ b/docs/glossary/roles-assumable-by-any-principal-with-conditions.md @@ -0,0 +1,45 @@ +IAM Roles that can be assumed by any principal (i.e. Principal: '*') but have conditions present can lead to unexpected outcomes. The conditions should be carefully reviewed to ensure they are not overly permissive. + +While the presence of conditions may appear to provide security controls, these configurations still present significant risk and require careful evaluation. The conditions may: + +- **Be overly permissive** - Allow broader access than intended +- **Contain logical flaws** - Have gaps that can be exploited +- **Be circumventable** - Use conditions that can be easily satisfied by attackers +- **Provide false security** - Give the impression of protection while being ineffective + +### Common Risky Condition Examples + +1. **IP-based restrictions that are too broad** - Allowing entire IP ranges instead of specific addresses +2. **Time-based conditions** - Allowing access during broad time windows +3. **User agent conditions** - Easily spoofable browser or client identifiers +4. **Weak external ID requirements** - Using predictable or well-known external IDs +5. **MFA conditions without proper validation** - Not properly verifying MFA device ownership + +### Why This Still Represents Risk + +Even with conditions, these roles are fundamentally different from properly scoped trust policies because: + +- **Attack surface remains large** - Any AWS user can attempt to assume the role +- **Condition bypass potential** - Attackers may find ways to satisfy the conditions +- **Complexity increases risk** - More complex policies are harder to audit and maintain +- **Future changes may weaken security** - Policy updates might inadvertently relax conditions + +### Recommended Actions + +1. **Audit conditions thoroughly** - Review each condition for potential weaknesses or bypass methods +2. **Consider principal-specific alternatives** - Replace wildcard principals with specific account IDs or ARNs where possible +3. **Implement defense in depth** - Use multiple complementary conditions rather than relying on a single control +4. **Regular review and testing** - Periodically test that conditions work as expected and cannot be bypassed +5. **Monitor assumption attempts** - Set up CloudTrail monitoring for role assumption attempts that fail condition checks +6. **Document intended use cases** - Clearly document why wildcard principals are necessary and what the conditions are meant to protect against + +### Best Practices for Conditions + +If wildcard principals are truly necessary, ensure conditions are: +- **Specific and restrictive** - Use narrow ranges and exact matches where possible +- **Multi-layered** - Combine multiple condition types for stronger protection +- **Regularly updated** - Keep IP ranges, time windows, and other dynamic values current +- **Properly tested** - Verify that legitimate use cases work and unauthorized access is blocked +- **Well-documented** - Maintain clear documentation of the security model and assumptions + +Roles with this finding should be prioritized for review to ensure the conditions provide adequate protection and align with your organization's security requirements. diff --git a/docs/glossary/roles-assumable-by-any-principal.md b/docs/glossary/roles-assumable-by-any-principal.md new file mode 100644 index 00000000..45179f83 --- /dev/null +++ b/docs/glossary/roles-assumable-by-any-principal.md @@ -0,0 +1,26 @@ +IAM Roles that can be assumed by any principal (i.e. Principal: '*') present a very high risk and should be remediated immediately. + +These configurations are extremely dangerous because they effectively make the role publicly assumable by anyone with valid AWS credentials, regardless of which AWS account they belong to. This creates a critical security vulnerability that can lead to: + +- **Unauthorized access**: Any AWS user from any account can assume the role +- **Privilege escalation**: Attackers with minimal AWS access can gain elevated permissions +- **Data breaches**: If the role has access to sensitive resources, those become exposed +- **Compliance violations**: Many security frameworks prohibit such open access patterns + +### Common Scenarios Where This Occurs + +1. **Misconfigured cross-account access** - Attempting to allow access from multiple accounts but using wildcard instead of specific account IDs +2. **Development/testing shortcuts** - Using overly permissive policies during development that make it to production +3. **Copy-paste errors** - Reusing policy templates without proper customization + +### Immediate Remediation Required + +Roles with this finding should be considered a **critical security incident** and require immediate attention: + +1. **Identify the intended use case** - Determine what legitimate access the role was meant to provide +2. **Replace wildcards with specific principals** - Use exact account IDs, user ARNs, or service principals +3. **Add conditions** - Implement additional constraints like IP restrictions, MFA requirements, or time-based access +4. **Review role permissions** - Ensure the role follows principle of least privilege +5. **Audit access logs** - Check CloudTrail for any unauthorized role assumptions + +Unlike roles with conditions that might provide some protection, roles flagged with this finding have **no safeguards** and should be treated as publicly accessible resources. diff --git a/docs/glossary/roles-assumable-by-compute-service.md b/docs/glossary/roles-assumable-by-compute-service.md index a0c2aa90..b595fe85 100644 --- a/docs/glossary/roles-assumable-by-compute-service.md +++ b/docs/glossary/roles-assumable-by-compute-service.md @@ -1,6 +1,3 @@ -## Roles Assumable by Compute Services - IAM Roles can be assumed by AWS Compute Services (such as EC2, ECS, EKS, or Lambda) can present greater risk than user-defined roles, especially if the AWS Compute service is on an instance that is directly or indirectly exposed to the internet. Flagging these roles is particularly useful to penetration testers (or attackers) under certain scenarios. For example, if an attacker obtains privileges to execute `ssm:SendCommand` and there are privileged EC2 instances with the SSM agent installed, they can effectively have the privileges of those EC2 instances. Remote Code Execution via AWS Systems Manager Agent was already a known escalation/exploitation path, but Cloudsplaining can make the process of identifying theses cases easier. - diff --git a/docs/glossary/roles-assumable-by-cross-account-principal.md b/docs/glossary/roles-assumable-by-cross-account-principal.md new file mode 100644 index 00000000..a72c4b84 --- /dev/null +++ b/docs/glossary/roles-assumable-by-cross-account-principal.md @@ -0,0 +1,15 @@ +IAM Roles that can be assumed from other AWS accounts can present a greater risk than roles that can only be assumed within the same AWS account. This is especially true if the trusting account is not owned by your organization. + +Cross-account role assumption is a common pattern for legitimate use cases such as: +- Service providers accessing customer resources +- Third-party integrations +- Multi-account AWS Organizations setups +- Partner integrations + +However, these configurations require careful review to ensure that: +- The external account is trusted and belongs to your organization or a legitimate partner +- The role's permissions are appropriately scoped for the cross-account use case +- Monitoring and logging are in place to track cross-account access +- The trust relationship includes appropriate conditions to limit access scope + +Roles flagged with this finding should be reviewed to verify that cross-account access is intentional and properly secured. Pay particular attention to roles that grant broad permissions or access to sensitive resources, as compromise of the external account could lead to unauthorized access to your AWS resources. diff --git a/mkdocs.yml b/mkdocs.yml index 9befda47..10abdb80 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -6,11 +6,11 @@ repo_url: https://github.com/salesforce/cloudsplaining theme: material use_directory_urls: true markdown_extensions: - - codehilite - - tables - - admonition - - pymdownx.details - - pymdownx.superfences + - codehilite + - tables + - admonition + - pymdownx.details + - pymdownx.superfences plugins: - search - mkdocstrings: @@ -22,45 +22,48 @@ plugins: watch: - cloudsplaining/ nav: - - "Home": 'index.md' + - "Home": "index.md" - "User Guide": - - Introduction: 'user-guide/overview.md' - - Installation: 'user-guide/installation.md' - - Downloading Account IAM Details: 'user-guide/download.md' - - Creating an Exclusions File: 'user-guide/create-exclusions-file.md' - - Scanning a single AWS account: 'user-guide/scan-account.md' - - Scanning a single policy: 'user-guide/scan-policy-file.md' - - Scanning multiple AWS accounts: 'user-guide/scan-multiple-accounts.md' - - Troubleshooting: 'user-guide/troubleshooting.md' + - Introduction: "user-guide/overview.md" + - Installation: "user-guide/installation.md" + - Downloading Account IAM Details: "user-guide/download.md" + - Creating an Exclusions File: "user-guide/create-exclusions-file.md" + - Scanning a single AWS account: "user-guide/scan-account.md" + - Scanning a single policy: "user-guide/scan-policy-file.md" + - Scanning multiple AWS accounts: "user-guide/scan-multiple-accounts.md" + - Troubleshooting: "user-guide/troubleshooting.md" - "Report Contents": - - Overview: 'report/overview.md' - - Triage: 'report/triage.md' - - Remediation: 'report/remediation.md' - - Validation: 'report/validation.md' + - Overview: "report/overview.md" + - Triage: "report/triage.md" + - Remediation: "report/remediation.md" + - Validation: "report/validation.md" - "Glossary": - - Privilege Escalation: 'glossary/privilege-escalation.md' - - Resource Exposure: 'glossary/resource-exposure.md' - - Data Exfiltration: 'glossary/data-exfiltration.md' - - Credentials Exposure: 'glossary/credentials-exposure.md' - - Infrastructure Modification: 'glossary/infrastructure-modification.md' - - Service Wildcard: 'glossary/service-wildcard.md' - - Roles Assumable by Compute Service: 'glossary/roles-assumable-by-compute-service.md' - - Trust Policy: 'glossary/trust-policy.md' + - Privilege Escalation: "glossary/privilege-escalation.md" + - Resource Exposure: "glossary/resource-exposure.md" + - Data Exfiltration: "glossary/data-exfiltration.md" + - Credentials Exposure: "glossary/credentials-exposure.md" + - Infrastructure Modification: "glossary/infrastructure-modification.md" + - Service Wildcard: "glossary/service-wildcard.md" + - Roles Assumable by Any Principal: "glossary/roles-assumable-by-any-principal.md" + - Roles Assumable by Any Principal with Condition: "glossary/roles-assumable-by-any-principal-with-conditions.md" + - Roles Assumable by Compute Service: "glossary/roles-assumable-by-compute-service.md" + - Roles Assumable by Cross Account Principal: "glossary/roles-assumable-by-cross-account-principal.md" + - Trust Policy: "glossary/trust-policy.md" - "Contributing": - - Contributing: 'contributing/contributing.md' - - Documentation: 'contributing/documentation.md' - - JavaScript: 'contributing/javascript.md' - - Python: 'contributing/python.md' - - JSON Schema: 'contributing/results-json-schema.md' - - Release Drafter: 'contributing/release-drafter.md' - - Report Generation: 'contributing/report.md' - - Testing: 'contributing/testing.md' - - Versioning: 'contributing/versioning.md' + - Contributing: "contributing/contributing.md" + - Documentation: "contributing/documentation.md" + - JavaScript: "contributing/javascript.md" + - Python: "contributing/python.md" + - JSON Schema: "contributing/results-json-schema.md" + - Release Drafter: "contributing/release-drafter.md" + - Report Generation: "contributing/report.md" + - Testing: "contributing/testing.md" + - Versioning: "contributing/versioning.md" - "Appendices": - - FAQ: 'appendices/faq.md' - - Comparison to other tools: 'appendices/comparison-to-other-tools.md' - - Jira Ticket Automation: 'appendices/jira-ticket-automation.md' + - FAQ: "appendices/faq.md" + - Comparison to other tools: "appendices/comparison-to-other-tools.md" + - Jira Ticket Automation: "appendices/jira-ticket-automation.md" diff --git a/test/files/scanning/test_role_detail_results.json b/test/files/scanning/test_role_detail_results.json index 92bc5078..5571ac22 100644 --- a/test/files/scanning/test_role_detail_results.json +++ b/test/files/scanning/test_role_detail_results.json @@ -64,12 +64,12 @@ "AssumableByCrossAccountPrincipal": { "description": "

IAM Roles that can be assumed from other AWS accounts can present a greater risk than roles that can only be assumed within the same AWS account. This is especially true if the trusting account is not owned by your organization.

", "findings": [], - "severity": "low" + "severity": "medium" }, "AssumableByAnyPrincipal": { "description": "

IAM Roles that can be assumed by any principal (i.e. Principal: '*') present a very high risk and should be remediated immediately.

", "findings": [], - "severity": "high" + "severity": "critical" }, "AssumableByAnyPrincipalWithConditions": { "description": "

IAM Roles that can be assumed by any principal (i.e. Principal: '*') but have conditions present can lead to unexpected outcomes. The conditions should be carefully reviewed to ensure they are not overly permissive.

", From e226ccc52c944cd7039aec7cb82304734f45f4fc Mon Sep 17 00:00:00 2001 From: Brian Henderson Date: Fri, 26 Sep 2025 10:02:49 +0200 Subject: [PATCH 8/9] Add trust policy severity caching to avoid repeated list scans Streamlined the trust policy aggregation to eliminate repeated loops over statements and cache the severity filter before building findings to improve performance. --- .../scan/assume_role_policy_document.py | 44 ++++++++++--------- cloudsplaining/scan/role_details.py | 19 +++----- 2 files changed, 29 insertions(+), 34 deletions(-) diff --git a/cloudsplaining/scan/assume_role_policy_document.py b/cloudsplaining/scan/assume_role_policy_document.py index f4cd1e28..55e933c6 100644 --- a/cloudsplaining/scan/assume_role_policy_document.py +++ b/cloudsplaining/scan/assume_role_policy_document.py @@ -54,38 +54,42 @@ def __init__( @property def role_assumable_by_compute_services(self) -> list[str]: """Determines whether or not the role is assumed from a compute service, and if so which ones.""" - assumable_by_compute_services = [] - for statement in self.statements: - if statement.role_assumable_by_compute_services: - assumable_by_compute_services.extend(statement.role_assumable_by_compute_services) - return assumable_by_compute_services + return [ + principal + for statement in self.statements + if statement.role_assumable_by_compute_services + for principal in statement.role_assumable_by_compute_services + ] @property def role_assumable_by_cross_account_principals(self) -> list[str]: """Determines whether or not the role can be assumed from principals in other accounts, and if so which ones.""" - assumable_from_other_accounts = [] - for statement in self.statements: - if statement.role_assumable_by_cross_account_principals: - assumable_from_other_accounts.extend(statement.role_assumable_by_cross_account_principals) - return assumable_from_other_accounts + return [ + principal + for statement in self.statements + if statement.role_assumable_by_cross_account_principals + for principal in statement.role_assumable_by_cross_account_principals + ] @property def role_assumable_by_any_principal(self) -> list[str]: """Determines whether or not the role can be assumed by any principal (*) or any AWS account root.""" - any_principals = [] - for statement in self.statements: - if statement.role_assumable_by_any_principal: - any_principals.extend(statement.role_assumable_by_any_principal) - return any_principals + return [ + principal + for statement in self.statements + if statement.role_assumable_by_any_principal + for principal in statement.role_assumable_by_any_principal + ] @property def role_assumable_by_any_principal_with_conditions(self) -> list[str]: """Determines whether or not the role can be assumed by any principal (*) or any AWS account root with conditions.""" - any_principals_with_conditions = [] - for statement in self.statements: - if statement.role_assumable_by_any_principal_with_conditions: - any_principals_with_conditions.extend(statement.role_assumable_by_any_principal_with_conditions) - return any_principals_with_conditions + return [ + principal + for statement in self.statements + if statement.role_assumable_by_any_principal_with_conditions + for principal in statement.role_assumable_by_any_principal_with_conditions + ] class AssumeRoleStatement(ResourceStatement): diff --git a/cloudsplaining/scan/role_details.py b/cloudsplaining/scan/role_details.py index c85da48c..d7e9f078 100644 --- a/cloudsplaining/scan/role_details.py +++ b/cloudsplaining/scan/role_details.py @@ -352,6 +352,7 @@ def json(self) -> dict[str, Any]: ) if self.flag_trust_policies: + severities = {x.lower() for x in self.severity} this_role_detail.update( { "AssumableByComputeServices": { @@ -360,10 +361,7 @@ def json(self) -> dict[str, Any]: "findings": ( self.assume_role_policy_document.role_assumable_by_compute_services if self.assume_role_policy_document - and ( - ISSUE_SEVERITY["AssumableByComputeService"] in [x.lower() for x in self.severity] - or not self.severity - ) + and (ISSUE_SEVERITY["AssumableByComputeService"] in severities or not self.severity) else [] ), }, @@ -373,10 +371,7 @@ def json(self) -> dict[str, Any]: "findings": ( self.assume_role_policy_document.role_assumable_by_cross_account_principals if self.assume_role_policy_document - and ( - ISSUE_SEVERITY["AssumableByCrossAccountPrincipal"] in [x.lower() for x in self.severity] - or not self.severity - ) + and (ISSUE_SEVERITY["AssumableByCrossAccountPrincipal"] in severities or not self.severity) else [] ), }, @@ -386,10 +381,7 @@ def json(self) -> dict[str, Any]: "findings": ( self.assume_role_policy_document.role_assumable_by_any_principal if self.assume_role_policy_document - and ( - ISSUE_SEVERITY["AssumableByAnyPrincipal"] in [x.lower() for x in self.severity] - or not self.severity - ) + and (ISSUE_SEVERITY["AssumableByAnyPrincipal"] in severities or not self.severity) else [] ), }, @@ -400,8 +392,7 @@ def json(self) -> dict[str, Any]: self.assume_role_policy_document.role_assumable_by_any_principal_with_conditions if self.assume_role_policy_document and ( - ISSUE_SEVERITY["AssumableByAnyPrincipalWithConditions"] - in [x.lower() for x in self.severity] + ISSUE_SEVERITY["AssumableByAnyPrincipalWithConditions"] in severities or not self.severity ) else [] From 14dcd6ea3ec3d45515f95447743639130afea1a0 Mon Sep 17 00:00:00 2001 From: Brian Henderson Date: Fri, 26 Sep 2025 10:45:35 +0200 Subject: [PATCH 9/9] Handle account ID principals in cross-account trust checks Trust policies can list 12-digit account IDs in addition to arns ARNs. Normalizing principals through a shared helper (and exercising exclusions/current-account filtering in tests) keeps --flag-trust-policies from missing those roles that are assumable from other accounts when only the account ID is supplied. --- .../scan/assume_role_policy_document.py | 24 ++++------- cloudsplaining/shared/exclusions.py | 4 +- cloudsplaining/shared/utils.py | 14 ++++++ test/scanning/test_trust_policies.py | 43 ++++++++++++++++++- test/shared/test_utils.py | 14 +++++- 5 files changed, 78 insertions(+), 21 deletions(-) diff --git a/cloudsplaining/scan/assume_role_policy_document.py b/cloudsplaining/scan/assume_role_policy_document.py index 55e933c6..083d42f6 100644 --- a/cloudsplaining/scan/assume_role_policy_document.py +++ b/cloudsplaining/scan/assume_role_policy_document.py @@ -7,12 +7,9 @@ # or https://opensource.org/licenses/BSD-3-Clause from __future__ import annotations -import contextlib import logging from typing import Any -from policy_sentry.util.arns import get_account_from_arn - from cloudsplaining.scan.resource_policy_document import ( ResourcePolicyDocument, ResourceStatement, @@ -22,6 +19,7 @@ DEFAULT_EXCLUSIONS, Exclusions, ) +from cloudsplaining.shared.utils import get_account_id_from_principal logger = logging.getLogger(__name__) @@ -158,19 +156,13 @@ def role_assumable_by_cross_account_principals(self) -> list[str]: if self.effect.lower() != "allow": return [] - other_account_principals = [] - for principal in self.principals: - # Check if this is an AWS IAM principal from another account - if principal.startswith("arn:aws:iam::"): - with contextlib.suppress(Exception): - principal_account_id = get_account_from_arn(principal) - # Only include if it's from a different account (or if we don't know our current account) - # and the account is not in the known-accounts exclusion list - if ( - self.current_account_id is None or principal_account_id != self.current_account_id - ) and principal_account_id not in self.exclusions.known_accounts: - other_account_principals.append(principal) - return other_account_principals + return [ + principal + for principal in self.principals + if (principal_account_id := get_account_id_from_principal(principal)) + and (self.current_account_id is None or principal_account_id != self.current_account_id) + and principal_account_id not in self.exclusions.known_accounts + ] @property def role_assumable_by_any_principal(self) -> list[str]: diff --git a/cloudsplaining/shared/exclusions.py b/cloudsplaining/shared/exclusions.py index 006a89b4..d20c393a 100644 --- a/cloudsplaining/shared/exclusions.py +++ b/cloudsplaining/shared/exclusions.py @@ -66,10 +66,10 @@ def _exclude_actions(self) -> list[str]: always_exclude_actions = [x.lower() for x in exclude_actions] return always_exclude_actions - def _known_accounts(self) -> list[str]: + def _known_accounts(self) -> set[str]: provided_known_accounts = self.config.get("known-accounts", []) # Normalize for comparisons - remove empty strings - known_accounts = [account for account in provided_known_accounts if account.strip()] + known_accounts = {account for account in provided_known_accounts if account.strip()} return known_accounts def is_action_always_included(self, action_in_question: str) -> bool | str: diff --git a/cloudsplaining/shared/utils.py b/cloudsplaining/shared/utils.py index 01f47dc1..bdd6363b 100644 --- a/cloudsplaining/shared/utils.py +++ b/cloudsplaining/shared/utils.py @@ -7,6 +7,7 @@ # or https://opensource.org/licenses/BSD-3-Clause from __future__ import annotations +import contextlib import json import logging import os @@ -20,6 +21,7 @@ remove_actions_not_matching_access_level, ) from policy_sentry.querying.all import get_all_service_prefixes +from policy_sentry.util.arns import get_account_from_arn all_service_prefixes = get_all_service_prefixes() logger = logging.getLogger(__name__) @@ -169,3 +171,15 @@ def write_json_to_file(file: str, content: str) -> None: os.remove(file) Path(file).write_text(json.dumps(content, indent=4, default=str), encoding="utf-8") + + +def get_account_id_from_principal(principal: str) -> str | None: + """Return the AWS account ID for a principal ARN or bare 12-digit account identifier.""" + principal_stripped = principal.strip() + if principal_stripped.startswith("arn:aws:iam::"): + with contextlib.suppress(Exception): + return get_account_from_arn(principal_stripped) + return None + if principal_stripped.isdigit() and len(principal_stripped) == 12: + return principal_stripped + return None diff --git a/test/scanning/test_trust_policies.py b/test/scanning/test_trust_policies.py index 2943caad..7156b1e0 100644 --- a/test/scanning/test_trust_policies.py +++ b/test/scanning/test_trust_policies.py @@ -201,6 +201,36 @@ def test_assume_role_assumable_by_cross_account_principals(self): ["arn:aws:iam::098765432109:role/testrole"], ) + # Test bare account ID principals + statement_account_ids = dict( + Effect="Allow", + Principal={"AWS": ["123456789012", "210987654321"]}, + Action=["sts:AssumeRole"], + ) + assume_role_account_ids = AssumeRoleStatement(statement_account_ids) + self.assertListEqual( + assume_role_account_ids.role_assumable_by_cross_account_principals, + ["123456789012", "210987654321"], + ) + + # Test bare account ID filtering by current account + assume_role_account_ids_filtered = AssumeRoleStatement(statement_account_ids, current_account_id="123456789012") + self.assertListEqual( + assume_role_account_ids_filtered.role_assumable_by_cross_account_principals, + ["210987654321"], + ) + + # Test bare account ID filtering with exclusions + exclusions_account_ids_config = {"known-accounts": ["210987654321"]} + exclusions_account_ids = Exclusions(exclusions_account_ids_config) + assume_role_account_ids_exclusions = AssumeRoleStatement( + statement_account_ids, exclusions=exclusions_account_ids + ) + self.assertListEqual( + assume_role_account_ids_exclusions.role_assumable_by_cross_account_principals, + ["123456789012"], + ) + # Test conditions that should return empty results # No sts:AssumeRole action statement_no_assume_role = dict( @@ -232,14 +262,23 @@ def test_assume_role_assumable_by_cross_account_principals(self): "Principal": {"AWS": "arn:aws:iam::098765432109:user/testuser"}, "Action": "sts:AssumeRole", }, + { + "Effect": "Allow", + "Principal": {"AWS": "210987654321"}, + "Action": "sts:AssumeRole", + }, ], } policy_doc = AssumeRolePolicyDocument(policy) - expected_policy = ["arn:aws:iam::123456789012:root", "arn:aws:iam::098765432109:user/testuser"] + expected_policy = [ + "arn:aws:iam::123456789012:root", + "arn:aws:iam::098765432109:user/testuser", + "210987654321", + ] self.assertListEqual(policy_doc.role_assumable_by_cross_account_principals, expected_policy) # Test AssumeRolePolicyDocument with exclusions - partial_exclusions_config_doc = {"known-accounts": ["123456789012"]} + partial_exclusions_config_doc = {"known-accounts": ["123456789012", "210987654321"]} partial_exclusions_doc = Exclusions(partial_exclusions_config_doc) policy_doc_with_exclusions = AssumeRolePolicyDocument(policy, exclusions=partial_exclusions_doc) expected_policy_with_exclusions = ["arn:aws:iam::098765432109:user/testuser"] diff --git a/test/shared/test_utils.py b/test/shared/test_utils.py index d067216b..617fe287 100644 --- a/test/shared/test_utils.py +++ b/test/shared/test_utils.py @@ -1,5 +1,9 @@ import unittest -from cloudsplaining.shared.utils import remove_wildcard_only_actions, remove_read_level_actions +from cloudsplaining.shared.utils import ( + get_account_id_from_principal, + remove_read_level_actions, + remove_wildcard_only_actions, +) class TestUtils(unittest.TestCase): @@ -20,3 +24,11 @@ def test_remove_read_level_actions(self): result = remove_read_level_actions(actions) expected_result = ["ecr:PutImage"] self.assertListEqual(result, expected_result) + + def test_get_account_id_from_principal(self): + self.assertEqual( + get_account_id_from_principal("arn:aws:iam::123456789012:role/test"), + "123456789012", + ) + self.assertEqual(get_account_id_from_principal(" 210987654321"), "210987654321") + self.assertIsNone(get_account_id_from_principal("invalid"))