mirror of
https://github.com/prowler-cloud/prowler.git
synced 2026-10-04 02:04:06 +00:00
feat(iam): scope AgentCore PassRole and workload token grants, and flag unbound service trust (#12664)
This commit is contained in:
Jonathan Nguyen
@@ -0,0 +1 @@
|
||||
`iam_policy_passrole_to_bedrock_agentcore_restricted` check for AWS provider, flagging customer-managed IAM policies that allow `iam:PassRole` over every role where the passed role can reach Bedrock AgentCore, so any principal holding the policy can run agent code under any role in the account
|
||||
@@ -0,0 +1 @@
|
||||
`iam_policy_no_agentcore_workload_access_token_wildcard` check for AWS provider, flagging customer-managed IAM policies that allow `bedrock-agentcore:GetWorkloadAccessToken`, `GetWorkloadAccessTokenForJWT` or `GetWorkloadAccessTokenForUserId` on resources reaching workload identities other than the caller's own, which AWS documents as the only binding on the unverified user ID the token is issued for
|
||||
@@ -0,0 +1 @@
|
||||
`iam_role_service_trust_restricts_source_to_account` check for AWS provider, flagging IAM roles whose trust policy lets an AWS service principal assume the role without confining the request to a specific source account, including trust policies that `iam_role_cross_service_confused_deputy_prevention` does not evaluate
|
||||
@@ -241,6 +241,7 @@
|
||||
"iam_inline_policy_no_administrative_privileges",
|
||||
"iam_policy_allows_privilege_escalation",
|
||||
"iam_inline_policy_allows_privilege_escalation",
|
||||
"iam_policy_passrole_to_bedrock_agentcore_restricted",
|
||||
"iam_role_administratoraccess_policy",
|
||||
"iam_user_administrator_access_policy",
|
||||
"iam_group_administrator_access_policy",
|
||||
@@ -269,6 +270,7 @@
|
||||
"iam_user_no_setup_initial_access_key",
|
||||
"iam_user_two_active_access_key",
|
||||
"iam_user_console_access_unused",
|
||||
"iam_policy_no_agentcore_workload_access_token_wildcard",
|
||||
"bedrock_api_key_no_long_term_credentials"
|
||||
]
|
||||
},
|
||||
@@ -305,6 +307,7 @@
|
||||
],
|
||||
"Checks": [
|
||||
"iam_role_cross_service_confused_deputy_prevention",
|
||||
"iam_role_service_trust_restricts_source_to_account",
|
||||
"iam_role_access_not_stale_to_bedrock",
|
||||
"iam_user_access_not_stale_to_bedrock",
|
||||
"iam_role_cross_account_readonlyaccess_policy"
|
||||
|
||||
+44
@@ -0,0 +1,44 @@
|
||||
{
|
||||
"Provider": "aws",
|
||||
"CheckID": "iam_policy_no_agentcore_workload_access_token_wildcard",
|
||||
"CheckTitle": "Custom IAM policy scopes Bedrock AgentCore workload access token retrieval to workload identity ARNs",
|
||||
"CheckType": [
|
||||
"Software and Configuration Checks/AWS Security Best Practices",
|
||||
"TTPs/Credential Access",
|
||||
"Effects/Data Exposure"
|
||||
],
|
||||
"ServiceName": "iam",
|
||||
"SubServiceName": "",
|
||||
"ResourceIdTemplate": "",
|
||||
"Severity": "high",
|
||||
"ResourceType": "AwsIamPolicy",
|
||||
"ResourceGroup": "IAM",
|
||||
"Description": "**Customer-managed IAM policies** are examined for `Allow` statements granting `bedrock-agentcore:GetWorkloadAccessToken`, `GetWorkloadAccessTokenForJWT` or `GetWorkloadAccessTokenForUserId` over resources that reach a workload identity other than the caller's own -- `*`, or an AgentCore ARN whose resource field wildcards past `workload-identity-directory`.",
|
||||
"Risk": "A workload access token carries both user and agent identity and unlocks the outbound credential providers holding that user's stored credentials. `GetWorkloadAccessTokenForUserId` takes a caller-supplied user ID the platform does not verify, so AWS documents the IAM scope as the binding: with a wildcard resource, a compromised agent can mint tokens for **other users** of the same agent.",
|
||||
"RelatedUrl": "",
|
||||
"AdditionalURLs": [
|
||||
"https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/get-workload-access-token.html",
|
||||
"https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/identity.html",
|
||||
"https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/agent-identity-directory.html",
|
||||
"https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/identity-manage-agent-ids.html"
|
||||
],
|
||||
"Remediation": {
|
||||
"Code": {
|
||||
"CLI": "aws iam create-policy-version --policy-arn <POLICY_ARN> --policy-document '{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Action\":[\"bedrock-agentcore:GetWorkloadAccessToken\"],\"Resource\":[\"arn:aws:bedrock-agentcore:<REGION>:<ACCOUNT_ID>:workload-identity-directory/default\",\"arn:aws:bedrock-agentcore:<REGION>:<ACCOUNT_ID>:workload-identity-directory/default/workload-identity/<WORKLOAD_NAME>\"]}]}' --set-as-default",
|
||||
"NativeIaC": "```yaml\n# CloudFormation: scope the token actions to this workload's identity\nResources:\n <example_resource_name>:\n Type: AWS::IAM::ManagedPolicy\n Properties:\n PolicyDocument:\n Version: '2012-10-17'\n Statement:\n - Effect: Allow\n Action:\n - bedrock-agentcore:GetWorkloadAccessToken\n - bedrock-agentcore:GetWorkloadAccessTokenForJWT\n Resource: # FIX: replace '*' with the workload identity this policy is for\n - !Sub 'arn:${AWS::Partition}:bedrock-agentcore:${AWS::Region}:${AWS::AccountId}:workload-identity-directory/default'\n - !Sub 'arn:${AWS::Partition}:bedrock-agentcore:${AWS::Region}:${AWS::AccountId}:workload-identity-directory/default/workload-identity/<example_resource_id>'\n # Where a JWT is always available, deny the unverified user-ID path outright\n - Effect: Deny\n Action: bedrock-agentcore:GetWorkloadAccessTokenForUserId\n Resource: !Sub 'arn:${AWS::Partition}:bedrock-agentcore:${AWS::Region}:${AWS::AccountId}:workload-identity-directory/default'\n```",
|
||||
"Other": "1. In the AWS console, open IAM > Policies and select <example_resource_name>\n2. Choose Edit > JSON\n3. Find every statement whose Action includes bedrock-agentcore:GetWorkloadAccessToken, GetWorkloadAccessTokenForJWT or GetWorkloadAccessTokenForUserId\n4. Replace \"Resource\": \"*\" with the workload identity ARNs the holder legitimately acts for, for example arn:aws:bedrock-agentcore:<REGION>:<ACCOUNT_ID>:workload-identity-directory/default/workload-identity/<WORKLOAD_NAME>\n5. If a JWT identifying the end user is always available, prefer GetWorkloadAccessTokenForJWT and add an explicit Deny for GetWorkloadAccessTokenForUserId\n6. Save as a new default version and re-run the check",
|
||||
"Terraform": "```hcl\n# Scope the token actions to this workload's identity\nresource \"aws_iam_policy\" \"<example_resource_name>\" {\n name = \"<example_resource_name>\"\n policy = jsonencode({\n Version = \"2012-10-17\"\n Statement = [{\n Effect = \"Allow\"\n Action = [\n \"bedrock-agentcore:GetWorkloadAccessToken\",\n \"bedrock-agentcore:GetWorkloadAccessTokenForJWT\",\n ]\n # FIX: replace '*' with the workload identity this policy is for\n Resource = [\n \"arn:aws:bedrock-agentcore:${var.region}:${var.account_id}:workload-identity-directory/default\",\n \"arn:aws:bedrock-agentcore:${var.region}:${var.account_id}:workload-identity-directory/default/workload-identity/<example_resource_id>\",\n ]\n }]\n })\n}\n```"
|
||||
},
|
||||
"Recommendation": {
|
||||
"Text": "Scope the workload access token actions to the workload identity ARNs the policy holder acts for, never `*`. AWS states the security binding of `GetWorkloadAccessTokenForUserId` rests on IAM scope, since the platform treats the user ID as an opaque unverified string: **do not grant it broadly via managed policies or wildcard resource statements**. Prefer `GetWorkloadAccessTokenForJWT` and deny the user-ID path where a JWT is always available.",
|
||||
"Url": "https://hub.prowler.com/check/iam_policy_no_agentcore_workload_access_token_wildcard"
|
||||
}
|
||||
},
|
||||
"Categories": [
|
||||
"identity-access",
|
||||
"gen-ai"
|
||||
],
|
||||
"DependsOn": [],
|
||||
"RelatedTo": [],
|
||||
"Notes": "Scope is the customer-managed population, matching every other iam_policy_* check; inline policies are the subject of the iam_inline_policy_* checks.\n\nThree deliberate limits. A bare Action \"*\" is not read as a grant of these operations, because that is what the administrative-privileges checks report; the Action must carry the bedrock-agentcore prefix, wildcards within it included. The assertion is at ARN-type granularity rather than per workload identity: a resource confined to the workload-identity-directory namespace passes even with a wildcard inside it, because that is the scope the AgentCore console issues. A resource naming another AgentCore type passes for the opposite reason, since these actions accept no such resource and the grant reaches no workload identity.\n\nAn unconditional Deny of the same operation on Resource \"*\" clears the finding; a Deny scoped to one directory does not, because the Allow still reaches every other one."
|
||||
}
|
||||
+426
@@ -0,0 +1,426 @@
|
||||
from prowler.lib.check.models import Check, Check_Report_AWS
|
||||
from prowler.providers.aws.services.iam.iam_client import iam_client
|
||||
from prowler.providers.aws.services.iam.lib.policy import iam_pattern_matches
|
||||
|
||||
AGENTCORE_SERVICE_PREFIX = "bedrock-agentcore"
|
||||
# The three operations that hand a caller a workload access token. Confirmed against the
|
||||
# bedrock-agentcore service model: GetWorkloadAccessToken issues a token for the calling
|
||||
# workload, ForJWT exchanges a user JWT, ForUserId names the user directly.
|
||||
WORKLOAD_ACCESS_TOKEN_OPERATIONS = (
|
||||
"GetWorkloadAccessToken",
|
||||
"GetWorkloadAccessTokenForJWT",
|
||||
"GetWorkloadAccessTokenForUserId",
|
||||
)
|
||||
# Every workload identity ARN sits under the workload-identity-directory resource path, both the
|
||||
# directory itself and the workload-identity children beneath it.
|
||||
WORKLOAD_IDENTITY_SEGMENT = "workload-identity-directory"
|
||||
# The leading resource-path segment of every AgentCore type that is NOT a workload identity, from
|
||||
# AWS's machine-readable service reference
|
||||
# (servicereference.us-east-1.amazonaws.com/v1/bedrock-agentcore/bedrock-agentcore.json): 32 resource
|
||||
# types collapsing to 23 distinct leading segments, of which workload-identity-directory is the only
|
||||
# one hosting the two in-scope types.
|
||||
#
|
||||
# SEGMENTS, NOT NAMES, and that distinction is the whole point. Probing concrete resources -- say
|
||||
# token-vault/default, gateway/my-gateway, runtime/my-runtime and a workload identity called
|
||||
# another-workload -- makes their example NAMES load-bearing. A resource field keyed on any other name
|
||||
# then matches none of them, and the check concludes it is confined to the workload-identity
|
||||
# namespace: "...:*prod-*" reaches runtime/prod-chatbot, gateway/prod-chatbot-gw,
|
||||
# memory/prod-chatbot-mem and two distinct workload identities, while reading as reaching none of
|
||||
# them. Names cannot be enumerated -- the AgentCore devguide's own examples are prod-chatbot,
|
||||
# dev-chatbot and customer-support-agent, and its own example policy wildcards on the name -- so no
|
||||
# probe corpus can be completed. Resource TYPES can be enumerated, and are, above.
|
||||
#
|
||||
# This is the same correction already applied to the two short-ARN branches, which stopped pinning
|
||||
# region, account and partition for exactly this reason; here it stops pinning the resource NAME.
|
||||
NON_WORKLOAD_IDENTITY_SEGMENTS = (
|
||||
"ab-test",
|
||||
"batch-evaluate",
|
||||
"browser",
|
||||
"browser-custom",
|
||||
"browser-profile",
|
||||
"capacity-provider",
|
||||
"code-interpreter",
|
||||
"code-interpreter-custom",
|
||||
"configuration-bundle",
|
||||
"dataset",
|
||||
"evaluator",
|
||||
"gateway",
|
||||
"harness",
|
||||
"memory",
|
||||
"online-evaluation-config",
|
||||
"payment-manager",
|
||||
"policy-engine",
|
||||
"recommendation",
|
||||
"registry",
|
||||
"runtime",
|
||||
"token-vault",
|
||||
"tool",
|
||||
)
|
||||
|
||||
|
||||
def _as_list(value) -> list:
|
||||
"""Normalize to list: None -> [], scalar -> [val], list -> list."""
|
||||
if value is None:
|
||||
return []
|
||||
return value if isinstance(value, list) else [value]
|
||||
|
||||
|
||||
def _statements(document: dict) -> list:
|
||||
"""Extract Statement, normalizing single-statement dict to list."""
|
||||
statements = document.get("Statement", [])
|
||||
if not isinstance(statements, list):
|
||||
statements = [statements]
|
||||
return [statement for statement in statements if isinstance(statement, dict)]
|
||||
|
||||
|
||||
def _covered_token_operations(statement: dict) -> set:
|
||||
"""Return the workload access token operations this statement's Action covers.
|
||||
|
||||
Only actions that can name the bedrock-agentcore service are read, and the service field is
|
||||
matched as an IAM pattern rather than compared literally -- `bedrock-*:GetWorkloadAccessToken`
|
||||
reaches the operation, and an exact comparison read it as granting nothing at all.
|
||||
|
||||
A bare "*" is deliberately not treated as a grant of these operations: a statement allowing
|
||||
every action on every resource is what check_admin_access reports, and re-reporting it here
|
||||
would duplicate the administrative-privileges checks rather than add a claim. The
|
||||
`separator != ":"` guard is what preserves that, since "*" partitions to an empty separator.
|
||||
"""
|
||||
covered = set()
|
||||
for action in _as_list(statement.get("Action")):
|
||||
if not isinstance(action, str):
|
||||
continue
|
||||
service, separator, operation = action.strip().partition(":")
|
||||
# A statement allowing EVERY action is the administrative-privileges checks' finding,
|
||||
# whether it is spelled "*" or "*:*"; re-reporting it here would duplicate them. The
|
||||
# separator test covers the bare "*", which partitions to an empty separator.
|
||||
if separator != ":" or (service == "*" and operation == "*"):
|
||||
continue
|
||||
if not iam_pattern_matches(service, AGENTCORE_SERVICE_PREFIX):
|
||||
continue
|
||||
covered.update(
|
||||
token_operation
|
||||
for token_operation in WORKLOAD_ACCESS_TOKEN_OPERATIONS
|
||||
if iam_pattern_matches(operation, token_operation)
|
||||
)
|
||||
return covered
|
||||
|
||||
|
||||
def _token_operations_removed_by(statement: dict) -> set:
|
||||
"""Return the token operations a DENY statement's Action removes.
|
||||
|
||||
Separate from _covered_token_operations on purpose, because the bare-star exclusion that is
|
||||
right on the Allow side inverts on the Deny side. There, skipping ``*`` and ``*:*`` avoids
|
||||
duplicating the administrative-privileges checks; here it meant an unconditional Deny of EVERY
|
||||
action credited nothing, so a policy that grants nothing at all was reported FAIL at high
|
||||
severity -- contradicting this check's own published Notes, which say an unconditional Deny of
|
||||
the operation on Resource "*" clears the finding. The Allow-side guard is deliberately left
|
||||
alone: relaxing it would reverse the settled decision that admin-level grants belong to
|
||||
check_admin_access.
|
||||
|
||||
Deny expressed as NotAction is still NOT read, here or on the Allow side. Inverting it requires
|
||||
resolving the whole action namespace, which is more than this check can claim; not crediting it
|
||||
errs toward reporting rather than toward silence, so a policy denied that way may still FAIL.
|
||||
"""
|
||||
removed = set()
|
||||
for action in _as_list(statement.get("Action")):
|
||||
if not isinstance(action, str):
|
||||
continue
|
||||
service, separator, operation = action.strip().partition(":")
|
||||
if separator != ":":
|
||||
# A bare "*" denies every action, these three among them.
|
||||
if service == "*":
|
||||
removed.update(WORKLOAD_ACCESS_TOKEN_OPERATIONS)
|
||||
continue
|
||||
if not iam_pattern_matches(service, AGENTCORE_SERVICE_PREFIX):
|
||||
continue
|
||||
removed.update(
|
||||
token_operation
|
||||
for token_operation in WORKLOAD_ACCESS_TOKEN_OPERATIONS
|
||||
if iam_pattern_matches(operation, token_operation)
|
||||
)
|
||||
return removed
|
||||
|
||||
|
||||
def _may_reach_segment(resource_field: str, segment: str) -> bool:
|
||||
"""Return True if this resource field could name a resource whose path starts with ``segment``.
|
||||
|
||||
Decided from the field's own shape rather than by matching example resources, so no resource NAME
|
||||
is load-bearing. The two wildcards are NOT interchangeable and that distinction is the whole
|
||||
function: ``*`` spans any run of characters including none, while ``?`` consumes EXACTLY ONE. So
|
||||
the field's HEAD -- everything before its first ``*`` -- is a fixed-length template in which each
|
||||
``?`` stands for one unknown character, and every string the field matches begins with something
|
||||
that template accepts.
|
||||
|
||||
Compare the template to the segment position by position, then decide who supplies the remainder:
|
||||
|
||||
- A position where the template holds a literal that differs from the segment's character rules
|
||||
the segment out entirely, however long the field is.
|
||||
- If the template is at least as long as the segment and no position disagreed, the segment is
|
||||
covered, so the field may reach it.
|
||||
- If the template is SHORTER, only a ``*`` can supply what is missing. Without one the field has
|
||||
a fixed length too short to contain the segment, so it reaches nothing under it.
|
||||
|
||||
THE HEAD IS CUT AT ``*`` ONLY, NEVER AT ``?``, and the two must not be treated alike. ``?``
|
||||
consumes exactly one character, so it belongs to the fixed-length head and is skipped
|
||||
position-by-position above; ``*`` spans an unbounded run, so it terminates the head. Cutting at
|
||||
``?`` as well empties the head of any ``?``-leading field, an empty head is compatible with every
|
||||
segment, and the field then reads as reaching all 22 of them: ``?orkload-identity-directory/*``
|
||||
would be reported while its byte-neighbour ``w?rkload-identity-directory/*`` is not. Measured
|
||||
against an exact oracle over 1345 fields, that spelling costs 265 false FAILs.
|
||||
|
||||
THE COMPLEMENTARY ERROR IS TO RETURN False ON AN EMPTY HEAD, and it is worse. That repairs the
|
||||
``?`` rows and simultaneously turns bare ``*`` and ``*prod-*`` into "reaches nothing", which
|
||||
reinstates the false PASS this segment test exists to remove: 10 oracle mismatches against 0 for
|
||||
the rule as written. So the decision rests on whether a ``*`` is PRESENT, not on whether the head
|
||||
is empty, because ``*`` must keep absorbing everything.
|
||||
|
||||
Deliberately errs toward True on a coarse pair such as head ``browser-custom/x`` against segment
|
||||
``browser``: over-estimating reach can only move a verdict toward reporting, and the reach test
|
||||
that follows still decides the workload-identity question on its own terms.
|
||||
"""
|
||||
head, spans = resource_field.split("*", 1)[0], "*" in resource_field
|
||||
for index in range(min(len(head), len(segment))):
|
||||
if head[index] != "?" and head[index] != segment[index]:
|
||||
return False
|
||||
if len(head) >= len(segment):
|
||||
return True
|
||||
return spans
|
||||
|
||||
|
||||
def _reaches_other_workload_identities(resource: str) -> bool:
|
||||
"""Return True if this resource lets the token actions name a workload identity that
|
||||
is not the caller's own.
|
||||
|
||||
A resource confined to the workload-identity-directory namespace is accepted, wildcards
|
||||
within it included: the AgentCore console itself issues that scope, and the ARN type is
|
||||
the granularity this check asserts. A resource of some other AgentCore type -- a token
|
||||
vault, a gateway -- is accepted too, but for the opposite reason: the token actions
|
||||
accept no such resource, so the grant reaches no workload identity at all.
|
||||
|
||||
EVERY field of the pattern is matched as an IAM pattern, the first one included, because a
|
||||
leading star matches "arn" as surely as it matches anything else. Comparing that field to the
|
||||
literal "arn" instead would read ``*:aws:bedrock-agentcore:us-east-1:123456789012:*`` and
|
||||
``*:*:*:*:*:*`` as naming no workload identity, while their correctly spelled six-field
|
||||
equivalent names every one.
|
||||
|
||||
A pattern with fewer than six fields reaches a workload identity when a star in its LAST
|
||||
spelled-out field can span the fields it never spells out, because IAM wildcards match the
|
||||
colon. That question is answered structurally rather than by probing a concrete ARN, and
|
||||
deliberately so: matching against one ``us-east-1``/``123456789012`` ARN makes the region,
|
||||
account and partition load-bearing, so ``arn:aws:bedrock-agentcore:us-west-2:*`` reads as
|
||||
reaching nothing while the byte-identical ``us-east-1`` spelling is reported. No finite probe
|
||||
corpus fixes that -- an account PREFIX such as
|
||||
``arn:aws:bedrock-agentcore:us-east-1:111122223333*`` has nothing to enumerate. What the fields
|
||||
it does spell out must still do is name an ARN at all: ``arn:aws:s3:*`` is short and starred but
|
||||
names another service.
|
||||
|
||||
``arn:aws:bedrock-agentcore`` and ``arn:aws:bedrock-agentcore:us-east-1`` carry no star, so they
|
||||
match no ARN and reach nothing. The star is what separates them from the cases above, not the
|
||||
length.
|
||||
|
||||
THE RESOURCE FIELD IS DECIDED STRUCTURALLY TOO, by ``_may_reach_segment`` against the enumerable
|
||||
list of AgentCore resource-path segments, and NOT by comparison against concrete example
|
||||
resources. The tempting argument for a small probe corpus is that the namespace test intercepts
|
||||
everything confined to workload-identity-directory before this one runs, so a single probe cannot
|
||||
produce a false verdict. That premise does not hold:
|
||||
|
||||
``arn:aws:bedrock-agentcore:us-east-1:123456789012:*prod-*`` is not confined to the namespace, yet
|
||||
it matches none of a four-resource probe corpus, so the namespace test intercepts it anyway and
|
||||
clears a statement reaching ``runtime/prod-chatbot``, ``gateway/prod-chatbot-gw``,
|
||||
``memory/prod-chatbot-mem``, a token vault, a custom browser, and two distinct workload
|
||||
identities. The pair that shows such a corpus has no defensible boundary: resource field ``*`` is
|
||||
reported while ``*prod-*`` is not, and no rule stated anywhere separates them except "matches one
|
||||
of four example NAMES", which is an artifact of the corpus rather than a property of IAM.
|
||||
|
||||
So the lesson is the one the short-ARN branches already record, one level down: a probe corpus can
|
||||
only decide a question whose answer space it enumerates. Regions, accounts and partitions could
|
||||
not be enumerated there; resource NAMES cannot be enumerated here, since AgentCore creates
|
||||
identities named after the runtime or gateway that made them. Resource TYPES can be, so the test
|
||||
is built on those.
|
||||
"""
|
||||
resource = resource.strip()
|
||||
if resource == "*":
|
||||
return True
|
||||
arn_fields = resource.split(":", 5)
|
||||
if len(arn_fields) < 6:
|
||||
if "*" not in arn_fields[-1]:
|
||||
return False
|
||||
if not iam_pattern_matches(arn_fields[0], "arn"):
|
||||
return False
|
||||
return len(arn_fields) < 3 or iam_pattern_matches(
|
||||
arn_fields[2], AGENTCORE_SERVICE_PREFIX
|
||||
)
|
||||
if not iam_pattern_matches(arn_fields[0], "arn"):
|
||||
return False
|
||||
if not iam_pattern_matches(arn_fields[2], AGENTCORE_SERVICE_PREFIX):
|
||||
return False
|
||||
resource_field = arn_fields[5]
|
||||
# Confined to the workload-identity namespace when it can reach NO resource of another AgentCore
|
||||
# type. The startswith test this replaced asked whether the field begins with the literal
|
||||
# namespace prefix, which is a different question and got the ordering backwards: the confined
|
||||
# workload-identity-* was reported while the strictly broader workload-identity-directory* --
|
||||
# whose reach is a superset of it -- was accepted.
|
||||
#
|
||||
# BOTH TESTS ARE STRUCTURAL, and they have to stay that way together. Comparing a field against
|
||||
# concrete example resources instead lets a field keyed on any name those examples do not use
|
||||
# satisfy both at once: "*prod-*" matches none of the four non-workload-identity probes, so this
|
||||
# branch would call it confined, and it also misses the single another-workload probe below, so
|
||||
# neither test would report it. Making only one of them structural leaves the verdict unchanged,
|
||||
# so do not read this branch as a guard for the one below -- it does not intercept everything
|
||||
# confined to the namespace, and a pattern that is not confined at all can reach it.
|
||||
if not any(
|
||||
_may_reach_segment(resource_field, segment)
|
||||
for segment in NON_WORKLOAD_IDENTITY_SEGMENTS
|
||||
):
|
||||
return False
|
||||
return _may_reach_segment(resource_field, WORKLOAD_IDENTITY_SEGMENT)
|
||||
|
||||
|
||||
def _is_workload_identity_scoped(statement: dict) -> bool:
|
||||
"""Return True if no resource the statement names reaches another workload identity.
|
||||
|
||||
A statement using NotResource names no resource at all -- it grants everything except
|
||||
an excluded list -- so it is not scoped.
|
||||
|
||||
Only Resource and NotResource are read. An Allow-side Condition is NOT evaluated, so a
|
||||
statement narrowed solely by one -- aws:ResourceTag is a condition key on both workload
|
||||
identity resource types -- is still reported. That is deliberate conservatism, the mirror
|
||||
of the Deny-side decision below: a condition is not credited with confining a grant any
|
||||
more than it is credited with removing one. What it costs is that the finding may name a
|
||||
statement an unread condition already scopes, which is why the FAIL text claims only that
|
||||
the RESOURCES do not confine it.
|
||||
"""
|
||||
resources = _as_list(statement.get("Resource"))
|
||||
if not resources:
|
||||
return "NotResource" not in statement
|
||||
return not any(
|
||||
isinstance(resource, str) and _reaches_other_workload_identities(resource)
|
||||
for resource in resources
|
||||
)
|
||||
|
||||
|
||||
def _denied_token_operations(document: dict) -> set:
|
||||
"""Return the token operations an unconditional Deny removes across all resources.
|
||||
|
||||
A conditional Deny is not counted: it only applies when the condition holds, so it
|
||||
does not take the permission away from the request the Allow statement grants.
|
||||
"""
|
||||
denied = set()
|
||||
for statement in _statements(document):
|
||||
if statement.get("Effect") != "Deny" or statement.get("Condition"):
|
||||
continue
|
||||
if any(
|
||||
isinstance(resource, str) and resource.strip() == "*"
|
||||
for resource in _as_list(statement.get("Resource"))
|
||||
):
|
||||
denied.update(_token_operations_removed_by(statement))
|
||||
return denied
|
||||
|
||||
|
||||
def _has_unevaluated_notaction(document: dict) -> bool:
|
||||
"""True if an Allow statement expresses its actions as NotAction.
|
||||
|
||||
NotAction under Effect Allow grants everything EXCEPT what it lists, so a policy using
|
||||
it can grant the very actions this check looks for while carrying no Action key at all.
|
||||
Reading only Action would find nothing and report a clean policy. Inverting NotAction
|
||||
correctly means resolving it against the full action namespace and its interaction with
|
||||
Resource and NotResource, which is more than this check can honestly claim to do -- so
|
||||
the statement is declared unevaluated rather than guessed at.
|
||||
"""
|
||||
for statement in _statements(document):
|
||||
if statement.get("Effect") == "Allow" and "NotAction" in statement:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
class iam_policy_no_agentcore_workload_access_token_wildcard(Check):
|
||||
"""Check whether a customer-managed policy scopes AgentCore workload access token retrieval.
|
||||
|
||||
A workload access token identifies the agent to AgentCore, so a policy granting the retrieval
|
||||
operations on every resource lets its holder obtain a token for any workload identity and act as
|
||||
that agent. FAIL when the grant reaches workload identities beyond a named directory; PASS when
|
||||
the resource is confined to the workload-identity-directory namespace, when it names an AgentCore
|
||||
type these actions do not accept, or when an unconditional Deny on ``Resource: "*"`` clears it;
|
||||
MANUAL when the policy document could not be read or expresses a shape this check does not
|
||||
evaluate.
|
||||
|
||||
Caveats:
|
||||
Customer-managed policies only. The assertion is at ARN-type granularity rather than per
|
||||
workload identity, because a wildcard inside the directory namespace is the scope the
|
||||
AgentCore console itself issues. A bare ``Action: "*"`` is left to the
|
||||
administrative-privileges checks.
|
||||
"""
|
||||
|
||||
def execute(self) -> Check_Report_AWS:
|
||||
"""Flag policies granting token ops beyond caller's workload ID.
|
||||
|
||||
MANUAL is used deliberately below, where the document could not be read or expresses a shape
|
||||
this check does not evaluate. An unread document must not report as compliant: the grant it
|
||||
might contain is precisely what is being looked for, so PASS there would assert something never
|
||||
established. This is not off-contract -- 110 upstream checks emit MANUAL and
|
||||
`lib/check/models.py` places no restriction on it.
|
||||
|
||||
THE COST, recorded so it is not rediscovered: `lib/outputs/asff/asff.py` SKIPS findings whose
|
||||
status is MANUAL, because MANUAL is not a valid Security Hub compliance state. A Security Hub
|
||||
consumer therefore sees NOTHING for an unreadable policy, and absence there reads as
|
||||
compliance. CSV and OCSF keep the status, so the information survives in those outputs. That is
|
||||
a gap in one output format, not a reason to report an unread document as PASS or FAIL.
|
||||
"""
|
||||
findings = []
|
||||
for policy in iam_client.policies.values():
|
||||
# Only customer-managed policies: the inline population is a separate check,
|
||||
# and an AWS-managed policy cannot be edited to remediate a finding.
|
||||
if policy.type != "Custom":
|
||||
continue
|
||||
if not policy.attached and not iam_client.provider.scan_unused_services:
|
||||
continue
|
||||
|
||||
report = Check_Report_AWS(metadata=self.metadata(), resource=policy)
|
||||
report.region = iam_client.region
|
||||
|
||||
if not policy.document:
|
||||
report.status = "MANUAL"
|
||||
report.status_extended = (
|
||||
f"Custom Policy {policy.name} could not be evaluated because its "
|
||||
"policy document was not retrieved."
|
||||
)
|
||||
findings.append(report)
|
||||
continue
|
||||
|
||||
if _has_unevaluated_notaction(policy.document):
|
||||
report.status = "MANUAL"
|
||||
report.status_extended = (
|
||||
f"Custom Policy {policy.name} expresses an Allow statement with "
|
||||
"NotAction, which this check does not evaluate, so its effective grants "
|
||||
"could not be determined; review it manually."
|
||||
)
|
||||
findings.append(report)
|
||||
continue
|
||||
|
||||
denied = _denied_token_operations(policy.document)
|
||||
unscoped = set()
|
||||
for statement in _statements(policy.document):
|
||||
if statement.get("Effect") != "Allow":
|
||||
continue
|
||||
if _is_workload_identity_scoped(statement):
|
||||
continue
|
||||
unscoped.update(_covered_token_operations(statement) - denied)
|
||||
|
||||
if unscoped:
|
||||
report.status = "FAIL"
|
||||
report.status_extended = (
|
||||
f"Custom Policy {policy.name} allows "
|
||||
f"{', '.join(sorted(AGENTCORE_SERVICE_PREFIX + ':' + operation for operation in unscoped))} "
|
||||
"on resources outside a workload identity ARN, so its resources do not "
|
||||
"confine token retrieval to the workload's own identity; conditions on "
|
||||
"the statement are not evaluated."
|
||||
)
|
||||
else:
|
||||
report.status = "PASS"
|
||||
report.status_extended = (
|
||||
f"Custom Policy {policy.name} does not allow AgentCore workload access "
|
||||
"token retrieval outside a workload identity ARN."
|
||||
)
|
||||
findings.append(report)
|
||||
|
||||
return findings
|
||||
+45
@@ -0,0 +1,45 @@
|
||||
{
|
||||
"Provider": "aws",
|
||||
"CheckID": "iam_policy_passrole_to_bedrock_agentcore_restricted",
|
||||
"CheckTitle": "Custom IAM policy restricts iam:PassRole to Bedrock AgentCore to specific roles",
|
||||
"CheckType": [
|
||||
"Software and Configuration Checks/AWS Security Best Practices",
|
||||
"TTPs/Privilege Escalation"
|
||||
],
|
||||
"ServiceName": "iam",
|
||||
"SubServiceName": "",
|
||||
"ResourceIdTemplate": "",
|
||||
"Severity": "high",
|
||||
"ResourceType": "AwsIamPolicy",
|
||||
"ResourceGroup": "IAM",
|
||||
"Description": "**Customer-managed IAM policies** are examined for `Allow` statements granting `iam:PassRole` over every role -- `Resource` `*`, or an IAM ARN whose resource field is nothing but wildcards -- where the passed role can reach **Bedrock AgentCore**: the statement pins `iam:PassedToService` to an AgentCore principal, or sets no such condition while the policy allows an AgentCore action.",
|
||||
"Risk": "AgentCore runtimes, gateways, code interpreters, browsers and evaluation configs all run under a role the caller names in the create call.\n\nWith `iam:PassRole` unbounded, any principal holding the policy can hand AgentCore **any role in the account**, an administrator role included, then reach that role's permissions through agent code it controls. The role itself needs no change.",
|
||||
"RelatedUrl": "",
|
||||
"AdditionalURLs": [
|
||||
"https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/evaluations-prerequisites.html",
|
||||
"https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/runtime-permissions.html",
|
||||
"https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_use_passrole.html",
|
||||
"https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_iam-condition-keys.html"
|
||||
],
|
||||
"Remediation": {
|
||||
"Code": {
|
||||
"CLI": "aws iam create-policy-version --policy-arn <POLICY_ARN> --policy-document '{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Action\":[\"iam:PassRole\"],\"Resource\":\"arn:aws:iam::<ACCOUNT_ID>:role/<AGENTCORE_EXECUTION_ROLE_PREFIX>*\",\"Condition\":{\"StringEquals\":{\"iam:PassedToService\":\"bedrock-agentcore.amazonaws.com\"}}}]}' --set-as-default",
|
||||
"NativeIaC": "```yaml\n# CloudFormation: name the roles AgentCore may be handed\nResources:\n <example_resource_name>:\n Type: AWS::IAM::ManagedPolicy\n Properties:\n PolicyDocument:\n Version: '2012-10-17'\n Statement:\n - Effect: Allow\n Action: iam:PassRole\n # FIX: replace '*' with the execution roles AgentCore is meant to run as.\n # A name prefix is enough -- this is the scope AWS's own AgentCore\n # Evaluations reference policy uses.\n Resource: !Sub 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/<example_resource_id>*'\n Condition:\n StringEquals:\n iam:PassedToService: bedrock-agentcore.amazonaws.com\n```",
|
||||
"Other": "1. In the AWS console, open IAM > Policies and select <example_resource_name>\n2. Choose Edit > JSON\n3. Find every statement whose Action includes iam:PassRole (or iam:* / iam:Pass*) with \"Resource\": \"*\" or an IAM ARN such as arn:aws:iam::<ACCOUNT_ID>:role/*\n4. Replace that resource with the AgentCore execution roles the holder should be able to pass -- a role ARN, or a role-name prefix such as arn:aws:iam::<ACCOUNT_ID>:role/AgentCoreEvaluationRole*\n5. Add Condition StringEquals iam:PassedToService = bedrock-agentcore.amazonaws.com so the roles cannot be handed to any other service\n6. Save as a new default version and re-run the check",
|
||||
"Terraform": "```hcl\n# Name the roles AgentCore may be handed\nresource \"aws_iam_policy\" \"<example_resource_name>\" {\n name = \"<example_resource_name>\"\n policy = jsonencode({\n Version = \"2012-10-17\"\n Statement = [{\n Effect = \"Allow\"\n Action = [\"iam:PassRole\"]\n # FIX: replace '*' with the execution roles AgentCore is meant to run as\n Resource = \"arn:aws:iam::${var.account_id}:role/<example_resource_id>*\"\n Condition = {\n StringEquals = { \"iam:PassedToService\" = \"bedrock-agentcore.amazonaws.com\" }\n }\n }]\n })\n}\n```"
|
||||
},
|
||||
"Recommendation": {
|
||||
"Text": "Name the roles that may be passed instead of allowing every role. AWS's own AgentCore Evaluations reference policy shows the shape: `iam:PassRole` on `arn:aws:iam::*:role/AgentCoreEvaluationRole*` under `StringEquals iam:PassedToService = bedrock-agentcore.amazonaws.com`. A role-name prefix satisfies this check; `*` and `role/*` do not. Keep the condition as well, so the same roles cannot be handed to another service.",
|
||||
"Url": "https://hub.prowler.com/check/iam_policy_passrole_to_bedrock_agentcore_restricted"
|
||||
}
|
||||
},
|
||||
"Categories": [
|
||||
"identity-access",
|
||||
"gen-ai"
|
||||
],
|
||||
"DependsOn": [],
|
||||
"RelatedTo": [
|
||||
"iam_policy_allows_privilege_escalation"
|
||||
],
|
||||
"Notes": "Companion to iam_policy_allows_privilege_escalation, which reports a full AgentCore create-and-invoke action set but evaluates Action alone, so it reports that combination whatever the PassRole scope and nothing when part of it is absent. This check asserts what that leaves open, how far the PassRole grant reaches, and needs only one AgentCore action beside it.\n\nScope is the customer-managed population. A statement pinning iam:PassedToService to another service is out of scope, and a bare Action \"*\" counts as neither the grant nor the AgentCore action, so administrator policies are left to the administrative-privileges checks.\n\nA resource names every role when no role name escapes it, which is broader than a field of wildcards alone: role/?* and *role* both qualify, as does an ARN whose star spans the account and resource fields. A bounded set passes, since role/? names single-character roles only and role/AgentCoreEvaluationRole* is the scope AWS's own AgentCore Evaluations reference policy uses."
|
||||
}
|
||||
+606
@@ -0,0 +1,606 @@
|
||||
import re
|
||||
|
||||
from prowler.lib.check.models import Check, Check_Report_AWS
|
||||
from prowler.providers.aws.services.iam.iam_client import iam_client
|
||||
from prowler.providers.aws.services.iam.lib.policy import iam_pattern_matches
|
||||
|
||||
AGENTCORE_SERVICE_PREFIX = "bedrock-agentcore"
|
||||
AGENTCORE_SERVICE_PRINCIPAL = "bedrock-agentcore.amazonaws.com"
|
||||
# Bedrock AgentCore also reaches IAM through subdomain principals such as
|
||||
# runtime-identity.bedrock-agentcore.amazonaws.com, so a literal value in that family
|
||||
# pins the statement to AgentCore just as the base principal does.
|
||||
AGENTCORE_PRINCIPAL_FAMILY_PATTERN = re.compile(
|
||||
rf"^([a-z0-9-]+\.)?{AGENTCORE_SERVICE_PREFIX}(-[a-z0-9-]+)?\.amazonaws\.com$",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
# Concrete principals used to ask whether a condition value, READ AS AN IAM PATTERN, covers one of
|
||||
# them. That is a different question from the regex above, which asks whether the value IS a family
|
||||
# member, and asking only the second let a wildcard covering the family out of scope entirely. Both
|
||||
# are needed: the regex catches a literal subdomain nobody enumerated here, and the probes catch a
|
||||
# pattern that names no principal literally while reaching several.
|
||||
AGENTCORE_PRINCIPAL_PROBES = (
|
||||
AGENTCORE_SERVICE_PRINCIPAL,
|
||||
f"runtime-identity.{AGENTCORE_SERVICE_PRINCIPAL}",
|
||||
)
|
||||
# Two role names used to ask whether a resource field names EVERY role rather than some of them:
|
||||
# the shortest a role name can be, and the longest. Only "*" can span an arbitrary run of
|
||||
# characters -- "?" matches exactly one -- so a pattern built from literals and "?" alone covers a
|
||||
# bounded set of names and cannot match both probes, while any pattern that does match both leaves
|
||||
# no role name outside it.
|
||||
_SHORTEST_ROLE_NAME = "role/a"
|
||||
_LONGEST_ROLE_NAME = "role/" + "r0le-name-" * 6 + "abcd" # 64 chars, the IAM maximum
|
||||
EVERY_ROLE_RESOURCE_PROBES = (_SHORTEST_ROLE_NAME, _LONGEST_ROLE_NAME)
|
||||
# Operators that COMPARE the request's iam:PassedToService against the statement's own values, so a
|
||||
# value under one of them names a service this statement can hand a role to. An allow-list, because
|
||||
# the deny-list this replaced -- two substring tests for "not" and "ifexists" -- dropped the entire
|
||||
# condition on a one-word operator change, and a dropped condition fell through to the document-wide
|
||||
# fallback in _targets_agentcore as though the statement pinned nothing at all.
|
||||
#
|
||||
# *IfExists and ForAllValues ARE included, unlike _RESTRICTIVE_ATTESTATION_OPERATORS in
|
||||
# kms/lib/enclave.py which rejects both as vacuous-true when the key is absent. The difference is the
|
||||
# question being asked: this function asks which services a statement NAMES, not whether the
|
||||
# statement is restrictive. A value is named whether or not the operator would also admit a request
|
||||
# that omits the key.
|
||||
#
|
||||
# The ARN operators are DELIBERATELY absent, which is why this list is shorter than the trust check's
|
||||
# and not an oversight in it. iam:PassedToService is a STRING-typed key holding a service principal,
|
||||
# and AWS documents it as working with the string operators; an ARN operator on it cannot compare
|
||||
# meaningfully. The trust check needs ArnEquals and ArnLike because aws:SourceArn is ARN-typed. Two
|
||||
# allow-lists differing by the TYPE of the key they read is correct; differing for no stated reason is
|
||||
# what gets flagged, so the reason is here.
|
||||
#
|
||||
# Consequence worth naming: ArnLikeIfExists on iam:PassedToService contributes no value, so the
|
||||
# statement takes the no-pin path and reaches every service -- the same route as carrying no condition
|
||||
# at all. It still FAILs beside an AgentCore action, by that route rather than by being read as a pin.
|
||||
_PASSED_TO_SERVICE_OPERATORS = frozenset(
|
||||
f"{qualifier}{operator}{suffix}".lower()
|
||||
for qualifier in ("", "ForAnyValue:", "ForAllValues:")
|
||||
for operator in ("StringEquals", "StringEqualsIgnoreCase", "StringLike")
|
||||
for suffix in ("", "IfExists")
|
||||
)
|
||||
|
||||
|
||||
def _as_list(value) -> list:
|
||||
"""Normalize to list: None -> [], scalar -> [val], list -> list."""
|
||||
if value is None:
|
||||
return []
|
||||
return value if isinstance(value, list) else [value]
|
||||
|
||||
|
||||
def _statements(document: dict) -> list:
|
||||
"""Extract Statement, normalizing single-statement dict to list."""
|
||||
statements = document.get("Statement", [])
|
||||
if not isinstance(statements, list):
|
||||
statements = [statements]
|
||||
return [statement for statement in statements if isinstance(statement, dict)]
|
||||
|
||||
|
||||
def _covers_passrole(statement: dict) -> bool:
|
||||
"""Return True if the statement's Action covers iam:PassRole.
|
||||
|
||||
The service field is matched as an IAM pattern rather than compared literally, so
|
||||
`*:PassRole` is read as covering it. A bare "*" still does not count: a statement allowing
|
||||
every action on every resource is what the administrative-privileges checks report, and
|
||||
re-reporting it here would duplicate them instead of adding a claim. The `separator != ":"`
|
||||
guard is what preserves that, since "*" partitions to an empty separator.
|
||||
"""
|
||||
for action in _as_list(statement.get("Action")):
|
||||
if not isinstance(action, str):
|
||||
continue
|
||||
service, separator, operation = action.strip().partition(":")
|
||||
# A statement allowing EVERY action is the administrative-privileges checks' finding,
|
||||
# whether it is spelled "*" or "*:*"; the separator test covers the bare "*".
|
||||
if separator != ":" or (service == "*" and operation == "*"):
|
||||
continue
|
||||
if not iam_pattern_matches(service, "iam"):
|
||||
continue
|
||||
if iam_pattern_matches(operation, "PassRole"):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _grants_agentcore_action(document: dict) -> bool:
|
||||
"""Return True if the policy allows at least one bedrock-agentcore action.
|
||||
|
||||
This is what puts an otherwise service-agnostic PassRole grant in scope: the same
|
||||
policy can both create an AgentCore resource and choose the role it runs as. A bare
|
||||
"*" is again excluded, so an administrator policy is not pulled in on that basis.
|
||||
|
||||
The service field is matched as an IAM pattern, as it is everywhere else these checks read
|
||||
one. It matters most here: ``bedrock-*:CreateAgentRuntime`` is a plausible thing to write, since
|
||||
one prefix covers bedrock and bedrock-agentcore together. A literal comparison would read it as
|
||||
no AgentCore reach, which takes an unpinned PassRole grant beside it out of scope entirely and
|
||||
clears the policy.
|
||||
"""
|
||||
for statement in _statements(document):
|
||||
if statement.get("Effect") != "Allow":
|
||||
continue
|
||||
for action in _as_list(statement.get("Action")):
|
||||
if not isinstance(action, str):
|
||||
continue
|
||||
service, separator, _ = action.strip().partition(":")
|
||||
if separator == ":" and iam_pattern_matches(
|
||||
service, AGENTCORE_SERVICE_PREFIX
|
||||
):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _passed_to_service_values(statement: dict) -> list:
|
||||
"""Collect the services a statement's iam:PassedToService condition names.
|
||||
|
||||
Read through the allow-list above rather than by rejecting operator names. An operator this code
|
||||
does not recognise must never be silently skipped, because ``_targets_agentcore`` treats "no
|
||||
values" as "reaches every service" and then consults the whole document. So a skipped condition
|
||||
inverts the verdict in BOTH directions, on nothing more than a one-word change of operator:
|
||||
|
||||
- ``StringEqualsIfExists`` naming AgentCore, on ``Resource: "*"``, yields no values, so a policy
|
||||
carrying no other AgentCore action falls out of scope entirely -- a PassRole grant on every
|
||||
role in the account, cleared by a high-severity privilege-escalation check.
|
||||
- ``StringEqualsIfExists`` naming another service yields no values too, so the document-wide
|
||||
fallback pulls the statement back in beside any AgentCore action and reports it, when a
|
||||
statement pinned to sagemaker is out of scope by the same rule spelled ``StringEquals``.
|
||||
|
||||
A negated operator is deliberately not read: it names the services the statement will NOT pass
|
||||
to, and the set it does reach is everything else, which is what "no values" already means here.
|
||||
"""
|
||||
values = []
|
||||
condition = statement.get("Condition", {})
|
||||
if not isinstance(condition, dict):
|
||||
return values
|
||||
for operator, block in condition.items():
|
||||
if not isinstance(operator, str) or not isinstance(block, dict):
|
||||
continue
|
||||
lowered_operator = operator.lower()
|
||||
if lowered_operator not in _PASSED_TO_SERVICE_OPERATORS:
|
||||
continue
|
||||
for key, value in block.items():
|
||||
if isinstance(key, str) and key.lower() == "iam:passedtoservice":
|
||||
values.extend(_as_list(value))
|
||||
return values
|
||||
|
||||
|
||||
def _null_guarded_keys(condition: dict) -> set:
|
||||
"""Return the lowercased condition keys a ``Null: "false"`` test forces to be present.
|
||||
|
||||
The helper the trust check in this PR defines, for the same reason, and byte-identical to it
|
||||
EXCEPT for the value type read -- see below, and see that file's docstring for the other half.
|
||||
Both spellings are handled, the scalar and the list, as kms/lib/enclave.py does.
|
||||
|
||||
A JSON ``false`` counts as well as the string ``"false"``, because on THIS check's surface IAM
|
||||
stores and returns both. Measured on a customer-managed policy, which is exactly this check's
|
||||
population: ``create_policy`` with ``{"Null": {"iam:PassedToService": false}}`` is accepted and
|
||||
``get_policy_version`` returns a Python ``bool``, unconverted. Reading only the string leaves the
|
||||
guard invisible, and the consequence is a false report on AWS's own prescribed hardening: a pin
|
||||
AWS's simulator holds to one service -- absent key ``implicitDeny``, AgentCore ``implicitDeny``
|
||||
-- would be reported as passing every role to AgentCore.
|
||||
|
||||
The trust sibling stays string-only ON PURPOSE, because a trust policy normalizes its scalars
|
||||
and no bool can reach it: the divergence is measured, not drift.
|
||||
|
||||
This also DIVERGES from kms/lib/enclave.py deliberately: that copy tests ``isinstance(value,
|
||||
str)`` only and carries the same blind spot. Diverging toward the correct reading rather than
|
||||
inheriting the house copy's defect.
|
||||
|
||||
EVERY value must be ``false``, not merely one of them, because IAM ORs the values inside a single
|
||||
condition operator. ``Null: {key: ["true","false"]}`` therefore means "key absent OR key present",
|
||||
which is always true and binds NOTHING, yet reading it with ``any`` credited it as a guard and
|
||||
rescued a defeasible pin -- so ADDING the word ``true`` to a guard list improved the score.
|
||||
Measured on IAM's own evaluator with the key omitted: ``allowed`` for ``["true","false"]`` and
|
||||
``["false","true"]``, indistinguishable from carrying no Null block, against ``implicitDeny`` for
|
||||
``"false"``, ``["false"]`` and ``["false","false"]``. Reachable: ``create_policy`` stores a
|
||||
multi-value list and ``get_policy_version`` returns it unchanged, and the trust surface preserves
|
||||
a multi-element list too even though it collapses a single-element one to a scalar.
|
||||
|
||||
The ``candidates and`` guard is not decoration: ``all()`` over an empty list is True, so an empty
|
||||
value list would otherwise be read as the strongest possible guard.
|
||||
|
||||
``0`` is NOT a guard, and it is the VALUE comparison that excludes it, not the type test:
|
||||
``str(0)`` is ``"0"``, which is simply not ``"false"``. The ``isinstance`` merely narrows the
|
||||
accepted types to the two JSON scalars IAM actually returns here. Spelled out because the
|
||||
tempting formulation is the broken one -- ``not candidate`` or ``candidate is False`` reads
|
||||
``0``, ``""``, ``None`` and ``[]`` as guards, since ``isinstance(False, int)`` is True in Python
|
||||
and falsiness is not the question being asked.
|
||||
|
||||
ACCESS ANALYZER DOES NOT CORROBORATE THIS BOUNDARY, so do not cite it as support. Measured: it
|
||||
reports TYPE_MISMATCH_BOOLEAN for ``"FALSE"`` and for ``" false "``, both of which this helper
|
||||
CREDITS, as well as for ``0``, which it does not. Its type-checking is therefore stricter than
|
||||
this helper's casing tolerance in one direction and looser in the other, and the boundary drawn
|
||||
here is this check's own decision rather than an external one. Over-recognising is the dangerous
|
||||
direction, because crediting a guard turns a FAIL into a PASS.
|
||||
"""
|
||||
guarded = set()
|
||||
for operator, block in condition.items():
|
||||
if not isinstance(operator, str) or operator.lower() != "null":
|
||||
continue
|
||||
if not isinstance(block, dict):
|
||||
continue
|
||||
for key, value in block.items():
|
||||
if not isinstance(key, str):
|
||||
continue
|
||||
candidates = value if isinstance(value, list) else [value]
|
||||
if candidates and all(
|
||||
isinstance(candidate, (str, bool))
|
||||
and str(candidate).strip().lower() == "false"
|
||||
for candidate in candidates
|
||||
):
|
||||
guarded.add(key.lower())
|
||||
return guarded
|
||||
|
||||
|
||||
def _passed_to_service_pin_is_defeasible(statement: dict) -> bool:
|
||||
"""Return True if every operator naming iam:PassedToService can be skipped by the caller.
|
||||
|
||||
An *IfExists operator is not evaluated when the request omits the key, and ForAllValues is
|
||||
vacuous-true for an absent key -- kms/lib/enclave.py records these as the same trap. So a
|
||||
statement whose only pin is one of those reaches every service as well as the one it names,
|
||||
and cannot be treated as confined to it.
|
||||
|
||||
UNLESS the same statement carries ``Null: "false"`` on the same key, which forces the key to be
|
||||
present and removes the skip. Reading that guard is what stops the check penalising the spelling
|
||||
AWS prescribes -- "You should always include the Null condition operator ... with a false value".
|
||||
Without it the hardened form scores worse than the plain one, which is backwards: a caller cannot
|
||||
omit a key the guard requires, so the guard can only narrow the grant.
|
||||
|
||||
Conditions are ANDed, so one non-defeasible operator naming the key holds the request to that
|
||||
key's values whatever else the statement carries; the pin is defeasible only if all of them are.
|
||||
"""
|
||||
defeasible = []
|
||||
condition = statement.get("Condition", {})
|
||||
if not isinstance(condition, dict):
|
||||
return False
|
||||
guarded = "iam:passedtoservice" in _null_guarded_keys(condition)
|
||||
for operator, block in condition.items():
|
||||
if not isinstance(operator, str) or not isinstance(block, dict):
|
||||
continue
|
||||
lowered_operator = operator.lower()
|
||||
if lowered_operator not in _PASSED_TO_SERVICE_OPERATORS:
|
||||
continue
|
||||
if not any(
|
||||
isinstance(key, str) and key.lower() == "iam:passedtoservice"
|
||||
for key in block
|
||||
):
|
||||
continue
|
||||
defeasible.append(
|
||||
(
|
||||
lowered_operator.endswith("ifexists")
|
||||
or lowered_operator.startswith("forallvalues:")
|
||||
)
|
||||
and not guarded
|
||||
)
|
||||
return bool(defeasible) and all(defeasible)
|
||||
|
||||
|
||||
def _names_agentcore(values: list) -> bool:
|
||||
"""Return True if any value can name an AgentCore service principal.
|
||||
|
||||
Two questions, and asking only the second was a false PASS on the exact grant this check exists
|
||||
to catch. Probing the value as a pattern against the ONE base principal, plus a regex asking
|
||||
whether the value IS a family member, left every wildcard that COVERS the family unrecognised:
|
||||
|
||||
*.bedrock-agentcore.amazonaws.com covers 2 known principals read as another service
|
||||
*-identity.bedrock-agentcore.amazonaws.com covers runtime-identity read as another service
|
||||
runtime-identity.* covers runtime-identity read as another service
|
||||
*.bedrock-agentcore.* covers 2 read as another service
|
||||
|
||||
Read that way each would pin iam:PassedToService to something other than AgentCore, taking the
|
||||
statement out of scope -- while both the narrower literal and the no-condition case are reported.
|
||||
Broadening the grant would flip the verdict the safe way round, which is the shape that never
|
||||
self-corrects.
|
||||
|
||||
So the value is now matched as a pattern against several concrete principals, not one, and a
|
||||
literal outside that list is still caught by the family regex. ``?`` covers nothing here on
|
||||
purpose: it matches exactly one character and no principal has a single-character subdomain --
|
||||
that ``?`` cannot match zero characters is pinned by iam/lib/policy_test.py, not assumed here.
|
||||
"""
|
||||
return any(
|
||||
isinstance(value, str)
|
||||
and (
|
||||
AGENTCORE_PRINCIPAL_FAMILY_PATTERN.match(value.strip())
|
||||
or any(
|
||||
iam_pattern_matches(value, probe)
|
||||
for probe in AGENTCORE_PRINCIPAL_PROBES
|
||||
)
|
||||
)
|
||||
for value in values
|
||||
)
|
||||
|
||||
|
||||
def _targets_agentcore(statement: dict, document: dict) -> bool:
|
||||
"""Return True if the statement can hand a role to Bedrock AgentCore.
|
||||
|
||||
Three cases, in the order they are decided:
|
||||
|
||||
1. A condition NAMES AgentCore. The statement is in scope on its own terms, under any operator
|
||||
that compares the key, *IfExists included. Dropping a spelling before its value is read is
|
||||
what would clear a PassRole grant on every role in the account: with no values the statement
|
||||
looks unpinned, so scope falls to a document that allows no AgentCore action.
|
||||
2. A condition pins the key elsewhere and cannot be skipped. The statement cannot reach
|
||||
AgentCore however the rest of the policy is shaped, so it is out of scope -- this is what
|
||||
keeps a grant pinned to sagemaker.amazonaws.com out of the check.
|
||||
3. Anything else -- no condition on the key, or only a defeasible one -- reaches every service,
|
||||
so the rest of the policy decides: in scope when the policy also allows an AgentCore action.
|
||||
"""
|
||||
values = _passed_to_service_values(statement)
|
||||
if _names_agentcore(values):
|
||||
return True
|
||||
if values and not _passed_to_service_pin_is_defeasible(statement):
|
||||
return False
|
||||
return _grants_agentcore_action(document)
|
||||
|
||||
|
||||
def _names_every_role(resource: str) -> bool:
|
||||
"""Return True if this one resource names every role rather than specific roles.
|
||||
|
||||
Decided by matching the resource against the shortest and longest role name a pattern would
|
||||
have to cover, rather than by a regex demanding the resource field be a run of asterisks. That
|
||||
regex was both too narrow and, being anchored on a literal ``arn:``, blind to a wildcarded
|
||||
partition. Four resources naming every role in the account read as specific ones:
|
||||
|
||||
role/?* every role whose name has at least one character
|
||||
*role* every role/... resource there is, since the stars absorb the prefix and the name
|
||||
arn:aws:iam::* the star spans the account and resource fields
|
||||
*:aws:iam::123456789012:role/* a leading star matches "arn"
|
||||
|
||||
``role/?`` still passes and is the case that shows the rule is not "contains a metacharacter":
|
||||
``?`` matches exactly one character -- pinned by iam/lib/policy_test.py rather than assumed here
|
||||
-- so it names single-character roles and nothing else. A name
|
||||
prefix such as ``role/AgentCoreEvaluationRole*`` passes for the same reason -- it covers a set,
|
||||
but not every role -- and that is the scope AWS's own AgentCore Evaluations reference policy
|
||||
uses, so reporting it would report the documented configuration.
|
||||
|
||||
Residual, and it is a judgement not a hole: a pattern of exactly 64 ``?`` would match the long
|
||||
probe and not the short one, so it reads as specific. It names every role whose name is exactly
|
||||
64 characters, which is a set no operator writes by hand.
|
||||
|
||||
A pattern with fewer than six fields is decided structurally, and NOT by probing concrete ARNs.
|
||||
A probe corpus pins the partition and account it happens to carry, which makes both load-bearing
|
||||
in the short branch while the six-field branch ignores them: ``arn:aws:iam::555555555555*`` reads
|
||||
as specific while the identical shape in the probe's own account is reported, and
|
||||
``arn:aws-us-gov:iam::*`` and ``arn:aws-cn:iam::*`` read as specific merely because no probe
|
||||
carries those partitions. No probe corpus can fix an account PREFIX -- there is nothing to
|
||||
enumerate. So: a star in the LAST spelled-out field spans every
|
||||
field after it, because IAM wildcards match the colon, and what remains is that the fields
|
||||
actually spelled out must be able to name a role ARN.
|
||||
|
||||
The region and account positions get an extra test, and both rest on a fixed property of an IAM
|
||||
ARN rather than on a corpus:
|
||||
|
||||
account an account is twelve digits, so a literal that is not twelve digits names no
|
||||
account. This is what keeps ``arn:aws:iam::role/Prod*`` and
|
||||
``arn:aws:iam::12345:role/*`` specific.
|
||||
region every IAM ARN has an EMPTY region, so any region pattern that cannot match the
|
||||
empty string names no region. This is what keeps ``arn:aws:iam:role/Prod*``
|
||||
specific -- the same shape one colon short -- and, at six fields,
|
||||
``arn:aws:iam:us-east-1:123456789012:role/*``.
|
||||
|
||||
BOTH BRANCHES APPLY BOTH TESTS, and the six-field branch not applying them was the defect that
|
||||
reached review. It tested only that the partition and service fields could name an IAM ARN and
|
||||
then probed the resource field, so a fully spelled-out ARN naming a region IAM does not have, or
|
||||
an account of the wrong length, was read as naming every role: a high-severity
|
||||
privilege-escalation FAIL on a pattern matching no role ARN at all. That is the same defect the
|
||||
short branch had already been fixed for, surviving in the branch nobody re-read.
|
||||
|
||||
The two branches phrase the SAME question differently because the field means something
|
||||
different in each, and this is the part that is easy to get wrong:
|
||||
|
||||
short branch the last spelled-out field carries a star that spans every field after it, so
|
||||
only its literal HEAD is pinned to a position. ``?`` is discounted there because
|
||||
it can match the colon and slide the rest of the pattern into a later field --
|
||||
which is what keeps ``arn:aws:iam:?*`` naming every role.
|
||||
six fields the field is delimited on both sides, so the WHOLE field must be able to name a
|
||||
region or an account, and ``?`` is NOT discounted -- it has no colon to match
|
||||
and must consume exactly one character of a region that has none. That is why
|
||||
``arn:aws:iam:?:123456789012:role/*`` names no role while ``arn:aws:iam:?*``
|
||||
names every one.
|
||||
|
||||
The PARTITION position deliberately gets no such test, which is why ``arn:xyz*`` still reads as
|
||||
naming every role. A partition is not a fixed shape -- ``aws``, ``aws-cn``, ``aws-us-gov`` and the
|
||||
iso partitions differ -- and a PREFIX of one cannot be enumerated, which is the same reason the
|
||||
probe corpus was abandoned above. Over-reporting an unspellable partition is the safe direction
|
||||
for a privilege-escalation check; guessing the partition set is not.
|
||||
"""
|
||||
candidate = resource.strip()
|
||||
if candidate == "*":
|
||||
return True
|
||||
arn_fields = candidate.split(":", 5)
|
||||
if len(arn_fields) < 6:
|
||||
if "*" not in arn_fields[-1]:
|
||||
return False
|
||||
if not iam_pattern_matches(arn_fields[0], "arn"):
|
||||
return False
|
||||
if len(arn_fields) > 2 and not iam_pattern_matches(arn_fields[2], "iam"):
|
||||
return False
|
||||
if len(arn_fields) == 4:
|
||||
# The last field sits in the REGION position, and every IAM ARN has an EMPTY region. Any
|
||||
# literal head -- role/Prod in arn:aws:iam:role/Prod* -- can name no region, so the
|
||||
# pattern matches no role ARN however its star spans. Discounting ? keeps
|
||||
# arn:aws:iam:?* naming every role, since ? matches the colon.
|
||||
region_head = arn_fields[3].split("*", 1)[0].replace("?", "")
|
||||
if region_head:
|
||||
return False
|
||||
if len(arn_fields) == 5:
|
||||
# The last field sits in the ACCOUNT position, and an account is twelve digits. A
|
||||
# literal head that is not digits -- role/Prod in arn:aws:iam::role/Prod* -- can name no
|
||||
# account, so the pattern matches no role ARN however its star spans.
|
||||
account_head = arn_fields[4].split("*", 1)[0].replace("?", "")
|
||||
if account_head and not account_head.isdigit():
|
||||
return False
|
||||
return True
|
||||
if not iam_pattern_matches(arn_fields[0], "arn"):
|
||||
return False
|
||||
if not iam_pattern_matches(arn_fields[2], "iam"):
|
||||
return False
|
||||
# Every IAM ARN has an EMPTY region, so a region field that cannot match the empty string names
|
||||
# no region and the pattern reaches no role. Asked as a pattern match against "" rather than by
|
||||
# stripping metacharacters, because that is the whole question here: "" and "*" match it, while
|
||||
# "us-east-1" and "?" do not. ? is deliberately NOT discounted, unlike the short branch above --
|
||||
# this field is delimited on both sides, so there is no colon for it to match and it must consume
|
||||
# one character of a region that has none.
|
||||
if not iam_pattern_matches(arn_fields[3], ""):
|
||||
return False
|
||||
account_field = arn_fields[4]
|
||||
# An account is twelve digits. Two ways a spelled-out field can fail to name one, and the second
|
||||
# is unreachable in the short branch, where a trailing star always spans the field:
|
||||
# a literal head that is not digits role/Prod in arn:aws:iam::role/Prod:...
|
||||
# a starless field of the wrong width 12345, which is digits but names no account
|
||||
account_head = account_field.split("*", 1)[0].replace("?", "")
|
||||
if account_head and not account_head.isdigit():
|
||||
return False
|
||||
if "*" not in account_field and len(account_field) != 12:
|
||||
return False
|
||||
return all(
|
||||
iam_pattern_matches(arn_fields[5], probe)
|
||||
for probe in EVERY_ROLE_RESOURCE_PROBES
|
||||
)
|
||||
|
||||
|
||||
def _allows_any_role(statement: dict) -> bool:
|
||||
"""Return True if the statement's resources name every role rather than named roles."""
|
||||
resources = _as_list(statement.get("Resource"))
|
||||
if not resources:
|
||||
# NotResource grants every resource but the excluded ones, so every role outside
|
||||
# that list stays passable.
|
||||
return "NotResource" in statement
|
||||
return any(
|
||||
isinstance(resource, str) and _names_every_role(resource)
|
||||
for resource in resources
|
||||
)
|
||||
|
||||
|
||||
def _deny_removes_passrole(statement: dict) -> bool:
|
||||
"""Return True if a DENY statement's Action removes iam:PassRole.
|
||||
|
||||
Separate from _covers_passrole on purpose, because the bare-star exclusion that is right on the
|
||||
Allow side inverts on the Deny side. There, skipping ``*`` and ``*:*`` avoids duplicating the
|
||||
administrative-privileges checks; here it meant an unconditional Deny of EVERY action credited
|
||||
nothing, so a policy that grants no PassRole at all was reported FAIL. The Allow-side guard is
|
||||
deliberately left alone: relaxing it would reverse the settled decision that admin-level grants
|
||||
belong to those checks.
|
||||
|
||||
Deny expressed as NotAction is still NOT read. Inverting it needs the whole action namespace,
|
||||
and not crediting it errs toward reporting rather than toward silence.
|
||||
"""
|
||||
for action in _as_list(statement.get("Action")):
|
||||
if not isinstance(action, str):
|
||||
continue
|
||||
service, separator, operation = action.strip().partition(":")
|
||||
if separator != ":":
|
||||
# A bare "*" denies every action, iam:PassRole among them.
|
||||
if service == "*":
|
||||
return True
|
||||
continue
|
||||
if not iam_pattern_matches(service, "iam"):
|
||||
continue
|
||||
if iam_pattern_matches(operation, "PassRole"):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _denies_passrole_everywhere(document: dict) -> bool:
|
||||
"""Return True if an unconditional Deny removes iam:PassRole on every resource."""
|
||||
for statement in _statements(document):
|
||||
if statement.get("Effect") != "Deny" or statement.get("Condition"):
|
||||
continue
|
||||
if not _deny_removes_passrole(statement):
|
||||
continue
|
||||
if any(
|
||||
isinstance(resource, str) and resource.strip() == "*"
|
||||
for resource in _as_list(statement.get("Resource"))
|
||||
):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _has_unevaluated_notaction(document: dict) -> bool:
|
||||
"""True if an Allow statement expresses its actions as NotAction.
|
||||
|
||||
NotAction under Effect Allow grants everything EXCEPT what it lists, so a policy using
|
||||
it can grant iam:PassRole while carrying no Action key at all. Reading only Action would
|
||||
find nothing and report a clean policy. Inverting NotAction correctly means resolving it
|
||||
against the full action namespace and its interaction with Resource and NotResource,
|
||||
which is more than this check can honestly claim to do -- so the statement is declared
|
||||
unevaluated rather than guessed at.
|
||||
"""
|
||||
for statement in _statements(document):
|
||||
if statement.get("Effect") == "Allow" and "NotAction" in statement:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
class iam_policy_passrole_to_bedrock_agentcore_restricted(Check):
|
||||
"""Check whether a customer-managed policy scopes iam:PassRole to Bedrock AgentCore.
|
||||
|
||||
A statement granting ``iam:PassRole`` on every role beside any Bedrock AgentCore action lets the
|
||||
holder hand an arbitrary role to an agent runtime and assume its permissions, which is a
|
||||
privilege-escalation path. FAIL when the PassRole resource names every role; PASS when it names
|
||||
a bounded set, when the statement pins ``iam:PassedToService`` to another service, or when no
|
||||
AgentCore action accompanies it; MANUAL when the policy document could not be read.
|
||||
|
||||
Caveats:
|
||||
Customer-managed policies only, since inline policies are covered by the
|
||||
``iam_inline_policy_*`` checks and an AWS-managed policy cannot be edited to remediate a
|
||||
finding. A bare ``Action: "*"`` is left to the administrative-privileges checks. Conditions
|
||||
do not rescue an unbounded resource, because ``iam:PassedToService`` binds the service and
|
||||
``iam:AssociatedResourceArn`` the consuming resource, neither the set of roles.
|
||||
"""
|
||||
|
||||
def execute(self) -> Check_Report_AWS:
|
||||
"""Flag policies allowing PassRole to AgentCore on all roles."""
|
||||
findings = []
|
||||
for policy in iam_client.policies.values():
|
||||
# Only customer-managed policies: the inline population is a separate check,
|
||||
# and an AWS-managed policy cannot be edited to remediate a finding.
|
||||
if policy.type != "Custom":
|
||||
continue
|
||||
if not policy.attached and not iam_client.provider.scan_unused_services:
|
||||
continue
|
||||
|
||||
report = Check_Report_AWS(metadata=self.metadata(), resource=policy)
|
||||
report.region = iam_client.region
|
||||
|
||||
if not policy.document:
|
||||
report.status = "MANUAL"
|
||||
report.status_extended = (
|
||||
f"Custom Policy {policy.name} could not be evaluated because its "
|
||||
"policy document was not retrieved."
|
||||
)
|
||||
findings.append(report)
|
||||
continue
|
||||
|
||||
if _has_unevaluated_notaction(policy.document):
|
||||
report.status = "MANUAL"
|
||||
report.status_extended = (
|
||||
f"Custom Policy {policy.name} expresses an Allow statement with "
|
||||
"NotAction, which this check does not evaluate, so whether it allows "
|
||||
"iam:PassRole could not be determined; review it manually."
|
||||
)
|
||||
findings.append(report)
|
||||
continue
|
||||
|
||||
unrestricted = False
|
||||
if not _denies_passrole_everywhere(policy.document):
|
||||
unrestricted = any(
|
||||
statement.get("Effect") == "Allow"
|
||||
and _covers_passrole(statement)
|
||||
and _allows_any_role(statement)
|
||||
and _targets_agentcore(statement, policy.document)
|
||||
for statement in _statements(policy.document)
|
||||
)
|
||||
|
||||
if unrestricted:
|
||||
report.status = "FAIL"
|
||||
report.status_extended = (
|
||||
f"Custom Policy {policy.name} allows iam:PassRole to Bedrock AgentCore "
|
||||
"on every role instead of the specific execution roles AgentCore is "
|
||||
"meant to run as."
|
||||
)
|
||||
else:
|
||||
report.status = "PASS"
|
||||
report.status_extended = (
|
||||
f"Custom Policy {policy.name} does not allow iam:PassRole to Bedrock "
|
||||
"AgentCore on every role."
|
||||
)
|
||||
findings.append(report)
|
||||
|
||||
return findings
|
||||
+46
@@ -0,0 +1,46 @@
|
||||
{
|
||||
"Provider": "aws",
|
||||
"CheckID": "iam_role_service_trust_restricts_source_to_account",
|
||||
"CheckTitle": "IAM role trust policy confines AWS service principals to a specific source account",
|
||||
"CheckType": [
|
||||
"Software and Configuration Checks/AWS Security Best Practices",
|
||||
"TTPs/Privilege Escalation"
|
||||
],
|
||||
"ServiceName": "iam",
|
||||
"SubServiceName": "",
|
||||
"ResourceIdTemplate": "",
|
||||
"Severity": "medium",
|
||||
"ResourceType": "AwsIamRole",
|
||||
"ResourceGroup": "IAM",
|
||||
"Description": "Trust-policy statements letting an **AWS service principal** call `sts:AssumeRole` confine the request source to one account -- via `aws:SourceAccount`, an account-bearing `aws:SourceArn`, or an organization-scoped source. Scope: statements whose condition binds no account, and trust policies that are not a plain service role. Unconditional service roles go to the related check.",
|
||||
"Risk": "A condition can look protective while binding nothing: a `*IfExists` operator is skipped when the calling service omits the key, a negated operator never matches, and an `aws:SourceArn` that is wildcarded or carries no account field (an S3 bucket ARN) names no account. Any account can then steer the service into assuming the role -- a **cross-service confused deputy** path to its permissions.",
|
||||
"RelatedUrl": "",
|
||||
"AdditionalURLs": [
|
||||
"https://docs.aws.amazon.com/IAM/latest/UserGuide/confused-deputy.html#cross-service-confused-deputy-prevention",
|
||||
"https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_condition-keys.html#condition-keys-sourcearn",
|
||||
"https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_condition-keys.html#condition-keys-sourceaccount",
|
||||
"https://aws.amazon.com/blogs/security/use-scalable-controls-for-aws-services-accessing-your-resources/",
|
||||
"https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_common-scenarios_third-party.html"
|
||||
],
|
||||
"Remediation": {
|
||||
"Code": {
|
||||
"CLI": "aws iam update-assume-role-policy --role-name <example_resource_name> --policy-document '{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"<service>.amazonaws.com\"},\"Action\":\"sts:AssumeRole\",\"Condition\":{\"StringEquals\":{\"aws:SourceAccount\":\"<ACCOUNT_ID>\"}}}]}'",
|
||||
"NativeIaC": "```yaml\n# CloudFormation: confine the service-principal trust to this account\nResources:\n <example_resource_name>:\n Type: AWS::IAM::Role\n Properties:\n AssumeRolePolicyDocument:\n Version: '2012-10-17'\n Statement:\n - Effect: Allow\n Principal:\n Service: <service>.amazonaws.com\n Action: sts:AssumeRole\n Condition:\n StringEquals:\n aws:SourceAccount: !Ref AWS::AccountId # binds the call to this account\n ArnLike:\n # Optional but preferred: bind to the calling resource as well.\n # An ARN whose account field is empty (an S3 bucket ARN) or wildcarded\n # does NOT bind the account -- keep aws:SourceAccount in that case.\n aws:SourceArn: !Sub 'arn:${AWS::Partition}:<service>:${AWS::Region}:${AWS::AccountId}:<resource-type>/<resource-name>'\n```",
|
||||
"Other": "1. In the AWS console, go to IAM > Roles\n2. Open <example_resource_name> and select the Trust relationships tab\n3. Click Edit trust policy\n4. For every statement whose Principal is a Service, add a Condition block that binds the source to an account, using either:\n - StringEquals: aws:SourceAccount = <ACCOUNT_ID>, or\n - ArnLike / ArnEquals: aws:SourceArn = an ARN whose account field is <ACCOUNT_ID>\n5. If the aws:SourceArn value has no account field (for example arn:aws:s3:::amzn-s3-demo-bucket), add aws:SourceAccount as well -- the ARN alone cannot bind the account\n6. Do not rely on a *IfExists operator: it is skipped when the calling service omits the key\n7. Save changes and re-run the check",
|
||||
"Terraform": "```hcl\n# Confine the service-principal trust to this account\nresource \"aws_iam_role\" \"<example_resource_name>\" {\n name = \"<example_resource_name>\"\n\n assume_role_policy = jsonencode({\n Version = \"2012-10-17\"\n Statement = [\n {\n Effect = \"Allow\"\n Principal = { Service = \"<service>.amazonaws.com\" }\n Action = \"sts:AssumeRole\"\n Condition = {\n StringEquals = { \"aws:SourceAccount\" = data.aws_caller_identity.current.account_id }\n # Optional but preferred: bind to the calling resource as well.\n ArnLike = { \"aws:SourceArn\" = aws_<service>_<resource>.example.arn }\n }\n }\n ]\n })\n}\n```"
|
||||
},
|
||||
"Recommendation": {
|
||||
"Text": "Bind every service-principal trust statement to an account with `aws:SourceAccount`, or with an `aws:SourceArn` whose account field holds the account ID. AWS documents `aws:SourceArn`, `aws:SourceAccount`, `aws:SourceOrgID` and `aws:SourceOrgPaths` as alternatives, so any one of them satisfies this check -- except an ARN with no account field, which needs `aws:SourceAccount` alongside it.",
|
||||
"Url": "https://hub.prowler.com/check/iam_role_service_trust_restricts_source_to_account"
|
||||
}
|
||||
},
|
||||
"Categories": [
|
||||
"identity-access",
|
||||
"trust-boundaries"
|
||||
],
|
||||
"DependsOn": [],
|
||||
"RelatedTo": [
|
||||
"iam_role_cross_service_confused_deputy_prevention"
|
||||
],
|
||||
"Notes": "Companion to iam_role_cross_service_confused_deputy_prevention, not a replacement. That check reports a service-principal trust statement carrying no restrictive condition at all, and only on roles it classifies as service roles. This check asserts the clause it leaves open: statements where a condition IS present but confines nothing, and statements no service-role check evaluates. Both can report one role.\n\nA condition confines nothing when it uses a negated operator, an *IfExists or ForAllValues operator with no Null:\"false\" guard on the same key, a wildcarded aws:SourceArn, or an ARN whose account field is empty such as an S3 bucket ARN. A role leaves the other check's population when its trust policy carries a Deny statement, an action outside the assume-role family, or a non-Service principal.\n\nAll four aws:Source* keys count as bindings. sts:ExternalId does not: AWS documents it for third-party access where the third party supplies the value, while a calling service passes source context instead."
|
||||
}
|
||||
+484
@@ -0,0 +1,484 @@
|
||||
import re
|
||||
|
||||
from prowler.lib.check.models import Check, Check_Report_AWS
|
||||
from prowler.providers.aws.services.iam.iam_client import iam_client
|
||||
from prowler.providers.aws.services.iam.lib.policy import iam_pattern_matches
|
||||
|
||||
ASSUME_ROLE_ACTION = "sts:AssumeRole"
|
||||
ACCOUNT_ID_PATTERN = re.compile(r"^\d{12}$")
|
||||
ORGANIZATION_ID_PATTERN = re.compile(r"^o-[a-z0-9]{10,32}$")
|
||||
ORGANIZATION_PATH_PATTERN = re.compile(r"^o-[a-z0-9]{10,32}/")
|
||||
|
||||
|
||||
def _as_list(value) -> list:
|
||||
"""Normalize to list: None -> [], scalar -> [val], list -> list.
|
||||
|
||||
The same helper both sibling checks in this PR define. A present-but-null key -- ``"Action":
|
||||
null`` -- makes ``.get("Action", [])`` return None rather than the default, and iterating that
|
||||
raises TypeError out of execute(), discarding every finding for the account rather than one
|
||||
role. IAM will not store such a document, so this is consistency with the siblings and not a
|
||||
security fix.
|
||||
"""
|
||||
if value is None:
|
||||
return []
|
||||
return value if isinstance(value, list) else [value]
|
||||
|
||||
|
||||
def _grants_assume_role(statement: dict) -> bool:
|
||||
"""Return True if the statement's Action covers sts:AssumeRole.
|
||||
|
||||
Exactly that one operation, not the wider assume-role family. ``sts:AssumeRoleWithWebIdentity``
|
||||
and ``sts:AssumeRoleWithSAML`` are how a federated or web identity assumes a role; an AWS service
|
||||
principal uses ``sts:AssumeRole``, so a statement granting only one of the other two is not a
|
||||
service-principal trust grant and is correctly outside this check's population.
|
||||
|
||||
The direction matters and is easy to read backwards: the statement's Action is the PATTERN and
|
||||
``sts:AssumeRole`` is the value, so ``sts:*`` and ``sts:Assume*`` match while
|
||||
``sts:AssumeRoleWithSAML`` does not.
|
||||
|
||||
Matched with the shared IAM matcher this PR already ships, rather than a literal tuple plus a
|
||||
trailing-star test. That pair recognised sts:AssumeRole, sts:* and * and any prefix ending in a
|
||||
star, but nothing else IAM honours: sts:*Role, sts:A*Role, sts:Assume?ole and sts:AssumeRol?
|
||||
each grant the action and each produced NO REPORT at all, because a statement that does not
|
||||
grant assume-role drops out of the evaluated population. Beside a second statement the same
|
||||
miss was worse than silence -- the role reported PASS, asserting it confines every AWS service
|
||||
principal in its trust policy to a specific account.
|
||||
"""
|
||||
for action in _as_list(statement.get("Action")):
|
||||
if not isinstance(action, str):
|
||||
continue
|
||||
if iam_pattern_matches(action, ASSUME_ROLE_ACTION):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _trusts_service_principal(statement: dict) -> bool:
|
||||
"""Return True if the statement trusts at least one AWS service principal.
|
||||
|
||||
A statement that trusts a service principal *alongside* other principal types
|
||||
still qualifies: the service principal is reachable regardless of what else the
|
||||
statement trusts, so it needs the same confused-deputy scoping.
|
||||
"""
|
||||
principal = statement.get("Principal", {})
|
||||
if not isinstance(principal, dict):
|
||||
# Principal: "*" is a string, not a mapping, and trusts every principal there is --
|
||||
# including every service principal. Returning False here dropped the statement from
|
||||
# the population and the role produced no finding at all.
|
||||
return principal == "*"
|
||||
return any(
|
||||
isinstance(service, str) and service
|
||||
for service in _as_list(principal.get("Service"))
|
||||
)
|
||||
|
||||
|
||||
# Operators that COMPARE a request value against the statement's own, so a value under one of them
|
||||
# genuinely pins the request source. An allow-list, because the deny-list this replaced -- two
|
||||
# substring tests for "not" and "ifexists" -- admitted every operator it did not recognise, Null
|
||||
# among them. Modelled on _RESTRICTIVE_ATTESTATION_OPERATORS in kms/lib/enclave.py, which solved the
|
||||
# same problem for attestation keys; the Arn forms are added here because aws:SourceArn is compared
|
||||
# with them. Matched lowercased, which keeps an oddly-cased operator as admissible as it was before.
|
||||
# The IfExists suffix is included, and admitted only under the same Null:"false" guard ForAllValues
|
||||
# needs. kms/lib/enclave.py calls it "the same trap as ForAllValues without a Null:false guard", so
|
||||
# rejecting one outright while rescuing the other was inconsistent on that file's own reading; and
|
||||
# secretsmanager_has_restrictive_resource_policy already ships IfExists paired with Null as its
|
||||
# accepted restrictive form, so the pairing is a shape prowler recognises rather than a new rule.
|
||||
_COMPARING_CONDITION_OPERATORS = frozenset(
|
||||
f"{qualifier}{operator}{suffix}".lower()
|
||||
for qualifier in ("", "ForAnyValue:", "ForAllValues:")
|
||||
for suffix in ("", "IfExists")
|
||||
for operator in (
|
||||
"StringEquals",
|
||||
"StringEqualsIgnoreCase",
|
||||
"StringLike",
|
||||
"ArnEquals",
|
||||
"ArnLike",
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
def _null_guarded_keys(condition: dict) -> set:
|
||||
"""Return the lowercased condition keys a ``Null: "false"`` test forces to be present.
|
||||
|
||||
Both spellings are handled, the scalar and the list, as kms/lib/enclave.py does.
|
||||
|
||||
EVERY value must be ``false``, not merely one of them, because IAM ORs the values inside a single
|
||||
condition operator. ``Null: {key: ["true","false"]}`` means "key absent OR key present", which is
|
||||
always true and binds NOTHING, yet ``any`` credited it and rescued a defeasible pin. This axis is
|
||||
shared with the PassRole sibling and fixed identically there, because a MULTI-element list survives
|
||||
on this surface: ``create_role`` stores ``["true","false"]`` and ``get_role`` returns it as a list.
|
||||
A single-element ``["false"]`` is collapsed to the scalar ``"false"`` here, which is why only the
|
||||
multi-value spelling matters. ``candidates and`` is load-bearing: ``all()`` over an empty list is
|
||||
True, so an empty value list would read as the strongest possible guard.
|
||||
|
||||
TWO AXES, MEASURED SEPARATELY, AND THEY DIVERGE. On VALUE TYPE this stays string-only while the
|
||||
PassRole sibling also reads a JSON boolean, because a trust policy normalizes scalar types and no
|
||||
bool can reach here. On LIST ARITY the two agree, because a multi-element list is preserved on both
|
||||
surfaces. Both halves are stated at both ends rather than left to look like drift, since a helper
|
||||
pair that agrees on one axis and differs on another is exactly what decays when nobody wrote down
|
||||
which axis was which.
|
||||
|
||||
A trust policy NORMALIZES its condition scalars, measured both ways round: ``create_role`` with
|
||||
``{"Null": {"aws:SourceAccount": false}}`` is ACCEPTED, and the document comes back carrying the
|
||||
string ``"false"`` -- from ``list_roles``, which is the call this check's collector actually
|
||||
makes, and identically from ``get_role``. An unquoted ``123456789012`` comes back quoted too.
|
||||
So no bool or int can reach this helper, and reading one would be dead code with no fixture able
|
||||
to exercise it. A customer-managed POLICY document, which is all the sibling reads,
|
||||
PRESERVES both types instead -- two IAM surfaces, two behaviours, which is exactly why this was
|
||||
measured per surface rather than inferred from one.
|
||||
|
||||
If IAM ever stops normalizing here, take the sibling's reading: ``isinstance(candidate, (str,
|
||||
bool)) and str(candidate).strip().lower() == "false"``. That admits a JSON ``false`` and still
|
||||
excludes ``0``, because ``str(0)`` is ``"0"`` -- the value comparison does that work, not the
|
||||
type test. What it must not become is ``not candidate`` or ``candidate is False``, which read
|
||||
``0``, ``""``, ``None`` and ``[]`` as guards; ``isinstance(False, int)`` is True in Python and
|
||||
falsiness is not the question. Over-recognising a guard turns a FAIL into a PASS.
|
||||
"""
|
||||
guarded = set()
|
||||
for operator, block in condition.items():
|
||||
if not isinstance(operator, str) or operator.lower() != "null":
|
||||
continue
|
||||
if not isinstance(block, dict):
|
||||
continue
|
||||
for key, value in block.items():
|
||||
if not isinstance(key, str):
|
||||
continue
|
||||
candidates = value if isinstance(value, list) else [value]
|
||||
if candidates and all(
|
||||
isinstance(candidate, str) and candidate.strip().lower() == "false"
|
||||
for candidate in candidates
|
||||
):
|
||||
guarded.add(key.lower())
|
||||
return guarded
|
||||
|
||||
|
||||
def _enforced_condition_value_groups(statement: dict, condition_key: str) -> list:
|
||||
"""Collect the values a statement pins to condition_key, GROUPED BY OPERATOR.
|
||||
|
||||
One group per operator, because IAM ANDs the operators in a Condition while ORing the values
|
||||
inside one operator. The grouping follows that structure directly:
|
||||
|
||||
- ACROSS groups, a caller asks whether ANY ONE confines the source. If one operator holds the
|
||||
request to literal account IDs, the request is confined whatever else it must also satisfy --
|
||||
an ANDed operator can only narrow. Pooling operators together would let a broad ``StringLike``
|
||||
beside a pinned ``StringEquals`` widen the verdict, which inverts the semantics.
|
||||
- WITHIN a group, EVERY value must qualify, since IAM lets the request match any one of them.
|
||||
|
||||
This is the per-operator evaluation ``kms/lib/enclave.py`` performs.
|
||||
|
||||
Operators are taken from an ALLOW-LIST, not a deny-list, so an operator this code does not
|
||||
recognise is never credited. Two consequences worth naming:
|
||||
|
||||
- Negated operators invert the match and so pin the source to nothing. They are absent from the
|
||||
allow-list by design.
|
||||
- ``Null`` is a presence test rather than a comparison, so it never contributes a value here. Its
|
||||
role is only as the guard below. A deny-list would admit it and feed the literal string
|
||||
``"false"`` in as though it were an account ID.
|
||||
|
||||
TWO operator families are vacuous on an Allow, and both are credited only under the same guard:
|
||||
|
||||
- ``ForAllValues:*`` "returns true if there are no context keys in the request", which AWS
|
||||
documents with an explicit warning against using it with an Allow effect. This check evaluates
|
||||
Allow statements only, so that is the reachable case.
|
||||
- ``*IfExists`` is not evaluated at all when the request omits the key, which kms/lib/enclave.py
|
||||
names as "the same trap as ForAllValues without a Null:false guard".
|
||||
|
||||
Both are credited only when the same statement carries a ``Null: "false"`` guard on the SAME key,
|
||||
which forces the key to be present and removes the vacuity. The guard defeats it identically for
|
||||
every spelling, so all four of ``ForAllValues:StringEquals``, ``StringEqualsIfExists``,
|
||||
``ArnLikeIfExists`` and ``ForAllValues:StringLikeIfExists`` are treated alike --
|
||||
``secretsmanager_has_restrictive_resource_policy`` already ships IfExists paired with Null as its
|
||||
accepted restrictive form. Refusing the guarded spellings outright would also be wrong because
|
||||
``aws:SourceOrgPaths`` is multivalued, making a set operator the only correct way to write it.
|
||||
|
||||
``ForAnyValue:*`` needs no guard. AWS documents that for no matching context key, or if the key
|
||||
does not exist, it returns false -- so it fails closed on an Allow.
|
||||
"""
|
||||
groups = []
|
||||
condition = statement.get("Condition", {})
|
||||
if not isinstance(condition, dict):
|
||||
return groups
|
||||
null_guarded = _null_guarded_keys(condition)
|
||||
for operator, block in condition.items():
|
||||
if not isinstance(operator, str) or not isinstance(block, dict):
|
||||
continue
|
||||
lowered_operator = operator.lower()
|
||||
if lowered_operator not in _COMPARING_CONDITION_OPERATORS:
|
||||
continue
|
||||
group = []
|
||||
for key, value in block.items():
|
||||
if not isinstance(key, str) or key.lower() != condition_key:
|
||||
continue
|
||||
if (
|
||||
lowered_operator.startswith("forallvalues:")
|
||||
or lowered_operator.endswith("ifexists")
|
||||
) and key.lower() not in null_guarded:
|
||||
continue
|
||||
group.extend(value if isinstance(value, list) else [value])
|
||||
if group:
|
||||
groups.append(group)
|
||||
return groups
|
||||
|
||||
|
||||
def _pins_every_value(statement: dict, condition_key: str, qualifies) -> bool:
|
||||
"""Return True if some one operator holds condition_key to values that all qualify."""
|
||||
return any(
|
||||
all(qualifies(value) for value in group)
|
||||
for group in _enforced_condition_value_groups(statement, condition_key)
|
||||
)
|
||||
|
||||
|
||||
def _is_account_id(value) -> bool:
|
||||
"""Return True if the value is a literal 12-digit account ID."""
|
||||
return isinstance(value, str) and bool(ACCOUNT_ID_PATTERN.match(value))
|
||||
|
||||
|
||||
def _pins_source_account(statement: dict) -> bool:
|
||||
"""Return True if aws:SourceAccount is pinned to literal account IDs only."""
|
||||
return _pins_every_value(statement, "aws:sourceaccount", _is_account_id)
|
||||
|
||||
|
||||
def _arn_carries_account(value) -> bool:
|
||||
"""Return True if the ARN's account field is a literal account ID.
|
||||
|
||||
An ARN whose account field is absent (an S3 bucket ARN) or wildcarded does not
|
||||
confine the caller to one account, so aws:SourceAccount is still required.
|
||||
"""
|
||||
if not isinstance(value, str):
|
||||
return False
|
||||
arn_fields = value.split(":")
|
||||
return len(arn_fields) >= 5 and _is_account_id(arn_fields[4])
|
||||
|
||||
|
||||
def _pins_source_arn_to_account(statement: dict) -> bool:
|
||||
"""Return True if some one operator holds every aws:SourceArn value to an account."""
|
||||
return _pins_every_value(statement, "aws:sourcearn", _arn_carries_account)
|
||||
|
||||
|
||||
def _pins_source_organization(statement: dict) -> bool:
|
||||
"""Return True if the source is pinned to an organization or an OU path.
|
||||
|
||||
AWS documents aws:SourceOrgID and aws:SourceOrgPaths as confused-deputy mitigations
|
||||
in their own right, so an organization-scoped statement is not reported.
|
||||
"""
|
||||
return _pins_every_value(
|
||||
statement,
|
||||
"aws:sourceorgid",
|
||||
lambda value: isinstance(value, str)
|
||||
and bool(ORGANIZATION_ID_PATTERN.match(value)),
|
||||
) or _pins_every_value(
|
||||
statement,
|
||||
"aws:sourceorgpaths",
|
||||
lambda value: isinstance(value, str)
|
||||
and bool(ORGANIZATION_PATH_PATTERN.match(value)),
|
||||
)
|
||||
|
||||
|
||||
def _prevents_confused_deputy(statement: dict) -> bool:
|
||||
"""Return True if the statement carries a control AWS documents for *cross-service*
|
||||
confused-deputy prevention.
|
||||
|
||||
sts:ExternalId is deliberately absent. AWS documents it only for third-party access --
|
||||
an external ID is a value the third party supplies -- and an AWS service passes source
|
||||
account and source ARN context, never an external ID. Crediting it here would accept a
|
||||
control the calling service can never satisfy.
|
||||
"""
|
||||
return (
|
||||
_pins_source_account(statement)
|
||||
or _pins_source_arn_to_account(statement)
|
||||
or _pins_source_organization(statement)
|
||||
)
|
||||
|
||||
|
||||
def _has_enforced_condition(statement: dict) -> bool:
|
||||
"""Return True if the statement gates access on at least one enforced condition key.
|
||||
|
||||
A statement with no enforced condition at all places no constraint whatsoever on the
|
||||
caller. That wholly-unconditional state is a different (and more severe) posture than
|
||||
a constraint that is present but does not confine the source, and it is what
|
||||
iam_role_cross_service_confused_deputy_prevention reports.
|
||||
|
||||
This filter DELIBERATELY differs from the allow-list _enforced_condition_value_groups applies, and
|
||||
the two must not be unified. This one asks only whether the statement is gated at all, so a
|
||||
``Null`` presence test counts: it does gate access, even while binding the source to nothing.
|
||||
Pulling such a statement into scope is the safe direction -- it gets evaluated and reported
|
||||
rather than silently skipped. The other function asks what the statement PINS, where a presence
|
||||
test contributes no value and crediting one poisoned the shape test beside it.
|
||||
"""
|
||||
condition = statement.get("Condition", {})
|
||||
if not isinstance(condition, dict):
|
||||
return False
|
||||
for operator, block in condition.items():
|
||||
if not isinstance(operator, str) or not isinstance(block, dict):
|
||||
continue
|
||||
lowered_operator = operator.lower()
|
||||
if "not" in lowered_operator or lowered_operator.endswith("ifexists"):
|
||||
continue
|
||||
if any(isinstance(key, str) for key in block):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _is_plain_service_trust_policy(statements: list) -> bool:
|
||||
"""Return True if every statement is an Allow of assume-role to services only.
|
||||
|
||||
This is the trust-policy shape that the existing service-role checks assume. A policy
|
||||
that departs from it -- by carrying a Deny statement, an action outside the
|
||||
assume-role family such as sts:SetContext, or a non-service principal -- falls outside
|
||||
their evaluated population entirely, so its service principals go unassessed.
|
||||
"""
|
||||
for statement in statements:
|
||||
if not isinstance(statement, dict):
|
||||
return False
|
||||
if statement.get("Effect") != "Allow" or not _grants_assume_role(statement):
|
||||
return False
|
||||
principal = statement.get("Principal", {})
|
||||
if not isinstance(principal, dict) or set(principal.keys()) != {"Service"}:
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
class iam_role_service_trust_restricts_source_to_account(Check):
|
||||
"""Check whether a role's service-principal trust confines the request to a source account.
|
||||
|
||||
An AWS service principal permitted to assume a role without a source-account or source-ARN
|
||||
binding exposes the role to the confused-deputy problem: another customer's resource can induce
|
||||
the service to assume it. FAIL when a trust statement carries a condition that confines nothing;
|
||||
PASS when every in-scope statement binds a source; MANUAL for a statement using ``NotAction``,
|
||||
which is the one shape here that cannot be evaluated, since inverting it correctly is more than
|
||||
this check can claim. A trust policy is always present on a role, so there is no unreadable-document
|
||||
branch on this surface, unlike the two policy checks beside it.
|
||||
|
||||
Caveats:
|
||||
Companion to ``iam_role_cross_service_confused_deputy_prevention`` rather than a replacement.
|
||||
That check reports statements with no restrictive condition at all, on roles it classifies as
|
||||
service roles; this one asserts the clause it leaves open, so the wholly unconditional
|
||||
statement produces no finding here and the two can both report one role. Bindings are read
|
||||
from Allow statements only, so a trust policy confined solely through a Deny is still
|
||||
reported.
|
||||
"""
|
||||
|
||||
def execute(self) -> Check_Report_AWS:
|
||||
"""Flag service-principal trust whose present condition binds no account.
|
||||
|
||||
Account bindings are read from Allow statements ONLY. A Deny can also confine the source --
|
||||
`StringNotEquals` on `aws:SourceAccount` denies every account but one -- and this check does
|
||||
not evaluate that, so a trust policy confined solely through a Deny is reported even though
|
||||
it is confined. Rare, and it errs toward reporting rather than toward silence, but it is an
|
||||
unevaluated shape and is declared here rather than left to be inferred, as `NotAction`
|
||||
already is. A Deny still matters for scope: it takes the policy outside the plain-service
|
||||
shape the sibling checks assume, which is what brings the Allow statements beside it into
|
||||
this check's population.
|
||||
|
||||
MANUAL is used deliberately for the NotAction shape, and is not off-contract: 110 upstream
|
||||
checks emit it and `lib/check/models.py` places no restriction on it. THE COST, recorded so it
|
||||
is not rediscovered: `lib/outputs/asff/asff.py` SKIPS MANUAL findings, since MANUAL is not a
|
||||
valid Security Hub compliance state, so a Security Hub consumer sees NOTHING for a trust policy
|
||||
this check could not evaluate, and absence there reads as compliance. CSV and OCSF keep the
|
||||
status. The sibling token-wildcard check carries the same note, for the same reason.
|
||||
"""
|
||||
findings = []
|
||||
for role in iam_client.roles:
|
||||
# Service-linked roles are excluded: their trust relationship is managed by
|
||||
# the service and cannot be edited, so a finding would not be actionable.
|
||||
if "aws-service-role" in role.arn:
|
||||
continue
|
||||
|
||||
trust_policy = role.assume_role_policy or {}
|
||||
statements = trust_policy.get("Statement", [])
|
||||
if not isinstance(statements, list):
|
||||
statements = [statements]
|
||||
|
||||
# NotAction under Effect Allow grants everything except what it lists, so a
|
||||
# trust statement using it can permit sts:AssumeRole while carrying no Action
|
||||
# key. _grants_assume_role reads only Action, so such a statement would drop out
|
||||
# of service_statements below and the role would produce no finding at all.
|
||||
# Inverting NotAction correctly is more than this check can claim, so the role
|
||||
# is declared unevaluated rather than silently skipped.
|
||||
if any(
|
||||
isinstance(statement, dict)
|
||||
and statement.get("Effect") == "Allow"
|
||||
and "NotAction" in statement
|
||||
for statement in statements
|
||||
):
|
||||
report = Check_Report_AWS(metadata=self.metadata(), resource=role)
|
||||
report.region = iam_client.region
|
||||
report.status = "MANUAL"
|
||||
report.status_extended = (
|
||||
f"IAM Role {role.name} has a trust policy statement using NotAction, "
|
||||
"which this check does not evaluate, so whether a service principal is "
|
||||
"confined to this account could not be determined; review it manually."
|
||||
)
|
||||
findings.append(report)
|
||||
continue
|
||||
|
||||
service_statements = [
|
||||
statement
|
||||
for statement in statements
|
||||
if isinstance(statement, dict)
|
||||
and statement.get("Effect") == "Allow"
|
||||
and _grants_assume_role(statement)
|
||||
and _trusts_service_principal(statement)
|
||||
]
|
||||
if not service_statements:
|
||||
continue
|
||||
|
||||
# A wholly unconditional service-principal trust statement on an otherwise
|
||||
# plain service role is the fully-unprotected posture that
|
||||
# iam_role_cross_service_confused_deputy_prevention already reports. This
|
||||
# check asserts the narrower clause it does not: that a constraint which IS
|
||||
# present actually confines the source to an account. Statements are therefore
|
||||
# in scope when they carry an enforced condition, or when the trust policy
|
||||
# departs from the plain-service shape and so is evaluated by no other check.
|
||||
is_plain = _is_plain_service_trust_policy(statements)
|
||||
in_scope = [
|
||||
statement
|
||||
for statement in service_statements
|
||||
if _has_enforced_condition(statement) or not is_plain
|
||||
]
|
||||
if not in_scope:
|
||||
continue
|
||||
|
||||
unscoped = [
|
||||
statement
|
||||
for statement in in_scope
|
||||
if not _prevents_confused_deputy(statement)
|
||||
]
|
||||
|
||||
report = Check_Report_AWS(metadata=self.metadata(), resource=role)
|
||||
report.region = iam_client.region
|
||||
if unscoped:
|
||||
# The finding says no condition PINS the key to a literal of the right shape, not
|
||||
# that the statement sets no key. Most inputs reaching here do set one: StringLike
|
||||
# aws:SourceAccount "1234*", an unguarded ForAllValues, and Null "false" all set the
|
||||
# key while pinning nothing. The weaker claim is the one the code supports.
|
||||
report.status = "FAIL"
|
||||
report.status_extended = (
|
||||
f"IAM Role {role.name} trusts an AWS service principal without confining the "
|
||||
"request source, since no condition pins aws:SourceAccount to a literal "
|
||||
"account ID, aws:SourceArn to an ARN carrying one, or aws:SourceOrgID or "
|
||||
"aws:SourceOrgPaths to an organization."
|
||||
)
|
||||
elif all(
|
||||
_pins_source_account(statement)
|
||||
or _pins_source_arn_to_account(statement)
|
||||
for statement in in_scope
|
||||
):
|
||||
report.status = "PASS"
|
||||
report.status_extended = (
|
||||
f"IAM Role {role.name} confines every AWS service principal in its trust "
|
||||
"policy to a specific account."
|
||||
)
|
||||
else:
|
||||
# The organization route reaches PASS through _pins_source_organization, and an
|
||||
# organization may hold hundreds of accounts. Reporting it with the sentence above
|
||||
# told the operator something categorically stronger than was verified, so the two
|
||||
# postures get separate sentences: a reader needs to know which one they have.
|
||||
report.status = "PASS"
|
||||
report.status_extended = (
|
||||
f"IAM Role {role.name} confines every AWS service principal in its trust "
|
||||
"policy, but at least one statement is scoped to an organization rather than "
|
||||
"to a single account, so the trusted source may be any account within it."
|
||||
)
|
||||
findings.append(report)
|
||||
|
||||
return findings
|
||||
@@ -21,6 +21,62 @@ def _get_patterns_from_standard_value(value):
|
||||
return patterns
|
||||
|
||||
|
||||
def iam_pattern_matches(pattern: str, value: str) -> bool:
|
||||
"""Whether an IAM wildcard pattern from a policy document names a given value.
|
||||
|
||||
IAM honours exactly two metacharacters, ``*`` for any run of characters and ``?`` for exactly
|
||||
one; every other character is literal, ``[seq]`` character classes included. Matching is
|
||||
case-insensitive, as IAM matches action names.
|
||||
|
||||
Args:
|
||||
pattern: A value taken from a policy document -- an Action's service or operation, a
|
||||
Resource ARN field, a condition value. Surrounding whitespace is stripped, which IAM
|
||||
tolerates.
|
||||
value: The concrete string to test the pattern against.
|
||||
|
||||
Returns:
|
||||
True when IAM would consider the pattern to name that value.
|
||||
|
||||
MATCHED WITH A TWO-POINTER SCAN RATHER THAN A TRANSLATED REGEX, and the scan below is the same
|
||||
one as ``_action_matches`` in ``bedrockagentcore_full_access_policy_attached`` -- taken from it
|
||||
rather than written again, so there is one implementation of this under review and not two.
|
||||
Building ``.*`` for every ``*`` and calling ``re``, which is what this file's callers previously
|
||||
did, backtracks catastrophically on input the ACCOUNT controls: against the 17-character
|
||||
``bedrock-agentcore``, a pattern of N stars, a literal absent from the value, then N more stars
|
||||
took 0.7 ms at N=6, 62 ms at N=10 and 2287 ms at N=14 -- a 31-character policy value, where a
|
||||
managed policy document allows 6144. Cost rises with the number of quantifiers, which is the
|
||||
account's side of the input, so a short value does not bound it. A hang raises nothing, so the
|
||||
bare ``except Exception`` in ``prowler/lib/check/check.py`` cannot catch it and every finding for
|
||||
the account is discarded in silence. This scan is O(len(pattern) x len(value)) with no
|
||||
backtracking path at all: 0.006 ms on that same 31-character pattern, 0.214 ms at 4003.
|
||||
"""
|
||||
# Normalised here rather than inside the scan, so the scan stays identical to the one already
|
||||
# under review. IAM tolerates surrounding whitespace and matches case-insensitively, both of
|
||||
# which the regex form this replaced provided through .strip() and re.IGNORECASE.
|
||||
pattern = pattern.strip().lower()
|
||||
action = value.lower()
|
||||
|
||||
p = a = 0
|
||||
star = resume = -1
|
||||
while a < len(action):
|
||||
if p < len(pattern) and pattern[p] in ("?", action[a]):
|
||||
p += 1
|
||||
a += 1
|
||||
elif p < len(pattern) and pattern[p] == "*":
|
||||
star = p
|
||||
resume = a
|
||||
p += 1
|
||||
elif star >= 0:
|
||||
# Backtrack to the most recent star and let it absorb one more character. Only ever
|
||||
# one star is reconsidered, which is what bounds this at a product rather than a power.
|
||||
resume += 1
|
||||
a = resume
|
||||
p = star + 1
|
||||
else:
|
||||
return False
|
||||
return all(char == "*" for char in pattern[p:])
|
||||
|
||||
|
||||
def get_effective_actions(policy: dict) -> set[str]:
|
||||
"""
|
||||
Calculates the set of effectively allowed IAM actions from a policy document.
|
||||
|
||||
+1169
File diff suppressed because it is too large
Load Diff
+1369
File diff suppressed because it is too large
Load Diff
+1410
File diff suppressed because it is too large
Load Diff
@@ -8,6 +8,7 @@ from prowler.providers.aws.services.iam.lib.policy import (
|
||||
has_codebuild_trusted_principal,
|
||||
has_public_principal,
|
||||
has_restrictive_source_arn_condition,
|
||||
iam_pattern_matches,
|
||||
is_codebuild_using_allowed_github_org,
|
||||
is_condition_block_restrictive,
|
||||
is_condition_block_restrictive_organization,
|
||||
@@ -3062,3 +3063,102 @@ class Test_has_restrictive_source_arn_condition:
|
||||
},
|
||||
}
|
||||
assert has_restrictive_source_arn_condition(statement) is True
|
||||
|
||||
|
||||
class Test_iam_pattern_matches:
|
||||
"""IAM wildcard matching: only * and ? are metacharacters, and it must stay linear."""
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"pattern,value,expected",
|
||||
[
|
||||
("bedrock-agentcore", "bedrock-agentcore", True),
|
||||
("bedrock-*", "bedrock-agentcore", True),
|
||||
("*-agentcore", "bedrock-agentcore", True),
|
||||
("*agentcore*", "bedrock-agentcore", True),
|
||||
("bedrock-agentcor?", "bedrock-agentcore", True),
|
||||
("bedrock?agentcore", "bedrock-agentcore", True),
|
||||
("*", "bedrock-agentcore", True),
|
||||
("**", "bedrock-agentcore", True),
|
||||
("s3", "bedrock-agentcore", False),
|
||||
("bedrock", "bedrock-agentcore", False),
|
||||
("bedrock-agentcore?", "bedrock-agentcore", False),
|
||||
("", "bedrock-agentcore", False),
|
||||
("*", "", True),
|
||||
("", "", True),
|
||||
("?", "", False),
|
||||
(" bedrock-* ", "bedrock-agentcore", True),
|
||||
("BEDROCK-AGENTCORE", "bedrock-agentcore", True),
|
||||
("bedrock-agentcore", "BEDROCK-AGENTCORE", True),
|
||||
("a.c", "abc", False),
|
||||
("[bs]3", "b3", False),
|
||||
("[bs]3", "[bs]3", True),
|
||||
],
|
||||
)
|
||||
def test_semantics(self, pattern, value, expected):
|
||||
"""`*` and `?` are the only metacharacters; everything else is literal.
|
||||
|
||||
`a.c` against `abc` must be False or a regex dot has leaked in, and `[bs]3` against `b3`
|
||||
must be False or bracket classes have -- the latter being the false PASS that a `fnmatch`
|
||||
implementation shipped earlier in this campaign, where a Deny of
|
||||
`agent-registry:[Dd]eleteRegistry` denied nothing while appearing to deny everything.
|
||||
"""
|
||||
assert iam_pattern_matches(pattern, value) is expected
|
||||
|
||||
def test_adversarial_pattern_stays_fast(self):
|
||||
"""A wildcard-dense pattern must not blow up: this is a DoS guard, not a style preference.
|
||||
|
||||
Leading wildcards, a literal that cannot occur in the value, then more wildcards is the
|
||||
shape that forces a backtracking engine to try every distribution of the value's characters
|
||||
across the star groups. Translating to a regex and matching took 2287 ms on exactly this
|
||||
31-character pattern, growing about sevenfold per added wildcard pair, and a hang raises
|
||||
nothing so `check.py`'s bare `except Exception` cannot catch it -- the account's findings are
|
||||
discarded in silence. Policy values reach this from the account and managed policy documents
|
||||
allow 6144 characters.
|
||||
|
||||
The budget is deliberately loose: the linear form measures ~0.006 ms, so a 0.5 s ceiling
|
||||
cannot flake under load while still failing hard on a regex reimplementation.
|
||||
"""
|
||||
import time
|
||||
|
||||
pattern = "*" * 14 + "zzz" + "*" * 14
|
||||
assert len(pattern) == 31
|
||||
start = time.perf_counter()
|
||||
assert iam_pattern_matches(pattern, "bedrock-agentcore") is False
|
||||
assert time.perf_counter() - start < 0.5
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"subject",
|
||||
[
|
||||
"bedrock-agentcore",
|
||||
"iam",
|
||||
"PassRole",
|
||||
"GetWorkloadAccessTokenForUserId",
|
||||
"bedrock-agentcore.amazonaws.com",
|
||||
"workload-identity-directory/default/workload-identity/another-workload",
|
||||
],
|
||||
)
|
||||
def test_every_call_site_subject_stays_fast(self, subject):
|
||||
"""Each subject the iam checks match against must be fast, not just one of them.
|
||||
|
||||
These six are every distinct value passed as the second argument across the eight call sites
|
||||
in the two AgentCore policy checks -- an Action's service and operation field, an ARN's
|
||||
service and resource field, and an iam:PassedToService condition value. Probing once against
|
||||
a single constant would not do: cost rises with the subject's length as well as the pattern's,
|
||||
so the 3-character `iam` is cheap enough to come back clean while the 17- and 70-character
|
||||
subjects were taking seconds.
|
||||
"""
|
||||
import time
|
||||
|
||||
pattern = "*" * 14 + "zzz" + "*" * 14
|
||||
start = time.perf_counter()
|
||||
iam_pattern_matches(pattern, subject)
|
||||
assert time.perf_counter() - start < 0.5
|
||||
|
||||
def test_long_pattern_stays_fast(self):
|
||||
"""Cost must grow with the product of the lengths, not as a power of them."""
|
||||
import time
|
||||
|
||||
pattern = "*" * 2000 + "zzz" + "*" * 2000
|
||||
start = time.perf_counter()
|
||||
assert iam_pattern_matches(pattern, "bedrock-agentcore.amazonaws.com") is False
|
||||
assert time.perf_counter() - start < 0.5
|
||||
|
||||
Reference in New Issue
Block a user