feat(iam): enforce AssumeRole trust policies when enforcement is enabled - #1552
Conversation
When floci.iam.enforcement-enabled is true, STS AssumeRole now evaluates the target role's trust policy (AssumeRolePolicyDocument) against the caller and returns AccessDenied if it is not permitted. Enforcement is off by default, so existing behavior is unchanged. Trust policies are principal-centric and carry no Resource element, so the identity/resource-oriented IamPolicyEvaluator cannot evaluate them. A focused AssumeRolePolicyEvaluator matches each statement's Action (sts:AssumeRole) and AWS Principal (account id, account-root ARN, exact principal ARN, or "*") against the caller, with explicit Deny taking precedence. The role is resolved in its own account (from the role ARN) via a new IamService.findRole overload, since roles are account-namespaced. Roles unknown to Floci stay permissive to preserve backward compatibility. Tests: unit coverage for the evaluator across principal/action/deny/malformed cases, plus an enforcement-enabled integration test asserting a permitted caller succeeds, an unauthorized caller is denied, and an unknown role stays permissive.
|
This is really clean, Alex, and the evaluator is nicely tested. The wire side checks out: AccessDenied at 403 is the right denial, and the action, principal, and explicit deny precedence all match how AWS evaluates a trust policy. Two gaps worth noting, both fine to defer as documented limitations since they are about evaluating more than the trust policy itself:
{
"Effect": "Allow",
"Principal": { "AWS": "arn:aws:iam::111111111111:root" },
"Action": "sts:AssumeRole",
"Condition": { "StringEquals": { "sts:ExternalId": "secret-123" } }
}So the caller gets in without passing
One tiny thing: AWS prefixes the denial message with the caller, like: so adding the Thanks for gating this behind the opt in flag and keeping the default behavior untouched, nice and safe. |
Resolve conflicts in IamService.java and StsQueryHandler.java against main's account-aware session changes. Also address review feedback: - prefix the AssumeRole AccessDenied message with 'User: <arn>' to match AWS - document that Condition blocks (sts:ExternalId) and the caller's identity policy are not evaluated yet (known limitations)
|
Thanks @hectorvent! Addressed in 5f4a3e8 (also merged latest
The merge against main was non-trivial — it reworked |
|
| Filename | Overview |
|---|---|
| src/main/java/io/github/hectorvent/floci/services/iam/AssumeRolePolicyEvaluator.java | New evaluator correctly handles Action/NotAction, AWS principal forms, assumed-role ARN resolution, and Deny-wins precedence; a statement with a missing Effect key silently defaults to ALLOW. |
| src/main/java/io/github/hectorvent/floci/services/iam/StsQueryHandler.java | Enforcement hook is correctly placed before session registration; callerArn resolution and permissive fallback for unknown roles look correct. |
| src/main/java/io/github/hectorvent/floci/services/iam/IamService.java | Adds account-scoped findRole overload; falls back to flat lookup ignoring accountId when AccountAwareStorageBackend is not in use (known limitation from prior review thread). |
| src/test/java/io/github/hectorvent/floci/services/iam/AssumeRolePolicyEvaluatorTest.java | Good unit coverage of all principal forms, NotAction, Deny-wins, assumed-role ARN resolution, service-only principal, and malformed documents. |
| src/test/java/io/github/hectorvent/floci/services/iam/AssumeRoleTrustPolicyIntegrationTest.java | Covers the three critical end-to-end cases (permitted caller, denied caller, unknown role stays permissive) with enforcement-enabled profile; no issues. |
| docs/services/sts.md | Documents the new enforcement flag and its known limitations (Condition blocks, caller-side identity policy); accurate and complete for the feature scope. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Client
participant StsQueryHandler
participant IamService
participant AssumeRolePolicyEvaluator
Client->>StsQueryHandler: AssumeRole(RoleArn, RoleSessionName)
StsQueryHandler->>StsQueryHandler: Extract roleName, accountId from ARN
alt enforcement disabled
StsQueryHandler-->>Client: 200 OK (credentials)
else enforcement enabled
StsQueryHandler->>IamService: findRole(roleAccountId, roleName)
alt role unknown to Floci
IamService-->>StsQueryHandler: Optional.empty()
StsQueryHandler-->>Client: 200 OK (permissive fallback)
else role known
IamService-->>StsQueryHandler: Optional[IamRole]
StsQueryHandler->>StsQueryHandler: Resolve callerArn from Authorization header
StsQueryHandler->>AssumeRolePolicyEvaluator: allows(trustPolicy, callerArn, callerAccount)
AssumeRolePolicyEvaluator->>AssumeRolePolicyEvaluator: Evaluate Action / NotAction
AssumeRolePolicyEvaluator->>AssumeRolePolicyEvaluator: Match AWS Principal
AssumeRolePolicyEvaluator->>AssumeRolePolicyEvaluator: Apply Deny-wins precedence
AssumeRolePolicyEvaluator-->>StsQueryHandler: true / false
alt caller permitted
StsQueryHandler-->>Client: 200 OK (credentials)
else caller denied
StsQueryHandler-->>Client: 403 AccessDenied
end
end
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Client
participant StsQueryHandler
participant IamService
participant AssumeRolePolicyEvaluator
Client->>StsQueryHandler: AssumeRole(RoleArn, RoleSessionName)
StsQueryHandler->>StsQueryHandler: Extract roleName, accountId from ARN
alt enforcement disabled
StsQueryHandler-->>Client: 200 OK (credentials)
else enforcement enabled
StsQueryHandler->>IamService: findRole(roleAccountId, roleName)
alt role unknown to Floci
IamService-->>StsQueryHandler: Optional.empty()
StsQueryHandler-->>Client: 200 OK (permissive fallback)
else role known
IamService-->>StsQueryHandler: Optional[IamRole]
StsQueryHandler->>StsQueryHandler: Resolve callerArn from Authorization header
StsQueryHandler->>AssumeRolePolicyEvaluator: allows(trustPolicy, callerArn, callerAccount)
AssumeRolePolicyEvaluator->>AssumeRolePolicyEvaluator: Evaluate Action / NotAction
AssumeRolePolicyEvaluator->>AssumeRolePolicyEvaluator: Match AWS Principal
AssumeRolePolicyEvaluator->>AssumeRolePolicyEvaluator: Apply Deny-wins precedence
AssumeRolePolicyEvaluator-->>StsQueryHandler: true / false
alt caller permitted
StsQueryHandler-->>Client: 200 OK (credentials)
else caller denied
StsQueryHandler-->>Client: 403 AccessDenied
end
end
end
Reviews (4): Last reviewed commit: "fix(iam): match an assumed-role caller a..." | Re-trigger Greptile
Greptile review on floci-io#1552: - Restore the FLOCI_SERVICES_STS_ENABLED Configuration section that was dropped when the Trust Policy Enforcement section was added. - Document that NotAction is not evaluated (a Deny with NotAction won't block).
|
Thanks @greptile-apps — went through all three:
|
|
All three responses are reasonable — thanks for the thorough follow-up.
No further concerns on these. The PR looks good to go from my end. |
evaluateStatement only inspected the Action key, so a trust statement using NotAction returned NO_MATCH and was skipped entirely. A Deny that should block sts:AssumeRole (NotAction excluding some other action) was therefore ignored, over-permitting AssumeRole when IAM enforcement is enabled. Add actionApplies() mirroring IamPolicyEvaluator's Action/NotAction semantics: Action applies when any pattern matches, NotAction applies when none match. Covered by three new AssumeRolePolicyEvaluatorTest cases.
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
…ipal When a caller uses assumed-role temporary credentials, resolveCallerArn (rightly, for GetCallerIdentity) yields the STS assumed-role ARN (arn:aws:sts::ACCT:assumed-role/Role/session). enforceTrustPolicy passes that to the trust-policy evaluator, but role trust policies name the role's IAM principal ARN (arn:aws:iam::ACCT:role/Role) — the canonical form AWS resolves the session back to. So a legitimate role-to-role AssumeRole was denied. matchesAwsPrincipal now also normalizes an assumed-role caller ARN to its underlying IAM role ARN and matches the principal against that, while still matching an exact session-ARN principal. resolveCallerArn is unchanged, so GetCallerIdentity keeps returning the assumed-role ARN. Adds AssumeRolePolicyEvaluatorTest cases for role-ARN match, differing-role denial, and exact session-ARN match.
Summary
When
FLOCI_SERVICES_IAM_ENFORCEMENT_ENABLED=true, STSAssumeRolenow evaluates the target role's trust policy (AssumeRolePolicyDocument) against the caller and returnsAccessDeniedif it isn't permitted. Enforcement is off by default, so existing behavior is unchanged.Trust policies are principal-centric and carry no
Resourceelement, so the identity/resource-orientedIamPolicyEvaluatorcan't evaluate them. This adds a focusedAssumeRolePolicyEvaluatorthat:Actionagainststs:AssumeRole(supportssts:*/*),Principal—"*", a bare account id, an account-root ARN (arn:aws:iam::<acct>:root), or an exact principal ARN,Denywins, otherwise a matchingAllowgrants.The role is resolved in its own account (parsed from the role ARN) via a new
IamService.findRole(account, roleName)overload, since IAM roles are account-namespaced. Roles that Floci has no record of stay permissive, so this only affects roles created through IAM with a real trust policy.Type of change
feat:) — gated behind an existing opt-in flagAWS Compatibility
Query protocol, unchanged wire format. Only adds an authorization decision on
AssumeRolewhen enforcement is enabled.Service/Federatedprincipals are intentionally not matched against SigV4 callers (function-before-fidelity).Checklist
./mvnw testpasses locally (targeted:AssumeRolePolicyEvaluatorTest12 +AssumeRoleTrustPolicyIntegrationTest3 +IamIntegrationTest37 green; built in a JDK 25 container)Independent of #1549 and #1551, though it composes with them for full cross-account assume-role fidelity.