diff --git a/crates/auth/src/policy/condition.rs b/crates/auth/src/policy/condition.rs index 6164cce58..1618dfce4 100644 --- a/crates/auth/src/policy/condition.rs +++ b/crates/auth/src/policy/condition.rs @@ -32,15 +32,33 @@ use super::matcher::wildcard_match; /// Absent key is vacuously true. /// - `ForAnyValue`: at least one context value must match some policy value. /// Absent key is false (unless wrapped in `IfExists`). +/// +/// Policy variables in the values are read as in a `2012-10-17` document; +/// [`evaluate_condition_with`] takes the document's choice. pub fn evaluate_condition(condition: &Condition, context: &impl ConditionContext) -> bool { + evaluate_condition_with(condition, true, context) +} + +/// [`evaluate_condition`] for a statement of a document whose `Version` is +/// `2012-10-17` when `version_2012` is true +/// ([`super::document::PolicyDocument::expands_policy_variables`]). +/// +/// The value says how the text between `${` and `}` in a condition value is +/// read ([`reads_iam_variable_syntax`]); `${key}` expands whatever it is. +pub fn evaluate_condition_with( + condition: &Condition, + version_2012: bool, + context: &impl ConditionContext, +) -> bool { let context_values = context.resolve_key(&condition.key); let is_multivalued = context.is_multivalued_key(&condition.key); + let iam_syntax = reads_iam_variable_syntax(&condition.operator, version_2012); // Expand policy variables (e.g. `${aws:PrincipalTag/Team}`) in condition values. let expanded_values: Vec = condition .values .iter() - .map(|v| expand_policy_variables(v, context)) + .map(|v| expand_policy_variables(v, iam_syntax, context)) .collect(); match &condition.operator { @@ -179,17 +197,24 @@ pub enum ConditionMatch { /// /// Returns [`ConditionMatch::Unresolvable`] when the key is unresolved and the /// context reports it as service-populated (case 3), and when a condition value -/// reads such a key through a `${name}` policy variable, since the variable has -/// no value to expand to. Otherwise returns `Match` or `NoMatch` from the -/// ordinary evaluation, so cases 1 and 2 are unchanged. +/// reads such a key through a `${name}` policy variable, or through a +/// `${name, 'default'}` one where [`reads_iam_variable_syntax`] holds, since +/// the variable has no value to expand to and the service, which has one, +/// would not use the default. Otherwise returns `Match` or `NoMatch` from the +/// ordinary evaluation, so cases 1 and 2 are unchanged. `version_2012` is as +/// in [`evaluate_condition_with`]. pub fn evaluate_condition_match( condition: &Condition, + version_2012: bool, context: &impl ConditionContext, ) -> ConditionMatch { - if is_unresolvable(&condition.key, context) || reads_unresolvable_variable(condition, context) { + let iam_syntax = reads_iam_variable_syntax(&condition.operator, version_2012); + if is_unresolvable(&condition.key, context) + || reads_unresolvable_variable(condition, iam_syntax, context) + { return ConditionMatch::Unresolvable; } - if evaluate_condition(condition, context) { + if evaluate_condition_with(condition, version_2012, context) { ConditionMatch::Match } else { ConditionMatch::NoMatch @@ -202,27 +227,58 @@ fn is_unresolvable(key: &str, context: &impl ConditionContext) -> bool { context.resolve_key(key).is_none() && context.is_unresolvable_key(key) } -/// Whether a value of `condition` names, as a `${name}` policy variable, a key -/// [`is_unresolvable`] reports. -fn reads_unresolvable_variable(condition: &Condition, context: &impl ConditionContext) -> bool { - condition - .values - .iter() - .any(|value| policy_variable_names(value).any(|name| is_unresolvable(name, context))) +/// Whether a value of `condition` names, as a `${name}` or `${name, 'default'}` +/// policy variable, a key [`is_unresolvable`] reports. +/// +/// The names come from [`super::variables::variable_key_names`], which reads +/// the text between the braces the way [`expand_policy_variables`] expands +/// it with the same `iam_syntax`, so with it a default written beside such a +/// key does not hide the key from this check. +fn reads_unresolvable_variable( + condition: &Condition, + iam_syntax: bool, + context: &impl ConditionContext, +) -> bool { + condition.values.iter().any(|value| { + super::variables::variable_key_names(value, iam_syntax) + .any(|name| is_unresolvable(name, context)) + }) } -/// The names of the `${name}` policy variables in `value`, in the order -/// [`expand_policy_variables`] reads them. A `${` with no closing brace names -/// nothing, as it expands to itself. -fn policy_variable_names(value: &str) -> impl Iterator { - let mut rest = value; - std::iter::from_fn(move || { - let start = rest.find("${")?; - let after_start = &rest[start + 2..]; - let end = after_start.find('}')?; - rest = &after_start[end + 1..]; - Some(&after_start[..end]) - }) +/// Whether the values of a condition under `operator`, in a document whose +/// `Version` is `2012-10-17` when `version_2012` is true, are read with IAM's +/// variable syntax: the `${key, 'default'}` form, and a key name trimmed of +/// the code points up to U+0020. +/// +/// IAM reads that syntax only in a `2012-10-17` document and under the string +/// and ARN operators, with or without a set qualifier or `IfExists` (IAM +/// policy simulator, 2026-11-03, two samples per row). In a `2008-10-17` +/// document and in one with no `Version`, `${aws:username, 'x'}` is compared +/// as written under `StringEquals` and `StringNotEquals`. In a `2012-10-17` +/// document a default does not take part under `NumericEquals`, +/// `DateGreaterThan`, `IpAddress`, `Bool` or `Null`, while it does under +/// `StringEquals` and `ArnLike`. Elsewhere the text between the braces is the +/// key name as written, which is how condition values were read before the +/// default form: `${key}` still expands there, as it did. +fn reads_iam_variable_syntax(operator: &ConditionOperator, version_2012: bool) -> bool { + version_2012 && operator_reads_variables(operator) +} + +/// Whether `operator`, unwrapped from its set qualifier and `IfExists`, is a +/// string or ARN operator. +fn operator_reads_variables(operator: &ConditionOperator) -> bool { + match operator { + ConditionOperator::ForAllValues(inner) + | ConditionOperator::ForAnyValue(inner) + | ConditionOperator::IfExists(inner) => operator_reads_variables(inner), + ConditionOperator::StringEquals + | ConditionOperator::StringNotEquals + | ConditionOperator::StringEqualsIgnoreCase + | ConditionOperator::StringNotEqualsIgnoreCase + | ConditionOperator::StringLike + | ConditionOperator::StringNotLike => true, + other => is_arn_operator(other), + } } /// Evaluate a statement's whole condition block (all conditions are ANDed). @@ -230,14 +286,17 @@ fn policy_variable_names(value: &str) -> impl Iterator { /// `NoMatch` dominates: if any condition is definitely not satisfied the /// statement does not apply, even if another condition is unresolvable. Failing /// that, any unresolvable condition makes the block `Unresolvable` (fail -/// closed). Otherwise every condition matched. +/// closed). Otherwise every condition matched. `version_2012` is whether the +/// statement's document has `Version` `2012-10-17`, as in +/// [`evaluate_condition_with`]. pub fn evaluate_statement_conditions( conditions: &[Condition], + version_2012: bool, context: &impl ConditionContext, ) -> ConditionMatch { let mut any_unresolvable = false; for condition in conditions { - match evaluate_condition_match(condition, context) { + match evaluate_condition_match(condition, version_2012, context) { ConditionMatch::Match => {} ConditionMatch::NoMatch => return ConditionMatch::NoMatch, ConditionMatch::Unresolvable => any_unresolvable = true, @@ -252,52 +311,29 @@ pub fn evaluate_statement_conditions( /// Expand IAM policy variables in a string value. /// -/// Replaces `${variable}` patterns with their resolved values from the context. -/// Supported variables: `aws:PrincipalTag/*` and any single-valued key the -/// context can resolve. A variable that names an unresolvable key stays -/// literal (IAM behavior), and so does one that names a multivalued key -/// (`aws:TagKeys`, `dynamodb:LeadingKeys`, `dynamodb:Attributes`): a set of -/// values has no one value to expand to. IAM leaves such a variable as text -/// (measured 2026-10-27: a Deny on `aws:ResourceTag/team` equal to -/// `${aws:TagKeys}` applies to no request, whatever order the request lists -/// its tag keys in, and the same Deny under `StringNotEquals` applies to -/// every request). -fn expand_policy_variables(value: &str, context: &impl ConditionContext) -> String { - if !value.contains("${") { - return value.to_owned(); - } - - let mut result = String::with_capacity(value.len()); - let mut rest = value; - - while let Some(start) = rest.find("${") { - result.push_str(&rest[..start]); - let after_start = &rest[start + 2..]; - if let Some(end) = after_start.find('}') { - let var_name = &after_start[..end]; - let resolved = if context.is_multivalued_key(var_name) { - None - } else { - context.resolve_key(var_name) - }; - if let Some(vals) = resolved { - // A single-valued key resolves to one value. - if let Some(v) = vals.first() { - result.push_str(v); - } - } else { - // Unresolvable or multivalued: leave the variable literal. - result.push_str(&rest[start..start + 3 + end]); - } - rest = &after_start[end + 1..]; - } else { - // No closing brace — copy literally. - result.push_str(&rest[start..]); - rest = ""; - } - } - result.push_str(rest); - result +/// Replaces `${variable}` and `${variable, 'default'}` patterns with their +/// resolved values from the context (see [`super::variables`]). Supported +/// variables: `aws:PrincipalTag/*` and any single-valued key the context can +/// resolve. A variable whose key the request does not carry, with no default, +/// stays literal, which gives IAM's answer except when the compared value +/// holds the variable's text. A variable that names a multivalued key +/// (`aws:TagKeys`, `dynamodb:LeadingKeys`, `dynamodb:Attributes`) also stays +/// literal: a set of values has no one value to expand to. IAM leaves such a +/// variable as text (measured 2026-10-27: a Deny on `aws:ResourceTag/team` +/// equal to `${aws:TagKeys}` applies to no request, whatever order the +/// request lists its tag keys in, and the same Deny under `StringNotEquals` +/// applies to every request). A variable naming a key the context reports +/// unresolvable also stays literal here, with or without a default; the +/// condition fails closed on it before this expansion is compared +/// ([`evaluate_condition_match`]). Without `iam_syntax` +/// ([`reads_iam_variable_syntax`]) the default form is not read, so +/// `${key, 'default'}` stays literal too. +fn expand_policy_variables( + value: &str, + iam_syntax: bool, + context: &impl ConditionContext, +) -> String { + super::variables::expand_condition_value(value, iam_syntax, context) } /// Unwrap an `IfExists` wrapper if present. diff --git a/crates/auth/src/policy/condition/tests/three_case.rs b/crates/auth/src/policy/condition/tests/three_case.rs index f0aabd82f..896a5086e 100644 --- a/crates/auth/src/policy/condition/tests/three_case.rs +++ b/crates/auth/src/policy/condition/tests/three_case.rs @@ -18,13 +18,19 @@ fn present_key_matches_or_not() { "aws:PrincipalArn", vec!["arn:a"], ); - assert_eq!(evaluate_condition_match(&eq, &ctx), ConditionMatch::Match); + assert_eq!( + evaluate_condition_match(&eq, true, &ctx), + ConditionMatch::Match + ); let ne = cond( ConditionOperator::StringEquals, "aws:PrincipalArn", vec!["arn:b"], ); - assert_eq!(evaluate_condition_match(&ne, &ctx), ConditionMatch::NoMatch); + assert_eq!( + evaluate_condition_match(&ne, true, &ctx), + ConditionMatch::NoMatch + ); } #[test] @@ -38,7 +44,7 @@ fn absent_key_keeps_iam_semantics() { vec!["x"], ); assert_eq!( - evaluate_condition_match(&pos, &ctx), + evaluate_condition_match(&pos, true, &ctx), ConditionMatch::NoMatch ); let neg = cond( @@ -46,7 +52,10 @@ fn absent_key_keeps_iam_semantics() { "aws:PrincipalTag/team", vec!["x"], ); - assert_eq!(evaluate_condition_match(&neg, &ctx), ConditionMatch::Match); + assert_eq!( + evaluate_condition_match(&neg, true, &ctx), + ConditionMatch::Match + ); } #[test] @@ -62,12 +71,69 @@ fn unresolvable_key_is_unresolvable_for_every_operator() { ] { let c = cond(op, "aws:PrincipalOrgID", vec!["o-1"]); assert_eq!( - evaluate_condition_match(&c, &ctx), + evaluate_condition_match(&c, true, &ctx), ConditionMatch::Unresolvable ); } } +#[test] +fn variable_naming_unresolvable_key_is_unresolvable_with_or_without_default() { + // Case 3 read through a condition value: the variable has no value to + // expand to, and the service, which has one, would not use the default. + // Every operator yields Unresolvable, so a Deny applies and an Allow does + // not grant whatever the operator says. A later value of the condition + // that names the key counts as the first one does. + let ctx = TestContext::new() + .with("aws:PrincipalTag/team", vec!["blue"]) + .unresolvable("aws:PrincipalOrgID"); + for values in [ + vec!["${aws:PrincipalOrgID}"], + vec!["${aws:PrincipalOrgID, 'blue'}"], + vec!["${aws:PrincipalOrgID,'blue'}"], + vec!["x-${aws:PrincipalOrgID, 'blue'}-y"], + vec!["red", "${aws:PrincipalOrgID, 'blue'}"], + ] { + for op in [ + ConditionOperator::StringEquals, + ConditionOperator::StringNotEquals, + ConditionOperator::StringLike, + ] { + let c = cond(op, "aws:PrincipalTag/team", values.clone()); + assert_eq!( + evaluate_condition_match(&c, true, &ctx), + ConditionMatch::Unresolvable, + "{values:?}" + ); + } + } + // The key present: its value is used, not the default. + let present = TestContext::new() + .with("aws:PrincipalTag/team", vec!["blue"]) + .with("aws:PrincipalOrgID", vec!["blue"]) + .unresolvable("aws:PrincipalOrgID"); + let c = cond( + ConditionOperator::StringEquals, + "aws:PrincipalTag/team", + vec!["${aws:PrincipalOrgID, 'red'}"], + ); + assert_eq!( + evaluate_condition_match(&c, true, &present), + ConditionMatch::Match + ); + // The key absent, not unresolvable: the default is used. + let absent = TestContext::new().with("aws:PrincipalTag/team", vec!["blue"]); + let c = cond( + ConditionOperator::StringEquals, + "aws:PrincipalTag/team", + vec!["${aws:PrincipalOrgID, 'blue'}"], + ); + assert_eq!( + evaluate_condition_match(&c, true, &absent), + ConditionMatch::Match + ); +} + #[test] fn resolved_unresolvable_key_uses_its_value() { // If the context both lists a key as unresolvable and resolves it, the @@ -80,7 +146,10 @@ fn resolved_unresolvable_key_uses_its_value() { "aws:PrincipalArn", vec!["arn:a"], ); - assert_eq!(evaluate_condition_match(&c, &ctx), ConditionMatch::Match); + assert_eq!( + evaluate_condition_match(&c, true, &ctx), + ConditionMatch::Match + ); } #[test] @@ -103,7 +172,7 @@ fn statement_nomatch_dominates_unresolvable() { ), ]; assert_eq!( - evaluate_statement_conditions(&conds, &ctx), + evaluate_statement_conditions(&conds, true, &ctx), ConditionMatch::NoMatch ); } @@ -126,7 +195,7 @@ fn statement_unresolvable_when_only_unresolvable_present() { ), ]; assert_eq!( - evaluate_statement_conditions(&conds, &ctx), + evaluate_statement_conditions(&conds, true, &ctx), ConditionMatch::Unresolvable ); } @@ -140,12 +209,12 @@ fn statement_all_match() { vec!["contractors"], )]; assert_eq!( - evaluate_statement_conditions(&conds, &ctx), + evaluate_statement_conditions(&conds, true, &ctx), ConditionMatch::Match ); // An empty condition block always matches. assert_eq!( - evaluate_statement_conditions(&[], &ctx), + evaluate_statement_conditions(&[], true, &ctx), ConditionMatch::Match ); } diff --git a/crates/auth/src/policy/context.rs b/crates/auth/src/policy/context.rs index be764624c..85ca0288f 100755 --- a/crates/auth/src/policy/context.rs +++ b/crates/auth/src/policy/context.rs @@ -128,6 +128,15 @@ fn strip_prefix_ci<'a>(key: &'a str, prefix: &str) -> Option<&'a str> { /// IAM absent-key semantics. The slice stays so a future global key the service /// begins populating before ExtendDB resolves it can be made to fail closed by /// listing it. +/// +/// Two places read the set, through `is_unresolvable_key`. A condition fails +/// closed when its key is listed, or when one of its values names a listed key +/// through a `${key}` or `${key, 'default'}` variable +/// (`condition::evaluate_condition_match`). A `Resource` or `NotResource` +/// pattern whose variable names a listed key fails closed in the same way +/// (`variables::resource_pattern_is_unresolvable`). In both, a default written +/// beside a listed key is not used, because the service would have a value +/// for the key. const FAIL_CLOSED_GLOBAL_KEYS: &[&str] = &[]; /// Whether an unresolved condition key must fail closed for the context that @@ -442,10 +451,10 @@ impl ConditionContext for RequestContext { // A request tag key the request names under two or more spellings that // fold to one name and do not all carry one value cannot be read as one // value: the statement fails closed (a Deny blocks, an Allow does not - // grant). Every other unresolved aws: global key that is neither a tag - // sub-key nor on the absent-ok allowlist is one the service populates - // but this context has no value for (for example aws:PrincipalOrgID, - // aws:SourceIp): it fails closed. Keys resolved above never reach here. + // grant). Any other key is unresolvable only when it is listed in + // FAIL_CLOSED_GLOBAL_KEYS, which is empty in this build, so every other + // unresolved key, aws:PrincipalOrgID and aws:SourceIp among them, keeps + // absent-key semantics. Keys resolved above never reach here. if let Some(sub) = strip_prefix_ci(key, "aws:RequestTag/") { return self.request_tag_values(sub) == RequestTagLookup::Ambiguous; } @@ -476,11 +485,11 @@ impl ConditionContext for RequestContext { /// `aws:RequestTag/*` / `aws:TagKeys`, and the request-wide keys /// `aws:SecureTransport`, `aws:RequestedRegion`, `aws:CurrentTime`, /// `aws:EpochTime`, `aws:ViaAWSService` and `aws:PrincipalIsAWSService`, all of -/// which this context resolves. A global key STS populates that this context -/// cannot resolve (for example `aws:PrincipalOrgID` or `aws:SourceIp`, neither -/// on the absent-ok allowlist) is reported as unresolvable (fail closed). Keys -/// that are legitimately absent for this caller model keep absent-key -/// semantics. `DynamoDB`-specific keys are not applicable. +/// which this context resolves. Another key is reported as unresolvable (fail +/// closed) only when it is listed in `FAIL_CLOSED_GLOBAL_KEYS`, which is +/// empty in this build; every other key this context does not resolve, such +/// as `aws:PrincipalOrgID` or `aws:SourceIp`, keeps absent-key semantics. +/// `DynamoDB`-specific keys are not applicable. #[derive(Debug)] pub struct AssumeRoleContext { /// Tags on the calling principal. @@ -584,10 +593,10 @@ impl ConditionContext for AssumeRoleContext { fn is_unresolvable_key(&self, key: &str) -> bool { // Keys this context resolves (tags, external id, principal ARN and // account, region, secure transport, time, principal type, user name) - // never reach here. Any other unresolved aws: global key that is not on - // the absent-ok allowlist is one STS populates but this context has no - // value for, so a Deny gated on it fails closed and an Allow does not - // grant. + // never reach here. Another key is unresolvable only when it is listed + // in FAIL_CLOSED_GLOBAL_KEYS, which is empty in this build: a Deny gated + // on a listed key fails closed and an Allow does not grant, and every + // other unresolved key keeps absent-key semantics. aws_key_is_unresolvable(key) } } diff --git a/crates/auth/src/policy/context/request_tag_tests.rs b/crates/auth/src/policy/context/request_tag_tests.rs index 38b91e40d..d4bf1539a 100644 --- a/crates/auth/src/policy/context/request_tag_tests.rs +++ b/crates/auth/src/policy/context/request_tag_tests.rs @@ -127,13 +127,13 @@ fn ambiguous_request_tag_fails_the_statement_closed() { values: vec!["prod".to_owned()], }]; assert_eq!( - evaluate_statement_conditions(&deny_on_prod, &ctx), + evaluate_statement_conditions(&deny_on_prod, true, &ctx), ConditionMatch::Unresolvable ); // The same statement against a request naming the tag once reads its value. let one = ctx_with(&[("env", "prod")], &["env"]); assert_eq!( - evaluate_statement_conditions(&deny_on_prod, &one), + evaluate_statement_conditions(&deny_on_prod, true, &one), ConditionMatch::Match ); } @@ -158,7 +158,7 @@ fn ambiguous_request_tag_as_a_policy_variable_fails_the_statement_closed() { // One spelling expands to its value. for tags in [[("env", "prod")], [("Env", "prod")]] { assert_eq!( - evaluate_statement_conditions(&deny, &with_resource_tag(&tags)), + evaluate_statement_conditions(&deny, true, &with_resource_tag(&tags)), ConditionMatch::Match ); } @@ -168,7 +168,7 @@ fn ambiguous_request_tag_as_a_policy_variable_fails_the_statement_closed() { [("env", "dev"), ("Env", "prod")], ] { assert_eq!( - evaluate_statement_conditions(&deny, &with_resource_tag(&tags)), + evaluate_statement_conditions(&deny, true, &with_resource_tag(&tags)), ConditionMatch::Unresolvable ); } @@ -181,13 +181,14 @@ fn ambiguous_request_tag_as_a_policy_variable_fails_the_statement_closed() { assert_eq!( evaluate_statement_conditions( &embedded, + true, &with_resource_tag(&[("Env", "prod"), ("env", "dev")]) ), ConditionMatch::Unresolvable ); // An absent request tag is not ambiguous: the variable stays literal, as before. assert_eq!( - evaluate_statement_conditions(&deny, &with_resource_tag(&[("team", "x")])), + evaluate_statement_conditions(&deny, true, &with_resource_tag(&[("team", "x")])), ConditionMatch::NoMatch ); // A brace that never closes names no variable. @@ -199,12 +200,58 @@ fn ambiguous_request_tag_as_a_policy_variable_fails_the_statement_closed() { assert_eq!( evaluate_statement_conditions( &unclosed, + true, &with_resource_tag(&[("Env", "prod"), ("env", "dev")]) ), ConditionMatch::NoMatch ); } +#[test] +fn ambiguous_request_tag_as_a_defaulted_policy_variable_fails_the_statement_closed() { + use crate::policy::condition::{ConditionMatch, evaluate_statement_conditions}; + use crate::policy::document::{Condition, ConditionOperator}; + + let mut resource = HashMap::new(); + resource.insert("env".to_owned(), "prod".to_owned()); + let with_resource_tag = |tags: &[(&str, &str)]| { + let mut ctx = ctx_with(tags, &[]); + ctx.resource_tags = resource.clone(); + ctx + }; + let deny = [Condition { + operator: ConditionOperator::StringEquals, + key: "aws:ResourceTag/env".to_owned(), + values: vec!["${aws:RequestTag/env, 'prod'}".to_owned()], + }]; + // Two spellings with differing values have no value to expand to, and the + // default is not used: the statement fails closed as it does for the plain + // form, so the default cannot route around the rule. + for tags in [ + [("Env", "prod"), ("env", "dev")], + [("env", "dev"), ("Env", "prod")], + ] { + assert_eq!( + evaluate_statement_conditions(&deny, true, &with_resource_tag(&tags)), + ConditionMatch::Unresolvable + ); + } + // One spelling expands to its value, and the default is not used. + assert_eq!( + evaluate_statement_conditions(&deny, true, &with_resource_tag(&[("Env", "prod")])), + ConditionMatch::Match + ); + assert_eq!( + evaluate_statement_conditions(&deny, true, &with_resource_tag(&[("env", "dev")])), + ConditionMatch::NoMatch + ); + // An absent request tag is not ambiguous: the default is used. + assert_eq!( + evaluate_statement_conditions(&deny, true, &with_resource_tag(&[("team", "x")])), + ConditionMatch::Match + ); +} + #[test] fn policy_variable_naming_aws_tagkeys_is_left_literal_for_every_tag_order() { use crate::policy::condition::{ConditionMatch, evaluate_statement_conditions}; @@ -223,7 +270,7 @@ fn policy_variable_naming_aws_tagkeys_is_left_literal_for_every_tag_order() { ctx.resource_tags .insert("team".to_owned(), "env".to_owned()); assert_eq!( - evaluate_statement_conditions(&deny, &ctx), + evaluate_statement_conditions(&deny, true, &ctx), ConditionMatch::NoMatch, "tag keys {keys:?}" ); @@ -280,7 +327,7 @@ fn trust_policy_variable_naming_aws_tagkeys_is_left_literal() { let ctx = assume_role_ctx(&[("key_env", "env"), ("key_owner", "owner")], &session); for tag in ["key_env", "key_owner"] { assert_eq!( - evaluate_statement_conditions(&compare_with_tag_keys(tag), &ctx), + evaluate_statement_conditions(&compare_with_tag_keys(tag), true, &ctx), ConditionMatch::NoMatch, "${{aws:TagKeys}} expanded to the session tag key in {tag}" ); @@ -288,7 +335,7 @@ fn trust_policy_variable_naming_aws_tagkeys_is_left_literal() { // The variable is text, so it equals the same text. let ctx = assume_role_ctx(&[("team", "${aws:TagKeys}")], &session); assert_eq!( - evaluate_statement_conditions(&compare_with_tag_keys("team"), &ctx), + evaluate_statement_conditions(&compare_with_tag_keys("team"), true, &ctx), ConditionMatch::Match ); } diff --git a/crates/auth/src/policy/document.rs b/crates/auth/src/policy/document.rs index dedcd5e2a..998f08357 100755 --- a/crates/auth/src/policy/document.rs +++ b/crates/auth/src/policy/document.rs @@ -13,8 +13,10 @@ use std::fmt; /// A parsed IAM policy document. #[derive(Debug, Clone)] pub struct PolicyDocument { - /// Policy version string (e.g., "2012-10-17"). - pub version: String, + /// The `Version` string (e.g., "2012-10-17"); `None` when the document has + /// no `Version` or its value is not a string. Writes accept only + /// `2012-10-17` and `2008-10-17`; a stored document may hold any value. + pub version: Option, /// The policy statements. pub statements: Vec, } @@ -172,7 +174,7 @@ impl PolicyDocument { ))); } let raw: Value = serde_json::from_str(json).map_err(|e| PolicyParseError(e.to_string()))?; - let version = raw["Version"].as_str().unwrap_or("2012-10-17").to_owned(); + let version = raw["Version"].as_str().map(ToOwned::to_owned); let statements = parse_statements(&raw["Statement"])?; validate_statement_cidrs(&statements)?; Ok(Self { @@ -185,17 +187,21 @@ impl PolicyDocument { /// a managed or boundary policy) and apply the checks the service applies /// only when a policy is written. /// - /// Everything `from_json` checks, plus the `Sid` charset. The `Sid` check - /// runs at write only: a `Sid` is a statement label that does not affect - /// evaluation, so a policy stored before this check existed must keep - /// parsing on the request path rather than fail the principal closed. + /// Everything `from_json` checks, plus the `Version` value and the `Sid` + /// charset. Both run at write only, so a policy stored before either check + /// existed keeps parsing on the request path rather than failing the + /// principal closed: a `Sid` is a label that does not affect evaluation, + /// and a stored document with another `Version` matches its `Resource` + /// patterns as written, as it did before. /// /// # Errors /// - /// Returns `PolicyParseError` if `from_json` rejects the document or a - /// statement `Sid` is outside the service's charset. + /// Returns `PolicyParseError` if `from_json` rejects the document, its + /// `Version` is not `2012-10-17` or `2008-10-17`, or a statement `Sid` is + /// outside the service's charset. pub fn from_json_for_write(json: &str) -> Result { let doc = Self::from_json(json)?; + super::variables::validate_version(json)?; validate_statement_sids(&doc.statements)?; Ok(doc) } @@ -503,7 +509,7 @@ mod tests { }] }"#; let doc = PolicyDocument::from_json(json).unwrap(); - assert_eq!(doc.version, "2012-10-17"); + assert_eq!(doc.version.as_deref(), Some("2012-10-17")); assert_eq!(doc.statements.len(), 1); assert_eq!(doc.statements[0].effect, Effect::Allow); assert!(matches!( diff --git a/crates/auth/src/policy/evaluator.rs b/crates/auth/src/policy/evaluator.rs index cf17858e3..125a01876 100644 --- a/crates/auth/src/policy/evaluator.rs +++ b/crates/auth/src/policy/evaluator.rs @@ -10,7 +10,8 @@ use super::condition::{ConditionMatch, evaluate_statement_conditions}; use super::context::ConditionContext; use super::document::{ActionMatch, Effect, PolicyDocument, ResourceMatch, Statement}; -use super::matcher::{arn_match, wildcard_match_ignore_case}; +use super::matcher::wildcard_match_ignore_case; +use super::variables::{resource_pattern_is_unresolvable, resource_pattern_matches}; /// The result of policy evaluation. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -100,8 +101,8 @@ pub fn explicit_deny_matches_arc( policy.statements.iter().any(|stmt| { stmt.effect == Effect::Deny && action_matches(stmt, action) - && resource_matches(stmt, resource_arn) - && deny_conditions_apply(stmt, context) + && resource_matches(policy, stmt, resource_arn, context) + && deny_conditions_apply(policy, stmt, context) }) }) } @@ -129,8 +130,8 @@ where for stmt in &policy.statements { if stmt.effect == Effect::Deny && action_matches(stmt, action) - && resource_matches(stmt, resource_arn) - && deny_conditions_apply(stmt, context) + && resource_matches(policy, stmt, resource_arn, context) + && deny_conditions_apply(policy, stmt, context) { return AuthzDecision::Deny; } @@ -142,8 +143,8 @@ where let boundary_allows = boundary.statements.iter().any(|stmt| { stmt.effect == Effect::Allow && action_matches(stmt, action) - && resource_matches(stmt, resource_arn) - && allow_conditions_grant(stmt, context) + && resource_matches(boundary, stmt, resource_arn, context) + && allow_conditions_grant(boundary, stmt, context) }); if !boundary_allows { return AuthzDecision::Deny; @@ -155,8 +156,8 @@ where let session_allows = session.statements.iter().any(|stmt| { stmt.effect == Effect::Allow && action_matches(stmt, action) - && resource_matches(stmt, resource_arn) - && allow_conditions_grant(stmt, context) + && resource_matches(session, stmt, resource_arn, context) + && allow_conditions_grant(session, stmt, context) }); if !session_allows { return AuthzDecision::Deny; @@ -168,8 +169,8 @@ where for stmt in &policy.statements { if stmt.effect == Effect::Allow && action_matches(stmt, action) - && resource_matches(stmt, resource_arn) - && allow_conditions_grant(stmt, context) + && resource_matches(policy, stmt, resource_arn, context) + && allow_conditions_grant(policy, stmt, context) { return AuthzDecision::Allow; } @@ -194,37 +195,83 @@ fn action_matches(statement: &Statement, request_action: &str) -> bool { } /// Check if the request resource matches the statement's resource constraint. -fn resource_matches(statement: &Statement, request_resource: &str) -> bool { +/// +/// In a `2012-10-17` document, `${key}` policy variables in each pattern are +/// expanded from `context` before matching, and a pattern with an unresolved +/// variable matches nothing. For `Resource` the statement then does not +/// apply; for `NotResource` every resource is outside the pattern, so the +/// statement applies to all of them. +/// +/// When no pattern matches and one names a key the service populates but the +/// context cannot resolve, the statement fails closed, as a condition on such +/// a key does: a `Deny` applies and an `Allow` does not grant, for `Resource` +/// and `NotResource` alike. +fn resource_matches( + policy: &PolicyDocument, + statement: &Statement, + request_resource: &str, + context: &impl ConditionContext, +) -> bool { + let expand = policy.expands_policy_variables(); + let any_match = |patterns: &[String]| { + patterns + .iter() + .any(|p| resource_pattern_matches(p, request_resource, expand, context)) + }; + let unresolvable = |patterns: &[String]| { + patterns + .iter() + .any(|p| resource_pattern_is_unresolvable(p, expand, context)) + }; + let deny = statement.effect == Effect::Deny; match &statement.resource_match { ResourceMatch::Resources(patterns) => { - patterns.iter().any(|p| arn_match(p, request_resource)) + any_match(patterns) || (deny && unresolvable(patterns)) } ResourceMatch::NotResources(patterns) => { - !patterns.iter().any(|p| arn_match(p, request_resource)) + !any_match(patterns) && (deny || !unresolvable(patterns)) } } } -/// Outcome of a statement's `Condition` block, three-valued. -fn statement_conditions(statement: &Statement, context: &impl ConditionContext) -> ConditionMatch { - evaluate_statement_conditions(&statement.conditions, context) +/// Outcome of a statement's `Condition` block, three-valued. `policy` is the +/// statement's document: its `Version` decides how the condition values read +/// policy variables (`condition::evaluate_condition_with`). +fn statement_conditions( + policy: &PolicyDocument, + statement: &Statement, + context: &impl ConditionContext, +) -> ConditionMatch { + evaluate_statement_conditions( + &statement.conditions, + policy.expands_policy_variables(), + context, + ) } /// Whether a `Deny` applies: its conditions match, or depend on a key the /// service populates but the context cannot resolve (fail closed, so the Deny /// blocks rather than being silently dropped). -fn deny_conditions_apply(statement: &Statement, context: &impl ConditionContext) -> bool { +fn deny_conditions_apply( + policy: &PolicyDocument, + statement: &Statement, + context: &impl ConditionContext, +) -> bool { matches!( - statement_conditions(statement, context), + statement_conditions(policy, statement, context), ConditionMatch::Match | ConditionMatch::Unresolvable ) } /// Whether an `Allow` grants: its conditions definitely match. An unresolvable /// key does not grant (fail closed). -fn allow_conditions_grant(statement: &Statement, context: &impl ConditionContext) -> bool { +fn allow_conditions_grant( + policy: &PolicyDocument, + statement: &Statement, + context: &impl ConditionContext, +) -> bool { matches!( - statement_conditions(statement, context), + statement_conditions(policy, statement, context), ConditionMatch::Match ) } diff --git a/crates/auth/src/policy/evaluator/tests.rs b/crates/auth/src/policy/evaluator/tests.rs index dcc8e403e..ecb25aab5 100644 --- a/crates/auth/src/policy/evaluator/tests.rs +++ b/crates/auth/src/policy/evaluator/tests.rs @@ -59,4 +59,10 @@ fn parse(json: &str) -> PolicyDocument { mod case_three; mod combined; +mod condition_variable_scope; mod core; +mod empty_values; +mod resource_variables; +mod resource_variables_fields; +mod variable_spelling; +mod variable_values; diff --git a/crates/auth/src/policy/evaluator/tests/case_three.rs b/crates/auth/src/policy/evaluator/tests/case_three.rs index f6ab83e30..55314cb85 100644 --- a/crates/auth/src/policy/evaluator/tests/case_three.rs +++ b/crates/auth/src/policy/evaluator/tests/case_three.rs @@ -3,7 +3,17 @@ //! Identity-policy evaluation of a key the service populates but the context //! cannot resolve (case 3): a Deny fails closed and blocks, an Allow gated on -//! it does not grant, and the negated-operator-on-absent rule cannot fire. +//! it does not grant, and the negated-operator-on-absent rule cannot fire. The +//! same holds when the key is named by a `${key}` variable in `Resource` or +//! `NotResource` of a `2012-10-17` document; a `2008-10-17` or no-`Version` +//! document matches such a pattern as written. A variable naming the key in a +//! condition value fails the condition closed in the same way, with or without +//! a default. +//! +//! Every expectation here is an ExtendDB rule with no service measurement: +//! the service has a value for such a key, so there is no service case to +//! compare with, and no key is listed in `FAIL_CLOSED_GLOBAL_KEYS` in this +//! build. use super::*; @@ -120,3 +130,199 @@ fn resolved_principal_arn_allows_normally() { AuthzDecision::Allow ); } + +// --- A `${key}` variable in Resource or NotResource naming such a key --- + +const TABLE: &str = "arn:aws:dynamodb:us-east-1:123456789012:table/"; + +fn only(statement: &str) -> PolicyDocument { + parse(&format!( + r#"{{"Version":"2012-10-17","Statement":[{statement}]}}"# + )) +} + +/// `Allow dynamodb:*` on `*` beside `statement`. +fn allow_all_and(statement: &str) -> Vec { + vec![ + only(r#"{"Effect":"Allow","Action":"dynamodb:*","Resource":"*"}"#), + only(statement), + ] +} + +/// GetItem on table `T` with `aws:PrincipalOrgID` unresolvable. +fn get_t(policies: &[PolicyDocument]) -> AuthzDecision { + let ctx = Ctx::empty().unresolvable("aws:PrincipalOrgID"); + evaluate_policies(policies, None, None, "dynamodb:GetItem", RES, &ctx) +} + +#[test] +fn deny_naming_an_unresolvable_key_in_resource_blocks_fail_closed() { + // As for the Deny gated on the key in a Condition above. The default is + // not used, because the service would have a value for the key. + for resource in ["${aws:PrincipalOrgID}", "${aws:PrincipalOrgID, 'none'}"] { + let p = allow_all_and(&format!( + r#"{{"Effect":"Deny","Action":"dynamodb:GetItem","Resource":"{TABLE}{resource}"}}"# + )); + assert_eq!(get_t(&p), AuthzDecision::Deny, "{resource}"); + } + let p = allow_all_and(&format!( + r#"{{"Effect":"Deny","Action":"dynamodb:GetItem","NotResource":"{TABLE}${{aws:PrincipalOrgID}}"}}"# + )); + assert_eq!(get_t(&p), AuthzDecision::Deny); +} + +#[test] +fn allow_naming_an_unresolvable_key_in_resource_does_not_grant() { + // Read as an absent key, the default would grant `table/T` and the + // NotResource would grant every table. + for (element, resource) in [ + ("Resource", "${aws:PrincipalOrgID, 'T'}"), + ("NotResource", "${aws:PrincipalOrgID}"), + ] { + let p = [only(&format!( + r#"{{"Effect":"Allow","Action":"dynamodb:GetItem","{element}":"{TABLE}{resource}"}}"# + ))]; + assert_eq!(get_t(&p), AuthzDecision::Deny, "{element} {resource}"); + } +} + +#[test] +fn a_pattern_that_decides_alone_wins_over_an_unresolvable_one() { + // `table/T` in the list decides whatever the unresolvable key holds, and + // so does an absent key with no default, which makes its pattern match + // nothing, before or after the unresolvable variable. + let allow = [only(&format!( + r#"{{"Effect":"Allow","Action":"dynamodb:GetItem","Resource":["{TABLE}${{aws:PrincipalOrgID}}","{TABLE}T"]}}"# + ))]; + assert_eq!(get_t(&allow), AuthzDecision::Allow); + let spared = allow_all_and(&format!( + r#"{{"Effect":"Deny","Action":"dynamodb:GetItem","NotResource":["{TABLE}T","{TABLE}${{aws:PrincipalOrgID}}"]}}"# + )); + assert_eq!(get_t(&spared), AuthzDecision::Allow); + for resource in [ + "${aws:PrincipalTag/team}${aws:PrincipalOrgID}", + "${aws:PrincipalOrgID}${aws:PrincipalTag/team}", + ] { + let absent = allow_all_and(&format!( + r#"{{"Effect":"Deny","Action":"dynamodb:GetItem","Resource":"{TABLE}{resource}"}}"# + )); + assert_eq!(get_t(&absent), AuthzDecision::Allow, "{resource}"); + } +} + +#[test] +fn a_multivalued_key_the_context_reports_unresolvable_fails_closed_in_resource() { + // The unresolvable rule comes before the multivalued one, so the default + // is not used: a Deny applies and an Allow does not grant. Read as an + // absent multivalued key, the default `T` would grant `table/T` and the + // NotResource would grant every table. + let ctx = Ctx::empty().unresolvable("dynamodb:LeadingKeys"); + for (element, resource) in [ + ("Resource", "${dynamodb:LeadingKeys, 'T'}"), + ("NotResource", "${dynamodb:LeadingKeys}"), + ] { + let allow = [only(&format!( + r#"{{"Effect":"Allow","Action":"dynamodb:GetItem","{element}":"{TABLE}{resource}"}}"# + ))]; + assert_eq!( + evaluate_policies(&allow, None, None, "dynamodb:GetItem", RES, &ctx), + AuthzDecision::Deny, + "Allow {element} {resource}" + ); + } + for (element, resource) in [ + ("Resource", "${dynamodb:LeadingKeys}"), + ("NotResource", "${dynamodb:LeadingKeys, 'T'}"), + ] { + let deny = allow_all_and(&format!( + r#"{{"Effect":"Deny","Action":"dynamodb:GetItem","{element}":"{TABLE}{resource}"}}"# + )); + assert_eq!( + evaluate_policies(&deny, None, None, "dynamodb:GetItem", RES, &ctx), + AuthzDecision::Deny, + "Deny {element} {resource}" + ); + } +} + +#[test] +fn an_unresolvable_key_in_a_2008_or_no_version_document_is_matched_as_written() { + // Only a `2012-10-17` document expands variables, so here the pattern is + // text: it names the table `${aws:PrincipalOrgID}` and no other, and the + // Deny does not fail closed. + let ctx = Ctx::empty().unresolvable("aws:PrincipalOrgID"); + let literal = format!("{TABLE}${{aws:PrincipalOrgID}}"); + for (label, version) in [ + ("2008-10-17", r#""Version":"2008-10-17","#), + ("no Version", ""), + ] { + let deny = parse(&format!( + r#"{{{version}"Statement":[{{"Effect":"Deny","Action":"dynamodb:GetItem","Resource":"{literal}"}}]}}"# + )); + let p = [ + only(r#"{"Effect":"Allow","Action":"dynamodb:*","Resource":"*"}"#), + deny, + ]; + assert_eq!(get_t(&p), AuthzDecision::Allow, "{label}"); + assert_eq!( + evaluate_policies(&p, None, None, "dynamodb:GetItem", &literal, &ctx), + AuthzDecision::Deny, + "{label}" + ); + } +} + +// --- A `${key}` variable in a condition value naming such a key --- + +/// GetItem on table `T` by a caller tagged `team=blue`, with `ctx` for the +/// other keys. +fn get_t_as_blue(policies: &[PolicyDocument], ctx: Ctx) -> AuthzDecision { + let ctx = ctx.with("aws:PrincipalTag/team", vec!["blue"]); + evaluate_policies(policies, None, None, "dynamodb:GetItem", RES, &ctx) +} + +#[test] +fn a_condition_value_naming_an_unresolvable_key_fails_closed_with_or_without_a_default() { + // As for the Deny gated on the key in a Condition above: the variable has + // no value to expand to, and the default is not used, because the service + // would have a value for the key. The Deny applies whatever the operator. + for (operator, value) in [ + ("StringEquals", "${aws:PrincipalOrgID}"), + ("StringEquals", "${aws:PrincipalOrgID, 'blue'}"), + ("StringNotEquals", "${aws:PrincipalOrgID, 'blue'}"), + ] { + let p = allow_all_and(&format!( + r#"{{"Effect":"Deny","Action":"dynamodb:GetItem","Resource":"*", + "Condition":{{"{operator}":{{"aws:PrincipalTag/team":"{value}"}}}}}}"# + )); + let unresolvable = Ctx::empty().unresolvable("aws:PrincipalOrgID"); + assert_eq!( + get_t_as_blue(&p, unresolvable), + AuthzDecision::Deny, + "{operator} {value}" + ); + } + // An Allow gated the same way does not grant, under either operator. + for operator in ["StringEquals", "StringNotEquals"] { + let allow = [only(&format!( + r#"{{"Effect":"Allow","Action":"dynamodb:GetItem","Resource":"*", + "Condition":{{"{operator}":{{"aws:PrincipalTag/team":"${{aws:PrincipalOrgID, 'blue'}}"}}}}}}"# + ))]; + let unresolvable = Ctx::empty().unresolvable("aws:PrincipalOrgID"); + assert_eq!( + get_t_as_blue(&allow, unresolvable), + AuthzDecision::Deny, + "{operator}" + ); + } + let p = allow_all_and( + r#"{"Effect":"Deny","Action":"dynamodb:GetItem","Resource":"*", + "Condition":{"StringEquals":{"aws:PrincipalTag/team":"${aws:PrincipalOrgID, 'blue'}"}}}"#, + ); + // Absent: the default `blue` is used and the Deny applies. + assert_eq!(get_t_as_blue(&p, Ctx::empty()), AuthzDecision::Deny); + // Present: the key's value is used, not the default. + let org = |value| Ctx::empty().with("aws:PrincipalOrgID", vec![value]); + assert_eq!(get_t_as_blue(&p, org("blue")), AuthzDecision::Deny); + assert_eq!(get_t_as_blue(&p, org("red")), AuthzDecision::Allow); +} diff --git a/crates/auth/src/policy/evaluator/tests/condition_variable_scope.rs b/crates/auth/src/policy/evaluator/tests/condition_variable_scope.rs new file mode 100644 index 000000000..a21a405d3 --- /dev/null +++ b/crates/auth/src/policy/evaluator/tests/condition_variable_scope.rs @@ -0,0 +1,309 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! Where a condition value reads the `${key, 'default'}` form and a trimmed +//! key name: only in a `2012-10-17` document and under a string or ARN +//! operator. Elsewhere the text between the braces is the key name as +//! written, as it was before the default form was read, so the default form +//! is compared as written and `${key}` expands as before. +//! +//! The tests named after rows repeat cases measured with the IAM policy +//! simulator on 2026-11-03, `dynamodb:GetItem`, two samples per row with the +//! same answer; a `Deny` row has `Allow dynamodb:*` on `*` in the same +//! document. The group, boundary and session policy cases, the trimmed key +//! in a `2008-10-17` document, the set qualifier and `IfExists` wrapping a +//! numeric operator, and the unresolvable key follow from the rows and were +//! not measured. Row N4b is not repeated: IAM reads a `Bool` value written as +//! a variable as `false`, which ExtendDB does not do for either form. Row N6 +//! (`IpAddress`) is not repeated either: ExtendDB refuses a document whose +//! `IpAddress` value is not a CIDR, so such a value never reaches evaluation. + +use super::*; +use serde_json::{Value, json}; + +const ARN: &str = "arn:aws:dynamodb:us-east-1:123456789012:table/"; +const ALLOW_ALL: &str = r#"{"Effect":"Allow","Action":"dynamodb:*","Resource":"*"}"#; + +/// A document with `version` (none when `None`) and `statements`. +fn document(version: Option<&str>, statements: &[Value]) -> PolicyDocument { + let mut doc = json!({"Statement": statements}); + if let Some(version) = version { + doc["Version"] = json!(version); + } + parse(&doc.to_string()) +} + +/// A statement on `dynamodb:GetItem` and every resource whose condition +/// compares `key` with `value` under `operator`. +fn get_if(effect: &str, operator: &str, key: &str, value: &str) -> Value { + json!({ + "Effect": effect, "Action": "dynamodb:GetItem", "Resource": "*", + "Condition": {operator: {key: value}}, + }) +} + +fn allow_all() -> Value { + serde_json::from_str(ALLOW_ALL).unwrap() +} + +/// `Allow dynamodb:*` on `*` and a `Deny` gated on the condition, in one +/// document. +fn deny_if(version: Option<&str>, operator: &str, key: &str, value: &str) -> Vec { + vec![document( + version, + &[allow_all(), get_if("Deny", operator, key, value)], + )] +} + +/// Only an `Allow` gated on the condition. +fn allow_only_if( + version: Option<&str>, + operator: &str, + key: &str, + value: &str, +) -> Vec { + vec![document(version, &[get_if("Allow", operator, key, value)])] +} + +fn get(policies: &[PolicyDocument], ctx: &Ctx) -> AuthzDecision { + get_with(policies, None, None, ctx) +} + +fn get_with( + policies: &[PolicyDocument], + boundary: Option<&PolicyDocument>, + session: Option<&PolicyDocument>, + ctx: &Ctx, +) -> AuthzDecision { + evaluate_policies( + policies, + boundary, + session, + "dynamodb:GetItem", + &format!("{ARN}x"), + ctx, + ) +} + +const TEAM: &str = "aws:PrincipalTag/team"; +const DEFAULTED_USERNAME: &str = "${aws:username, 'x'}"; + +/// A user `alice` tagged `team=alice`. +fn alice_on_team_alice() -> Ctx { + Ctx::empty() + .with("aws:username", vec!["alice"]) + .with(TEAM, vec!["alice"]) +} + +// --- The `Version` of the document --- + +#[test] +fn v1_a_2008_deny_compares_the_default_form_as_written() { + let p = deny_if( + Some("2008-10-17"), + "StringNotEquals", + TEAM, + DEFAULTED_USERNAME, + ); + assert_eq!(get(&p, &alice_on_team_alice()), AuthzDecision::Deny); +} + +#[test] +fn v2_v3_v4_v5_an_allow_reads_the_default_form_only_in_a_2012_document() { + let ctx = alice_on_team_alice(); + for version in [Some("2008-10-17"), None] { + let p = allow_only_if(version, "StringEquals", TEAM, DEFAULTED_USERNAME); + assert_eq!(get(&p, &ctx), AuthzDecision::Deny, "{version:?}"); + } + let p = allow_only_if(Some("2012-10-17"), "StringEquals", TEAM, DEFAULTED_USERNAME); + assert_eq!(get(&p, &ctx), AuthzDecision::Allow); + // V5: the tag holds the text as written. + let p = allow_only_if(Some("2008-10-17"), "StringEquals", TEAM, DEFAULTED_USERNAME); + let literal = Ctx::empty() + .with("aws:username", vec!["alice"]) + .with(TEAM, vec![DEFAULTED_USERNAME]); + assert_eq!(get(&p, &literal), AuthzDecision::Allow); +} + +#[test] +fn a_2008_document_does_not_trim_the_key_name() { + let p = allow_only_if( + Some("2008-10-17"), + "StringEquals", + TEAM, + "${ aws:username }", + ); + assert_eq!(get(&p, &alice_on_team_alice()), AuthzDecision::Deny); + let p = allow_only_if( + Some("2012-10-17"), + "StringEquals", + TEAM, + "${ aws:username }", + ); + assert_eq!(get(&p, &alice_on_team_alice()), AuthzDecision::Allow); +} + +#[test] +fn a_group_policy_follows_its_own_version() { + // A group policy is one more identity document beside the user's own. + let user = document( + Some("2012-10-17"), + &[json!({"Effect": "Allow", "Action": "dynamodb:PutItem", "Resource": "*"})], + ); + let group = document( + Some("2008-10-17"), + &[get_if("Allow", "StringEquals", TEAM, DEFAULTED_USERNAME)], + ); + assert_eq!( + get(&[user, group], &alice_on_team_alice()), + AuthzDecision::Deny + ); +} + +#[test] +fn a_boundary_and_a_session_policy_follow_their_own_version() { + let identity = [document(Some("2012-10-17"), &[allow_all()])]; + let ctx = alice_on_team_alice(); + for version in [Some("2008-10-17"), None] { + let gate = document( + version, + &[get_if("Allow", "StringEquals", TEAM, DEFAULTED_USERNAME)], + ); + assert_eq!( + get_with(&identity, Some(&gate), None, &ctx), + AuthzDecision::Deny, + "boundary {version:?}" + ); + assert_eq!( + get_with(&identity, None, Some(&gate), &ctx), + AuthzDecision::Deny, + "session {version:?}" + ); + } + let gate = document( + Some("2012-10-17"), + &[get_if("Allow", "StringEquals", TEAM, DEFAULTED_USERNAME)], + ); + assert_eq!( + get_with(&identity, Some(&gate), None, &ctx), + AuthzDecision::Allow + ); + assert_eq!( + get_with(&identity, None, Some(&gate), &ctx), + AuthzDecision::Allow + ); +} + +// --- The operator --- + +#[test] +fn n1_n2_numeric_equals_does_not_read_a_default() { + let age = Ctx::empty().with("aws:MultiFactorAuthAge", vec!["300"]); + let value = "${aws:PrincipalTag/m, '300'}"; + let p = allow_only_if( + Some("2012-10-17"), + "NumericEquals", + "aws:MultiFactorAuthAge", + value, + ); + assert_eq!(get(&p, &age), AuthzDecision::Deny); + let p = deny_if( + Some("2012-10-17"), + "NumericEquals", + "aws:MultiFactorAuthAge", + value, + ); + assert_eq!(get(&p, &age), AuthzDecision::Allow); + // The same under a set qualifier and `IfExists`. + for operator in ["ForAnyValue:NumericEquals", "NumericEqualsIfExists"] { + let p = allow_only_if( + Some("2012-10-17"), + operator, + "aws:MultiFactorAuthAge", + value, + ); + assert_eq!(get(&p, &age), AuthzDecision::Deny, "{operator}"); + } +} + +#[test] +fn n3_n5_date_and_null_operators_do_not_read_a_default() { + let now = Ctx::empty().with("aws:CurrentTime", vec!["2026-11-03T12:00:00Z"]); + let p = deny_if( + Some("2012-10-17"), + "DateGreaterThan", + "aws:CurrentTime", + "${aws:PrincipalTag/d, '2000-01-01T00:00:00Z'}", + ); + assert_eq!(get(&p, &now), AuthzDecision::Allow, "N3"); + let p = deny_if( + Some("2012-10-17"), + "Null", + TEAM, + "${aws:PrincipalTag/f, 'true'}", + ); + assert_eq!(get(&p, &Ctx::empty()), AuthzDecision::Allow, "N5"); +} + +#[test] +fn n4_bool_does_not_read_a_default() { + let secure = Ctx::empty().with("aws:SecureTransport", vec!["true"]); + let p = deny_if( + Some("2012-10-17"), + "Bool", + "aws:SecureTransport", + "${aws:PrincipalTag/f, 'true'}", + ); + assert_eq!(get(&p, &secure), AuthzDecision::Allow); +} + +#[test] +fn s1_a1_string_and_arn_operators_read_a_default() { + let owner = Ctx::empty().with("aws:ResourceTag/owner", vec!["anon"]); + let p = deny_if( + Some("2012-10-17"), + "StringEquals", + "aws:ResourceTag/owner", + "${aws:username, 'anon'}", + ); + assert_eq!(get(&p, &owner), AuthzDecision::Deny, "S1"); + let source = Ctx::empty().with( + "aws:SourceArn", + vec!["arn:aws:iam::123456789012:user/alice"], + ); + let p = deny_if( + Some("2012-10-17"), + "ArnLike", + "aws:SourceArn", + "arn:aws:iam::123456789012:user/${aws:PrincipalTag/n, 'alice'}", + ); + assert_eq!(get(&p, &source), AuthzDecision::Deny, "A1"); +} + +// --- A defaulted variable naming an unresolvable key --- + +#[test] +fn a_defaulted_unresolvable_key_fails_closed_only_where_the_default_form_is_read() { + let ctx = Ctx::empty() + .with(TEAM, vec!["blue"]) + .unresolvable("aws:PrincipalOrgID"); + let value = "${aws:PrincipalOrgID, 'blue'}"; + // Read with IAM's syntax, the variable names the unresolvable key and the + // Deny fails closed. + let p = deny_if(Some("2012-10-17"), "StringEquals", TEAM, value); + assert_eq!(get(&p, &ctx), AuthzDecision::Deny); + // Elsewhere the value names the key `aws:PrincipalOrgID, 'blue'`, which + // the request does not carry, and is compared as written. + let p = deny_if(Some("2008-10-17"), "StringEquals", TEAM, value); + assert_eq!(get(&p, &ctx), AuthzDecision::Allow); + let p = deny_if( + Some("2012-10-17"), + "NumericEquals", + "aws:MultiFactorAuthAge", + "${aws:PrincipalOrgID, '300'}", + ); + let age = Ctx::empty() + .with("aws:MultiFactorAuthAge", vec!["300"]) + .unresolvable("aws:PrincipalOrgID"); + assert_eq!(get(&p, &age), AuthzDecision::Allow); +} diff --git a/crates/auth/src/policy/evaluator/tests/empty_values.rs b/crates/auth/src/policy/evaluator/tests/empty_values.rs new file mode 100644 index 000000000..48c2d9532 --- /dev/null +++ b/crates/auth/src/policy/evaluator/tests/empty_values.rs @@ -0,0 +1,110 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! A tag whose value is the empty string, in a `Resource` or `NotResource` +//! pattern: the variable expands to no characters, so +//! `table/${aws:PrincipalTag/team}*` becomes `table/*` for a caller tagged +//! `team=""`, while with no `team` tag the pattern matches nothing. A +//! `StringLike` condition `?*` on the tag keeps such a caller out. +//! +//! Each test repeats rows measured with the IAM policy simulator on +//! 2026-11-03, `dynamodb:GetItem`, two samples per row with the same answer; +//! a `Deny` row has `Allow dynamodb:*` on `*` beside it. The rows that read a +//! table with an empty name (`table/`) are not repeated: the simulator +//! denies that resource whatever the pattern, and a table name has at least +//! three characters. + +use super::*; + +const TEAM: &str = "aws:PrincipalTag/team"; +const ARN: &str = "arn:aws:dynamodb:us-east-1:123456789012:table/"; +const TEAM_STAR: &str = "${aws:PrincipalTag/team}*"; + +fn document(statements: &str) -> Vec { + vec![parse(&format!( + r#"{{"Version":"2012-10-17","Statement":[{statements}]}}"# + ))] +} + +/// Only an `Allow dynamodb:GetItem` on `table/`, with `condition`. +fn allow_only(resource: &str, condition: &str) -> Vec { + document(&format!( + r#"{{"Effect":"Allow","Action":"dynamodb:GetItem","Resource":"{ARN}{resource}"{condition}}}"# + )) +} + +/// `Allow dynamodb:*` on `*` and a `Deny dynamodb:GetItem` with `element` +/// (`Resource` or `NotResource`) on `table/`. +fn deny(element: &str, resource: &str) -> Vec { + document(&format!( + r#"{{"Effect":"Allow","Action":"dynamodb:*","Resource":"*"}}, + {{"Effect":"Deny","Action":"dynamodb:GetItem","{element}":"{ARN}{resource}"}}"# + )) +} + +fn get(policies: &[PolicyDocument], target: &str, ctx: &Ctx) -> AuthzDecision { + evaluate_policies( + policies, + None, + None, + "dynamodb:GetItem", + &format!("{ARN}{target}"), + ctx, + ) +} + +fn team(value: &str) -> Ctx { + Ctx::empty().with(TEAM, vec![value]) +} + +#[test] +fn e1_to_e5_an_empty_value_before_a_wildcard_matches_every_table() { + let p = allow_only(TEAM_STAR, ""); + assert_eq!(get(&p, "red", &team("")), AuthzDecision::Allow, "E1"); + assert_eq!(get(&p, "blue", &team("")), AuthzDecision::Allow, "E2"); + assert_eq!(get(&p, "red", &Ctx::empty()), AuthzDecision::Deny, "E3"); + assert_eq!(get(&p, "red", &team("blue")), AuthzDecision::Deny, "E4"); + assert_eq!(get(&p, "blue-x", &team("blue")), AuthzDecision::Allow, "E5"); +} + +#[test] +fn e6_e7_a_deny_on_an_empty_value_before_a_wildcard() { + let p = deny("Resource", TEAM_STAR); + assert_eq!(get(&p, "red", &team("")), AuthzDecision::Deny, "E6"); + let p = deny("NotResource", TEAM_STAR); + assert_eq!(get(&p, "red", &team("")), AuthzDecision::Allow, "E7"); +} + +#[test] +fn e8_an_empty_value_after_a_star_leaves_the_star() { + let p = document( + r#"{"Effect":"Allow","Action":"dynamodb:GetItem","Resource":"*${aws:PrincipalTag/suffix}"}"#, + ); + let ctx = Ctx::empty().with("aws:PrincipalTag/suffix", vec![""]); + assert_eq!(get(&p, "red", &ctx), AuthzDecision::Allow); +} + +#[test] +fn e9_an_empty_value_beside_written_text_adds_nothing() { + let empty = team(""); + let p = allow_only("${aws:PrincipalTag/team}x", ""); + assert_eq!(get(&p, "x", &empty), AuthzDecision::Allow, "E9d"); + let p = allow_only("x${aws:PrincipalTag/team}", ""); + assert_eq!(get(&p, "x", &empty), AuthzDecision::Allow, "E9e"); + let p = allow_only("${aws:PrincipalTag/team}", ""); + assert_eq!(get(&p, "x", &empty), AuthzDecision::Deny, "E9f"); + let p = allow_only("x${aws:PrincipalTag/team}y", ""); + assert_eq!(get(&p, "xy", &empty), AuthzDecision::Allow, "E9h"); +} + +#[test] +fn e10_e11_a_string_like_guard_keeps_an_empty_value_out() { + let guard = r#","Condition":{"StringLike":{"aws:PrincipalTag/team":"?*"}}"#; + let p = allow_only(TEAM_STAR, guard); + assert_eq!(get(&p, "red", &team("")), AuthzDecision::Deny, "E10"); + assert_eq!( + get(&p, "blue-x", &team("blue")), + AuthzDecision::Allow, + "E11" + ); +} diff --git a/crates/auth/src/policy/evaluator/tests/resource_variables.rs b/crates/auth/src/policy/evaluator/tests/resource_variables.rs new file mode 100644 index 000000000..f40700feb --- /dev/null +++ b/crates/auth/src/policy/evaluator/tests/resource_variables.rs @@ -0,0 +1,319 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! Policy variables in `Resource` and `NotResource` (issue #533). +//! +//! The tests `row1` to `row11` repeat the rows of the issue's table, and +//! `row12` the `${*}` case measured with them, with the IAM policy simulator +//! on 2026-10-24 and `dynamodb:GetItem`. Three more repeat cases measured +//! the same day for the PR #535 body table: +//! `not_resource_unresolved_variable_applies_to_every_resource`, +//! `allow_not_resource_with_unresolved_variable_grants_every_table` and +//! `default_form_without_a_space_expands_when_key_absent`. A check on a +//! second target or context beside the measured one is derived: `red-x` in +//! `row4`, `table/blue` and the present-key checks in `row11`, and `table/*` +//! in `row12`. The other tests follow from those rules and were not +//! measured: the `${?}` and `${$}` escapes, `NotResource` in a `2008-10-17` +//! or no-`Version` document, the default form in a condition value, and the +//! boundary, session policy and `explicit_deny_matches_arc` paths. + +use super::*; + +const TEAM: &str = "aws:PrincipalTag/team"; +const ARN: &str = "arn:aws:dynamodb:us-east-1:123456789012:table/"; + +fn table(name: &str) -> String { + format!("{ARN}{name}") +} + +fn allow_all() -> PolicyDocument { + parse( + r#"{"Version":"2012-10-17","Statement":[{ + "Effect":"Allow","Action":"dynamodb:*","Resource":"*" + }]}"#, + ) +} + +/// `Allow dynamodb:* on *` beside a `Deny dynamodb:GetItem` on `resource`. +fn allow_all_deny_get(version: Option<&str>, element: &str, resource: &str) -> Vec { + let version = version.map_or(String::new(), |v| format!(r#""Version":"{v}","#)); + vec![ + allow_all(), + parse(&format!( + r#"{{{version}"Statement":[{{ + "Effect":"Deny","Action":"dynamodb:GetItem","{element}":"{ARN}{resource}" + }}]}}"# + )), + ] +} + +fn get(policies: &[PolicyDocument], target: &str, ctx: &Ctx) -> AuthzDecision { + evaluate_policies( + policies, + None, + None, + "dynamodb:GetItem", + &table(target), + ctx, + ) +} + +fn team_blue() -> Ctx { + Ctx::empty().with(TEAM, vec!["blue"]) +} + +// --- Rows 1 to 3: Deny Resource table/${aws:PrincipalTag/team} --- + +#[test] +fn row1_deny_variable_resource_denies_own_team_table() { + let p = allow_all_deny_get(Some("2012-10-17"), "Resource", "${aws:PrincipalTag/team}"); + assert_eq!(get(&p, "blue", &team_blue()), AuthzDecision::Deny); +} + +#[test] +fn row2_deny_variable_resource_leaves_other_table_allowed() { + let p = allow_all_deny_get(Some("2012-10-17"), "Resource", "${aws:PrincipalTag/team}"); + assert_eq!(get(&p, "red", &team_blue()), AuthzDecision::Allow); +} + +#[test] +fn row3_unresolved_variable_resource_denies_nothing() { + let p = allow_all_deny_get(Some("2012-10-17"), "Resource", "${aws:PrincipalTag/team}"); + assert_eq!(get(&p, "blue", &Ctx::empty()), AuthzDecision::Allow); +} + +// --- Row 4: wildcard after the variable --- + +#[test] +fn row4_wildcard_after_variable_still_matches() { + let p = allow_all_deny_get(Some("2012-10-17"), "Resource", "${aws:PrincipalTag/team}*"); + assert_eq!(get(&p, "blue-x", &team_blue()), AuthzDecision::Deny); + assert_eq!(get(&p, "red-x", &team_blue()), AuthzDecision::Allow); +} + +// --- Rows 5 and 6: Allow only table/${aws:username} --- + +fn allow_own_table() -> Vec { + vec![parse(&format!( + r#"{{"Version":"2012-10-17","Statement":[{{ + "Effect":"Allow","Action":"dynamodb:GetItem","Resource":"{ARN}${{aws:username}}" + }}]}}"# + ))] +} + +#[test] +fn row5_allow_variable_resource_grants_own_table() { + let ctx = Ctx::empty().with("aws:username", vec!["alice"]); + assert_eq!(get(&allow_own_table(), "alice", &ctx), AuthzDecision::Allow); +} + +#[test] +fn row6_allow_variable_resource_does_not_grant_other_table() { + let ctx = Ctx::empty().with("aws:username", vec!["alice"]); + assert_eq!(get(&allow_own_table(), "bob", &ctx), AuthzDecision::Deny); +} + +// --- Rows 7 and 8: 2008-10-17 and no Version match the text as written --- + +#[test] +fn row7_version_2008_does_not_expand_resource() { + let p = allow_all_deny_get(Some("2008-10-17"), "Resource", "${aws:PrincipalTag/team}"); + assert_eq!(get(&p, "blue", &team_blue()), AuthzDecision::Allow); +} + +#[test] +fn row8_no_version_does_not_expand_resource() { + let p = allow_all_deny_get(None, "Resource", "${aws:PrincipalTag/team}"); + assert_eq!(get(&p, "blue", &team_blue()), AuthzDecision::Allow); +} + +// --- Rows 9 and 10: Deny NotResource table/${aws:PrincipalTag/team} --- + +#[test] +fn row9_deny_not_resource_variable_spares_own_team_table() { + let p = allow_all_deny_get( + Some("2012-10-17"), + "NotResource", + "${aws:PrincipalTag/team}", + ); + assert_eq!(get(&p, "blue", &team_blue()), AuthzDecision::Allow); +} + +#[test] +fn row10_deny_not_resource_variable_denies_other_table() { + let p = allow_all_deny_get( + Some("2012-10-17"), + "NotResource", + "${aws:PrincipalTag/team}", + ); + assert_eq!(get(&p, "red", &team_blue()), AuthzDecision::Deny); +} + +// --- Row 11: default form --- + +#[test] +fn row11_default_form_expands_when_key_absent() { + let p = allow_all_deny_get( + Some("2012-10-17"), + "Resource", + "${aws:PrincipalTag/team, 'none'}", + ); + assert_eq!(get(&p, "none", &Ctx::empty()), AuthzDecision::Deny); + assert_eq!(get(&p, "blue", &Ctx::empty()), AuthzDecision::Allow); + // With the key present the value wins over the default. + assert_eq!(get(&p, "blue", &team_blue()), AuthzDecision::Deny); + assert_eq!(get(&p, "none", &team_blue()), AuthzDecision::Allow); +} + +// --- Row 12: ${*} is a literal star (`table/x` measured, `table/*` derived) --- + +#[test] +fn row12_escaped_star_in_resource_is_literal() { + let p = allow_all_deny_get(Some("2012-10-17"), "Resource", "${*}"); + assert_eq!(get(&p, "x", &Ctx::empty()), AuthzDecision::Allow); + assert_eq!(get(&p, "*", &Ctx::empty()), AuthzDecision::Deny); +} + +#[test] +fn escaped_question_and_dollar_in_resource_are_literal() { + let q = allow_all_deny_get(Some("2012-10-17"), "Resource", "${?}"); + assert_eq!(get(&q, "x", &Ctx::empty()), AuthzDecision::Allow); + assert_eq!(get(&q, "?", &Ctx::empty()), AuthzDecision::Deny); + let d = allow_all_deny_get(Some("2012-10-17"), "Resource", "${$}x"); + assert_eq!(get(&d, "$x", &Ctx::empty()), AuthzDecision::Deny); + assert_eq!(get(&d, "x", &Ctx::empty()), AuthzDecision::Allow); +} + +// --- NotResource with 2008-10-17, no Version, and an unresolved variable --- + +#[test] +fn not_resource_version_2008_matches_text_as_written() { + // Unexpanded, the pattern matches no real table, so every table is + // "not in" it and the Deny applies to all of them. + let p = allow_all_deny_get( + Some("2008-10-17"), + "NotResource", + "${aws:PrincipalTag/team}", + ); + assert_eq!(get(&p, "blue", &team_blue()), AuthzDecision::Deny); + assert_eq!(get(&p, "red", &team_blue()), AuthzDecision::Deny); +} + +#[test] +fn not_resource_no_version_matches_text_as_written() { + let p = allow_all_deny_get(None, "NotResource", "${aws:PrincipalTag/team}"); + assert_eq!(get(&p, "blue", &team_blue()), AuthzDecision::Deny); +} + +#[test] +fn not_resource_unresolved_variable_applies_to_every_resource() { + // Measured (PR #535 body table): with no `team` tag the pattern matches + // nothing, so every resource is outside it and the Deny applies. + let p = allow_all_deny_get( + Some("2012-10-17"), + "NotResource", + "${aws:PrincipalTag/team}", + ); + assert_eq!(get(&p, "blue", &Ctx::empty()), AuthzDecision::Deny); +} + +#[test] +fn allow_not_resource_with_unresolved_variable_grants_every_table() { + // Measured: an `Allow` alone, with no `team` tag. The pattern matches + // nothing, so every table is outside it and the Allow grants both. + let p = vec![parse(&format!( + r#"{{"Version":"2012-10-17","Statement":[{{ + "Effect":"Allow","Action":"dynamodb:GetItem","NotResource":"{ARN}${{aws:PrincipalTag/team}}" + }}]}}"# + ))]; + assert_eq!(get(&p, "blue", &Ctx::empty()), AuthzDecision::Allow); + assert_eq!(get(&p, "red", &Ctx::empty()), AuthzDecision::Allow); +} + +#[test] +fn default_form_without_a_space_expands_when_key_absent() { + // Measured: no space after the comma. + let p = allow_all_deny_get( + Some("2012-10-17"), + "Resource", + "${aws:PrincipalTag/team,'none'}", + ); + assert_eq!(get(&p, "none", &Ctx::empty()), AuthzDecision::Deny); +} + +// --- Default form in a condition value --- + +#[test] +fn condition_value_default_form_expands_when_key_absent() { + let p = parse(&format!( + r#"{{"Version":"2012-10-17","Statement":[{{ + "Effect":"Allow","Action":"dynamodb:GetItem","Resource":"{ARN}*", + "Condition":{{"StringEquals":{{"aws:ResourceTag/owner":"${{aws:username, 'anon'}}"}}}} + }}]}}"# + )); + let p = std::slice::from_ref(&p); + let owned_by_anon = Ctx::empty().with("aws:ResourceTag/owner", vec!["anon"]); + assert_eq!(get(p, "T", &owned_by_anon), AuthzDecision::Allow); + let owned_by_alice = Ctx::empty().with("aws:ResourceTag/owner", vec!["alice"]); + assert_eq!(get(p, "T", &owned_by_alice), AuthzDecision::Deny); + let alice_on_alice = owned_by_alice.with("aws:username", vec!["alice"]); + assert_eq!(get(p, "T", &alice_on_alice), AuthzDecision::Allow); +} + +// --- Boundary and session policy paths --- + +fn allow_own_table_doc() -> PolicyDocument { + allow_own_table().remove(0) +} + +#[test] +fn permissions_boundary_expands_resource_variable() { + let ctx = Ctx::empty().with("aws:username", vec!["alice"]); + let boundary = allow_own_table_doc(); + let decide = |target: &str| { + evaluate_policies( + &[allow_all()], + Some(&boundary), + None, + "dynamodb:GetItem", + &table(target), + &ctx, + ) + }; + assert_eq!(decide("alice"), AuthzDecision::Allow); + assert_eq!(decide("bob"), AuthzDecision::Deny); +} + +#[test] +fn session_policy_expands_resource_variable() { + let ctx = Ctx::empty().with("aws:username", vec!["alice"]); + let session = allow_own_table_doc(); + let decide = |target: &str| { + evaluate_policies( + &[allow_all()], + None, + Some(&session), + "dynamodb:GetItem", + &table(target), + &ctx, + ) + }; + assert_eq!(decide("alice"), AuthzDecision::Allow); + assert_eq!(decide("bob"), AuthzDecision::Deny); +} + +#[test] +fn explicit_deny_matches_arc_expands_resource_variable() { + use std::sync::Arc; + let p: Vec> = + allow_all_deny_get(Some("2012-10-17"), "Resource", "${aws:PrincipalTag/team}") + .into_iter() + .map(Arc::new) + .collect(); + let deny_on = |target: &str, ctx: &Ctx| { + explicit_deny_matches_arc(&p, None, None, "dynamodb:GetItem", &table(target), ctx) + }; + assert!(deny_on("blue", &team_blue())); + assert!(!deny_on("red", &team_blue())); + assert!(!deny_on("blue", &Ctx::empty())); +} diff --git a/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs b/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs new file mode 100644 index 000000000..04e98107d --- /dev/null +++ b/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs @@ -0,0 +1,229 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! Policy variables in `Resource`: multivalued keys, variables outside the +//! resource field, a `:` inside a value, and the wildcards beside a variable +//! (issue #533). Every test repeats a case measured with the IAM policy +//! simulator on 2026-10-24 with `dynamodb:GetItem` (PR #535 body table). Two +//! checks beside a measured one are derived: `table/red` with one +//! `dynamodb:LeadingKeys` value, and `table/blue` in the `blue:x` test. + +use super::*; + +const P: &str = "arn:aws:dynamodb:us-east-1:123456789012"; +const LEADING_KEYS: &str = "dynamodb:LeadingKeys"; + +fn allow_all_and(statement: &str) -> Vec { + vec![ + parse( + r#"{"Version":"2012-10-17","Statement":[{ + "Effect":"Allow","Action":"dynamodb:*","Resource":"*" + }]}"#, + ), + parse(&format!( + r#"{{"Version":"2012-10-17","Statement":[{statement}]}}"# + )), + ] +} + +fn deny_get_on(resource: &str) -> Vec { + allow_all_and(&format!( + r#"{{"Effect":"Deny","Action":"dynamodb:GetItem","Resource":"{resource}"}}"# + )) +} + +fn get(policies: &[PolicyDocument], target: &str, ctx: &Ctx) -> AuthzDecision { + evaluate_policies(policies, None, None, "dynamodb:GetItem", target, ctx) +} + +fn leading_keys(values: Vec<&str>) -> Ctx { + Ctx::empty().with(LEADING_KEYS, values) +} + +// --- A multivalued key is never a variable (`dynamodb:LeadingKeys` with one +// value and with two, PR #535 body table) --- + +#[test] +fn a_deny_on_a_multivalued_key_variable_denies_no_table() { + let p = deny_get_on(&format!("{P}:table/${{dynamodb:LeadingKeys}}")); + for ctx in [ + leading_keys(vec!["blue", "red"]), + leading_keys(vec!["blue"]), + ] { + assert_eq!( + get(&p, &format!("{P}:table/blue"), &ctx), + AuthzDecision::Allow + ); + assert_eq!( + get(&p, &format!("{P}:table/red"), &ctx), + AuthzDecision::Allow + ); + } +} + +#[test] +fn an_allow_on_a_multivalued_key_variable_grants_no_table() { + let p = vec![parse(&format!( + r#"{{"Version":"2012-10-17","Statement":[{{ + "Effect":"Allow","Action":"dynamodb:GetItem","Resource":"{P}:table/${{dynamodb:LeadingKeys}}" + }}]}}"# + ))]; + let ctx = leading_keys(vec!["blue", "red"]); + assert_eq!( + get(&p, &format!("{P}:table/blue"), &ctx), + AuthzDecision::Deny + ); +} + +#[test] +fn a_condition_value_naming_a_multivalued_key_is_not_expanded() { + let p = allow_all_and( + r#"{"Effect":"Deny","Action":"dynamodb:GetItem","Resource":"*", + "Condition":{"StringEquals":{"aws:PrincipalTag/team":"${dynamodb:LeadingKeys}"}}}"#, + ); + for values in [vec!["blue", "red"], vec!["blue"]] { + let ctx = leading_keys(values).with("aws:PrincipalTag/team", vec!["blue"]); + assert_eq!(get(&p, &format!("{P}:table/x"), &ctx), AuthzDecision::Allow); + } +} + +// --- A multivalued key with a default: the default is used only when the +// request does not carry the key. Measured with the IAM policy simulator on +// 2026-10-28 with `aws:TagKeys` as `[x, y]`, `[x]` and absent, against +// `table/${aws:TagKeys, 'x'}` in `Resource` (implicitDeny, implicitDeny, +// allowed) and against `${aws:TagKeys, 'x'}` as a `StringEquals` condition +// value with `aws:PrincipalTag/team` = `x` (the same three answers). Run here +// on `dynamodb:LeadingKeys`, the multivalued key the test context declares. --- + +#[test] +fn a_multivalued_key_variable_with_a_default_matches_only_when_the_key_is_absent() { + let p = vec![parse(&format!( + r#"{{"Version":"2012-10-17","Statement":[{{ + "Effect":"Allow","Action":"dynamodb:GetItem","Resource":"{P}:table/${{dynamodb:LeadingKeys, 'x'}}" + }}]}}"# + ))]; + for values in [vec!["x", "y"], vec!["x"]] { + assert_eq!( + get(&p, &format!("{P}:table/x"), &leading_keys(values)), + AuthzDecision::Deny + ); + } + assert_eq!( + get(&p, &format!("{P}:table/x"), &Ctx::empty()), + AuthzDecision::Allow + ); +} + +#[test] +fn a_condition_value_naming_a_multivalued_key_with_a_default_uses_it_only_when_absent() { + let p = vec![parse( + r#"{"Version":"2012-10-17","Statement":[{ + "Effect":"Allow","Action":"dynamodb:GetItem","Resource":"*", + "Condition":{"StringEquals":{"aws:PrincipalTag/team":"${dynamodb:LeadingKeys, 'x'}"}} + }]}"#, + )]; + for values in [vec!["x", "y"], vec!["x"]] { + let ctx = leading_keys(values).with("aws:PrincipalTag/team", vec!["x"]); + assert_eq!(get(&p, &format!("{P}:table/x"), &ctx), AuthzDecision::Deny); + } + let ctx = Ctx::empty().with("aws:PrincipalTag/team", vec!["x"]); + assert_eq!(get(&p, &format!("{P}:table/x"), &ctx), AuthzDecision::Allow); +} + +// --- Variables before the fifth colon expand (`aws:RequestedRegion` in the +// region field, `aws:PrincipalAccount` in the account field, PR #535 body +// table) --- +// +// These two simulator rows contradict IAM's policy variables page, which +// says a variable cannot replace parts of the ARN before the fifth colon. +// The tests follow the measurement. A variable in the service field goes +// through the same code and was not measured, so change neither rule +// without measuring both. + +#[test] +fn a_variable_in_the_region_field_expands() { + let p = deny_get_on("arn:aws:dynamodb:${aws:RequestedRegion}:123456789012:table/blue"); + let ctx = Ctx::empty().with("aws:RequestedRegion", vec!["us-east-1"]); + assert_eq!( + get(&p, &format!("{P}:table/blue"), &ctx), + AuthzDecision::Deny + ); +} + +#[test] +fn a_variable_in_the_account_field_expands() { + let p = deny_get_on("arn:aws:dynamodb:us-east-1:${aws:PrincipalAccount}:table/blue"); + let ctx = Ctx::empty().with("aws:PrincipalAccount", vec!["123456789012"]); + assert_eq!( + get(&p, &format!("{P}:table/blue"), &ctx), + AuthzDecision::Deny + ); +} + +// --- A ':' inside a value stays in its field (a region tag value +// `us-east-1:123456789012`, and `team=blue:x`, PR #535 body table) --- + +#[test] +fn a_colon_in_a_region_field_value_does_not_separate_fields() { + let p = deny_get_on("arn:aws:dynamodb:${aws:PrincipalTag/region}:*:table/T"); + let ctx = Ctx::empty().with("aws:PrincipalTag/region", vec!["us-east-1:123456789012"]); + assert_eq!(get(&p, &format!("{P}:table/T"), &ctx), AuthzDecision::Allow); + assert_eq!( + get(&p, &format!("{P}:table/A/stream/x:table/T"), &ctx), + AuthzDecision::Allow + ); +} + +#[test] +fn a_colon_in_a_resource_field_value_matches_the_same_name() { + let p = deny_get_on(&format!("{P}:table/${{aws:PrincipalTag/team}}")); + let ctx = Ctx::empty().with("aws:PrincipalTag/team", vec!["blue:x"]); + assert_eq!( + get(&p, &format!("{P}:table/blue:x"), &ctx), + AuthzDecision::Deny + ); + assert_eq!( + get(&p, &format!("{P}:table/blue"), &ctx), + AuthzDecision::Allow + ); +} + +// --- Wildcards beside a variable (`table/${aws:PrincipalTag/team}*` with no +// `team` tag, and `table/b?${aws:PrincipalTag/team}`, PR #535 body table) --- + +#[test] +fn an_unresolved_variable_before_a_wildcard_matches_nothing() { + // An unresolved variable makes the pattern match nothing; expanding it to + // the empty string would leave `table/*` and deny every table. + let p = deny_get_on(&format!("{P}:table/${{aws:PrincipalTag/team}}*")); + assert_eq!( + get(&p, &format!("{P}:table/blue"), &Ctx::empty()), + AuthzDecision::Allow + ); +} + +#[test] +fn a_question_mark_written_beside_a_variable_is_a_wildcard() { + let p = deny_get_on(&format!("{P}:table/b?${{aws:PrincipalTag/team}}")); + let ctx = Ctx::empty().with("aws:PrincipalTag/team", vec!["ue"]); + assert_eq!( + get(&p, &format!("{P}:table/blue"), &ctx), + AuthzDecision::Deny + ); +} + +// --- A wildcard inside a value is literal --- + +#[test] +fn a_wildcard_in_a_value_matches_only_the_same_text() { + let p = deny_get_on(&format!("{P}:table/${{aws:PrincipalTag/team}}")); + let ctx = Ctx::empty().with("aws:PrincipalTag/team", vec!["bl*"]); + assert_eq!( + get(&p, &format!("{P}:table/blue"), &ctx), + AuthzDecision::Allow + ); + assert_eq!( + get(&p, &format!("{P}:table/bl*"), &ctx), + AuthzDecision::Deny + ); +} diff --git a/crates/auth/src/policy/evaluator/tests/variable_spelling.rs b/crates/auth/src/policy/evaluator/tests/variable_spelling.rs new file mode 100644 index 000000000..6a49fa6ca --- /dev/null +++ b/crates/auth/src/policy/evaluator/tests/variable_spelling.rs @@ -0,0 +1,165 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! How the text between `${` and `}` is read: the code points up to U+0020 +//! are trimmed around the key name, around the comma of the default form and +//! before the closing brace, and around an escape, in `Resource` and in a +//! condition value. The text inside the quotes of a default keeps its spaces. +//! +//! Each test repeats rows measured with the IAM policy simulator on +//! 2026-11-03, `dynamodb:GetItem` with `Allow dynamodb:*` on `*` beside each +//! `Deny`, two samples per row with the same answer. The row names are in +//! the comments. + +use super::*; +use serde_json::json; + +const TEAM: &str = "aws:PrincipalTag/team"; +const ARN: &str = "arn:aws:dynamodb:us-east-1:123456789012:table/"; + +/// `Allow dynamodb:*` on `*` and `Deny dynamodb:GetItem` with `deny_fields`. +fn allow_all_and_deny(deny_fields: &serde_json::Value) -> Vec { + let mut deny = json!({"Effect": "Deny", "Action": "dynamodb:GetItem"}); + for (field, value) in deny_fields.as_object().unwrap() { + deny[field] = value.clone(); + } + let document = json!({ + "Version": "2012-10-17", + "Statement": [{"Effect": "Allow", "Action": "dynamodb:*", "Resource": "*"}, deny], + }); + vec![parse(&document.to_string())] +} + +fn deny_get_on(resource: &str) -> Vec { + allow_all_and_deny(&json!({"Resource": format!("{ARN}{resource}")})) +} + +fn get(policies: &[PolicyDocument], target: &str, ctx: &Ctx) -> AuthzDecision { + evaluate_policies( + policies, + None, + None, + "dynamodb:GetItem", + &format!("{ARN}{target}"), + ctx, + ) +} + +fn team(value: &str) -> Ctx { + Ctx::empty().with(TEAM, vec![value]) +} + +#[test] +fn spaces_around_the_key_are_trimmed() { + // W1, W2, W3. + for written in [ + "${ aws:PrincipalTag/team }", + "${aws:PrincipalTag/team }", + "${ aws:PrincipalTag/team}", + ] { + let p = deny_get_on(written); + assert_eq!( + get(&p, "blue", &team("blue")), + AuthzDecision::Deny, + "{written}" + ); + assert_eq!( + get(&p, "red", &team("blue")), + AuthzDecision::Allow, + "{written}" + ); + } + // W1b: the table named with the text as written is not the one denied. + let p = deny_get_on("${ aws:PrincipalTag/team }"); + assert_eq!( + get(&p, "${ aws:PrincipalTag/team }", &team("blue")), + AuthzDecision::Allow + ); +} + +#[test] +fn spaces_around_the_comma_and_the_brace_are_trimmed() { + // W4 and W5: spaces on both sides of the key and before the comma. + let p = deny_get_on("${ aws:PrincipalTag/team , 'none'}"); + assert_eq!(get(&p, "blue", &team("blue")), AuthzDecision::Deny); + assert_eq!(get(&p, "none", &Ctx::empty()), AuthzDecision::Deny); + // W6: a space before the comma and none after it. + let p = deny_get_on("${aws:PrincipalTag/team ,'none'}"); + assert_eq!(get(&p, "blue", &team("blue")), AuthzDecision::Deny); + // W7: two spaces after the comma. + let p = deny_get_on("${aws:PrincipalTag/team, 'none'}"); + assert_eq!(get(&p, "none", &Ctx::empty()), AuthzDecision::Deny); + // W8: a space before the closing brace. + let p = deny_get_on("${aws:PrincipalTag/team, 'none' }"); + assert_eq!(get(&p, "none", &Ctx::empty()), AuthzDecision::Deny); +} + +#[test] +fn spaces_inside_the_quotes_of_a_default_are_kept() { + // T4 and T4b. + let p = deny_get_on("${aws:PrincipalTag/team, ' none '}"); + assert_eq!(get(&p, " none ", &Ctx::empty()), AuthzDecision::Deny); + assert_eq!(get(&p, "none", &Ctx::empty()), AuthzDecision::Allow); +} + +#[test] +fn code_points_up_to_space_are_trimmed_and_no_others() { + // T1 (tab), T2 (newline, before the key only), T5 (U+000B), T6 (U+0001), + // T7 (U+000C), T8 (U+000D) and T9 (U+001F) are trimmed. + for (before, after) in [ + ("\t", "\t"), + ("\n", ""), + ("\u{b}", "\u{b}"), + ("\u{1}", "\u{1}"), + ("\u{c}", "\u{c}"), + ("\r", "\r"), + ("\u{1f}", "\u{1f}"), + ] { + let p = deny_get_on(&format!("${{{before}{TEAM}{after}}}")); + assert_eq!( + get(&p, "blue", &team("blue")), + AuthzDecision::Deny, + "{before:?}" + ); + } + // T3 (U+00A0, before the key only), T10 (U+0085) and T11 (U+3000) are + // not, so the key the variable names is not in the request. + for (before, after) in [ + ("\u{a0}", ""), + ("\u{85}", "\u{85}"), + ("\u{3000}", "\u{3000}"), + ] { + let p = deny_get_on(&format!("${{{before}{TEAM}{after}}}")); + assert_eq!( + get(&p, "blue", &team("blue")), + AuthzDecision::Allow, + "{before:?}" + ); + } +} + +#[test] +fn spaces_around_an_escape_are_trimmed() { + // X1, X1b and the control X1c: `${ * }` stands for a literal `*`. + let p = deny_get_on("${ * }"); + assert_eq!(get(&p, "*", &Ctx::empty()), AuthzDecision::Deny); + assert_eq!(get(&p, "x", &Ctx::empty()), AuthzDecision::Allow); + let p = deny_get_on("${*}"); + assert_eq!(get(&p, "*", &Ctx::empty()), AuthzDecision::Deny); + // X2: `${ $ }` stands for a literal `$`. + let p = deny_get_on("a${ $ }"); + assert_eq!(get(&p, "a$", &Ctx::empty()), AuthzDecision::Deny); +} + +#[test] +fn spaces_around_the_key_are_trimmed_in_a_condition_value() { + // W9. + let p = allow_all_and_deny(&json!({ + "Resource": "*", + "Condition": {"StringEquals": {"aws:ResourceTag/owner": "${ aws:username }"}}, + })); + let ctx = Ctx::empty() + .with("aws:username", vec!["alice"]) + .with("aws:ResourceTag/owner", vec!["alice"]); + assert_eq!(get(&p, "x", &ctx), AuthzDecision::Deny); +} diff --git a/crates/auth/src/policy/evaluator/tests/variable_values.rs b/crates/auth/src/policy/evaluator/tests/variable_values.rs new file mode 100644 index 000000000..7f3ec7099 --- /dev/null +++ b/crates/auth/src/policy/evaluator/tests/variable_values.rs @@ -0,0 +1,186 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! Text that comes from a variable value or a default: it is not expanded a +//! second time, and a `*` or `?` in it is not a wildcard. Also the two +//! condition-value rules, the `${key, 'default'}` form and no expansion for a +//! multivalued key, under the set operators and `IfExists`, and the default +//! form under `Null`, which does not read it. +//! +//! Except for the `Null` row, none of these expectations was measured on its +//! own. Each follows from a rule the PR #535 body table measured with the IAM +//! policy simulator on 2026-10-24: text from a value is matched literally +//! (`team=bl*` matches only `table/bl*`), and a condition value naming +//! `dynamodb:LeadingKeys` is not expanded under `StringEquals`. The default +//! form in a condition value was measured on 2026-11-03 under `StringEquals`, +//! `ArnLike` and `Null` (`condition_variable_scope.rs`). + +use super::*; + +const ARN: &str = "arn:aws:dynamodb:us-east-1:123456789012:table/"; + +fn table(name: &str) -> String { + format!("{ARN}{name}") +} + +fn doc(statement: &str) -> PolicyDocument { + parse(&format!( + r#"{{"Version":"2012-10-17","Statement":[{statement}]}}"# + )) +} + +fn allow_all() -> PolicyDocument { + doc(r#"{"Effect":"Allow","Action":"dynamodb:*","Resource":"*"}"#) +} + +/// `Allow dynamodb:*` on `*` beside a `Deny dynamodb:GetItem` on +/// `table/`. +fn deny_get_on(resource: &str) -> Vec { + vec![ + allow_all(), + doc(&format!( + r#"{{"Effect":"Deny","Action":"dynamodb:GetItem","Resource":"{ARN}{resource}"}}"# + )), + ] +} + +/// `Allow dynamodb:*` on `*` beside a `Deny dynamodb:GetItem` on `*` whose +/// condition compares `key` with `value` under `operator`. +fn deny_get_if(operator: &str, key: &str, value: &str) -> Vec { + vec![ + allow_all(), + doc(&format!( + r#"{{"Effect":"Deny","Action":"dynamodb:GetItem","Resource":"*", + "Condition":{{"{operator}":{{"{key}":"{value}"}}}}}}"# + )), + ] +} + +/// Only an `Allow dynamodb:GetItem` on every table whose condition compares +/// `key` with `value` under `operator`. +fn allow_get_if(operator: &str, key: &str, value: &str) -> Vec { + vec![doc(&format!( + r#"{{"Effect":"Allow","Action":"dynamodb:GetItem","Resource":"{ARN}*", + "Condition":{{"{operator}":{{"{key}":"{value}"}}}}}}"# + ))] +} + +fn get(policies: &[PolicyDocument], target: &str, ctx: &Ctx) -> AuthzDecision { + evaluate_policies( + policies, + None, + None, + "dynamodb:GetItem", + &table(target), + ctx, + ) +} + +// --- A substituted value is not expanded again --- + +#[test] +fn a_resource_value_holding_a_variable_is_not_expanded_again() { + // A resource tag can hold any text. The value `${aws:username}` names the + // table `${aws:username}`, not the caller's own table. + let p = deny_get_on("${aws:ResourceTag/alias}"); + let ctx = Ctx::empty() + .with("aws:ResourceTag/alias", vec!["${aws:username}"]) + .with("aws:username", vec!["alice"]); + assert_eq!(get(&p, "alice", &ctx), AuthzDecision::Allow); + assert_eq!(get(&p, "${aws:username}", &ctx), AuthzDecision::Deny); +} + +#[test] +fn a_condition_value_holding_a_variable_is_not_expanded_again() { + // `${aws:UserAgent}` expands to the text `${aws:username}`, which is + // compared as written, so the user name `alice` does not equal it. + let p = deny_get_if("StringEquals", "aws:username", "${aws:UserAgent}"); + let ctx = Ctx::empty() + .with("aws:UserAgent", vec!["${aws:username}"]) + .with("aws:username", vec!["alice"]); + assert_eq!(get(&p, "T", &ctx), AuthzDecision::Allow); + let equal = Ctx::empty() + .with("aws:UserAgent", vec!["alice"]) + .with("aws:username", vec!["alice"]); + assert_eq!(get(&p, "T", &equal), AuthzDecision::Deny); +} + +// --- A `*` or `?` in a default is literal --- + +#[test] +fn a_wildcard_in_a_resource_default_is_literal() { + let star = deny_get_on("${aws:PrincipalTag/team, '*'}"); + assert_eq!(get(&star, "x", &Ctx::empty()), AuthzDecision::Allow); + assert_eq!(get(&star, "*", &Ctx::empty()), AuthzDecision::Deny); + let question = deny_get_on("${aws:PrincipalTag/team, 'b?'}"); + assert_eq!(get(&question, "bx", &Ctx::empty()), AuthzDecision::Allow); + assert_eq!(get(&question, "b?", &Ctx::empty()), AuthzDecision::Deny); +} + +// --- The default form in a condition value, under every operator form --- + +/// A request on a table tagged `owner=` by a caller with no +/// `aws:username`. +fn owned_by(owner: &str) -> Ctx { + Ctx::empty().with("aws:ResourceTag/owner", vec![owner]) +} + +#[test] +fn the_default_form_expands_under_set_operators_and_if_exists() { + for operator in [ + "ForAnyValue:StringEquals", + "ForAllValues:StringEquals", + "StringEqualsIfExists", + ] { + let p = allow_get_if(operator, "aws:ResourceTag/owner", "${aws:username, 'anon'}"); + assert_eq!( + get(&p, "T", &owned_by("anon")), + AuthzDecision::Allow, + "{operator}" + ); + assert_eq!( + get(&p, "T", &owned_by("alice")), + AuthzDecision::Deny, + "{operator}" + ); + } +} + +#[test] +fn the_default_form_is_not_read_in_a_null_condition() { + // Row N5 of `condition_variable_scope.rs`: the Deny does not apply to a + // caller with neither tag. Read with its default, the value would be + // `true` and the Deny would apply. + let p = deny_get_if( + "Null", + "aws:PrincipalTag/team", + "${aws:PrincipalTag/f, 'true'}", + ); + assert_eq!(get(&p, "T", &Ctx::empty()), AuthzDecision::Allow); +} + +// --- A multivalued key in a condition value, under every operator form --- + +#[test] +fn a_multivalued_key_in_a_condition_value_is_not_expanded_under_any_form() { + // Expanded to its first value, `${dynamodb:LeadingKeys}` would equal the + // request's own key `blue` and each Deny would apply. + let ctx = Ctx::empty() + .with("dynamodb:LeadingKeys", vec!["blue"]) + .with("aws:PrincipalTag/team", vec!["blue"]); + for (operator, key) in [ + ("ForAnyValue:StringEquals", "dynamodb:LeadingKeys"), + ("ForAllValues:StringEquals", "dynamodb:LeadingKeys"), + ("StringEqualsIfExists", "aws:PrincipalTag/team"), + ] { + let p = deny_get_if(operator, key, "${dynamodb:LeadingKeys}"); + assert_eq!(get(&p, "T", &ctx), AuthzDecision::Allow, "{operator}"); + } + // Under `Null`, the first value `false` would say that `team` is present, + // which it is, and the Deny would apply. + let null = Ctx::empty() + .with("dynamodb:LeadingKeys", vec!["false"]) + .with("aws:PrincipalTag/team", vec!["blue"]); + let p = deny_get_if("Null", "aws:PrincipalTag/team", "${dynamodb:LeadingKeys}"); + assert_eq!(get(&p, "T", &null), AuthzDecision::Allow, "Null"); +} diff --git a/crates/auth/src/policy/mod.rs b/crates/auth/src/policy/mod.rs index 2eac6a912..ceaf66376 100755 --- a/crates/auth/src/policy/mod.rs +++ b/crates/auth/src/policy/mod.rs @@ -14,3 +14,4 @@ pub mod document; pub mod evaluator; pub mod matcher; pub mod principal_id; +pub mod variables; diff --git a/crates/auth/src/policy/variables.rs b/crates/auth/src/policy/variables.rs new file mode 100644 index 000000000..4fab6d4a8 --- /dev/null +++ b/crates/auth/src/policy/variables.rs @@ -0,0 +1,455 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! IAM policy variable expansion. +//! +//! A policy may write `${key}` in a `Resource` or `NotResource` pattern or in +//! a condition value, and IAM replaces it with the request's value for `key` +//! before matching. The forms are: +//! +//! - `${key}`: the value of `key` in the request context. A multivalued key +//! is never a variable: when the request carries it, with one value or +//! several, the pattern matches nothing, default or not; a written default +//! is used only when the request does not carry the key (measured with +//! `dynamodb:LeadingKeys` holding one value and two, in `Resource` and in a +//! condition value, PR #535 body table; and with `aws:TagKeys` present and +//! absent against `${aws:TagKeys, 'x'}` on 2026-10-28). +//! - `${key, 'default'}`: the value of `key`, or `default` when the request +//! does not carry `key`. +//! - `${*}`, `${?}`, `${$}`: a literal `*`, `?` or `$`. +//! +//! In a `Resource` or `NotResource` pattern the text written in the policy +//! keeps `*` and `?` as wildcards, while text that comes from a variable +//! value, a default or an escape is matched literally, so a tag value or an +//! escaped `*` cannot widen a pattern. A variable whose key the request does +//! not carry, with no default, makes the pattern match nothing. Both rules +//! were measured with the IAM policy simulator (issue #533). A variable whose +//! key the service populates but the context cannot resolve (a key listed in +//! `context::FAIL_CLOSED_GLOBAL_KEYS`) leaves the match unknown, and the +//! evaluator fails closed on it as it does on such a key in a condition. +//! Only a document whose `Version` is `2012-10-17` expands variables; the +//! caller decides that with [`PolicyDocument::expands_policy_variables`]. +//! +//! ARN matching here follows the same field rule as +//! [`super::matcher::arn_match`]: the first five `:` separate fields, the +//! resource field keeps its own `:` and `/`, and both sides need the same +//! number of fields. Only a `:` written in the policy text separates fields; +//! a `:` inside a variable value is matched as a character of its field +//! (measured: a region-field value `us-east-1:123456789012` matches no table +//! ARN). `*` and `?` count UTF-16 code units, as in +//! [`super::matcher::wildcard_match`] (#581). + +use super::context::ConditionContext; +use super::document::{PolicyDocument, PolicyParseError}; +use super::matcher::arn_match; +use serde_json::Value; + +impl PolicyDocument { + /// Whether `${key}` policy variables in `Resource` and `NotResource` + /// patterns are expanded. + /// + /// IAM expands them only in a `2012-10-17` document; a `2008-10-17` + /// document, or one with no `Version`, matches the pattern as written. + /// Measured with the IAM policy simulator (issue #533). + #[must_use] + pub fn expands_policy_variables(&self) -> bool { + self.version.as_deref() == Some("2012-10-17") + } +} + +/// Refuse a `Version` that is not the string `2012-10-17` or `2008-10-17`, +/// the two values of IAM's policy grammar. +/// +/// The value decides whether `Resource` and `NotResource` variables expand, +/// so a mistyped `Version` would otherwise turn that off for the whole +/// document with no error. +/// `PolicyDocument::from_json_for_write` calls this, so a stored document with +/// another value keeps parsing. A document with no `Version` passes here, +/// because each write path refuses it first. Whether IAM refuses such a +/// document at write, and with what message, was not measured. +pub(super) fn validate_version(json: &str) -> Result<(), PolicyParseError> { + let raw: Value = serde_json::from_str(json).map_err(|e| PolicyParseError(e.to_string()))?; + match raw.get("Version") { + None => Ok(()), + Some(Value::String(v)) if v == "2012-10-17" || v == "2008-10-17" => Ok(()), + Some(other) => Err(PolicyParseError(format!( + "invalid Version (must be the string 2012-10-17 or 2008-10-17): {other}" + ))), + } +} + +/// `*` in UTF-16. +const STAR: u16 = 0x2A; +/// `?` in UTF-16. +const QUESTION: u16 = 0x3F; +/// `:` in UTF-16. +const COLON: u16 = 0x3A; + +/// One unit of an expanded pattern. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Token { + /// `*` written in the policy text: matches zero or more UTF-16 code units. + Star, + /// `?` written in the policy text: matches exactly one UTF-16 code unit. + Question, + /// `:` written in the policy text: a field separator among the first five, + /// a literal `:` in the resource field. + Colon, + /// One UTF-16 code unit matched literally. + Unit(u16), +} + +impl Token { + /// The token for a UTF-16 code unit of the policy text, where `*`, `?` + /// and `:` keep their meaning. + fn written(unit: u16) -> Self { + match unit { + STAR => Self::Star, + QUESTION => Self::Question, + COLON => Self::Colon, + other => Self::Unit(other), + } + } +} + +/// A `${...}` reference as written in a policy. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Reference<'a> { + /// `${key}` or `${key, 'default'}`. + Key { + name: &'a str, + default: Option<&'a str>, + }, + /// `${*}`, `${?}` or `${$}`. + Escape(u8), +} + +/// Trim the code points up to U+0020 from both ends of `text`, the set Java's +/// `String.trim` removes. +/// +/// IAM trims this set around a variable's key name and its quoted default, +/// in both forms and in the escapes (measured with the IAM policy simulator: +/// space, tab, newline, U+000B, U+000C, U+000D, U+0001 and U+001F are +/// trimmed; U+0085, U+00A0 and U+3000 are not, so a key name written with +/// one of them names a key the request does not carry). +fn trim_up_to_space(text: &str) -> &str { + text.trim_matches(|c: char| c <= ' ') +} + +/// Parse the text between `${` and `}`. +/// +/// The text is trimmed with [`trim_up_to_space`] first. Then `*`, `?` and +/// `$` are the escapes, and `name, 'default'` is the default form, where the +/// name and the quoted default are trimmed again and the text inside the +/// quotes is kept as written. Anything else, including a `,` whose right +/// side is not single-quoted, is taken as the key name. +fn parse_reference(inner: &str) -> Reference<'_> { + let inner = trim_up_to_space(inner); + match inner { + "*" => return Reference::Escape(b'*'), + "?" => return Reference::Escape(b'?'), + "$" => return Reference::Escape(b'$'), + _ => {} + } + if let Some((name, default)) = inner.split_once(',') { + let default = trim_up_to_space(default); + if default.len() >= 2 && default.starts_with('\'') && default.ends_with('\'') { + return Reference::Key { + name: trim_up_to_space(name), + default: Some(&default[1..default.len() - 1]), + }; + } + } + Reference::Key { + name: inner, + default: None, + } +} + +/// One piece of a policy string: plain text, or a `${...}` reference with its +/// raw text including the braces. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Part<'a> { + Text(&'a str), + Ref(&'a str, Reference<'a>), +} + +/// Split `value` into plain text and `${...}` references. A `${` with no +/// closing `}` is plain text, as before. +/// +/// With `iam_syntax` the text between the braces is read by +/// [`parse_reference`]. Without it the whole text, as written, is the key +/// name, which is how a condition value was read before the default form: +/// nothing is trimmed, and `${key, 'default'}` names the key +/// `key, 'default'`, which no request carries. +fn parts(value: &str, iam_syntax: bool) -> Vec> { + let mut out = Vec::new(); + let mut rest = value; + while let Some(start) = rest.find("${") { + out.push(Part::Text(&rest[..start])); + let after_start = &rest[start + 2..]; + match after_start.find('}') { + Some(end) => { + let raw = &rest[start..start + 3 + end]; + let inner = &after_start[..end]; + let reference = if iam_syntax { + parse_reference(inner) + } else { + Reference::Key { + name: inner, + default: None, + } + }; + out.push(Part::Ref(raw, reference)); + rest = &after_start[end + 1..]; + } + None => { + out.push(Part::Text(&rest[start..])); + rest = ""; + } + } + } + out.push(Part::Text(rest)); + out +} + +/// Why a variable has no text to substitute. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Missing { + /// The request does not carry the key and no default was written. + Absent, + /// The service populates the key but the context cannot resolve it + /// (`ConditionContext::is_unresolvable_key`). A default is not used, + /// because the service would have a value. + Unresolvable, +} + +/// The request's value for `key`: the value of a present single-valued key, +/// or the default when the key is absent and a default was written. A present +/// key with no values expands to the empty string. A multivalued key is never +/// a value: when the request carries it the pattern matches nothing, default +/// or not, and when it is absent a written default is used (IAM policy +/// simulator, 2026-10-28, `aws:TagKeys` as `[x, y]`, `[x]` and absent against +/// `table/${aws:TagKeys, 'x'}`: implicitDeny, implicitDeny, allowed). A key +/// the request does not carry and the context reports unresolvable is +/// [`Missing::Unresolvable`] before either rule, multivalued or not, so its +/// default is not used. +fn resolve<'a>( + name: &str, + default: Option<&'a str>, + context: &'a impl ConditionContext, +) -> Result<&'a str, Missing> { + let values = context.resolve_key(name); + if values.is_none() && context.is_unresolvable_key(name) { + return Err(Missing::Unresolvable); + } + if context.is_multivalued_key(name) { + return match values { + Some(_) => Err(Missing::Absent), + None => default.ok_or(Missing::Absent), + }; + } + match values { + Some(values) => Ok(values.first().copied().unwrap_or("")), + None => default.ok_or(Missing::Absent), + } +} + +/// Expand `pattern` into tokens. Fails with [`Missing::Absent`] when a +/// variable names a key the request does not carry and gives no default, +/// since that alone makes the pattern match nothing, and otherwise with +/// [`Missing::Unresolvable`] when a variable names a key the context cannot +/// resolve. +fn expand_tokens(pattern: &str, context: &impl ConditionContext) -> Result, Missing> { + let mut tokens = Vec::with_capacity(pattern.len()); + let mut unresolvable = false; + for part in parts(pattern, true) { + match part { + Part::Text(text) => tokens.extend(text.encode_utf16().map(Token::written)), + Part::Ref(_, Reference::Escape(byte)) => tokens.push(Token::Unit(u16::from(byte))), + Part::Ref(_, Reference::Key { name, default }) => { + match resolve(name, default, context) { + Ok(value) => tokens.extend(value.encode_utf16().map(Token::Unit)), + Err(Missing::Unresolvable) => unresolvable = true, + Err(Missing::Absent) => return Err(Missing::Absent), + } + } + } + } + if unresolvable { + Err(Missing::Unresolvable) + } else { + Ok(tokens) + } +} + +/// Whether the `Resource` or `NotResource` pattern `pattern` matches +/// `resource_arn`. +/// +/// With `expand` false, or when `pattern` has no `${`, this is exactly +/// [`arn_match`]. Otherwise variables are expanded first, and a pattern with +/// an unresolved variable matches nothing, as does one that +/// [`resource_pattern_is_unresolvable`] reports. +#[must_use] +pub fn resource_pattern_matches( + pattern: &str, + resource_arn: &str, + expand: bool, + context: &impl ConditionContext, +) -> bool { + if !expand || !pattern.contains("${") { + return arn_match(pattern, resource_arn); + } + expand_tokens(pattern, context).is_ok_and(|tokens| arn_match_tokens(&tokens, resource_arn)) +} + +/// Whether expanding `pattern` stops on a variable whose key the service +/// populates but the context cannot resolve, so whether the pattern matches +/// is unknown. A pattern that also names an absent key with no default is not +/// reported: it matches nothing whatever the other value is. +/// +/// The caller fails closed on such a pattern, as on a condition key the +/// context cannot resolve: a `Deny` applies and an `Allow` does not grant. +#[must_use] +pub fn resource_pattern_is_unresolvable( + pattern: &str, + expand: bool, + context: &impl ConditionContext, +) -> bool { + expand + && pattern.contains("${") + && expand_tokens(pattern, context) == Err(Missing::Unresolvable) +} + +/// The key names the `${key}` and `${key, 'default'}` variables in `value` +/// read, in the order [`expand_condition_value`] reads them with the same +/// `iam_syntax`. +/// +/// With `iam_syntax` the default is not part of the name, so a caller that +/// asks the context about each name sees the key behind +/// `${aws:RequestTag/env, 'x'}` as it sees the one behind +/// `${aws:RequestTag/env}`. Without it the name is the text between the +/// braces as written. The `${*}`, `${?}` and `${$}` escapes name no key, and +/// neither does a `${` with no closing brace, as both expand to themselves. +pub fn variable_key_names(value: &str, iam_syntax: bool) -> impl Iterator { + parts(value, iam_syntax) + .into_iter() + .filter_map(|part| match part { + Part::Ref(_, Reference::Key { name, .. }) => Some(name), + Part::Text(_) | Part::Ref(_, Reference::Escape(_)) => None, + }) +} + +/// Expand variables in a condition value. +/// +/// `${key}` is replaced by its value, and with `iam_syntax` so is +/// `${key, 'default'}`; a variable whose key the request does not carry, +/// with no default, is left as written, and so are the `${*}`, `${?}` and +/// `${$}` escapes, as before. A variable whose key the context reports +/// unresolvable is also left as written, with or without a default: the +/// service would have a value for the key, so the default is not used, and +/// the caller fails the condition closed instead of comparing the text +/// (`condition::evaluate_condition_match`, which finds the key through +/// [`variable_key_names`]). +/// +/// The caller passes `iam_syntax` only where IAM reads the default form: in +/// a `2012-10-17` document and under a string or ARN operator. Elsewhere the +/// text between the braces is the key name as written ([`parts`]), so +/// `${key, 'default'}` stays as written, and `${key}` expands as it did +/// before the default form was read. +#[must_use] +pub fn expand_condition_value( + value: &str, + iam_syntax: bool, + context: &impl ConditionContext, +) -> String { + if !value.contains("${") { + return value.to_owned(); + } + let mut result = String::with_capacity(value.len()); + for part in parts(value, iam_syntax) { + match part { + Part::Text(text) | Part::Ref(text, Reference::Escape(_)) => result.push_str(text), + Part::Ref(raw, Reference::Key { name, default }) => { + result.push_str(resolve(name, default, context).unwrap_or(raw)); + } + } + } + result +} + +/// Match an expanded ARN pattern against an ARN value with the field rule of +/// [`arn_match`]. +fn arn_match_tokens(pattern: &[Token], value: &str) -> bool { + if pattern == [Token::Star] { + return true; + } + let p_fields = split_fields(pattern); + let v_fields: Vec<&str> = value.splitn(6, ':').collect(); + if p_fields.len() != v_fields.len() { + return false; + } + p_fields.iter().zip(v_fields.iter()).all(|(p, v)| { + let units: Vec = v.encode_utf16().collect(); + match_tokens(p, &units) + }) +} + +/// Split at the first five `:` written in the policy text, so the resource +/// field keeps its own `:` and a `:` from a variable value stays in its field. +fn split_fields(pattern: &[Token]) -> Vec<&[Token]> { + let mut fields = Vec::with_capacity(6); + let mut rest = pattern; + while fields.len() < 5 { + match rest.iter().position(|t| *t == Token::Colon) { + Some(i) => { + fields.push(&rest[..i]); + rest = &rest[i + 1..]; + } + None => break, + } + } + fields.push(rest); + fields +} + +/// Greedy wildcard match over tokens against the UTF-16 code units of a +/// value: the algorithm of `matcher::wildcard_match`, which counts what `?` +/// and `*` match in UTF-16 code units (#581), with `Star` and `Question` as +/// the wildcards. +fn match_tokens(p: &[Token], v: &[u16]) -> bool { + let (plen, vlen) = (p.len(), v.len()); + let mut pi = 0; + let mut vi = 0; + let mut last_star = usize::MAX; + let mut match_from = 0; + + while vi < vlen { + if pi < plen && p[pi] == Token::Star { + last_star = pi + 1; + match_from = vi; + pi += 1; + } else if pi < plen + && (p[pi] == Token::Question + || p[pi] == Token::Unit(v[vi]) + || (p[pi] == Token::Colon && v[vi] == COLON)) + { + pi += 1; + vi += 1; + } else if last_star != usize::MAX { + match_from += 1; + vi = match_from; + pi = last_star; + } else { + return false; + } + } + + while pi < plen && p[pi] == Token::Star { + pi += 1; + } + pi == plen +} + +#[cfg(test)] +mod tests; diff --git a/crates/auth/src/policy/variables/tests.rs b/crates/auth/src/policy/variables/tests.rs new file mode 100644 index 000000000..f45e0a474 --- /dev/null +++ b/crates/auth/src/policy/variables/tests.rs @@ -0,0 +1,533 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! Unit tests for policy variable parsing, expansion and token matching. + +use super::*; +use std::collections::HashMap; + +struct Ctx(HashMap>); + +impl Ctx { + fn empty() -> Self { + Self(HashMap::new()) + } + + fn with(mut self, key: &str, values: &[&str]) -> Self { + self.0.insert( + key.to_owned(), + values.iter().map(|v| (*v).to_owned()).collect(), + ); + self + } +} + +impl ConditionContext for Ctx { + fn resolve_key(&self, key: &str) -> Option> { + self.0 + .get(key) + .map(|v| v.iter().map(String::as_str).collect()) + } + + fn is_multivalued_key(&self, _key: &str) -> bool { + false + } +} + +const TEAM: &str = "aws:PrincipalTag/team"; +const BLUE: &str = "arn:aws:dynamodb:us-east-1:123456789012:table/blue"; + +fn team_blue() -> Ctx { + Ctx::empty().with(TEAM, &["blue"]) +} + +// --- PolicyDocument::expands_policy_variables --- + +#[test] +fn only_version_2012_expands_variables() { + let stmt = r#""Statement":[{"Effect":"Allow","Action":"dynamodb:*","Resource":"*"}]"#; + let v2012 = + PolicyDocument::from_json(&format!(r#"{{"Version":"2012-10-17",{stmt}}}"#)).unwrap(); + assert!(v2012.expands_policy_variables()); + let v2008 = + PolicyDocument::from_json(&format!(r#"{{"Version":"2008-10-17",{stmt}}}"#)).unwrap(); + assert_eq!(v2008.version.as_deref(), Some("2008-10-17")); + assert!(!v2008.expands_policy_variables()); + let none = PolicyDocument::from_json(&format!("{{{stmt}}}")).unwrap(); + assert_eq!(none.version, None); + assert!(!none.expands_policy_variables()); +} + +#[test] +fn a_write_refuses_a_version_that_is_not_one_of_the_two_values() { + // The rule and its message are ExtendDB's own. Whether IAM refuses these + // values at write, and with what message, was not measured. + let stmt = r#""Statement":[{"Effect":"Allow","Action":"dynamodb:*","Resource":"*"}]"#; + for version in [ + r#""2012-10-18""#, + r#""2012-10-17 ""#, + "20121017", + "null", + r#"["2012-10-17"]"#, + r#""2012-10-17T""#, + ] { + let json = format!(r#"{{"Version":{version},{stmt}}}"#); + let refused = PolicyDocument::from_json_for_write(&json) + .err() + .map(|e| e.0); + let expected = + format!("invalid Version (must be the string 2012-10-17 or 2008-10-17): {version}"); + assert_eq!(refused, Some(expected), "{version}"); + // A stored document holding the value still parses, and does not expand. + let stored = PolicyDocument::from_json(&json).unwrap(); + assert!(!stored.expands_policy_variables(), "{version}"); + } + for version in ["2012-10-17", "2008-10-17"] { + let json = format!(r#"{{"Version":"{version}",{stmt}}}"#); + assert!( + PolicyDocument::from_json_for_write(&json).is_ok(), + "{version}" + ); + } + // A missing Version is refused by each write path before parsing. + assert!(PolicyDocument::from_json_for_write(&format!("{{{stmt}}}")).is_ok()); +} + +// --- parse_reference --- + +#[test] +fn parse_plain_key() { + assert_eq!( + parse_reference("aws:username"), + Reference::Key { + name: "aws:username", + default: None + } + ); +} + +#[test] +fn parse_default_form_with_and_without_space() { + for inner in [ + "aws:username, 'anon'", + "aws:username,'anon'", + " aws:username , 'anon' ", + ] { + assert_eq!( + parse_reference(inner), + Reference::Key { + name: "aws:username", + default: Some("anon") + }, + "{inner}" + ); + } +} + +/// Both forms trim the key name, and the default form trims before and after +/// the comma and before the closing brace; the text inside the quotes keeps +/// its spaces. Rows W1 to W8 and T4 of `evaluator/tests/variable_spelling.rs`. +#[test] +fn parse_trims_spaces_around_the_key_in_both_forms() { + let plain = Reference::Key { + name: "aws:username", + default: None, + }; + for inner in [" aws:username ", "aws:username ", " aws:username"] { + assert_eq!(parse_reference(inner), plain, "{inner:?}"); + } + let defaulted = Reference::Key { + name: "aws:username", + default: Some("anon"), + }; + for inner in [ + " aws:username , 'anon'", + "aws:username ,'anon'", + "aws:username, 'anon'", + "aws:username, 'anon' ", + ] { + assert_eq!(parse_reference(inner), defaulted, "{inner:?}"); + } + assert_eq!( + parse_reference("aws:username, ' anon '"), + Reference::Key { + name: "aws:username", + default: Some(" anon ") + } + ); +} + +/// The code points up to U+0020 are trimmed and no others (rows T1 to T11 +/// of `evaluator/tests/variable_spelling.rs`). +#[test] +fn parse_trims_code_points_up_to_space_and_no_others() { + for c in [' ', '\t', '\n', '\u{b}', '\u{c}', '\r', '\u{1}', '\u{1f}'] { + let inner = format!("{c}aws:username{c}"); + assert_eq!( + parse_reference(&inner), + Reference::Key { + name: "aws:username", + default: None + }, + "{c:?}" + ); + } + for c in ['\u{85}', '\u{a0}', '\u{3000}'] { + let inner = format!("{c}aws:username{c}"); + assert_eq!( + parse_reference(&inner), + Reference::Key { + name: &inner, + default: None + }, + "{c:?}" + ); + } +} + +/// `${ * }` and `${ $ }` are the escapes (rows X1 and X2); `${ ? }` follows. +#[test] +fn parse_trims_the_escapes() { + assert_eq!(parse_reference(" * "), Reference::Escape(b'*')); + assert_eq!(parse_reference(" ? "), Reference::Escape(b'?')); + assert_eq!(parse_reference("\t$\t"), Reference::Escape(b'$')); +} + +#[test] +fn parse_comma_without_quotes_is_key_text() { + assert_eq!( + parse_reference("a, b"), + Reference::Key { + name: "a, b", + default: None + } + ); +} + +#[test] +fn parse_escapes() { + assert_eq!(parse_reference("*"), Reference::Escape(b'*')); + assert_eq!(parse_reference("?"), Reference::Escape(b'?')); + assert_eq!(parse_reference("$"), Reference::Escape(b'$')); +} + +// --- resource_pattern_matches --- + +#[test] +fn no_variable_is_arn_match() { + let ctx = Ctx::empty(); + assert!(resource_pattern_matches( + "arn:aws:dynamodb:*:*:table/bl*", + BLUE, + true, + &ctx + )); + assert!(resource_pattern_matches("*", BLUE, true, &ctx)); + assert!(!resource_pattern_matches( + "arn:aws:dynamodb:*:*:table/red", + BLUE, + true, + &ctx + )); +} + +#[test] +fn variable_expands_when_enabled_only() { + let pattern = "arn:aws:dynamodb:us-east-1:123456789012:table/${aws:PrincipalTag/team}"; + assert!(resource_pattern_matches(pattern, BLUE, true, &team_blue())); + assert!(!resource_pattern_matches( + pattern, + BLUE, + false, + &team_blue() + )); +} + +#[test] +fn unresolved_variable_matches_nothing() { + let pattern = "arn:aws:dynamodb:us-east-1:123456789012:table/${aws:PrincipalTag/team}"; + assert!(!resource_pattern_matches( + pattern, + BLUE, + true, + &Ctx::empty() + )); + assert!(!resource_pattern_matches( + "${aws:PrincipalTag/team}*", + BLUE, + true, + &Ctx::empty() + )); +} + +#[test] +fn default_used_when_key_absent() { + let pattern = "arn:aws:dynamodb:us-east-1:123456789012:table/${aws:PrincipalTag/team, 'blue'}"; + assert!(resource_pattern_matches(pattern, BLUE, true, &Ctx::empty())); + let red = Ctx::empty().with(TEAM, &["red"]); + assert!(!resource_pattern_matches(pattern, BLUE, true, &red)); +} + +#[test] +fn wildcard_after_variable_still_matches() { + let pattern = "arn:aws:dynamodb:us-east-1:123456789012:table/${aws:PrincipalTag/team}*"; + let blue_x = "arn:aws:dynamodb:us-east-1:123456789012:table/blue-x"; + assert!(resource_pattern_matches( + pattern, + blue_x, + true, + &team_blue() + )); + assert!(!resource_pattern_matches( + pattern, + "arn:aws:dynamodb:us-east-1:123456789012:table/red", + true, + &team_blue() + )); +} + +#[test] +fn wildcard_inside_variable_value_is_literal() { + let pattern = "arn:aws:dynamodb:us-east-1:123456789012:table/${aws:PrincipalTag/team}"; + let star = Ctx::empty().with(TEAM, &["*"]); + assert!(!resource_pattern_matches(pattern, BLUE, true, &star)); + assert!(resource_pattern_matches( + pattern, + "arn:aws:dynamodb:us-east-1:123456789012:table/*", + true, + &star + )); +} + +#[test] +fn escapes_are_literal_characters() { + let ctx = Ctx::empty(); + let star = "arn:aws:dynamodb:us-east-1:123456789012:table/${*}"; + assert!(!resource_pattern_matches(star, BLUE, true, &ctx)); + assert!(resource_pattern_matches( + star, + "arn:aws:dynamodb:us-east-1:123456789012:table/*", + true, + &ctx + )); + let question = "arn:aws:dynamodb:us-east-1:123456789012:table/${?}"; + assert!(!resource_pattern_matches( + question, + "arn:aws:dynamodb:us-east-1:123456789012:table/x", + true, + &ctx + )); + assert!(resource_pattern_matches( + question, + "arn:aws:dynamodb:us-east-1:123456789012:table/?", + true, + &ctx + )); + let dollar = "arn:aws:dynamodb:us-east-1:123456789012:table/${$}x"; + assert!(resource_pattern_matches( + dollar, + "arn:aws:dynamodb:us-east-1:123456789012:table/$x", + true, + &ctx + )); +} + +#[test] +fn a_present_single_valued_key_with_several_values_uses_the_first() { + // The request context lists one value for a single-valued key; a second + // value can only come from a test context. The first is used, as before. + let pattern = "arn:aws:dynamodb:us-east-1:123456789012:table/${aws:PrincipalTag/team}"; + let ctx = Ctx::empty().with(TEAM, &["blue", "red"]); + assert!(resource_pattern_matches(pattern, BLUE, true, &ctx)); +} + +#[test] +fn present_key_with_no_values_expands_empty() { + let pattern = "arn:aws:dynamodb:us-east-1:123456789012:table/${aws:PrincipalTag/team}"; + let ctx = Ctx::empty().with(TEAM, &[]); + assert!(resource_pattern_matches( + pattern, + "arn:aws:dynamodb:us-east-1:123456789012:table/", + true, + &ctx + )); + assert!(!resource_pattern_matches(pattern, BLUE, true, &ctx)); +} + +#[test] +fn field_count_rule_holds_after_expansion() { + let pattern = "arn:aws:dynamodb:${aws:RequestedRegion}:*:table/blue"; + let ctx = Ctx::empty().with("aws:RequestedRegion", &["us-east-1"]); + assert!(resource_pattern_matches(pattern, BLUE, true, &ctx)); + assert!(!resource_pattern_matches( + pattern, + "arn:aws:dynamodb:us-east-1:table/blue", + true, + &ctx + )); +} + +// --- `?` and `*` count UTF-16 code units, as `matcher::wildcard_match` does +// (#581) --- + +const TABLE: &str = "arn:aws:dynamodb:us-east-1:123456789012:table/"; + +/// Whether `table/` followed by a variable that expands to nothing +/// matches `table/`. The variable puts the pattern on the token path; +/// the answer must be the one `arn_match` gives for `table/`. +fn matches_beside_an_empty_variable(pattern: &str, name: &str) -> bool { + let arn = format!("{TABLE}{name}"); + let with_variable = format!("{TABLE}{pattern}${{aws:PrincipalTag/none, ''}}"); + let answer = resource_pattern_matches(&with_variable, &arn, true, &Ctx::empty()); + assert_eq!( + answer, + arn_match(&format!("{TABLE}{pattern}"), &arn), + "{pattern} against {name}" + ); + answer +} + +/// The `Resource` rows of `matcher/utf16_tests.rs`, measured with the IAM +/// policy simulator: `?` takes the one-unit `\u{e9}` and `\u{65e5}`, and the +/// two-unit `\u{1f600}` takes `??`. The `?zz` cases follow from them. +#[test] +fn question_mark_takes_one_utf16_unit_beside_a_variable() { + assert!(matches_beside_an_empty_variable("?", "\u{e9}")); + assert!(!matches_beside_an_empty_variable("??", "\u{e9}")); + assert!(matches_beside_an_empty_variable("?", "\u{65e5}")); + assert!(!matches_beside_an_empty_variable("??", "\u{65e5}")); + assert!(!matches_beside_an_empty_variable("?", "\u{1f600}")); + assert!(matches_beside_an_empty_variable("??", "\u{1f600}")); + assert!(!matches_beside_an_empty_variable("????", "\u{1f600}")); + assert!(matches_beside_an_empty_variable("?zz", "\u{e9}zz")); + assert!(matches_beside_an_empty_variable("?zz", "\u{65e5}zz")); +} + +/// Measured with the IAM policy simulator: `*??bc` does not match +/// `\u{65e5}bc` and matches `\u{1f600}bc`, and `*?bc` matches `\u{1f600}bc`. +#[test] +fn star_backtracks_by_utf16_units_beside_a_variable() { + assert!(!matches_beside_an_empty_variable("*??bc", "\u{65e5}bc")); + assert!(matches_beside_an_empty_variable("*??bc", "\u{1f600}bc")); + assert!(matches_beside_an_empty_variable("*?bc", "\u{1f600}bc")); +} + +#[test] +fn a_substituted_value_is_matched_by_utf16_units() { + // The value is literal, and a `?` written after it takes one unit. + let ctx = Ctx::empty().with(TEAM, &["\u{65e5}"]); + let matches = |suffix: &str, name: &str| { + let pattern = format!("{TABLE}${{aws:PrincipalTag/team}}{suffix}"); + resource_pattern_matches(&pattern, &format!("{TABLE}{name}"), true, &ctx) + }; + assert!(matches("?", "\u{65e5}\u{e9}")); + assert!(!matches("?", "\u{65e5}\u{1f600}")); + assert!(matches("??", "\u{65e5}\u{1f600}")); + assert!(!matches("?", "\u{65e5}")); + assert!(!matches("?", "\u{65e6}x")); +} + +#[test] +fn unclosed_brace_is_literal_text() { + let pattern = "arn:aws:dynamodb:us-east-1:123456789012:table/${aws"; + assert!(resource_pattern_matches( + pattern, + "arn:aws:dynamodb:us-east-1:123456789012:table/${aws", + true, + &Ctx::empty() + )); + assert!(!resource_pattern_matches( + pattern, + BLUE, + true, + &Ctx::empty() + )); +} + +// --- expand_condition_value --- + +#[test] +fn condition_value_expands_variable_and_default() { + let ctx = team_blue(); + assert_eq!( + expand_condition_value("team-${aws:PrincipalTag/team}", true, &ctx), + "team-blue" + ); + assert_eq!( + expand_condition_value("${aws:username, 'anon'}", true, &ctx), + "anon" + ); + assert_eq!( + expand_condition_value("${aws:PrincipalTag/team, 'none'}", true, &ctx), + "blue" + ); +} + +#[test] +fn condition_value_leaves_unresolved_and_escapes_literal() { + let ctx = Ctx::empty(); + assert_eq!( + expand_condition_value("${aws:username}", true, &ctx), + "${aws:username}" + ); + assert_eq!( + expand_condition_value("a${*}b${?}c${$}", true, &ctx), + "a${*}b${?}c${$}" + ); + assert_eq!(expand_condition_value("plain", true, &ctx), "plain"); + assert_eq!(expand_condition_value("${open", true, &ctx), "${open"); +} + +#[test] +fn variable_key_names_strip_the_default_and_skip_escapes() { + let names: Vec<&str> = variable_key_names( + "${aws:PrincipalTag/team}-${aws:RequestTag/env, 'x'}${*}${aws:username,'y'}${open", + true, + ) + .collect(); + assert_eq!( + names, + [ + "aws:PrincipalTag/team", + "aws:RequestTag/env", + "aws:username" + ] + ); + // A comma whose right side is not single-quoted is part of the name, as + // `expand_condition_value` reads it. + let names: Vec<&str> = variable_key_names("${a, b}", true).collect(); + assert_eq!(names, ["a, b"]); + assert_eq!(variable_key_names("plain", true).count(), 0); +} + +/// Without `iam_syntax`, as in a `2008-10-17` document or under an operator +/// other than a string or ARN one, the text between the braces is the key +/// name as written: the default form and a trimmed key are not read, and +/// `${key}` expands as it did before the default form was read. +#[test] +fn without_iam_syntax_the_text_between_the_braces_is_the_key_name() { + let ctx = Ctx::empty().with("aws:username", &["alice"]); + for value in [ + "${aws:username, 'anon'}", + "${aws:PrincipalTag/team, 'none'}", + "${ aws:username }", + ] { + assert_eq!(expand_condition_value(value, false, &ctx), value); + } + assert_eq!( + expand_condition_value("u-${aws:username}", false, &ctx), + "u-alice" + ); + let names: Vec<&str> = + variable_key_names("${aws:RequestTag/env, 'x'}${ aws:username }", false).collect(); + assert_eq!(names, ["aws:RequestTag/env, 'x'", " aws:username "]); +} + +#[test] +fn condition_value_present_key_without_values_is_empty() { + let ctx = Ctx::empty().with(TEAM, &[]); + assert_eq!( + expand_condition_value("x${aws:PrincipalTag/team}y", true, &ctx), + "xy" + ); +} diff --git a/crates/server/src/management/assume_role.rs b/crates/server/src/management/assume_role.rs index 0be4b5295..e3495acd1 100755 --- a/crates/server/src/management/assume_role.rs +++ b/crates/server/src/management/assume_role.rs @@ -375,7 +375,22 @@ async fn evaluate_trust_policy( epoch_time, }; - trust_decision(&statements, caller_arn, &context, has_session_tags) + trust_decision_for( + &statements, + caller_arn, + &context, + has_session_tags, + trust_policy_is_version_2012(trust_policy), + ) +} + +/// Whether a trust policy's `Version` is `2012-10-17`. Its condition values +/// then read the `${key, 'default'}` form and trim the key name, as an +/// identity policy's do (`condition::evaluate_condition_with`); in a +/// `2008-10-17` trust policy, or one with no `Version`, the text between the +/// braces is the key name as written. +fn trust_policy_is_version_2012(trust_policy: &Value) -> bool { + trust_policy.get("Version").and_then(Value::as_str) == Some("2012-10-17") } /// The `aws:PrincipalType` for a caller ARN: `User` for an IAM user ARN, @@ -410,17 +425,31 @@ fn statements_vec(trust_policy: &Value) -> Option> { } } +/// [`trust_decision_for`] for a `2012-10-17` trust policy. The unit tests' +/// statements stand for such a document. +#[cfg(test)] +fn trust_decision( + statements: &[Value], + caller_arn: &str, + context: &AssumeRoleContext, + has_session_tags: bool, +) -> bool { + trust_decision_for(statements, caller_arn, context, has_session_tags, true) +} + /// Decide assumption from resolved statements and context. Split out so the /// per-action Deny-over-Allow ordering is unit-testable without a store. -fn trust_decision( +/// `version_2012` is [`trust_policy_is_version_2012`] for the trust policy. +fn trust_decision_for( statements: &[Value], caller_arn: &str, context: &AssumeRoleContext, has_session_tags: bool, + version_2012: bool, ) -> bool { requested_actions(has_session_tags) .iter() - .all(|action| action_permitted(statements, caller_arn, context, action)) + .all(|action| action_permitted(statements, caller_arn, context, action, version_2012)) } /// The STS actions an `AssumeRole` request exercises. @@ -440,17 +469,18 @@ fn action_permitted( caller_arn: &str, context: &AssumeRoleContext, action: &str, + version_2012: bool, ) -> bool { for stmt in statements { if effect_of(stmt).eq_ignore_ascii_case("Deny") - && deny_statement_blocks(stmt, caller_arn, context, action) + && deny_statement_blocks(stmt, caller_arn, context, action, version_2012) { return false; } } for stmt in statements { if effect_of(stmt).eq_ignore_ascii_case("Allow") - && allow_statement_grants(stmt, caller_arn, context, action) + && allow_statement_grants(stmt, caller_arn, context, action, version_2012) { return true; } @@ -471,12 +501,13 @@ fn deny_statement_blocks( caller_arn: &str, context: &AssumeRoleContext, action: &str, + version_2012: bool, ) -> bool { if !principal_matches(&stmt["Principal"], caller_arn) || !action_matches(stmt, action) { return false; } matches!( - evaluate_conditions(stmt, context), + evaluate_conditions(stmt, context, version_2012), ConditionOutcome::Pass | ConditionOutcome::Unparseable ) } @@ -488,11 +519,15 @@ fn allow_statement_grants( caller_arn: &str, context: &AssumeRoleContext, action: &str, + version_2012: bool, ) -> bool { if !principal_matches(&stmt["Principal"], caller_arn) || !action_matches(stmt, action) { return false; } - matches!(evaluate_conditions(stmt, context), ConditionOutcome::Pass) + matches!( + evaluate_conditions(stmt, context, version_2012), + ConditionOutcome::Pass + ) } /// Outcome of a statement's `Condition` block for a given context. @@ -504,11 +539,16 @@ enum ConditionOutcome { /// Evaluate a statement's `Condition` block. Scalar condition values (numbers, /// booleans) are coerced to strings, matching IAM. An unknown operator or a -/// structurally invalid block is reported as `Unparseable`. -fn evaluate_conditions(stmt: &Value, context: &AssumeRoleContext) -> ConditionOutcome { +/// structurally invalid block is reported as `Unparseable`. `version_2012` is +/// whether the trust policy's `Version` is `2012-10-17`. +fn evaluate_conditions( + stmt: &Value, + context: &AssumeRoleContext, + version_2012: bool, +) -> ConditionOutcome { let normalized = normalize_condition_scalars(&stmt["Condition"]); match parse_conditions(&normalized) { - Ok(conds) => match evaluate_statement_conditions(&conds, context) { + Ok(conds) => match evaluate_statement_conditions(&conds, version_2012, context) { ConditionMatch::Match => ConditionOutcome::Pass, ConditionMatch::NoMatch => ConditionOutcome::Fail, // A condition on a key STS populates but ExtendDB cannot resolve @@ -1911,4 +1951,51 @@ mod tests { assert_eq!(caller_principal_type(root), None); assert_eq!(caller_username(root), None); } + + /// Decide `sts:AssumeRole` for alice under the whole trust policy `doc`, + /// with the `Version` it carries. + fn trust_document_allows(doc: &Value, context: &AssumeRoleContext) -> bool { + let statements = statements_vec(doc).unwrap(); + trust_decision_for( + &statements, + CALLER, + context, + false, + trust_policy_is_version_2012(doc), + ) + } + + fn trust_document(version: Option<&str>, statements: &[Value]) -> Value { + let mut doc = json!({"Statement": statements}); + if let Some(version) = version { + doc["Version"] = json!(version); + } + doc + } + + #[test] + fn trust_policy_reads_the_default_form_only_with_version_2012() { + // The identity policy rows V1 to V4 (IAM policy simulator, + // 2026-11-03) applied to a trust policy: outside a 2012-10-17 + // document `${aws:username, 'x'}` is compared as written. + let gate = |effect: &str, operator: &str| { + json!({"Effect": effect, "Principal": {"AWS": CALLER}, + "Action": "sts:AssumeRole", + "Condition": {operator: {"aws:PrincipalTag/team": "${aws:username, 'x'}"}}}) + }; + let alice = ctx_with_tags(&[("team", "alice")]); + for version in [None, Some("2008-10-17")] { + let allow = trust_document(version, &[gate("Allow", "StringEquals")]); + assert!(!trust_document_allows(&allow, &alice), "{version:?}"); + let deny = trust_document(version, &[allow_alice(), gate("Deny", "StringNotEquals")]); + assert!(!trust_document_allows(&deny, &alice), "{version:?}"); + } + let allow = trust_document(Some("2012-10-17"), &[gate("Allow", "StringEquals")]); + assert!(trust_document_allows(&allow, &alice)); + let deny = trust_document( + Some("2012-10-17"), + &[allow_alice(), gate("Deny", "StringNotEquals")], + ); + assert!(trust_document_allows(&deny, &alice)); + } } diff --git a/docs/backlog.md b/docs/backlog.md index d335df5a3..b2207b03b 100755 --- a/docs/backlog.md +++ b/docs/backlog.md @@ -8,7 +8,7 @@ Refreshed: v0.0.118 (P115) - ⬜ **Tagging rate limiting** — extenddb should implement `LimitExceededException` for rapid tag operations to match real DynamoDB behavior. 5 tagging tests fail against real DynamoDB due to this. (P114 follow-up) - ⬜ **Key-vs-item size gap** — batch/transact delete/update WCU uses key size, not old item size. Minor fidelity gap. - ⬜ **`f64` equality in `NumericEquals`/`NumericNotEquals`** — `policy/condition.rs` compares parsed numeric condition values with `f64` equality, violating the numeric-safety rule (and clippy `float_cmp`). Verify against AWS numeric-condition semantics (AWS treats these as arbitrary-precision decimals) and switch to `BigDecimal`/integer comparison. (BR-7085 follow-up) -- ⬜ **Non-`2012-10-17` policy `Version` accepted with warning** — extenddb accepts policy documents with an unrecognized `Version` string and logs a warning; real AWS rejects them with a `MalformedPolicyDocument` error. Confirm intended behavior and align. (BR-7085 follow-up) +- ⬜ **Policy `Version` refused at write, not compared with IAM**: the identity-policy, permissions-boundary and console writes refuse a `Version` other than the string `2012-10-17` or `2008-10-17`. Not yet compared with the service: whether IAM refuses such a document at write and with what message, and what STS does with one in an `AssumeRole` session policy, where ExtendDB checks only that the member exists. (BR-7085 follow-up) ## Test Gaps @@ -40,7 +40,7 @@ Refreshed: v0.0.118 (P115) ## Standing Items (need human decision) - ⬜ **22 unapproved license dependencies** — Unicode-3.0, CDLA-Permissive-2.0, MPL-2.0. All pre-existing. Human approved as-is (P99 session). -- ⬜ **Policy-variable expansion spec/code/AWS divergence** — `05-component-auth.md` §6.5 and `01-requirements.md` (REQ-ABAC-006) state variable substitution is deferred/literal, but `condition.rs::expand_policy_variables` **is** implemented and applied to Condition values only, not Resource ARNs (`evaluator.rs::resource_matches`/`arn_match` do not expand). AWS expands in both. Three-way inconsistent (spec vs code vs AWS). Fail-closed (a `${var}` in a Resource ARN silently fails to match), so not a leak. Resolve by either (a) documenting the implemented condition-value expansion and marking resource-ARN expansion deferred, or (b) completing REQ-ABAC-006. Needs human direction. (BR-7085) +- ⬜ **Policy-variable gaps in condition values (#534)**: a condition value in a `2008-10-17` or no-`Version` document is still expanded, where IAM treats it as literal text, and `${*}` in a condition value is left as text, where IAM expands it to `*`. Variables in `Resource` and `NotResource` patterns expand as IAM does (PR #535), and `05-component-auth.md` and `01-requirements.md` (REQ-ABAC-006) describe the expansion. (BR-7085) ## Recently Completed diff --git a/docs/design/01-requirements.md b/docs/design/01-requirements.md index 394412bd2..c6e9b1294 100755 --- a/docs/design/01-requirements.md +++ b/docs/design/01-requirements.md @@ -26,7 +26,6 @@ extenddb (ExtendDB) is a standalone, stateless Rust application that provides a | Contributor Insights (DescribeContributorInsights, ListContributorInsights, UpdateContributorInsights) | Excluded — analytics feature, not core API | | Kinesis Streaming Destination (DescribeKinesisStreamingDestination, EnableKinesisStreamingDestination, DisableKinesisStreamingDestination, UpdateKinesisStreamingDestination) | Excluded — AWS-specific integration | | Resource Policies (GetResourcePolicy, PutResourcePolicy, DeleteResourcePolicy) | Deferred — resource-based policies are a future enhancement | -| IAM Policy Variables (`${aws:PrincipalTag/key}`, `${aws:username}`, etc.) | Deferred — variable substitution in Resource ARNs and Condition values is a future enhancement (see REQ-ABAC-006) | | Federated Role Assumption (AssumeRoleWithSAML, AssumeRoleWithWebIdentity) | Deferred — v1 supports basic AssumeRole only (see REQ-IDENT-008) | | Role Chaining (role assumes role) | Deferred — v1 requires CallerArn to be a user; SourceIdentity and TransitiveTagKeys are deferred alongside (see REQ-IDENT-009) | | Role MaxSessionDuration | Deferred — v1 accepts any DurationSeconds with no per-role cap (see REQ-IDENT-010) | @@ -326,7 +325,7 @@ extenddb implements a local IAM identity model that mirrors the structure of AWS - `dynamodb:FullTableScan` — restricts whether Scan operations are allowed (`Bool` condition). - `dynamodb:EnclosingOperation` — identifies the parent operation for batch/transact sub-operations. - REQ-ABAC-005: The policy engine must build a **request context** for each operation containing all applicable context keys. The request context is populated by the server middleware before policy evaluation. -- REQ-ABAC-006 (Deferred): **Policy variables** (`${aws:PrincipalTag/key}`, `${aws:username}`, etc.) in Resource ARNs and Condition values are not supported in v1. In AWS IAM, policy variables enable a single policy to grant access to resources that match the caller's own tags — e.g., `"Resource": "arn:aws:dynamodb:*:*:table/${aws:PrincipalTag/Team}-*"`. Without policy variables, each team or department requires a separate policy with hardcoded resource ARNs, which reduces the ABAC testing value. This is a high-priority enhancement for a future version. +- REQ-ABAC-006: **Policy variables** (`${aws:PrincipalTag/key}`, `${aws:username}`, etc.) in Resource and NotResource ARNs and in Condition values are expanded from the request context, as AWS IAM expands them, so that one policy can grant access to resources that match the caller's own tags, for example `"Resource": "arn:aws:dynamodb:*:*:table/${aws:PrincipalTag/Team}-*"`. Expansion in `Resource` and `NotResource` applies to `2012-10-17` documents only, as in IAM. Two gaps in condition values remain and are filed as #534. ### 4.5 Authorization — Caching diff --git a/docs/design/05-component-auth-v3.md b/docs/design/05-component-auth-v3.md index 7a920770b..e8ae9f7ba 100755 --- a/docs/design/05-component-auth-v3.md +++ b/docs/design/05-component-auth-v3.md @@ -266,7 +266,7 @@ When `extenddb init` creates the first admin user, it seeds a default policy. Wh } ``` -Note: the `${account_id}` and `${user_name}` in the default policy are literal substitutions performed at user creation time, not IAM policy variables. Policy variable support (REQ-ABAC-006) remains deferred. +Note: the `${account_id}` and `${user_name}` in the default policy are literal substitutions performed at user creation time, not IAM policy variables. IAM policy variables are expanded when a policy is evaluated (REQ-ABAC-006, `policy/variables.rs`). ## 6. Catalog Schema Changes for Multi-Account diff --git a/docs/design/05-component-auth.md b/docs/design/05-component-auth.md index 277826217..b9e6bc65d 100755 --- a/docs/design/05-component-auth.md +++ b/docs/design/05-component-auth.md @@ -501,7 +501,7 @@ pub enum ConditionOperator { } ``` -> **Deferred: Policy Variables.** AWS IAM supports variable substitution in `Resource` ARNs and `Condition` values — e.g., `"Resource": "arn:aws:dynamodb:*:*:table/${aws:PrincipalTag/Team}-*"` grants access to tables matching the caller's own team tag. extenddb v1 does not support policy variables; all Resource and Condition values are treated as literal strings (with wildcard matching where applicable). This means ABAC policies that use variables to generalize across principals must be written as separate per-principal policies. Policy variable support is tracked as REQ-ABAC-006. +> **Policy Variables.** AWS IAM supports variable substitution in `Resource` ARNs and `Condition` values, for example `"Resource": "arn:aws:dynamodb:*:*:table/${aws:PrincipalTag/Team}-*"`, which grants access to tables matching the caller's own team tag. extenddb expands `${key}` and `${key, 'default'}` in the `Resource` and `NotResource` patterns of a `2012-10-17` document and in condition values, where the default form is read only in a `2012-10-17` document and under a string or ARN operator (`policy/variables.rs`). In a `Resource` or `NotResource` pattern, text that comes from a value or a default is matched literally, so a value cannot add a wildcard, and a variable whose key the request does not carry, with no default, makes the pattern match nothing. With a default, the default is used: `table/${aws:PrincipalTag/team, 'none'}` matches `table/none` when the request has no `team` tag (measured with the IAM policy simulator, issue #533). The full rules are in the security model manual (`docs/manuals/10-security-model.md`); two gaps that remain in condition values are filed as #534. Policy variable support is tracked as REQ-ABAC-006. **JSON deserialization:** @@ -1353,7 +1353,7 @@ The EC2 instance (or IAM role) running extenddb in Mode 3 needs the following pe 3. **Service control policies (SCPs) are not evaluated.** SCPs are an Organizations-level feature. extenddb does not query Organizations APIs. -4. **Policy variable substitution is not supported.** Consistent with REQ-ABAC-006 deferral. Fetched policies containing `${aws:PrincipalTag/key}` will treat the variable syntax as a literal string. +4. **Policy variables follow the identity-policy rules, with the condition-value gaps of #534.** Fetched policies are evaluated by the same engine, so `${aws:PrincipalTag/key}` in a `Resource`, `NotResource` or condition value expands from the request (REQ-ABAC-006), and the two condition-value gaps filed as #534 apply to them as well. 5. **Cross-account access is not evaluated.** extenddb assumes all callers are in the same account. Cross-account role assumption and resource policies are not supported. diff --git a/docs/manuals/07-upgrade-manual.md b/docs/manuals/07-upgrade-manual.md index 04e3111a6..3fe989dfc 100755 --- a/docs/manuals/07-upgrade-manual.md +++ b/docs/manuals/07-upgrade-manual.md @@ -330,6 +330,87 @@ An earlier PostgreSQL or SQLite binary refuses to start against the migrated cat --- +### v0.1.15: IAM policy variables expand in `Resource` and `NotResource` + +**What changed.** In a `2012-10-17` policy document, a `${key}` variable in a +`Resource` or `NotResource` pattern is now expanded from the request before the +pattern is matched, as IAM does. Before this release the pattern was compared as +literal text and matched no table. Text from a variable value or a default is +matched literally, a variable whose key the request does not carry, with no +default, makes the pattern match nothing, and a `2008-10-17` document, or one +with no `Version`, still matches the pattern as written. With a default, the +default is used: `table/${aws:PrincipalTag/team, 'none'}` matches `table/none` +when the request has no `team` tag (measured with the IAM policy simulator, +issue #533). Four decisions change for stored policies: + +1. A pattern with a variable now matches the table it names. `Allow + table/${aws:username}` now grants the caller's own table, where it granted + nothing. `Deny table/${aws:PrincipalTag/team}` now denies the team's table, + where it denied nothing. `Deny NotResource table/${aws:PrincipalTag/team}` now + spares the team's table, where it denied every table, and an `Allow + NotResource` written that way no longer grants the team's table. +2. A condition value naming a multivalued key, such as `${dynamodb:LeadingKeys}` + or `${aws:TagKeys}` in a trust policy, is no longer expanded to the key's first + value; the text is compared as written. Under a positive operator such as + `StringEquals` the condition no longer matches, and under a negated operator + such as `StringNotEquals` it now matches. For example, an `Allow` with the + condition `StringNotEquals` on `aws:PrincipalTag/team` and the value + `${dynamodb:LeadingKeys}` now grants a caller tagged `team=blue` whose request + has the leading key `blue`, where it did not, and a `Deny` with that condition + now applies to the same request. +3. In a `2012-10-17` document, a condition value written as + `${key, 'default'}` under a string or ARN operator now expands to the + request's value for `key`, or to `default` when the request does not carry + `key`. Before, the whole text was compared as written. For example, an + `Allow` with the condition `StringEquals` on `aws:ResourceTag/owner` and the + value `${aws:username, 'anon'}` now grants `alice` a table tagged + `owner=alice`, and a caller with no user name a table tagged `owner=anon`; + before, it granted neither. In a `2008-10-17` document, in one with no + `Version`, and under the numeric, date, `Bool` and `Null` operators, the + text is still compared as written; IAM does not read the default form + there either (measured with the IAM policy simulator). A user, group or + role policy or boundary write refuses an `IpAddress` value written as a + variable as a malformed CIDR, as before. A role trust policy follows its + own `Version`. +4. Spaces, and the other code points up to U+0020, around the key name of a + variable are now ignored, in `Resource` and `NotResource` patterns and in the + condition values item 3 covers. An `Allow` with the condition `StringEquals` + on `aws:ResourceTag/owner` and the value `${ aws:username }` now grants + `alice` a table tagged `owner=alice`. Before, the spaces were part of the + key name, so the variable named a key no request carries, the value was + compared as written, and the `Allow` granted nothing. + +**Policy writes check `Version`.** Writing a user, group or role policy or a +permissions boundary, through the management API or the console, now refuses a +document whose `Version` is not the string `2012-10-17` or `2008-10-17`, for +example `"2012-10-18"` or the number `20121017`, with `Invalid policy document: +policy parse error: invalid Version ...`, or for a permissions boundary `Invalid +permissions boundary document: policy parse error: invalid Version ...`. A +document already stored with such a value keeps evaluating, with its `Resource` +patterns matched as written, but sending it again unchanged is refused with the +same message. A stored `Deny` whose `Resource` or `NotResource` holds a variable +in such a document keeps denying what the pattern as written matches, which for +`table/${aws:PrincipalTag/team}` is nothing, until its `Version` is changed to +`2012-10-17`. To find such documents before upgrading, list the stored policies +and boundaries with the commands under **Who is affected** below and check each +document's `Version`. A session policy passed to `AssumeRole` is checked only +for its `Version` and `Statement` members. + +**Who is affected.** Any deployment with a stored policy whose `Resource`, +`NotResource` or condition values contain `${`: inline user, group and role +policies, permissions boundaries and session policies, and, for the condition +value changes in items 2 to 4, role trust policies. Search the stored policies for `${` +(`extenddb manage list-user-policies`, `list-group-policies` and +`list-role-policies` print inline policies, `get-user-boundary` and +`get-role-boundary` permissions boundaries, and `list-roles` trust policies) and +check each match against the cases above before upgrading, in particular an +`Allow` that now grants (a `Resource` pattern with a variable, or a negated +condition whose value names a multivalued key), a `Deny` that now applies (the +same two forms), and a `Deny NotResource` that now spares a table. No catalog +migration is required. + +--- + ### v0.1.14 LeadingKeys now enforced on Query, batch and transaction operations **What changed.** `dynamodb:LeadingKeys` is now evaluated on `Query`, diff --git a/docs/manuals/10-security-model.md b/docs/manuals/10-security-model.md index bb7a1c75d..0c1ee414f 100755 --- a/docs/manuals/10-security-model.md +++ b/docs/manuals/10-security-model.md @@ -83,7 +83,7 @@ Policy sources collected for evaluation: - **Auth before JSON parse**: SigV4 signature verification runs before the request body is parsed. Invalid signatures are rejected with constant-time comparison before any business logic executes. - **Concurrent policy fetching**: Identity policies, group policies, and permissions boundaries are fetched concurrently from PostgreSQL. All must succeed for evaluation to proceed. - **Constant-time rejection for inactive keys**: Inactive or expired access keys are rejected without timing differences that could reveal key existence. -- **Policy document validation on write**: Policy documents are validated for JSON structure and size-capped (6,144 bytes) when attached via the management API. Invalid documents are rejected before storage. +- **Policy document validation on write**: Policy documents are validated for JSON structure and size-capped (6,144 bytes) when attached via the management API. Invalid documents are rejected before storage. A `Version` other than the string `2012-10-17` or `2008-10-17` is refused; only a `2012-10-17` document expands policy variables. - **Expression depth and token limits**: Expression parsing enforces configurable depth (default 150) and token limits (default 4,096) to prevent resource exhaustion. ### Supported Condition Operators @@ -117,7 +117,7 @@ Set operators and `IfExists` can be combined: `ForAllValues:StringEqualsIfExists | `dynamodb:FullTableScan` | Boolean | `true` for Scan operations | | `dynamodb:EnclosingOperation` | String | The enclosing operation for batch/transact sub-operations | -Policy variables (e.g., `${aws:PrincipalTag/Team}`) are expanded in condition values. +Policy variables (e.g., `${aws:PrincipalTag/Team}`) are expanded in condition values. In a `2012-10-17` document, under a string or ARN operator (with or without `ForAnyValue:`, `ForAllValues:` or `IfExists`), the `${key, 'default'}` form expands to `default` when the request does not carry `key`, and code points up to U+0020 around the key name are ignored. Elsewhere the text between the braces is read as the key name as written, so `${key, 'default'}` is compared as written; IAM does not read the default form there either. A role trust policy follows its own `Version`. A multivalued key such as `dynamodb:LeadingKeys` is not expanded. ### Access Control Patterns @@ -141,7 +141,7 @@ Resources are identified by ARN: `arn:aws:dynamodb:::table/< - `Effect`: Allow, Deny - `Action` / `NotAction`: DynamoDB actions (e.g., `dynamodb:PutItem`, `dynamodb:*`) -- `Resource` / `NotResource`: ARN patterns with wildcards +- `Resource` / `NotResource`: ARN patterns with wildcards. In a `2012-10-17` document, `${key}` policy variables in the pattern are expanded from the request before matching (for example `arn:aws:dynamodb:*:*:table/${aws:username}`); a wildcard written after a variable still matches; `${key, 'default'}` falls back to `default`; `${*}`, `${?}` and `${$}` stand for literal characters; a variable whose key the request does not carry, or that names a multivalued key such as `dynamodb:LeadingKeys` that the request carries, makes the pattern match nothing (a written default is used only when the key is absent); a `:` inside a substituted value does not separate ARN fields. A `2008-10-17` document, or one with no `Version`, matches the pattern as written, as IAM does. Code points up to U+0020 around the key name, around the comma of the default form and before the closing brace are ignored, as in IAM, and a space inside the quotes of a default is kept. A tag whose value is the empty string expands to no characters, as in IAM: `table/${aws:PrincipalTag/team}*` matches every table for a caller tagged `team=""`, while with no `team` tag it matches nothing. To refuse such a caller, add a condition such as `StringLike` on `aws:PrincipalTag/team` with the value `?*` to the `Allow` statement (measured with the IAM policy simulator). A `Deny NotResource` statement on such a pattern spares every table for that caller (also measured), and that condition on it does not change this. - `Condition`: Full condition block support (see operators and keys above) ## Transport Security diff --git a/tests/test_resource_policy_variables.py b/tests/test_resource_policy_variables.py new file mode 100644 index 000000000..27c13f653 --- /dev/null +++ b/tests/test_resource_policy_variables.py @@ -0,0 +1,256 @@ +# Copyright 2026 ExtendDB contributors +# SPDX-License-Identifier: Apache-2.0 + +"""Policy variables in `Resource` and `NotResource`, and the `Version` a policy +write accepts (issue #533). + +`TestResourceVariable` gives a user tagged `team=blue` an `Allow dynamodb:*` on +`*` and a `Deny dynamodb:GetItem` on `table/${aws:PrincipalTag/team}`, written as +`Resource` or as `NotResource`, and reads the tables `blue` and `red`. The expected +answers repeat four rows measured with `iam simulate-custom-policy` on 2026-10-24 +(PR #535 body table): under `Resource` the read of `blue` is denied and the read +of `red` is allowed, and under `NotResource` the reverse. + +`TestVersionRefused` writes documents whose `Version` is not the string +`2012-10-17` or `2008-10-17` through the management API (user, group and role +policies, user and role permissions boundaries) and the console (user, group and +role policies). IAM refuses such a document: Access Analyzer reports +`INVALID_VERSION` and the policy simulator answers `InvalidInput` (measured +2026-10-28). The messages are ExtendDB's own. + +Both classes provision builtin IAM principals through the management API, so they +run against ExtendDB only. The dual-target harness has no way to create IAM users +on the service, and the expectations come from the measurements above. + +Prerequisites: + - extenddb running with `auth.provider = "builtin"` on EXTENDDB_TEST_ENDPOINT + - Admin credentials in EXTENDDB_ADMIN_USER / EXTENDDB_ADMIN_PASSWORD + +Run: + EXTENDDB_TEST_ENDPOINT=https://127.0.0.1:18443 \ + EXTENDDB_ADMIN_USER=admin \ + EXTENDDB_ADMIN_PASSWORD= \ + pytest tests/test_resource_policy_variables.py -v +""" + +from __future__ import annotations + +import json +import os +import uuid + +import pytest +from botocore.exceptions import ClientError + +from conftest import wait_for_active +from management_helpers import ManagementClient +from test_console_integration import ConsoleClient +from test_tag_request_conditions import ( + ALLOW_ALL, + _error_code, + _make_client, + _require_auth_env, + _user_with_key, +) + +TEAM_VARIABLE = "${aws:PrincipalTag/team}" + +# Each refused value, as JSON, and the text the refusal ends with. +REFUSED_VERSIONS = [ + pytest.param("2012-10-18", '"2012-10-18"', id="2012-10-18"), + pytest.param("2012-10-17 ", '"2012-10-17 "', id="trailing-space"), + pytest.param(20121017, "20121017", id="number"), +] + +VERSION_RULE = "invalid Version (must be the string 2012-10-17 or 2008-10-17)" + + +def _document(version) -> dict: + return { + "Version": version, + "Statement": [{"Effect": "Allow", "Action": "dynamodb:GetItem", "Resource": "*"}], + } + + +@pytest.fixture(scope="module") +def auth_env(): + return _require_auth_env() + + +@pytest.fixture(scope="module") +def mgmt(auth_env) -> ManagementClient: + endpoint, admin_user, admin_pass = auth_env + return ManagementClient(endpoint, admin_user, admin_pass) + + +@pytest.fixture(scope="module") +def region() -> str: + return os.environ.get("AWS_DEFAULT_REGION", "us-east-1") + + +@pytest.fixture(scope="module") +def account_id(mgmt) -> str: + acct_id = f"{uuid.uuid4().int % 10**12:012d}" + resp = mgmt.create_account(acct_id, f"resvar-{acct_id}") + assert resp.status_code == 201, resp.text + yield acct_id + mgmt.delete_account(acct_id) + + +class TestResourceVariable: + """`table/${aws:PrincipalTag/team}` names the caller's team table.""" + + @pytest.fixture(scope="class") + def tables(self, auth_env, mgmt, account_id, region): + name = f"adm-{uuid.uuid4().hex[:8]}" + ak, sk = _user_with_key(mgmt, account_id, name) + resp = mgmt.put_user_policy(account_id, name, "full", ALLOW_ALL) + assert resp.status_code == 204, resp.text + admin = _make_client(auth_env[0], ak, sk, region) + # The account is new, so the two names cannot collide with other tests. + for table in ("blue", "red"): + admin.create_table( + TableName=table, + AttributeDefinitions=[{"AttributeName": "pk", "AttributeType": "S"}], + KeySchema=[{"AttributeName": "pk", "KeyType": "HASH"}], + BillingMode="PAY_PER_REQUEST", + ) + wait_for_active(admin, table) + yield + for table in ("blue", "red"): + try: + admin.delete_table(TableName=table) + except ClientError: + pass + mgmt.delete_user(account_id, name) + + @pytest.mark.parametrize( + "element, denied, allowed", + [("Resource", "blue", "red"), ("NotResource", "red", "blue")], + ) + def test_deny_on_the_team_variable( + self, tables, auth_env, mgmt, account_id, region, element, denied, allowed + ): + name = f"u-{uuid.uuid4().hex[:8]}" + ak, sk = _user_with_key(mgmt, account_id, name) + resp = mgmt.tag_user(account_id, name, {"team": "blue"}) + assert resp.status_code in (200, 204), resp.text + pattern = f"arn:aws:dynamodb:{region}:{account_id}:table/" + TEAM_VARIABLE + policy = { + "Version": "2012-10-17", + "Statement": [ + {"Effect": "Allow", "Action": "dynamodb:*", "Resource": "*"}, + {"Effect": "Deny", "Action": "dynamodb:GetItem", element: pattern}, + ], + } + resp = mgmt.put_user_policy(account_id, name, "team", policy) + assert resp.status_code == 204, resp.text + client = _make_client(auth_env[0], ak, sk, region) + try: + with pytest.raises(ClientError) as exc: + client.get_item(TableName=denied, Key={"pk": {"S": "k"}}) + assert _error_code(exc.value) == "AccessDeniedException", exc.value.response + resp = client.get_item(TableName=allowed, Key={"pk": {"S": "k"}}) + assert "Item" not in resp + finally: + mgmt.delete_user(account_id, name) + + +class TestVersionRefused: + """A policy write refuses a `Version` other than the two IAM accepts.""" + + @pytest.fixture() + def user(self, mgmt, account_id): + name = f"vu-{uuid.uuid4().hex[:8]}" + resp = mgmt.create_user(account_id, name, "TestPass123!") + assert resp.status_code == 201, resp.text + yield name + mgmt.delete_user(account_id, name) + + @pytest.fixture() + def group(self, mgmt, account_id): + name = f"vg-{uuid.uuid4().hex[:8]}" + resp = mgmt.create_group(account_id, name) + assert resp.status_code == 201, resp.text + yield name + mgmt.delete_group(account_id, name) + + @pytest.fixture() + def role(self, mgmt, account_id): + name = f"vr-{uuid.uuid4().hex[:8]}" + trust = { + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Principal": {"AWS": f"arn:aws:iam::{account_id}:root"}, + "Action": "sts:AssumeRole", + }], + } + resp = mgmt.create_role(account_id, name, trust) + assert resp.status_code == 201, resp.text + yield name + mgmt.delete_role(account_id, name) + + @pytest.fixture() + def console(self, auth_env) -> ConsoleClient: + client = ConsoleClient(auth_env[0]) + resp = client.login(auth_env[1], auth_env[2]) + assert resp.status_code == 303, f"Login failed: {resp.status_code}" + return client + + @pytest.mark.parametrize("version, shown", REFUSED_VERSIONS) + @pytest.mark.parametrize("principal_type", ["user", "group", "role"]) + def test_management_api_policy_write( + self, request, mgmt, account_id, principal_type, version, shown + ): + principal = request.getfixturevalue(principal_type) + put = getattr(mgmt, f"put_{principal_type}_policy") + listing = getattr(mgmt, f"list_{principal_type}_policies") + resp = put(account_id, principal, "odd", _document(version)) + assert resp.status_code == 400, resp.text + assert resp.text == f"Invalid policy document: policy parse error: {VERSION_RULE}: {shown}" + names = [p["policy_name"] for p in listing(account_id, principal).json()] + assert "odd" not in names + # The same document with `Version` 2008-10-17 is stored. + resp = put(account_id, principal, "old", _document("2008-10-17")) + assert resp.status_code == 204, resp.text + + @pytest.mark.parametrize("version, shown", REFUSED_VERSIONS) + @pytest.mark.parametrize("principal_type", ["user", "role"]) + def test_management_api_boundary_write( + self, request, mgmt, account_id, principal_type, version, shown + ): + principal = request.getfixturevalue(principal_type) + put = getattr(mgmt, f"set_{principal_type}_boundary") + get = getattr(mgmt, f"get_{principal_type}_boundary") + resp = put(account_id, principal, _document(version)) + assert resp.status_code == 400, resp.text + assert resp.text == ( + f"Invalid permissions boundary document: policy parse error: {VERSION_RULE}: {shown}" + ) + assert get(account_id, principal).status_code == 404 + resp = put(account_id, principal, _document("2008-10-17")) + assert resp.status_code == 204, resp.text + + @pytest.mark.parametrize("version, shown", REFUSED_VERSIONS) + @pytest.mark.parametrize("principal_type", ["user", "group", "role"]) + def test_console_policy_write( + self, request, console, account_id, principal_type, version, shown + ): + principal = request.getfixturevalue(principal_type) + base = f"/accounts/{account_id}/{principal_type}s/{principal}" + resp = console.post( + f"{base}/policies/new", + {"policy_name": "odd", "policy_document": json.dumps(_document(version))}, + ) + # The form is shown again with the error instead of redirecting. + assert resp.status_code == 200 + assert f"Invalid policy document: policy parse error: {VERSION_RULE}" in resp.text + resp = console.get(base) + assert resp.status_code == 200 + assert "odd" not in resp.text + resp = console.post( + f"{base}/policies/new", + {"policy_name": "old", "policy_document": json.dumps(_document("2008-10-17"))}, + ) + assert resp.status_code == 303