From 34021d744a648608ae55c453482db5d180f2a042 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Wed, 15 Jul 2026 21:40:40 -0300 Subject: [PATCH 1/2] fix(iam): tighten service-principal trust + SAML audience binding (bug-hunt 5.6/5.7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 5.6 — a `Principal: { Service: }` trust (and the service-linked-role assumption gate) matched any AssumedRole ARN that merely *contained* the service host string, so a user who created and assumed a role literally named e.g. `lambda.amazonaws.com` satisfied a Service trust — a privilege escalation. Now a genuine service principal is required: its ARN must carry the AWS-reserved `aws-service-role//` path (new `evaluator::arn_denotes_service`, shared by `principal_is_service` and the STS SLR-assumption gate). - 5.7 — `AssumeRoleWithSAML` skipped audience binding for an unregistered SAML provider (fall-through soft pass) and for a registered provider when the assertion carried no audience claim. Now the named provider must be registered, and when its metadata declares an audience (`entityID`) the assertion must present a matching one; a missing audience is rejected. Tests: evaluator unit tests (genuine SLR allows, look-alike/impostor denied); STS unit tests (unregistered-provider denial, missing-audience rejection); existing SAML tests updated to register a provider (AWS requires it). 5.2 (cross-account AssumeRoot org-membership / management-account gating) is tracked as a separate PR — it needs a new cross-crate organization-membership resolver (IAM state has no org topology today), which is its own unit of work. --- crates/fakecloud-iam/src/evaluator.rs | 16 ++- crates/fakecloud-iam/src/evaluator_tests.rs | 73 +++++++++++- .../fakecloud-iam/src/sts_service/assume.rs | 53 +++++---- crates/fakecloud-iam/src/sts_service/mod.rs | 106 +++++++++++++++++- 4 files changed, 220 insertions(+), 28 deletions(-) diff --git a/crates/fakecloud-iam/src/evaluator.rs b/crates/fakecloud-iam/src/evaluator.rs index 593c25d01..f4316481f 100644 --- a/crates/fakecloud-iam/src/evaluator.rs +++ b/crates/fakecloud-iam/src/evaluator.rs @@ -882,9 +882,23 @@ fn principal_is_federated(principal: &Principal, provider: &str) -> bool { /// host string. False matches are avoided because unrelated role /// names would have to happen to contain `lambda.amazonaws.com` — /// unlikely in practice and never silently grant to user principals. +/// True when `arn` denotes the given AWS service principal — i.e. it is a +/// service-linked-role identity under the AWS-reserved `aws-service-role/ +/// /` path. The reserved path segment cannot be forged as an ordinary +/// role name, so this distinguishes a genuine service principal from a role a +/// user merely named after a service host. +pub(crate) fn arn_denotes_service(arn: &str, service: &str) -> bool { + arn.contains(&format!("aws-service-role/{service}")) +} + fn principal_is_service(principal: &Principal, service: &str) -> bool { + // Match the bare service host *anywhere* in the ARN would let a user create + // a role literally named e.g. `lambda.amazonaws.com`, assume it, and have + // its `assumed-role/lambda.amazonaws.com/...` ARN satisfy a + // `Principal: { Service: lambda.amazonaws.com }` trust — a privilege + // escalation. Require the reserved service-linked-role path instead. matches!(principal.principal_type, PrincipalType::AssumedRole) - && principal.arn.contains(service) + && arn_denotes_service(&principal.arn, service) } fn action_matches(action: &ActionMatch, request_action: &str) -> bool { diff --git a/crates/fakecloud-iam/src/evaluator_tests.rs b/crates/fakecloud-iam/src/evaluator_tests.rs index 4b090978e..26251be00 100644 --- a/crates/fakecloud-iam/src/evaluator_tests.rs +++ b/crates/fakecloud-iam/src/evaluator_tests.rs @@ -1091,11 +1091,19 @@ fn principal_user_arn_mismatch_is_deny() { } #[test] -fn principal_service_matches_assumed_role_containing_service_host() { - let p = assumed_role_principal( - ACCT_A, - "AWSServiceRoleForLambda.lambda.amazonaws.com/session", - ); +fn principal_service_matches_service_linked_role() { + // A genuine service principal is a service-linked-role identity under the + // reserved `aws-service-role//` path. + let slr = Principal { + arn: format!( + "arn:aws:iam::{ACCT_A}:role/aws-service-role/lambda.amazonaws.com/AWSServiceRoleForLambda" + ), + user_id: "AROASLR".into(), + account_id: ACCT_A.into(), + principal_type: PrincipalType::AssumedRole, + source_identity: None, + tags: None, + }; let resource = json!({ "Statement": [{ "Effect": "Allow", @@ -1105,9 +1113,16 @@ fn principal_service_matches_assumed_role_containing_service_host() { }] }); assert_eq!( - eval_cross(None, Some(resource), &p, ACCT_A), + eval_cross(None, Some(resource.clone()), &slr, ACCT_A), Decision::Allow ); + + // §5.6: a role a user merely NAMED after the service host must not match. + let impostor = assumed_role_principal(ACCT_A, "lambda.amazonaws.com/session"); + assert_eq!( + eval_cross(None, Some(resource), &impostor, ACCT_A), + Decision::ImplicitDeny + ); } #[test] @@ -2152,3 +2167,49 @@ fn unseeded_aws_managed_policy_grants_nothing() { "an unseeded managed ARN resolves to no document" ); } + +// --- §5.6 Service-principal trust must not be satisfied by a look-alike role - + +fn principal_assumed_role(arn: &str) -> Principal { + Principal { + arn: arn.to_string(), + user_id: "AROA:sess".into(), + account_id: "123456789012".into(), + principal_type: PrincipalType::AssumedRole, + source_identity: None, + tags: None, + } +} + +#[test] +fn service_principal_requires_service_linked_role_path() { + // A genuine service principal carries the reserved + // `aws-service-role//` path. + let slr = principal_assumed_role( + "arn:aws:iam::123456789012:role/aws-service-role/lambda.amazonaws.com/AWSServiceRoleForLambda", + ); + assert!(principal_is_service(&slr, "lambda.amazonaws.com")); + + // A user-created role merely NAMED after the service host must NOT satisfy + // a `Principal: { Service: lambda.amazonaws.com }` trust (§5.6). + let impostor = + principal_assumed_role("arn:aws:sts::123456789012:assumed-role/lambda.amazonaws.com/sess"); + assert!(!principal_is_service(&impostor, "lambda.amazonaws.com")); + + // And a plain unrelated role never matches. + let unrelated = + principal_assumed_role("arn:aws:sts::123456789012:assumed-role/my-app-role/sess"); + assert!(!principal_is_service(&unrelated, "lambda.amazonaws.com")); +} + +#[test] +fn arn_denotes_service_matches_only_reserved_path() { + assert!(arn_denotes_service( + "arn:aws:iam::123456789012:role/aws-service-role/ecs.amazonaws.com/AWSServiceRoleForECS", + "ecs.amazonaws.com" + )); + assert!(!arn_denotes_service( + "arn:aws:sts::123456789012:assumed-role/ecs.amazonaws.com/sess", + "ecs.amazonaws.com" + )); +} diff --git a/crates/fakecloud-iam/src/sts_service/assume.rs b/crates/fakecloud-iam/src/sts_service/assume.rs index bd20ec6dd..4302fe03a 100644 --- a/crates/fakecloud-iam/src/sts_service/assume.rs +++ b/crates/fakecloud-iam/src/sts_service/assume.rs @@ -134,10 +134,13 @@ impl StsService { .path .trim_start_matches("/aws-service-role/") .trim_end_matches('/'); + // Only a genuine service principal (identified by the reserved + // `aws-service-role//` path) may assume the SLR — a + // role a user named after the service host must not qualify. let caller_is_service = req .principal .as_ref() - .map(|p| p.arn.contains(expected_service)) + .map(|p| crate::evaluator::arn_denotes_service(&p.arn, expected_service)) .unwrap_or(false); if !caller_is_service { return Err(AwsServiceError::aws_error( @@ -714,25 +717,37 @@ impl StsService { ); let assumed_role_id_str = format!("{}:{}", role_id, role_session_name); - // If the named SAML provider IS registered, enforce its - // metadata-derived audience against the assertion's - // `` claim. Unregistered providers fall through — - // tests still in the pre-F1 era (and AWS itself, when the - // provider was nuked between assertion issue and use) get a - // soft pass; the trust policy below still gates the call. - if let Some(provider) = find_saml_provider(&accounts, &saml_provider_arn) { + // The named SAML provider must be registered before its assertion is + // trusted — AWS validates the assertion against the provider's IdP + // metadata, which is impossible for a provider that does not exist. + // Previously an unregistered provider fell through and skipped audience + // binding entirely, so a caller could dodge the check by naming a + // provider ARN that was never created. When the provider's metadata + // declares an audience (`entityID`), the assertion must carry a + // matching one; a *missing* audience claim previously slipped through + // and skipped the binding, so it is now rejected too. (A registered + // provider whose metadata declares no audience still skips the check by + // design — `expected_saml_audience` returns `None`.) + { + let provider = find_saml_provider(&accounts, &saml_provider_arn).ok_or_else(|| { + AwsServiceError::aws_error( + StatusCode::FORBIDDEN, + "AccessDenied", + format!( + "SAML provider {saml_provider_arn} is not registered; the assertion cannot be validated" + ), + ) + })?; if let Some(expected_aud) = expected_saml_audience(&provider.saml_metadata_document) { - if let Some(ref got) = saml_claims.audience { - if got != &expected_aud { - return Err(AwsServiceError::aws_error( - StatusCode::BAD_REQUEST, - "InvalidIdentityToken", - format!( - "SAML assertion audience '{got}' does not match SAML provider '{}'", - provider.arn - ), - )); - } + if saml_claims.audience.as_deref() != Some(expected_aud.as_str()) { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "InvalidIdentityToken", + format!( + "SAML assertion audience {:?} does not match the audience '{expected_aud}' declared by SAML provider '{}'", + saml_claims.audience, provider.arn + ), + )); } } } diff --git a/crates/fakecloud-iam/src/sts_service/mod.rs b/crates/fakecloud-iam/src/sts_service/mod.rs index ded59bf3f..300cca4f3 100644 --- a/crates/fakecloud-iam/src/sts_service/mod.rs +++ b/crates/fakecloud-iam/src/sts_service/mod.rs @@ -920,6 +920,26 @@ mod tests { arn } + /// Register a SAML provider in state so `AssumeRoleWithSAML` accepts its + /// assertion. The metadata deliberately carries no `entityID`, so + /// `expected_saml_audience` returns `None` and the audience check is + /// skipped (mirroring a provider whose metadata declares no audience). + fn register_saml_provider_in_state(state: &SharedIamState, arn: &str) { + let mut accounts = state.write(); + let s = accounts.get_or_create("123456789012"); + s.saml_providers.insert( + arn.to_string(), + crate::state::SamlProvider { + arn: arn.to_string(), + name: arn.rsplit('/').next().unwrap_or("idp").to_string(), + saml_metadata_document: "fake-saml-metadata-no-entity-id".to_string(), + created_at: Utc::now(), + valid_until: Utc::now() + chrono::Duration::days(365), + tags: Vec::new(), + }, + ); + } + // ── GetCallerIdentity ── #[tokio::test] @@ -1185,6 +1205,7 @@ mod tests { let (svc, state) = make_sts_service(); let trust = r#"{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Federated":"arn:aws:iam::123456789012:saml-provider/idp"},"Action":"sts:AssumeRoleWithSAML"}]}"#; let role_arn = create_role_in_state_with_trust(&state, "saml-role", trust); + register_saml_provider_in_state(&state, "arn:aws:iam::123456789012:saml-provider/idp"); let assertion = make_saml_assertion(); let req = sts_request( "AssumeRoleWithSAML", @@ -1205,7 +1226,8 @@ mod tests { #[tokio::test] async fn assume_role_with_saml_nonexistent_role_denied() { // §5.2: a missing role must NOT fall through to credential minting. - let (svc, _state) = make_sts_service(); + let (svc, state) = make_sts_service(); + register_saml_provider_in_state(&state, "arn:aws:iam::123456789012:saml-provider/idp"); let assertion = make_saml_assertion(); let req = sts_request( "AssumeRoleWithSAML", @@ -1229,6 +1251,85 @@ mod tests { ); } + #[tokio::test] + async fn assume_role_with_saml_unregistered_provider_denied() { + // §5.7: naming a SAML provider ARN that was never created must NOT + // fall through and skip assertion validation. + let (svc, state) = make_sts_service(); + let trust = r#"{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Federated":"arn:aws:iam::123456789012:saml-provider/ghost"},"Action":"sts:AssumeRoleWithSAML"}]}"#; + let role_arn = create_role_in_state_with_trust(&state, "saml-role", trust); + let assertion = make_saml_assertion(); + let req = sts_request( + "AssumeRoleWithSAML", + vec![ + ("RoleArn", &role_arn), + ( + "PrincipalArn", + "arn:aws:iam::123456789012:saml-provider/ghost", + ), + ("SAMLAssertion", &assertion), + ], + ); + let err = match svc.handle(req).await { + Err(e) => e, + Ok(_) => panic!("expected denial for an unregistered SAML provider"), + }; + assert_eq!(err.status(), StatusCode::FORBIDDEN); + assert!( + format!("{err:?}").contains("not registered"), + "expected provider-not-registered error, got {err:?}" + ); + } + + #[tokio::test] + async fn assume_role_with_saml_missing_audience_rejected() { + // §5.7: when the provider metadata declares an audience (`entityID`), + // an assertion with no matching audience claim must be rejected rather + // than skipping the binding. + let (svc, state) = make_sts_service(); + let trust = r#"{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Federated":"arn:aws:iam::123456789012:saml-provider/idp"},"Action":"sts:AssumeRoleWithSAML"}]}"#; + let role_arn = create_role_in_state_with_trust(&state, "saml-role", trust); + // Provider metadata declaring an entityID -> expected audience. + { + let mut accounts = state.write(); + let s = accounts.get_or_create("123456789012"); + s.saml_providers.insert( + "arn:aws:iam::123456789012:saml-provider/idp".to_string(), + crate::state::SamlProvider { + arn: "arn:aws:iam::123456789012:saml-provider/idp".to_string(), + name: "idp".to_string(), + saml_metadata_document: + r#""#.to_string(), + created_at: Utc::now(), + valid_until: Utc::now() + chrono::Duration::days(365), + tags: Vec::new(), + }, + ); + } + // `make_saml_assertion` carries no claim. + let assertion = make_saml_assertion(); + let req = sts_request( + "AssumeRoleWithSAML", + vec![ + ("RoleArn", &role_arn), + ( + "PrincipalArn", + "arn:aws:iam::123456789012:saml-provider/idp", + ), + ("SAMLAssertion", &assertion), + ], + ); + let err = match svc.handle(req).await { + Err(e) => e, + Ok(_) => panic!("expected InvalidIdentityToken for missing audience"), + }; + assert_eq!(err.status(), StatusCode::BAD_REQUEST); + assert!( + format!("{err:?}").contains("audience"), + "expected audience-mismatch error, got {err:?}" + ); + } + // ── GetSessionToken ── #[tokio::test] @@ -1668,9 +1769,10 @@ mod tests { let (svc, state) = make_sts_service(); let trust = r#"{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"AWS":"*"},"Action":"sts:AssumeRoleWithSAML"}]}"#; let role_arn = create_role_in_state_with_trust(&state, "saml-role", trust); + let provider_arn = "arn:aws:iam::123456789012:saml-provider/idp"; + register_saml_provider_in_state(&state, provider_arn); let saml_xml = r#"jane"#; let saml_b64 = base64::engine::general_purpose::STANDARD.encode(saml_xml); - let provider_arn = "arn:aws:iam::123456789012:saml-provider/idp"; let req = sts_request( "AssumeRoleWithSAML", vec![ From 3725a705424dc9c7e8ebfbbb4a01798eed868ebc Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Thu, 16 Jul 2026 00:26:59 -0300 Subject: [PATCH 2/2] fix(iam): don't require SAML provider registration for AssumeRoleWithSAML MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 5.7 change required the named SAML provider to be registered, but the conformance baseline (sts_assume_role_with_saml) deliberately assumes with an unregistered provider and expects success — fakecloud tolerates that (there is no provider metadata to bind an audience against anyway). Restore the soft-pass for unregistered providers; keep the real fix: a REGISTERED provider whose metadata declares an audience (entityID) still requires the assertion to carry a matching audience (a missing/mismatched one is rejected). Drop the now-invalid unregistered-provider-denied test. --- .../fakecloud-iam/src/sts_service/assume.rs | 32 +++++++------------ crates/fakecloud-iam/src/sts_service/mod.rs | 30 ----------------- 2 files changed, 11 insertions(+), 51 deletions(-) diff --git a/crates/fakecloud-iam/src/sts_service/assume.rs b/crates/fakecloud-iam/src/sts_service/assume.rs index 4302fe03a..59fb6cd48 100644 --- a/crates/fakecloud-iam/src/sts_service/assume.rs +++ b/crates/fakecloud-iam/src/sts_service/assume.rs @@ -717,27 +717,17 @@ impl StsService { ); let assumed_role_id_str = format!("{}:{}", role_id, role_session_name); - // The named SAML provider must be registered before its assertion is - // trusted — AWS validates the assertion against the provider's IdP - // metadata, which is impossible for a provider that does not exist. - // Previously an unregistered provider fell through and skipped audience - // binding entirely, so a caller could dodge the check by naming a - // provider ARN that was never created. When the provider's metadata - // declares an audience (`entityID`), the assertion must carry a - // matching one; a *missing* audience claim previously slipped through - // and skipped the binding, so it is now rejected too. (A registered - // provider whose metadata declares no audience still skips the check by - // design — `expected_saml_audience` returns `None`.) - { - let provider = find_saml_provider(&accounts, &saml_provider_arn).ok_or_else(|| { - AwsServiceError::aws_error( - StatusCode::FORBIDDEN, - "AccessDenied", - format!( - "SAML provider {saml_provider_arn} is not registered; the assertion cannot be validated" - ), - ) - })?; + // When the named SAML provider IS registered and its metadata declares + // an audience (`entityID`), the assertion must carry a *matching* + // audience. Previously a missing audience claim slipped through and + // skipped the binding entirely, so an assertion with no `` + // was accepted for a provider that pins one. A registered provider + // whose metadata declares no audience still skips the check by design + // (`expected_saml_audience` returns `None`), and an unregistered + // provider falls through here — fakecloud tolerates it, matching the + // recorded conformance baseline (`sts_assume_role_with_saml`), and there + // is no metadata to bind an audience against in that case anyway. + if let Some(provider) = find_saml_provider(&accounts, &saml_provider_arn) { if let Some(expected_aud) = expected_saml_audience(&provider.saml_metadata_document) { if saml_claims.audience.as_deref() != Some(expected_aud.as_str()) { return Err(AwsServiceError::aws_error( diff --git a/crates/fakecloud-iam/src/sts_service/mod.rs b/crates/fakecloud-iam/src/sts_service/mod.rs index 300cca4f3..4d38b960d 100644 --- a/crates/fakecloud-iam/src/sts_service/mod.rs +++ b/crates/fakecloud-iam/src/sts_service/mod.rs @@ -1251,36 +1251,6 @@ mod tests { ); } - #[tokio::test] - async fn assume_role_with_saml_unregistered_provider_denied() { - // §5.7: naming a SAML provider ARN that was never created must NOT - // fall through and skip assertion validation. - let (svc, state) = make_sts_service(); - let trust = r#"{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Federated":"arn:aws:iam::123456789012:saml-provider/ghost"},"Action":"sts:AssumeRoleWithSAML"}]}"#; - let role_arn = create_role_in_state_with_trust(&state, "saml-role", trust); - let assertion = make_saml_assertion(); - let req = sts_request( - "AssumeRoleWithSAML", - vec![ - ("RoleArn", &role_arn), - ( - "PrincipalArn", - "arn:aws:iam::123456789012:saml-provider/ghost", - ), - ("SAMLAssertion", &assertion), - ], - ); - let err = match svc.handle(req).await { - Err(e) => e, - Ok(_) => panic!("expected denial for an unregistered SAML provider"), - }; - assert_eq!(err.status(), StatusCode::FORBIDDEN); - assert!( - format!("{err:?}").contains("not registered"), - "expected provider-not-registered error, got {err:?}" - ); - } - #[tokio::test] async fn assume_role_with_saml_missing_audience_rejected() { // §5.7: when the provider metadata declares an audience (`entityID`),