From e7397ac621861424d9550fbbc978552c5300eb73 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Sat, 31 Oct 2026 00:45:49 +0000 Subject: [PATCH 01/29] fix(auth): expand policy variables in Resource and NotResource patterns In a 2012-10-17 document, every ${key} in a Resource or NotResource pattern is replaced by the request's value before ARN matching, with ${key, 'default'} falling back to its default and ${*}, ${?} and ${$} producing literal characters. Text substituted from a value, a default or an escape is matched literally; a wildcard written in the policy still matches. A variable whose key the request does not carry, with no default, makes the pattern match nothing. A 2008-10-17 document, or one with no Version, matches the pattern as written, so PolicyDocument now keeps an absent Version as None. Condition values read the ${key, 'default'} form through the same module. A multivalued key such as aws:TagKeys or dynamodb:LeadingKeys has no value, default or not, as main already treats it in a condition value: a condition value naming one stays as written, and a pattern naming one matches nothing. Measured with the IAM policy simulator. Closes #533. --- crates/auth/src/policy/condition.rs | 50 +--- crates/auth/src/policy/document.rs | 35 ++- crates/auth/src/policy/evaluator.rs | 40 ++- crates/auth/src/policy/mod.rs | 1 + crates/auth/src/policy/variables.rs | 268 +++++++++++++++++++ crates/auth/src/policy/variables/tests.rs | 305 ++++++++++++++++++++++ 6 files changed, 637 insertions(+), 62 deletions(-) create mode 100644 crates/auth/src/policy/variables.rs create mode 100644 crates/auth/src/policy/variables/tests.rs diff --git a/crates/auth/src/policy/condition.rs b/crates/auth/src/policy/condition.rs index 6164cce58..fd66ac9fe 100644 --- a/crates/auth/src/policy/condition.rs +++ b/crates/auth/src/policy/condition.rs @@ -252,52 +252,12 @@ 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). +/// Replaces `${variable}` and `${variable, 'default'}` patterns with their +/// resolved values from the context (see [`super::variables`]). Supported +/// variables: `aws:PrincipalTag/*` and any key the context can resolve. +/// Unresolvable variables with no default are left as-is (IAM behavior). 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 + super::variables::expand_condition_value(value, context) } /// Unwrap an `IfExists` wrapper if present. diff --git a/crates/auth/src/policy/document.rs b/crates/auth/src/policy/document.rs index dedcd5e2a..a5893169e 100755 --- a/crates/auth/src/policy/document.rs +++ b/crates/auth/src/policy/document.rs @@ -13,12 +13,26 @@ 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, + /// Policy version string (e.g., "2012-10-17"), `None` when the document + /// has no `Version`. + pub version: Option, /// The policy statements. pub statements: Vec, } +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") + } +} + /// A single policy statement. #[derive(Debug, Clone)] pub struct Statement { @@ -172,7 +186,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 { @@ -492,6 +506,18 @@ fn parse_string_or_array(value: &Value) -> Result, PolicyParseError> mod tests { use super::*; + #[test] + fn version_2008_or_absent_does_not_expand_variables() { + let stmt = r#""Statement":[{"Effect":"Allow","Action":"dynamodb:*","Resource":"*"}]"#; + 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 parse_simple_allow_policy() { let json = r#"{ @@ -503,7 +529,8 @@ 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!(doc.expands_policy_variables()); 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..049fb0737 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_matches; /// The result of policy evaluation. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -100,7 +101,7 @@ pub fn explicit_deny_matches_arc( policy.statements.iter().any(|stmt| { stmt.effect == Effect::Deny && action_matches(stmt, action) - && resource_matches(stmt, resource_arn) + && resource_matches(policy, stmt, resource_arn, context) && deny_conditions_apply(stmt, context) }) }) @@ -129,7 +130,7 @@ where for stmt in &policy.statements { if stmt.effect == Effect::Deny && action_matches(stmt, action) - && resource_matches(stmt, resource_arn) + && resource_matches(policy, stmt, resource_arn, context) && deny_conditions_apply(stmt, context) { return AuthzDecision::Deny; @@ -142,7 +143,7 @@ where let boundary_allows = boundary.statements.iter().any(|stmt| { stmt.effect == Effect::Allow && action_matches(stmt, action) - && resource_matches(stmt, resource_arn) + && resource_matches(boundary, stmt, resource_arn, context) && allow_conditions_grant(stmt, context) }); if !boundary_allows { @@ -155,7 +156,7 @@ where let session_allows = session.statements.iter().any(|stmt| { stmt.effect == Effect::Allow && action_matches(stmt, action) - && resource_matches(stmt, resource_arn) + && resource_matches(session, stmt, resource_arn, context) && allow_conditions_grant(stmt, context) }); if !session_allows { @@ -168,7 +169,7 @@ where for stmt in &policy.statements { if stmt.effect == Effect::Allow && action_matches(stmt, action) - && resource_matches(stmt, resource_arn) + && resource_matches(policy, stmt, resource_arn, context) && allow_conditions_grant(stmt, context) { return AuthzDecision::Allow; @@ -194,14 +195,27 @@ 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. +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)) + }; match &statement.resource_match { - ResourceMatch::Resources(patterns) => { - patterns.iter().any(|p| arn_match(p, request_resource)) - } - ResourceMatch::NotResources(patterns) => { - !patterns.iter().any(|p| arn_match(p, request_resource)) - } + ResourceMatch::Resources(patterns) => any_match(patterns), + ResourceMatch::NotResources(patterns) => !any_match(patterns), } } 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..f23f72a2b --- /dev/null +++ b/crates/auth/src/policy/variables.rs @@ -0,0 +1,268 @@ +// 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 +//! has no value, default or not: a set of values has no one value to +//! expand to. +//! - `${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). Only a document +//! whose `Version` is `2012-10-17` expands variables; the caller decides that +//! with [`super::document::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. + +use super::context::ConditionContext; +use super::matcher::arn_match; + +/// One unit of an expanded pattern. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Token { + /// `*` written in the policy text: matches zero or more bytes. + Star, + /// `?` written in the policy text: matches exactly one byte. + Question, + /// One byte matched literally. + Byte(u8), +} + +/// 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), +} + +/// Parse the text between `${` and `}`. +/// +/// `*`, `?` and `$` are escapes. `name, 'default'` (whitespace around either +/// part ignored) is the default form. Anything else, including a `,` whose +/// right side is not single-quoted, is taken as the key name as written, which +/// is what the condition path did before the default form was read. +fn parse_reference(inner: &str) -> Reference<'_> { + match inner { + "*" => return Reference::Escape(b'*'), + "?" => return Reference::Escape(b'?'), + "$" => return Reference::Escape(b'$'), + _ => {} + } + if let Some((name, default)) = inner.split_once(',') { + let default = default.trim(); + if default.len() >= 2 && default.starts_with('\'') && default.ends_with('\'') { + return Reference::Key { + name: name.trim(), + 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. +fn parts(value: &str) -> 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]; + out.push(Part::Ref(raw, parse_reference(&after_start[..end]))); + rest = &after_start[end + 1..]; + } + None => { + out.push(Part::Text(&rest[start..])); + rest = ""; + } + } + } + out.push(Part::Text(rest)); + out +} + +/// The request's value for `key`: the first value of a present key, the +/// default when the key is absent and a default was written, `None` when the +/// key is absent and no default was written. A present key with no values +/// expands to the empty string. A multivalued key is always `None`. +fn resolve<'a>( + name: &str, + default: Option<&'a str>, + context: &'a impl ConditionContext, +) -> Option<&'a str> { + if context.is_multivalued_key(name) { + return None; + } + match context.resolve_key(name) { + Some(values) => Some(values.first().copied().unwrap_or("")), + None => default, + } +} + +/// Expand `pattern` into tokens. Returns `None` when a variable names a key +/// the request does not carry and gives no default. +fn expand_tokens(pattern: &str, context: &impl ConditionContext) -> Option> { + let mut tokens = Vec::with_capacity(pattern.len()); + for part in parts(pattern) { + match part { + Part::Text(text) => tokens.extend(text.bytes().map(|b| match b { + b'*' => Token::Star, + b'?' => Token::Question, + other => Token::Byte(other), + })), + Part::Ref(_, Reference::Escape(byte)) => tokens.push(Token::Byte(byte)), + Part::Ref(_, Reference::Key { name, default }) => { + let value = resolve(name, default, context)?; + tokens.extend(value.bytes().map(Token::Byte)); + } + } + } + Some(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. +#[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); + } + match expand_tokens(pattern, context) { + Some(tokens) => arn_match_tokens(&tokens, resource_arn), + None => false, + } +} + +/// Expand variables in a condition value. +/// +/// `${key}` and `${key, 'default'}` are replaced by their value; a variable +/// whose key the request does not carry, with no default, is left as written, +/// and so are the `${*}`, `${?}` and `${$}` escapes, as before. +#[must_use] +pub fn expand_condition_value(value: &str, context: &impl ConditionContext) -> String { + if !value.contains("${") { + return value.to_owned(); + } + let mut result = String::with_capacity(value.len()); + for part in parts(value) { + 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)| match_tokens(p, v.as_bytes())) +} + +/// Split at the first five literal `:` bytes, so the resource field keeps its +/// own `:`. +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::Byte(b':')) { + Some(i) => { + fields.push(&rest[..i]); + rest = &rest[i + 1..]; + } + None => break, + } + } + fields.push(rest); + fields +} + +/// Greedy wildcard match over tokens, the same algorithm as +/// `matcher::wildcard_match` with `Star` and `Question` as the wildcards. +fn match_tokens(p: &[Token], v: &[u8]) -> 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::Byte(v[vi])) { + 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..e0f800c65 --- /dev/null +++ b/crates/auth/src/policy/variables/tests.rs @@ -0,0 +1,305 @@ +// 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"]) +} + +// --- 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}" + ); + } +} + +#[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 multivalued_key_uses_first_value() { + 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 + )); +} + +#[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}", &ctx), + "team-blue" + ); + assert_eq!( + expand_condition_value("${aws:username, 'anon'}", &ctx), + "anon" + ); + assert_eq!( + expand_condition_value("${aws:PrincipalTag/team, 'none'}", &ctx), + "blue" + ); +} + +#[test] +fn condition_value_leaves_unresolved_and_escapes_literal() { + let ctx = Ctx::empty(); + assert_eq!( + expand_condition_value("${aws:username}", &ctx), + "${aws:username}" + ); + assert_eq!( + expand_condition_value("a${*}b${?}c${$}", &ctx), + "a${*}b${?}c${$}" + ); + assert_eq!(expand_condition_value("plain", &ctx), "plain"); + assert_eq!(expand_condition_value("${open", &ctx), "${open"); +} + +#[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", &ctx), + "xy" + ); +} From 00eadf18c6f4c4d2741e0cbbc217fab38841740c Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Sat, 31 Oct 2026 00:45:49 +0000 Subject: [PATCH 02/29] test(auth): cover policy variables in Resource and NotResource at the evaluator One test per measured row of issue #533, plus the escapes, the default form in a condition value, the 2008-10-17 and no-Version documents for NotResource, the permissions boundary and session policy paths, and explicit_deny_matches_arc. PolicyDocument::expands_policy_variables moves beside the expansion code so document.rs does not grow. --- crates/auth/src/policy/document.rs | 26 -- crates/auth/src/policy/evaluator/tests.rs | 1 + .../evaluator/tests/resource_variables.rs | 285 ++++++++++++++++++ crates/auth/src/policy/variables.rs | 16 +- crates/auth/src/policy/variables/tests.rs | 17 ++ 5 files changed, 318 insertions(+), 27 deletions(-) create mode 100644 crates/auth/src/policy/evaluator/tests/resource_variables.rs diff --git a/crates/auth/src/policy/document.rs b/crates/auth/src/policy/document.rs index a5893169e..46d3d5415 100755 --- a/crates/auth/src/policy/document.rs +++ b/crates/auth/src/policy/document.rs @@ -20,19 +20,6 @@ pub struct PolicyDocument { pub statements: Vec, } -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") - } -} - /// A single policy statement. #[derive(Debug, Clone)] pub struct Statement { @@ -506,18 +493,6 @@ fn parse_string_or_array(value: &Value) -> Result, PolicyParseError> mod tests { use super::*; - #[test] - fn version_2008_or_absent_does_not_expand_variables() { - let stmt = r#""Statement":[{"Effect":"Allow","Action":"dynamodb:*","Resource":"*"}]"#; - 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 parse_simple_allow_policy() { let json = r#"{ @@ -530,7 +505,6 @@ mod tests { }"#; let doc = PolicyDocument::from_json(json).unwrap(); assert_eq!(doc.version.as_deref(), Some("2012-10-17")); - assert!(doc.expands_policy_variables()); assert_eq!(doc.statements.len(), 1); assert_eq!(doc.statements[0].effect, Effect::Allow); assert!(matches!( diff --git a/crates/auth/src/policy/evaluator/tests.rs b/crates/auth/src/policy/evaluator/tests.rs index dcc8e403e..651aba82f 100644 --- a/crates/auth/src/policy/evaluator/tests.rs +++ b/crates/auth/src/policy/evaluator/tests.rs @@ -60,3 +60,4 @@ fn parse(json: &str) -> PolicyDocument { mod case_three; mod combined; mod core; +mod resource_variables; 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..a2bab2795 --- /dev/null +++ b/crates/auth/src/policy/evaluator/tests/resource_variables.rs @@ -0,0 +1,285 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! Policy variables in `Resource` and `NotResource` (issue #533). +//! +//! One test per row of the issue's table, each measured with the IAM policy +//! simulator on 2026-10-24 with `dynamodb:GetItem`, plus the escapes, the +//! default form in a condition value, the `2008-10-17` and no-`Version` +//! documents for `NotResource`, and the boundary and session policy 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, not a wildcard (measured) --- + +#[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() { + // Derived from the measured rule, not measured: a pattern with an + // unresolved variable matches nothing, so every resource is outside it. + let p = allow_all_deny_get( + Some("2012-10-17"), + "NotResource", + "${aws:PrincipalTag/team}", + ); + assert_eq!(get(&p, "blue", &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 owned_by_anon = Ctx::empty().with("aws:ResourceTag/owner", vec!["anon"]); + assert_eq!(get(&[p.clone()], "T", &owned_by_anon), AuthzDecision::Allow); + let owned_by_alice = Ctx::empty().with("aws:ResourceTag/owner", vec!["alice"]); + assert_eq!(get(&[p.clone()], "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/variables.rs b/crates/auth/src/policy/variables.rs index f23f72a2b..b4de3ba47 100644 --- a/crates/auth/src/policy/variables.rs +++ b/crates/auth/src/policy/variables.rs @@ -21,7 +21,7 @@ //! not carry, with no default, makes the pattern match nothing. Both rules //! were measured with the IAM policy simulator (issue #533). Only a document //! whose `Version` is `2012-10-17` expands variables; the caller decides that -//! with [`super::document::PolicyDocument::expands_policy_variables`]. +//! with [`PolicyDocument::expands_policy_variables`]. //! //! ARN matching here follows the same field rule as //! [`super::matcher::arn_match`]: the first five `:` separate fields, the @@ -29,8 +29,22 @@ //! number of fields. use super::context::ConditionContext; +use super::document::PolicyDocument; use super::matcher::arn_match; +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") + } +} + /// One unit of an expanded pattern. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum Token { diff --git a/crates/auth/src/policy/variables/tests.rs b/crates/auth/src/policy/variables/tests.rs index e0f800c65..704696068 100644 --- a/crates/auth/src/policy/variables/tests.rs +++ b/crates/auth/src/policy/variables/tests.rs @@ -41,6 +41,23 @@ 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()); +} + // --- parse_reference --- #[test] From 810cd7b002ea822fead54d391c9c828b697d2e5e Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Sat, 31 Oct 2026 00:45:50 +0000 Subject: [PATCH 03/29] docs(security): state that Resource and NotResource expand policy variables --- docs/manuals/10-security-model.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/manuals/10-security-model.md b/docs/manuals/10-security-model.md index bb7a1c75d..5f1b2c609 100755 --- a/docs/manuals/10-security-model.md +++ b/docs/manuals/10-security-model.md @@ -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. The `${key, 'default'}` form expands to `default` when the request does not carry `key`. ### 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 makes the pattern match nothing. A `2008-10-17` document, or one with no `Version`, matches the pattern as written, as IAM does. - `Condition`: Full condition block support (see operators and keys above) ## Transport Security From d0644c9175681cc008bf853fb2eb588fcee2b448 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Sat, 31 Oct 2026 00:45:50 +0000 Subject: [PATCH 04/29] test(auth): borrow the condition default-form policy as a slice --- .../auth/src/policy/evaluator/tests/resource_variables.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/auth/src/policy/evaluator/tests/resource_variables.rs b/crates/auth/src/policy/evaluator/tests/resource_variables.rs index a2bab2795..f31ae99e4 100644 --- a/crates/auth/src/policy/evaluator/tests/resource_variables.rs +++ b/crates/auth/src/policy/evaluator/tests/resource_variables.rs @@ -218,12 +218,13 @@ fn condition_value_default_form_expands_when_key_absent() { "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.clone()], "T", &owned_by_anon), AuthzDecision::Allow); + 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.clone()], "T", &owned_by_alice), AuthzDecision::Deny); + 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); + assert_eq!(get(p, "T", &alice_on_alice), AuthzDecision::Allow); } // --- Boundary and session policy paths --- From b6dad4bed573af08b6ef9507d9cbcda4e994078d Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Sat, 31 Oct 2026 00:45:50 +0000 Subject: [PATCH 05/29] fix(auth): treat multivalued keys as unset and keep a value's colons in its field Two rules of IAM's policy variable expansion, measured with the policy simulator: - A multivalued condition key is never a policy variable. A Resource pattern naming one matches no resource and a condition value naming one is not expanded, with one value in the request or several. The resolver gave such a key no value, as main does in a condition value, but it also dropped a default written beside the key. It now reads the key as one the request does not carry, so the default form applies. - Only a colon written in the policy separates ARN fields. A colon inside a substituted value is a byte of its field, so a region-field value of us-east-1:123456789012 matches no table ARN, where it used to shift the fields and match a stream ARN. Tests pin both, variables in the region and account fields, an unresolved variable before a wildcard, and a policy-text ? beside a variable. --- crates/auth/src/policy/evaluator/tests.rs | 1 + .../tests/resource_variables_fields.rs | 157 ++++++++++++++++++ crates/auth/src/policy/variables.rs | 34 ++-- crates/auth/src/policy/variables/tests.rs | 4 +- 4 files changed, 183 insertions(+), 13 deletions(-) create mode 100644 crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs diff --git a/crates/auth/src/policy/evaluator/tests.rs b/crates/auth/src/policy/evaluator/tests.rs index 651aba82f..bdce768a5 100644 --- a/crates/auth/src/policy/evaluator/tests.rs +++ b/crates/auth/src/policy/evaluator/tests.rs @@ -61,3 +61,4 @@ mod case_three; mod combined; mod core; mod resource_variables; +mod resource_variables_fields; 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..a32d2d277 --- /dev/null +++ b/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs @@ -0,0 +1,157 @@ +// 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 expected decision was measured with the IAM policy +//! simulator on 2026-10-24 with `dynamodb:GetItem`. + +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 (cells C1, D1) --- + +#[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); + } +} + +// --- Variables before the fifth colon expand (cells C2, D3) --- + +#[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 (cells C3, D2) --- + +#[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 (cells C9, D4) --- + +#[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 + ); +} diff --git a/crates/auth/src/policy/variables.rs b/crates/auth/src/policy/variables.rs index b4de3ba47..9dbcf12eb 100644 --- a/crates/auth/src/policy/variables.rs +++ b/crates/auth/src/policy/variables.rs @@ -8,8 +8,8 @@ //! before matching. The forms are: //! //! - `${key}`: the value of `key` in the request context. A multivalued key -//! has no value, default or not: a set of values has no one value to -//! expand to. +//! is never a variable: it resolves as if the request did not carry it, +//! with one value or several (measured, issue #533 review cells C1 and D1). //! - `${key, 'default'}`: the value of `key`, or `default` when the request //! does not carry `key`. //! - `${*}`, `${?}`, `${$}`: a literal `*`, `?` or `$`. @@ -26,7 +26,9 @@ //! 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. +//! number of fields. Only a `:` written in the policy text separates fields; +//! a `:` inside a variable value is matched as a byte of its field (measured: +//! a region-field value `us-east-1:123456789012` matches no table ARN). use super::context::ConditionContext; use super::document::PolicyDocument; @@ -52,6 +54,9 @@ enum Token { Star, /// `?` written in the policy text: matches exactly one byte. Question, + /// `:` written in the policy text: a field separator among the first five, + /// a literal `:` in the resource field. + Colon, /// One byte matched literally. Byte(u8), } @@ -128,17 +133,17 @@ fn parts(value: &str) -> Vec> { out } -/// The request's value for `key`: the first value of a present key, the -/// default when the key is absent and a default was written, `None` when the -/// key is absent and no default was written. A present key with no values -/// expands to the empty string. A multivalued key is always `None`. +/// The request's value for `key`: the value of a present single-valued key, +/// the default when the key is absent and a default was written, `None` when +/// the key is absent and no default was written. A present key with no values +/// expands to the empty string. A multivalued key is treated as absent. fn resolve<'a>( name: &str, default: Option<&'a str>, context: &'a impl ConditionContext, ) -> Option<&'a str> { if context.is_multivalued_key(name) { - return None; + return default; } match context.resolve_key(name) { Some(values) => Some(values.first().copied().unwrap_or("")), @@ -155,6 +160,7 @@ fn expand_tokens(pattern: &str, context: &impl ConditionContext) -> Option tokens.extend(text.bytes().map(|b| match b { b'*' => Token::Star, b'?' => Token::Question, + b':' => Token::Colon, other => Token::Byte(other), })), Part::Ref(_, Reference::Escape(byte)) => tokens.push(Token::Byte(byte)), @@ -228,13 +234,13 @@ fn arn_match_tokens(pattern: &[Token], value: &str) -> bool { .all(|(p, v)| match_tokens(p, v.as_bytes())) } -/// Split at the first five literal `:` bytes, so the resource field keeps its -/// own `:`. +/// 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::Byte(b':')) { + match rest.iter().position(|t| *t == Token::Colon) { Some(i) => { fields.push(&rest[..i]); rest = &rest[i + 1..]; @@ -260,7 +266,11 @@ fn match_tokens(p: &[Token], v: &[u8]) -> bool { last_star = pi + 1; match_from = vi; pi += 1; - } else if pi < plen && (p[pi] == Token::Question || p[pi] == Token::Byte(v[vi])) { + } else if pi < plen + && (p[pi] == Token::Question + || p[pi] == Token::Byte(v[vi]) + || (p[pi] == Token::Colon && v[vi] == b':')) + { pi += 1; vi += 1; } else if last_star != usize::MAX { diff --git a/crates/auth/src/policy/variables/tests.rs b/crates/auth/src/policy/variables/tests.rs index 704696068..260d0e2dc 100644 --- a/crates/auth/src/policy/variables/tests.rs +++ b/crates/auth/src/policy/variables/tests.rs @@ -229,7 +229,9 @@ fn escapes_are_literal_characters() { } #[test] -fn multivalued_key_uses_first_value() { +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)); From 883e3ea99c61814e0f8cc73bf6f1ffa0caa0787a Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Sat, 31 Oct 2026 00:45:50 +0000 Subject: [PATCH 06/29] docs(security): multivalued keys and colons in policy variable values --- docs/manuals/10-security-model.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/manuals/10-security-model.md b/docs/manuals/10-security-model.md index 5f1b2c609..665525dd2 100755 --- a/docs/manuals/10-security-model.md +++ b/docs/manuals/10-security-model.md @@ -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. The `${key, 'default'}` form expands to `default` when the request does not carry `key`. +Policy variables (e.g., `${aws:PrincipalTag/Team}`) are expanded in condition values. The `${key, 'default'}` form expands to `default` when the request does not carry `key`. 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. 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 makes the pattern match nothing. A `2008-10-17` document, or one with no `Version`, matches the pattern as written, as IAM does. +- `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`, makes the pattern match nothing; 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. - `Condition`: Full condition block support (see operators and keys above) ## Transport Security From 3cc7663fe976a9c75a7cb06b45f851139a0e8fcd Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:40:12 +0000 Subject: [PATCH 07/29] test(auth): pin policy variable rules that had no evaluator test Text substituted into a Resource pattern or a condition value from a variable value or a default is matched as text: it is not expanded a second time, and a `*` or `?` in it is not a wildcard. An Allow whose NotResource pattern holds an unresolved variable grants every table. The `${key, 'default'}` form, and the rule that a multivalued key is never a variable, hold under every condition operator form, not only bare StringEquals. New evaluator tests pin each rule. A resource tag value of `${aws:username}` names only the table `${aws:username}`, and the same text reached through `${aws:UserAgent}` is compared as written in a condition. The defaults `'*'` and `'b?'` match only `table/*` and `table/b?`. `Allow NotResource table/${aws:PrincipalTag/team}` with no tag grants `table/blue` and `table/red`. `${aws:PrincipalTag/team,'none'}` without a space denies `table/none`. A `team` value of `bl*` denies only `table/bl*`. The default form expands under ForAnyValue, ForAllValues, IfExists and Null, and `${dynamodb:LeadingKeys}` stays text under ForAnyValue, ForAllValues and IfExists. The NotResource Allow, no-space default and `bl*` cases repeat rows the IAM policy simulator measured; the other cases follow from those rules and are labelled as not measured. --- crates/auth/src/policy/evaluator/tests.rs | 1 + .../evaluator/tests/resource_variables.rs | 24 +++ .../tests/resource_variables_fields.rs | 16 ++ .../policy/evaluator/tests/variable_values.rs | 177 ++++++++++++++++++ 4 files changed, 218 insertions(+) create mode 100644 crates/auth/src/policy/evaluator/tests/variable_values.rs diff --git a/crates/auth/src/policy/evaluator/tests.rs b/crates/auth/src/policy/evaluator/tests.rs index bdce768a5..0dbd08a9f 100644 --- a/crates/auth/src/policy/evaluator/tests.rs +++ b/crates/auth/src/policy/evaluator/tests.rs @@ -62,3 +62,4 @@ mod combined; mod core; mod resource_variables; mod resource_variables_fields; +mod variable_values; diff --git a/crates/auth/src/policy/evaluator/tests/resource_variables.rs b/crates/auth/src/policy/evaluator/tests/resource_variables.rs index f31ae99e4..c4941214d 100644 --- a/crates/auth/src/policy/evaluator/tests/resource_variables.rs +++ b/crates/auth/src/policy/evaluator/tests/resource_variables.rs @@ -208,6 +208,30 @@ fn not_resource_unresolved_variable_applies_to_every_resource() { 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] diff --git a/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs b/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs index a32d2d277..c1256414d 100644 --- a/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs +++ b/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs @@ -155,3 +155,19 @@ fn a_question_mark_written_beside_a_variable_is_a_wildcard() { 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_values.rs b/crates/auth/src/policy/evaluator/tests/variable_values.rs new file mode 100644 index 000000000..abdfa6878 --- /dev/null +++ b/crates/auth/src/policy/evaluator/tests/variable_values.rs @@ -0,0 +1,177 @@ +// 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, `IfExists` and `Null`. +//! +//! 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 +//! not measured. + +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_expands_in_a_null_condition() { + // With no `flag` tag the value is `false`, so the Allow needs `team`. + let p = allow_get_if( + "Null", + "aws:PrincipalTag/team", + "${aws:PrincipalTag/flag, 'false'}", + ); + let with_team = Ctx::empty().with("aws:PrincipalTag/team", vec!["blue"]); + assert_eq!(get(&p, "T", &with_team), AuthzDecision::Allow); + assert_eq!(get(&p, "T", &Ctx::empty()), AuthzDecision::Deny); +} + +// --- 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}"); + } +} From 07ba980018d84a8291b1dc736a438e9bdb12b7e2 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:40:12 +0000 Subject: [PATCH 08/29] fix(auth): fail closed on an unresolvable key in a Resource variable A condition on a key the service populates but the context cannot resolve fails closed: a Deny applies and an Allow does not grant. A `${key}` variable naming such a key in Resource or NotResource was read as an absent key instead, so `Deny table/${key}` did not apply and `Allow NotResource table/${key}` granted every table. `resolve` now reports such a key on its own, before the multivalued rule and without using a default, and `resource_matches` fails closed when no pattern matched and one names it. A pattern that matches, or one whose variable names an absent key with no default, still decides alone. In this build the context reports one such key, `aws:RequestTag/` when the request names the tag under two spellings with different values, so a pattern naming that tag fails closed for such a request. No key is listed in `FAIL_CLOSED_GLOBAL_KEYS`; the comment on that list now names both places that read it. Condition values do not change here: #522's check fails a condition closed when a plain `${key}` in one of its values names such a key, and a value with a default is compared as written. The tests mark `aws:PrincipalOrgID` unresolvable through the test context. Deny on `table/${aws:PrincipalOrgID}`, with and without a default, and Deny NotResource block GetItem on `table/T`. Allow on `table/${aws:PrincipalOrgID, 'T'}` and Allow NotResource do not grant it. `table/T` in the same list, or an absent key in the same pattern before or after the unresolvable one, still decides. The multivalued `dynamodb:LeadingKeys`, marked unresolvable, fails closed the same way in Resource and NotResource, with and without a default. --- crates/auth/src/policy/context.rs | 8 ++ crates/auth/src/policy/evaluator.rs | 21 +++- .../src/policy/evaluator/tests/case_three.rs | 118 +++++++++++++++++- crates/auth/src/policy/variables.rs | 97 ++++++++++---- 4 files changed, 216 insertions(+), 28 deletions(-) diff --git a/crates/auth/src/policy/context.rs b/crates/auth/src/policy/context.rs index be764624c..855f9c593 100755 --- a/crates/auth/src/policy/context.rs +++ b/crates/auth/src/policy/context.rs @@ -128,6 +128,14 @@ 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 whose +/// key is listed fails closed (`condition::evaluate_condition_match`), and so +/// does a `Resource` or `NotResource` pattern whose `${key}` variable names a +/// listed key (`variables::resource_pattern_is_unresolvable`). A variable that +/// names a listed key inside a condition value is left as written, as an +/// unresolved variable is there, so listing a key does not make such a +/// condition fail closed. const FAIL_CLOSED_GLOBAL_KEYS: &[&str] = &[]; /// Whether an unresolved condition key must fail closed for the context that diff --git a/crates/auth/src/policy/evaluator.rs b/crates/auth/src/policy/evaluator.rs index 049fb0737..f787fffc2 100644 --- a/crates/auth/src/policy/evaluator.rs +++ b/crates/auth/src/policy/evaluator.rs @@ -11,7 +11,7 @@ use super::condition::{ConditionMatch, evaluate_statement_conditions}; use super::context::ConditionContext; use super::document::{ActionMatch, Effect, PolicyDocument, ResourceMatch, Statement}; use super::matcher::wildcard_match_ignore_case; -use super::variables::resource_pattern_matches; +use super::variables::{resource_pattern_is_unresolvable, resource_pattern_matches}; /// The result of policy evaluation. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -201,6 +201,11 @@ fn action_matches(statement: &Statement, request_action: &str) -> bool { /// 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, @@ -213,9 +218,19 @@ fn resource_matches( .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) => any_match(patterns), - ResourceMatch::NotResources(patterns) => !any_match(patterns), + ResourceMatch::Resources(patterns) => { + any_match(patterns) || (deny && unresolvable(patterns)) + } + ResourceMatch::NotResources(patterns) => { + !any_match(patterns) && (deny || !unresolvable(patterns)) + } } } diff --git a/crates/auth/src/policy/evaluator/tests/case_three.rs b/crates/auth/src/policy/evaluator/tests/case_three.rs index f6ab83e30..814bb4e8f 100644 --- a/crates/auth/src/policy/evaluator/tests/case_three.rs +++ b/crates/auth/src/policy/evaluator/tests/case_three.rs @@ -3,7 +3,9 @@ //! 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`. use super::*; @@ -120,3 +122,117 @@ 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}" + ); + } +} diff --git a/crates/auth/src/policy/variables.rs b/crates/auth/src/policy/variables.rs index 9dbcf12eb..436ecfbaa 100644 --- a/crates/auth/src/policy/variables.rs +++ b/crates/auth/src/policy/variables.rs @@ -19,9 +19,12 @@ //! 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). Only a document -//! whose `Version` is `2012-10-17` expands variables; the caller decides that -//! with [`PolicyDocument::expands_policy_variables`]. +//! 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 @@ -133,28 +136,49 @@ fn parts(value: &str) -> Vec> { 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, -/// the default when the key is absent and a default was written, `None` when -/// the key is absent and no default was written. A present key with no values -/// expands to the empty string. A multivalued key is treated as absent. +/// 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 +/// treated as absent. 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, -) -> Option<&'a str> { +) -> 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 default; + return default.ok_or(Missing::Absent); } - match context.resolve_key(name) { - Some(values) => Some(values.first().copied().unwrap_or("")), - None => default, + match values { + Some(values) => Ok(values.first().copied().unwrap_or("")), + None => default.ok_or(Missing::Absent), } } -/// Expand `pattern` into tokens. Returns `None` when a variable names a key -/// the request does not carry and gives no default. -fn expand_tokens(pattern: &str, context: &impl ConditionContext) -> Option> { +/// 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) { match part { Part::Text(text) => tokens.extend(text.bytes().map(|b| match b { @@ -165,20 +189,28 @@ fn expand_tokens(pattern: &str, context: &impl ConditionContext) -> Option tokens.push(Token::Byte(byte)), Part::Ref(_, Reference::Key { name, default }) => { - let value = resolve(name, default, context)?; - tokens.extend(value.bytes().map(Token::Byte)); + match resolve(name, default, context) { + Ok(value) => tokens.extend(value.bytes().map(Token::Byte)), + Err(Missing::Unresolvable) => unresolvable = true, + Err(Missing::Absent) => return Err(Missing::Absent), + } } } } - Some(tokens) + 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. +/// [`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, @@ -189,17 +221,34 @@ pub fn resource_pattern_matches( if !expand || !pattern.contains("${") { return arn_match(pattern, resource_arn); } - match expand_tokens(pattern, context) { - Some(tokens) => arn_match_tokens(&tokens, resource_arn), - None => false, - } + 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) } /// Expand variables in a condition value. /// /// `${key}` and `${key, 'default'}` are replaced by their value; a variable /// whose key the request does not carry, with no default, is left as written, -/// and so are the `${*}`, `${?}` and `${$}` escapes, as before. +/// and so are the `${*}`, `${?}` and `${$}` escapes, as before. A variable +/// whose key the context cannot resolve is also left as written, with or +/// without a default; a condition value does not fail closed on it. #[must_use] pub fn expand_condition_value(value: &str, context: &impl ConditionContext) -> String { if !value.contains("${") { From e71157af05a9e637852aba536a98629fecaec8a6 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:40:12 +0000 Subject: [PATCH 09/29] docs(auth): name the measured cases instead of matrix cell labels Comments in the policy variable code and tests cited cells of a table in an issue comment (C1, D1 and so on), which a reader of the tree cannot look up. Each label is replaced by the case it stood for, with the PR #535 body table as the source: `dynamodb:LeadingKeys` with one value and with two, `aws:RequestedRegion` and `aws:PrincipalAccount` in the region and account fields, a region tag value `us-east-1:123456789012` and `team=blue:x`, and `table/${aws:PrincipalTag/team}*` with no tag beside `table/b?${aws:PrincipalTag/team}`. Comments only. --- .../evaluator/tests/resource_variables_fields.rs | 13 +++++++++---- crates/auth/src/policy/variables.rs | 4 +++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs b/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs index c1256414d..19e66cad4 100644 --- a/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs +++ b/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs @@ -38,7 +38,8 @@ fn leading_keys(values: Vec<&str>) -> Ctx { Ctx::empty().with(LEADING_KEYS, values) } -// --- A multivalued key is never a variable (cells C1, D1) --- +// --- 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() { @@ -84,7 +85,9 @@ fn a_condition_value_naming_a_multivalued_key_is_not_expanded() { } } -// --- Variables before the fifth colon expand (cells C2, D3) --- +// --- Variables before the fifth colon expand (`aws:RequestedRegion` in the +// region field, `aws:PrincipalAccount` in the account field, PR #535 body +// table) --- #[test] fn a_variable_in_the_region_field_expands() { @@ -106,7 +109,8 @@ fn a_variable_in_the_account_field_expands() { ); } -// --- A ':' inside a value stays in its field (cells C3, D2) --- +// --- 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() { @@ -133,7 +137,8 @@ fn a_colon_in_a_resource_field_value_matches_the_same_name() { ); } -// --- Wildcards beside a variable (cells C9, D4) --- +// --- 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() { diff --git a/crates/auth/src/policy/variables.rs b/crates/auth/src/policy/variables.rs index 436ecfbaa..0972ad6ec 100644 --- a/crates/auth/src/policy/variables.rs +++ b/crates/auth/src/policy/variables.rs @@ -9,7 +9,9 @@ //! //! - `${key}`: the value of `key` in the request context. A multivalued key //! is never a variable: it resolves as if the request did not carry it, -//! with one value or several (measured, issue #533 review cells C1 and D1). +//! with one value or several (measured with `dynamodb:LeadingKeys` holding +//! one value and two, in `Resource` and in a condition value; PR #535 body +//! table). //! - `${key, 'default'}`: the value of `key`, or `default` when the request //! does not carry `key`. //! - `${*}`, `${?}`, `${$}`: a literal `*`, `?` or `$`. From c63894752a4da1c99d5ab3f3ffbd8779956b9bcb Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:40:12 +0000 Subject: [PATCH 10/29] test(auth): say which policy variable expectations were measured The header of resource_variables.rs said every test was a measured row, while the `${?}` and `${$}` escapes, NotResource in a 2008-10-17 or no-Version document, the condition default form and the boundary, session policy and explicit_deny_matches_arc tests were never measured. The Deny NotResource test with no tag said "not measured" although the simulator measured that case, and the `${*}` heading said "measured" for both its checks although only `table/x` was measured. The header now names the tests that repeat a simulator case, the checks inside them that go beyond the measured target or context, and the tests that follow from the measured rules. The fields file header names its two derived checks. Comments only. --- .../evaluator/tests/resource_variables.rs | 23 +++++++++++++------ .../tests/resource_variables_fields.rs | 6 +++-- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/crates/auth/src/policy/evaluator/tests/resource_variables.rs b/crates/auth/src/policy/evaluator/tests/resource_variables.rs index c4941214d..f40700feb 100644 --- a/crates/auth/src/policy/evaluator/tests/resource_variables.rs +++ b/crates/auth/src/policy/evaluator/tests/resource_variables.rs @@ -3,10 +3,19 @@ //! Policy variables in `Resource` and `NotResource` (issue #533). //! -//! One test per row of the issue's table, each measured with the IAM policy -//! simulator on 2026-10-24 with `dynamodb:GetItem`, plus the escapes, the -//! default form in a condition value, the `2008-10-17` and no-`Version` -//! documents for `NotResource`, and the boundary and session policy paths. +//! 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::*; @@ -156,7 +165,7 @@ fn row11_default_form_expands_when_key_absent() { assert_eq!(get(&p, "none", &team_blue()), AuthzDecision::Allow); } -// --- Row 12: ${*} is a literal star, not a wildcard (measured) --- +// --- Row 12: ${*} is a literal star (`table/x` measured, `table/*` derived) --- #[test] fn row12_escaped_star_in_resource_is_literal() { @@ -198,8 +207,8 @@ fn not_resource_no_version_matches_text_as_written() { #[test] fn not_resource_unresolved_variable_applies_to_every_resource() { - // Derived from the measured rule, not measured: a pattern with an - // unresolved variable matches nothing, so every resource is outside it. + // 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", diff --git a/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs b/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs index 19e66cad4..11f73a978 100644 --- a/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs +++ b/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs @@ -3,8 +3,10 @@ //! Policy variables in `Resource`: multivalued keys, variables outside the //! resource field, a `:` inside a value, and the wildcards beside a variable -//! (issue #533). Every expected decision was measured with the IAM policy -//! simulator on 2026-10-24 with `dynamodb:GetItem`. +//! (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::*; From c2ac656c2a44ffaa9d4117bb35c925fa9ebcca63 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:40:12 +0000 Subject: [PATCH 11/29] fix(auth): refuse a policy Version other than the two IAM values The policy write paths checked that a `Version` member exists but not its value, and the value now decides whether Resource and NotResource variables expand. A document whose `Version` was `"2012-10-18"`, `"2012-10-17 "`, `20121017`, `null`, `["2012-10-17"]` or `"2012-10-17T"` was stored, and every variable in its Resource patterns then matched as literal text, with no error at write. `from_json_for_write` now refuses a `Version` that is not the string `2012-10-17` or `2008-10-17`, the two values in IAM's policy grammar, with `invalid Version (must be the string 2012-10-17 or 2008-10-17):` followed by the value. The check sits beside `expands_policy_variables` so document.rs does not grow. It runs at write only, so a stored document with another value keeps parsing. A missing `Version` is still refused by each write path before parsing. The user, group and role policy, permissions boundary and console writes reach the check; an AssumeRole session policy is still checked only for the member. The message IAM returns for such a document was not measured. The test writes each of the six values and expects the refusal, parses each as a stored document, and writes both accepted values. The security model manual and the backlog entry on `Version` are updated. --- crates/auth/src/policy/document.rs | 21 +++++++++------ crates/auth/src/policy/variables.rs | 24 ++++++++++++++++- crates/auth/src/policy/variables/tests.rs | 33 +++++++++++++++++++++++ docs/backlog.md | 2 +- docs/manuals/10-security-model.md | 2 +- 5 files changed, 71 insertions(+), 11 deletions(-) diff --git a/crates/auth/src/policy/document.rs b/crates/auth/src/policy/document.rs index 46d3d5415..998f08357 100755 --- a/crates/auth/src/policy/document.rs +++ b/crates/auth/src/policy/document.rs @@ -13,8 +13,9 @@ use std::fmt; /// A parsed IAM policy document. #[derive(Debug, Clone)] pub struct PolicyDocument { - /// Policy version string (e.g., "2012-10-17"), `None` when the document - /// has no `Version`. + /// 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, @@ -186,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) } diff --git a/crates/auth/src/policy/variables.rs b/crates/auth/src/policy/variables.rs index 0972ad6ec..885e4511a 100644 --- a/crates/auth/src/policy/variables.rs +++ b/crates/auth/src/policy/variables.rs @@ -36,8 +36,9 @@ //! a region-field value `us-east-1:123456789012` matches no table ARN). use super::context::ConditionContext; -use super::document::PolicyDocument; +use super::document::{PolicyDocument, PolicyParseError}; use super::matcher::arn_match; +use serde_json::Value; impl PolicyDocument { /// Whether `${key}` policy variables in `Resource` and `NotResource` @@ -52,6 +53,27 @@ impl PolicyDocument { } } +/// 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. The message IAM returns for such +/// a document 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}" + ))), + } +} + /// One unit of an expanded pattern. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum Token { diff --git a/crates/auth/src/policy/variables/tests.rs b/crates/auth/src/policy/variables/tests.rs index 260d0e2dc..de57257c3 100644 --- a/crates/auth/src/policy/variables/tests.rs +++ b/crates/auth/src/policy/variables/tests.rs @@ -58,6 +58,39 @@ fn only_version_2012_expands_variables() { assert!(!none.expands_policy_variables()); } +#[test] +fn a_write_refuses_a_version_that_is_not_one_of_the_two_values() { + 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] diff --git a/docs/backlog.md b/docs/backlog.md index d335df5a3..f13fe5581 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, message not measured**: 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: the message IAM returns for such a document, 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 diff --git a/docs/manuals/10-security-model.md b/docs/manuals/10-security-model.md index 665525dd2..a24448627 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 From 011c6cbd4259a52c1d521ab828323d334b18c48d Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:40:12 +0000 Subject: [PATCH 12/29] docs(design): describe policy variables in Resource as implemented The requirements, the two auth component designs and the backlog said that policy variables are deferred and that Resource and Condition values are literal strings, which the expansion in policy/variables.rs now contradicts. REQ-ABAC-006 states the requirement as met for Resource, NotResource and condition values, and the non-goals table drops its policy variables row. The policy variables note in 05-component-auth.md describes the expansion and points to the security model manual, the Mode 3 limitation names only the condition-value gaps, and the v3 note no longer calls the support deferred. The backlog entry keeps the two condition-value gaps filed as #534. --- docs/backlog.md | 2 +- docs/design/01-requirements.md | 3 +-- docs/design/05-component-auth-v3.md | 2 +- docs/design/05-component-auth.md | 4 ++-- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/backlog.md b/docs/backlog.md index f13fe5581..5304d0ea7 100755 --- a/docs/backlog.md +++ b/docs/backlog.md @@ -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..d1a75e692 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 (`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 makes the pattern match nothing. 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. From 026c861c8cf3d761ef68f9cc237d092110f4244e Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:40:12 +0000 Subject: [PATCH 13/29] docs(upgrade): note the policy variable decision changes in v0.1.15 Stored policies change decision in three ways when policy variables expand in Resource and NotResource, and operators had no note of it: a pattern with a variable now matches the table it names, a condition value naming a multivalued key is no longer expanded to its first value, and the `${key, 'default'}` form now expands in condition values. The entry names each change, including an Allow that now grants and a Deny NotResource that now spares the named table, the write-time refusal of a Version other than 2012-10-17 or 2008-10-17, who is affected, and how to find affected policies. No catalog migration is required. --- docs/manuals/07-upgrade-manual.md | 39 +++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/docs/manuals/07-upgrade-manual.md b/docs/manuals/07-upgrade-manual.md index f4451a0f5..2c8eb1517 100755 --- a/docs/manuals/07-upgrade-manual.md +++ b/docs/manuals/07-upgrade-manual.md @@ -319,6 +319,45 @@ Catalog upgrades change the schema; behavior changes alter how the running serve interprets existing data or configuration. Review these before upgrading a live deployment, even when no catalog migration is required. +### 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 makes the +pattern match nothing, and a `2008-10-17` document, or one with no `Version`, +still matches the pattern as written. Three 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, so such a condition no longer matches. +3. A condition value written as `${key, 'default'}` now expands to `default` when + the request does not carry `key`. Before, the whole text was compared. + +**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 ...`. A document already stored with such a +value keeps evaluating, with its `Resource` patterns matched as written. A +session policy passed to `AssumeRole` is not checked. + +**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 two condition +value changes, role trust policies. Search the stored policies for `${` +(`extenddb manage list-user-policies`, `list-group-policies` and +`list-role-policies`) and check each match against the cases above before +upgrading, in particular an `Allow` that now grants 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`, From cb153c12fdf246115fb737cd6bc903bc7e066784 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:40:12 +0000 Subject: [PATCH 14/29] test(auth): pin a condition value naming an unresolvable key A condition value `${key, 'default'}` whose key the context reports unresolvable keeps its text as written: resolve returns Missing::Unresolvable whatever the default, and expand_condition_value falls back to the raw text. No existing test put such a key in a condition value. The new case_three test puts `${aws:PrincipalOrgID, 'blue'}` in a StringEquals Deny on aws:PrincipalTag/team, for a caller tagged team=blue. With the key unresolvable the Deny does not apply. With the key absent the default is used and the Deny applies. With the key present its value is used, so `blue` denies and `red` does not. --- .../src/policy/evaluator/tests/case_three.rs | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/crates/auth/src/policy/evaluator/tests/case_three.rs b/crates/auth/src/policy/evaluator/tests/case_three.rs index 814bb4e8f..6793f1501 100644 --- a/crates/auth/src/policy/evaluator/tests/case_three.rs +++ b/crates/auth/src/policy/evaluator/tests/case_three.rs @@ -5,7 +5,8 @@ //! 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. The //! same holds when the key is named by a `${key}` variable in `Resource` or -//! `NotResource`. +//! `NotResource`. A variable naming the key in a condition value is compared +//! as written, with or without a default. use super::*; @@ -236,3 +237,30 @@ fn a_multivalued_key_the_context_reports_unresolvable_fails_closed_in_resource() ); } } + +// --- 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_is_compared_as_written() { + let p = allow_all_and( + r#"{"Effect":"Deny","Action":"dynamodb:GetItem","Resource":"*", + "Condition":{"StringEquals":{"aws:PrincipalTag/team":"${aws:PrincipalOrgID, 'blue'}"}}}"#, + ); + // Unresolvable: the value is compared as written, which is not `blue`, so + // the Deny does not apply. The default is not used. + let unresolvable = Ctx::empty().unresolvable("aws:PrincipalOrgID"); + assert_eq!(get_t_as_blue(&p, unresolvable), AuthzDecision::Allow); + // 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); +} From fd8d5138a47641148050da95d0ac5d4e8953320f Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:40:12 +0000 Subject: [PATCH 15/29] test(auth): pin the Version gate on an unresolvable Resource key resource_pattern_is_unresolvable starts with `expand &&`, so a `2008-10-17` document, or one with no Version, matches a pattern naming a key the context cannot resolve as written and does not fail closed. Every test of the fail-closed rule used a `2012-10-17` document, so no test covered the gate. The new case_three test puts `Deny table/${aws:PrincipalOrgID}` in each of the two documents beside `Allow dynamodb:*`, with the key unresolvable: GetItem on table `T` is allowed, and GetItem on the table named `${aws:PrincipalOrgID}`, which the text matches, is denied. --- .../src/policy/evaluator/tests/case_three.rs | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/crates/auth/src/policy/evaluator/tests/case_three.rs b/crates/auth/src/policy/evaluator/tests/case_three.rs index 6793f1501..957fce4f4 100644 --- a/crates/auth/src/policy/evaluator/tests/case_three.rs +++ b/crates/auth/src/policy/evaluator/tests/case_three.rs @@ -5,8 +5,9 @@ //! 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. The //! same holds when the key is named by a `${key}` variable in `Resource` or -//! `NotResource`. A variable naming the key in a condition value is compared -//! as written, with or without a default. +//! `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 is compared as written, with or without a default. use super::*; @@ -238,6 +239,33 @@ fn a_multivalued_key_the_context_reports_unresolvable_fails_closed_in_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 From 918f09a738804bca62a4f1fc2fcb6027aaf7f4a1 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:40:12 +0000 Subject: [PATCH 16/29] docs: say a Resource variable with a default uses the default The v0.1.15 upgrade entry and the policy variables paragraph of 05-component-auth.md said that a variable whose key the request does not carry makes the pattern match nothing. That holds only with no default. With a default, the default is used: `table/${aws:PrincipalTag/team, 'none'}` matches `table/none` when the request has no `team` tag, as measured with the IAM policy simulator for the PR #535 measurements table. Both places now say so. --- docs/design/05-component-auth.md | 2 +- docs/manuals/07-upgrade-manual.md | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/design/05-component-auth.md b/docs/design/05-component-auth.md index d1a75e692..df7ce422a 100755 --- a/docs/design/05-component-auth.md +++ b/docs/design/05-component-auth.md @@ -501,7 +501,7 @@ pub enum ConditionOperator { } ``` -> **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 (`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 makes the pattern match nothing. 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. +> **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 (`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:** diff --git a/docs/manuals/07-upgrade-manual.md b/docs/manuals/07-upgrade-manual.md index 2c8eb1517..ba6b6a453 100755 --- a/docs/manuals/07-upgrade-manual.md +++ b/docs/manuals/07-upgrade-manual.md @@ -325,9 +325,12 @@ deployment, even when no catalog migration is required. `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 makes the -pattern match nothing, and a `2008-10-17` document, or one with no `Version`, -still matches the pattern as written. Three decisions change for stored policies: +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). Three 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 From 06522e5b333146060876b520300e02b84f9ed675 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:40:12 +0000 Subject: [PATCH 17/29] docs(upgrade): cover negated operators in the multivalued key item Item 2 of the v0.1.15 entry said a condition value naming a multivalued key is compared as written, so such a condition no longer matches. That holds for a positive operator. Under a negated operator the comparison with the text now succeeds where the comparison with the first value failed: `Allow` if `StringNotEquals` on aws:PrincipalTag/team with the value `${dynamodb:LeadingKeys}`, for a caller tagged team=blue whose request has the leading key `blue`, was denied before and is allowed now, and the same condition in a Deny now applies. The item now names both operator kinds with that example, and the check-before-upgrading sentence names an Allow that now grants and a Deny that now applies, for a Resource pattern with a variable and for such a negated condition. --- docs/manuals/07-upgrade-manual.md | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/docs/manuals/07-upgrade-manual.md b/docs/manuals/07-upgrade-manual.md index ba6b6a453..332344a90 100755 --- a/docs/manuals/07-upgrade-manual.md +++ b/docs/manuals/07-upgrade-manual.md @@ -340,7 +340,13 @@ issue #533). Three decisions change for stored policies: 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, so such a condition no longer matches. + 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. A condition value written as `${key, 'default'}` now expands to `default` when the request does not carry `key`. Before, the whole text was compared. @@ -358,8 +364,10 @@ policies, permissions boundaries and session policies, and, for the two conditio value changes, role trust policies. Search the stored policies for `${` (`extenddb manage list-user-policies`, `list-group-policies` and `list-role-policies`) and check each match against the cases above before -upgrading, in particular an `Allow` that now grants and a `Deny NotResource` that -now spares a table. No catalog migration is required. +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 From 056e0b959e529fb4a03c2bda2fd5740b583dbab2 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:40:12 +0000 Subject: [PATCH 18/29] docs(auth): correct policy variable labels and upgrade wording - variable_values: the multivalued condition-value test also covers Null. With dynamodb:LeadingKeys holding `false`, expanding `${dynamodb:LeadingKeys}` to its first value would make `Null aws:PrincipalTag/team` match for a caller with a team tag, and the Deny would apply; the test expects Allow. The module header already listed Null for both rules. - case_three: the header says its expectations are ExtendDB rules with no service measurement. The Version write test says its message is ExtendDB's own and that whether IAM refuses those values at write was not measured; validate_version and the backlog row say the same. - condition.rs: the expand_policy_variables comment says "unresolved" for a key the request does not carry, and names the key the context cannot resolve, which is left as written with or without a default. - Upgrade manual, v0.1.15: a permissions boundary write refuses another Version with `Invalid permissions boundary document: ...`; a session policy is checked only for its Version and Statement members; item 3 names the present-key case of `${key, 'default'}`; the search advice names get-user-boundary, get-role-boundary and list-roles for boundaries and trust policies. --- crates/auth/src/policy/condition.rs | 4 ++- .../src/policy/evaluator/tests/case_three.rs | 5 ++++ .../policy/evaluator/tests/variable_values.rs | 7 +++++ crates/auth/src/policy/variables.rs | 4 +-- crates/auth/src/policy/variables/tests.rs | 2 ++ docs/backlog.md | 2 +- docs/manuals/07-upgrade-manual.md | 29 ++++++++++++------- 7 files changed, 39 insertions(+), 14 deletions(-) diff --git a/crates/auth/src/policy/condition.rs b/crates/auth/src/policy/condition.rs index fd66ac9fe..e11418299 100644 --- a/crates/auth/src/policy/condition.rs +++ b/crates/auth/src/policy/condition.rs @@ -255,7 +255,9 @@ pub fn evaluate_statement_conditions( /// Replaces `${variable}` and `${variable, 'default'}` patterns with their /// resolved values from the context (see [`super::variables`]). Supported /// variables: `aws:PrincipalTag/*` and any key the context can resolve. -/// Unresolvable variables with no default are left as-is (IAM behavior). +/// Unresolved variables with no default are left as-is (IAM behavior). A +/// variable naming a key the context cannot resolve is also left as-is, with +/// or without a default; see [`super::variables::expand_condition_value`]. fn expand_policy_variables(value: &str, context: &impl ConditionContext) -> String { super::variables::expand_condition_value(value, context) } diff --git a/crates/auth/src/policy/evaluator/tests/case_three.rs b/crates/auth/src/policy/evaluator/tests/case_three.rs index 957fce4f4..caea88b0c 100644 --- a/crates/auth/src/policy/evaluator/tests/case_three.rs +++ b/crates/auth/src/policy/evaluator/tests/case_three.rs @@ -8,6 +8,11 @@ //! `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 is compared as written, 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::*; diff --git a/crates/auth/src/policy/evaluator/tests/variable_values.rs b/crates/auth/src/policy/evaluator/tests/variable_values.rs index abdfa6878..fb06a4596 100644 --- a/crates/auth/src/policy/evaluator/tests/variable_values.rs +++ b/crates/auth/src/policy/evaluator/tests/variable_values.rs @@ -174,4 +174,11 @@ fn a_multivalued_key_in_a_condition_value_is_not_expanded_under_any_form() { 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/variables.rs b/crates/auth/src/policy/variables.rs index 885e4511a..c8dc297bc 100644 --- a/crates/auth/src/policy/variables.rs +++ b/crates/auth/src/policy/variables.rs @@ -61,8 +61,8 @@ impl PolicyDocument { /// 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. The message IAM returns for such -/// a document was not measured. +/// 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") { diff --git a/crates/auth/src/policy/variables/tests.rs b/crates/auth/src/policy/variables/tests.rs index de57257c3..9f990ca18 100644 --- a/crates/auth/src/policy/variables/tests.rs +++ b/crates/auth/src/policy/variables/tests.rs @@ -60,6 +60,8 @@ fn only_version_2012_expands_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""#, diff --git a/docs/backlog.md b/docs/backlog.md index 5304d0ea7..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) -- ⬜ **Policy `Version` refused at write, message not measured**: 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: the message IAM returns for such a document, and what STS does with one in an `AssumeRole` session policy, where ExtendDB checks only that the member exists. (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 diff --git a/docs/manuals/07-upgrade-manual.md b/docs/manuals/07-upgrade-manual.md index 332344a90..a0ad41108 100755 --- a/docs/manuals/07-upgrade-manual.md +++ b/docs/manuals/07-upgrade-manual.md @@ -347,27 +347,36 @@ issue #533). Three decisions change for stored policies: `${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. A condition value written as `${key, 'default'}` now expands to `default` when - the request does not carry `key`. Before, the whole text was compared. +3. A condition value written as `${key, 'default'}` 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. **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 ...`. A document already stored with such a -value keeps evaluating, with its `Resource` patterns matched as written. A -session policy passed to `AssumeRole` is not checked. +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. 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 two condition value changes, role trust policies. Search the stored policies for `${` (`extenddb manage list-user-policies`, `list-group-policies` and -`list-role-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. +`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 From a6aba3c2f8a50903ee524f98256e1cdbe585efdf Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Wed, 28 Oct 2026 18:40:13 +0000 Subject: [PATCH 19/29] fix(auth): a present multivalued key ignores a variable default Measured with the IAM policy simulator on 2026-10-28: with aws:TagKeys as [x, y] or [x], table/${aws:TagKeys, 'x'} in Resource and ${aws:TagKeys, 'x'} as a StringEquals condition value both answer implicitDeny; with aws:TagKeys absent both answer allowed. The code used the default whenever the key was multivalued, present or not. Now a present multivalued key makes the pattern match nothing and keeps a condition value as written, and the default is used only when the request does not carry the key. Two evaluator tests pin the six cells on dynamodb:LeadingKeys; the security model and the module comment say the rule. --- .../tests/resource_variables_fields.rs | 43 +++++++++++++++++++ crates/auth/src/policy/variables.rs | 27 ++++++++---- docs/manuals/10-security-model.md | 2 +- 3 files changed, 62 insertions(+), 10 deletions(-) diff --git a/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs b/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs index 11f73a978..20ffdf87f 100644 --- a/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs +++ b/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs @@ -87,6 +87,49 @@ fn a_condition_value_naming_a_multivalued_key_is_not_expanded() { } } +// --- 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) --- diff --git a/crates/auth/src/policy/variables.rs b/crates/auth/src/policy/variables.rs index c8dc297bc..2d255069a 100644 --- a/crates/auth/src/policy/variables.rs +++ b/crates/auth/src/policy/variables.rs @@ -8,10 +8,12 @@ //! before matching. The forms are: //! //! - `${key}`: the value of `key` in the request context. A multivalued key -//! is never a variable: it resolves as if the request did not carry it, -//! with one value or several (measured with `dynamodb:LeadingKeys` holding -//! one value and two, in `Resource` and in a condition value; PR #535 body -//! table). +//! 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 `$`. @@ -173,10 +175,14 @@ enum Missing { /// 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 -/// treated as absent. 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. +/// 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>, @@ -187,7 +193,10 @@ fn resolve<'a>( return Err(Missing::Unresolvable); } if context.is_multivalued_key(name) { - return default.ok_or(Missing::Absent); + return match values { + Some(_) => Err(Missing::Absent), + None => default.ok_or(Missing::Absent), + }; } match values { Some(values) => Ok(values.first().copied().unwrap_or("")), diff --git a/docs/manuals/10-security-model.md b/docs/manuals/10-security-model.md index a24448627..c1c7c52cb 100755 --- a/docs/manuals/10-security-model.md +++ b/docs/manuals/10-security-model.md @@ -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. 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`, makes the pattern match nothing; 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. +- `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. - `Condition`: Full condition block support (see operators and keys above) ## Transport Security From 3afcf10656fe7b49ed51c4b25df0a62a3a9d8294 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Sat, 31 Oct 2026 10:05:46 +0000 Subject: [PATCH 20/29] fix(auth): fail closed on a defaulted variable naming an unresolvable key A condition fails closed when one of its values reads, through a ${key} variable, a key the context reports unresolvable. In this build that key is aws:RequestTag/ when the request names the tag under two spellings with different values. The check took the whole text between ${ and } as the key name, so ${aws:RequestTag/env, 'x'} named `aws:RequestTag/env, 'x'`, which the context does not report. The value was then compared as written: a StringEquals Deny did not apply and a StringNotEquals Allow granted, where the plain form fails closed. reads_unresolvable_variable now takes its names from variables::variable_key_names, which parses each reference the way expand_condition_value does and leaves the default out of the name. Both forms fail closed: a Deny applies and an Allow does not grant, whatever the operator. The default is not used for such a key, because the service has a value for it. A key the request does not carry still takes the default, and a present key still uses its own value. The expand_policy_variables comment drops "(IAM behavior)": a variable left as written gives IAM's answer except when the compared value holds the variable's text. The case_three test that pinned the old reading is renamed. It now expects the Deny to apply under StringEquals and StringNotEquals, and an Allow gated the same way not to grant under either operator. New tests cover variable_key_names, the default form under three operators in three_case, including as the second of two values, and an ambiguous request tag with a default in request_tag_tests. --- crates/auth/src/policy/condition.rs | 51 +++++++++--------- .../src/policy/condition/tests/three_case.rs | 54 +++++++++++++++++++ .../src/policy/context/request_tag_tests.rs | 45 ++++++++++++++++ .../src/policy/evaluator/tests/case_three.rs | 41 +++++++++++--- crates/auth/src/policy/variables.rs | 22 +++++++- crates/auth/src/policy/variables/tests.rs | 21 ++++++++ 6 files changed, 200 insertions(+), 34 deletions(-) diff --git a/crates/auth/src/policy/condition.rs b/crates/auth/src/policy/condition.rs index e11418299..c69e1a124 100644 --- a/crates/auth/src/policy/condition.rs +++ b/crates/auth/src/policy/condition.rs @@ -179,9 +179,10 @@ 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}` or `${name, 'default'}` policy +/// variable, 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. pub fn evaluate_condition_match( condition: &Condition, context: &impl ConditionContext, @@ -202,26 +203,15 @@ 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. +/// 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 default form the way [`expand_policy_variables`] expands it, so a +/// default written beside such a key does not hide the key from this check. 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))) -} - -/// 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]) + condition.values.iter().any(|value| { + super::variables::variable_key_names(value).any(|name| is_unresolvable(name, context)) }) } @@ -254,10 +244,19 @@ pub fn evaluate_statement_conditions( /// /// Replaces `${variable}` and `${variable, 'default'}` patterns with their /// resolved values from the context (see [`super::variables`]). Supported -/// variables: `aws:PrincipalTag/*` and any key the context can resolve. -/// Unresolved variables with no default are left as-is (IAM behavior). A -/// variable naming a key the context cannot resolve is also left as-is, with -/// or without a default; see [`super::variables::expand_condition_value`]. +/// 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`]). fn expand_policy_variables(value: &str, context: &impl ConditionContext) -> String { super::variables::expand_condition_value(value, context) } diff --git a/crates/auth/src/policy/condition/tests/three_case.rs b/crates/auth/src/policy/condition/tests/three_case.rs index f0aabd82f..74d936d8d 100644 --- a/crates/auth/src/policy/condition/tests/three_case.rs +++ b/crates/auth/src/policy/condition/tests/three_case.rs @@ -68,6 +68,60 @@ fn unresolvable_key_is_unresolvable_for_every_operator() { } } +#[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, &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, &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, &absent), ConditionMatch::Match); +} + #[test] fn resolved_unresolvable_key_uses_its_value() { // If the context both lists a key as unresolvable and resolves it, the diff --git a/crates/auth/src/policy/context/request_tag_tests.rs b/crates/auth/src/policy/context/request_tag_tests.rs index 38b91e40d..e44e87968 100644 --- a/crates/auth/src/policy/context/request_tag_tests.rs +++ b/crates/auth/src/policy/context/request_tag_tests.rs @@ -205,6 +205,51 @@ fn ambiguous_request_tag_as_a_policy_variable_fails_the_statement_closed() { ); } +#[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, &with_resource_tag(&tags)), + ConditionMatch::Unresolvable + ); + } + // One spelling expands to its value, and the default is not used. + assert_eq!( + evaluate_statement_conditions(&deny, &with_resource_tag(&[("Env", "prod")])), + ConditionMatch::Match + ); + assert_eq!( + evaluate_statement_conditions(&deny, &with_resource_tag(&[("env", "dev")])), + ConditionMatch::NoMatch + ); + // An absent request tag is not ambiguous: the default is used. + assert_eq!( + evaluate_statement_conditions(&deny, &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}; diff --git a/crates/auth/src/policy/evaluator/tests/case_three.rs b/crates/auth/src/policy/evaluator/tests/case_three.rs index caea88b0c..55314cb85 100644 --- a/crates/auth/src/policy/evaluator/tests/case_three.rs +++ b/crates/auth/src/policy/evaluator/tests/case_three.rs @@ -7,7 +7,8 @@ //! 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 is compared as written, with or without a default. +//! 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 @@ -281,15 +282,43 @@ fn get_t_as_blue(policies: &[PolicyDocument], ctx: Ctx) -> AuthzDecision { } #[test] -fn a_condition_value_naming_an_unresolvable_key_is_compared_as_written() { +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'}"}}}"#, ); - // Unresolvable: the value is compared as written, which is not `blue`, so - // the Deny does not apply. The default is not used. - let unresolvable = Ctx::empty().unresolvable("aws:PrincipalOrgID"); - assert_eq!(get_t_as_blue(&p, unresolvable), AuthzDecision::Allow); // 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. diff --git a/crates/auth/src/policy/variables.rs b/crates/auth/src/policy/variables.rs index 2d255069a..f6e248d96 100644 --- a/crates/auth/src/policy/variables.rs +++ b/crates/auth/src/policy/variables.rs @@ -275,13 +275,31 @@ pub fn resource_pattern_is_unresolvable( && 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. +/// +/// 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}`. 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) -> impl Iterator { + parts(value).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}` and `${key, 'default'}` are replaced by their value; 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 cannot resolve is also left as written, with or -/// without a default; a condition value does not fail closed on it. +/// 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`]). #[must_use] pub fn expand_condition_value(value: &str, context: &impl ConditionContext) -> String { if !value.contains("${") { diff --git a/crates/auth/src/policy/variables/tests.rs b/crates/auth/src/policy/variables/tests.rs index 9f990ca18..909a62233 100644 --- a/crates/auth/src/policy/variables/tests.rs +++ b/crates/auth/src/policy/variables/tests.rs @@ -349,6 +349,27 @@ fn condition_value_leaves_unresolved_and_escapes_literal() { assert_eq!(expand_condition_value("${open", &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", + ) + .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}").collect(); + assert_eq!(names, ["a, b"]); + assert_eq!(variable_key_names("plain").count(), 0); +} + #[test] fn condition_value_present_key_without_values_is_empty() { let ctx = Ctx::empty().with(TEAM, &[]); From 57861f59280efde32e0557ec746cff34d0311ddf Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Sat, 31 Oct 2026 10:09:19 +0000 Subject: [PATCH 21/29] docs(auth): say a condition value naming an unresolvable key fails closed The FAIL_CLOSED_GLOBAL_KEYS comment said that a variable naming a listed key inside a condition value is left as written, so listing a key does not make such a condition fail closed. Main fails a condition closed when a ${key} variable in one of its values names such a key, and the previous commit does the same for ${key, 'default'}. The comment now says both forms fail closed, as a Resource or NotResource pattern does, and that a default written beside a listed key is not used. Three comments said an unresolved global key such as aws:PrincipalOrgID or aws:SourceIp fails closed: in RequestContext::is_unresolvable_key, on AssumeRoleContext and in its is_unresolvable_key. The list is empty, so such a key keeps absent-key semantics. Each comment now says which keys the context reports unresolvable: in RequestContext an ambiguous aws:RequestTag/, and in both contexts a key listed in FAIL_CLOSED_GLOBAL_KEYS. --- crates/auth/src/policy/context.rs | 41 ++++++++++++++++--------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/crates/auth/src/policy/context.rs b/crates/auth/src/policy/context.rs index 855f9c593..85ca0288f 100755 --- a/crates/auth/src/policy/context.rs +++ b/crates/auth/src/policy/context.rs @@ -129,13 +129,14 @@ fn strip_prefix_ci<'a>(key: &'a str, prefix: &str) -> Option<&'a str> { /// 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 whose -/// key is listed fails closed (`condition::evaluate_condition_match`), and so -/// does a `Resource` or `NotResource` pattern whose `${key}` variable names a -/// listed key (`variables::resource_pattern_is_unresolvable`). A variable that -/// names a listed key inside a condition value is left as written, as an -/// unresolved variable is there, so listing a key does not make such a -/// condition fail closed. +/// 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 @@ -450,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; } @@ -484,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. @@ -592,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) } } From 5a1f13f8357bd8fe34bffab8d20e933dbf863976 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:15:28 +0000 Subject: [PATCH 22/29] fix(auth): count wildcards in UTF-16 code units in patterns with a variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Resource or NotResource pattern with a policy variable is matched by match_tokens in variables.rs, which compared bytes of the value's UTF-8 encoding. #581 made matcher::wildcard_match and arn_match count UTF-16 code units, as IAM does, so after the merge with main a ? beside a variable took one byte where the same pattern without a variable took one unit. A Deny on table/?zz applied to a table named with é or 日 before zz, and the same Deny written as table/?zz${aws:PrincipalTag/none, ''} did not. The tokens now hold UTF-16 code units. The text written in the policy and each substituted value are encoded with encode_utf16, and each ARN field of the request is encoded the same way before the comparison. ASCII patterns and values match as before. The new tests run the Resource rows of matcher/utf16_tests.rs, measured with the IAM policy simulator, on the token path and check that the answer equals the one arn_match gives without the variable. With the byte comparison put back, three of them fail: question_mark_takes_one_utf16_unit_beside_a_variable, star_backtracks_by_utf16_units_beside_a_variable and a_substituted_value_is_matched_by_utf16_units. --- crates/auth/src/policy/variables.rs | 65 +++++++++++++++-------- crates/auth/src/policy/variables/tests.rs | 60 +++++++++++++++++++++ 2 files changed, 102 insertions(+), 23 deletions(-) diff --git a/crates/auth/src/policy/variables.rs b/crates/auth/src/policy/variables.rs index f6e248d96..77ae210c0 100644 --- a/crates/auth/src/policy/variables.rs +++ b/crates/auth/src/policy/variables.rs @@ -34,8 +34,10 @@ //! [`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 byte of its field (measured: -//! a region-field value `us-east-1:123456789012` matches no table ARN). +//! 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}; @@ -76,18 +78,38 @@ pub(super) fn validate_version(json: &str) -> Result<(), PolicyParseError> { } } +/// `*` 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 bytes. + /// `*` written in the policy text: matches zero or more UTF-16 code units. Star, - /// `?` written in the policy text: matches exactly one byte. + /// `?` 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 byte matched literally. - Byte(u8), + /// 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. @@ -214,16 +236,11 @@ fn expand_tokens(pattern: &str, context: &impl ConditionContext) -> Result tokens.extend(text.bytes().map(|b| match b { - b'*' => Token::Star, - b'?' => Token::Question, - b':' => Token::Colon, - other => Token::Byte(other), - })), - Part::Ref(_, Reference::Escape(byte)) => tokens.push(Token::Byte(byte)), + 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.bytes().map(Token::Byte)), + Ok(value) => tokens.extend(value.encode_utf16().map(Token::Unit)), Err(Missing::Unresolvable) => unresolvable = true, Err(Missing::Absent) => return Err(Missing::Absent), } @@ -328,10 +345,10 @@ fn arn_match_tokens(pattern: &[Token], value: &str) -> bool { if p_fields.len() != v_fields.len() { return false; } - p_fields - .iter() - .zip(v_fields.iter()) - .all(|(p, v)| match_tokens(p, v.as_bytes())) + 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 @@ -352,9 +369,11 @@ fn split_fields(pattern: &[Token]) -> Vec<&[Token]> { fields } -/// Greedy wildcard match over tokens, the same algorithm as -/// `matcher::wildcard_match` with `Star` and `Question` as the wildcards. -fn match_tokens(p: &[Token], v: &[u8]) -> bool { +/// 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; @@ -368,8 +387,8 @@ fn match_tokens(p: &[Token], v: &[u8]) -> bool { pi += 1; } else if pi < plen && (p[pi] == Token::Question - || p[pi] == Token::Byte(v[vi]) - || (p[pi] == Token::Colon && v[vi] == b':')) + || p[pi] == Token::Unit(v[vi]) + || (p[pi] == Token::Colon && v[vi] == COLON)) { pi += 1; vi += 1; diff --git a/crates/auth/src/policy/variables/tests.rs b/crates/auth/src/policy/variables/tests.rs index 909a62233..cde71ea3c 100644 --- a/crates/auth/src/policy/variables/tests.rs +++ b/crates/auth/src/policy/variables/tests.rs @@ -298,6 +298,66 @@ fn field_count_rule_holds_after_expansion() { )); } +// --- `?` 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"; From a9d43622ae3468ba4b7f5d4a75b2731d511ab987 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:19:46 +0000 Subject: [PATCH 23/29] fix(auth): trim the key name in both policy variable forms, as IAM does parse_reference trimmed the key name of the default form and not of the plain form, so ${ aws:PrincipalTag/team , 'none'} read the team tag while ${ aws:PrincipalTag/team } named a key no request carries. A Deny on table/${ aws:PrincipalTag/team } denied nothing to a caller tagged team=blue, and the defaulted spelling denied the table blue. The IAM policy simulator trims both forms (measured on 2026-11-03, two samples per row): ${ team }, ${team } and ${ team} read the tag in Resource and in a StringEquals condition value, and spaces before or after the comma and before the closing brace leave the default form intact. A space inside the quotes of the default is kept. IAM trims every code point up to U+0020: tab, newline, U+000B, U+000C, U+000D, U+0001 and U+001F are trimmed, and U+0085, U+00A0 and U+3000 are not. It trims around the escapes too: ${ * } stands for a literal * and ${ $ } for a literal $. parse_reference now trims that set from the text between the braces, and from the key name and the quoted default of the default form. Rust's str::trim is not used, because it trims U+0085, U+00A0 and U+3000 and keeps U+0001 and U+001F. The tests in evaluator/tests/variable_spelling.rs repeat the measured rows, and unit tests pin the parser. --- crates/auth/src/policy/evaluator/tests.rs | 1 + .../evaluator/tests/variable_spelling.rs | 165 ++++++++++++++++++ crates/auth/src/policy/variables.rs | 26 ++- crates/auth/src/policy/variables/tests.rs | 69 ++++++++ 4 files changed, 255 insertions(+), 6 deletions(-) create mode 100644 crates/auth/src/policy/evaluator/tests/variable_spelling.rs diff --git a/crates/auth/src/policy/evaluator/tests.rs b/crates/auth/src/policy/evaluator/tests.rs index 0dbd08a9f..afd0e38c0 100644 --- a/crates/auth/src/policy/evaluator/tests.rs +++ b/crates/auth/src/policy/evaluator/tests.rs @@ -62,4 +62,5 @@ mod combined; mod core; mod resource_variables; mod resource_variables_fields; +mod variable_spelling; mod variable_values; 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/variables.rs b/crates/auth/src/policy/variables.rs index 77ae210c0..0cabed21d 100644 --- a/crates/auth/src/policy/variables.rs +++ b/crates/auth/src/policy/variables.rs @@ -124,13 +124,27 @@ enum Reference<'a> { 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 `}`. /// -/// `*`, `?` and `$` are escapes. `name, 'default'` (whitespace around either -/// part ignored) is the default form. Anything else, including a `,` whose -/// right side is not single-quoted, is taken as the key name as written, which -/// is what the condition path did before the default form was read. +/// 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'?'), @@ -138,10 +152,10 @@ fn parse_reference(inner: &str) -> Reference<'_> { _ => {} } if let Some((name, default)) = inner.split_once(',') { - let default = default.trim(); + let default = trim_up_to_space(default); if default.len() >= 2 && default.starts_with('\'') && default.ends_with('\'') { return Reference::Key { - name: name.trim(), + name: trim_up_to_space(name), default: Some(&default[1..default.len() - 1]), }; } diff --git a/crates/auth/src/policy/variables/tests.rs b/crates/auth/src/policy/variables/tests.rs index cde71ea3c..a4f2b8636 100644 --- a/crates/auth/src/policy/variables/tests.rs +++ b/crates/auth/src/policy/variables/tests.rs @@ -124,6 +124,75 @@ fn parse_default_form_with_and_without_space() { } } +/// 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!( From 86cd1845bf177d611c65c086b733a0862a0c7b45 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:27:51 +0000 Subject: [PATCH 24/29] fix(auth): read the default form in condition values only where IAM reads it The condition path did not get the document's Version, so the ${key, 'default'} form this branch added expanded in every condition value: in a 2008-10-17 document, in one with no Version, in a trust policy whatever its Version, and under every operator. main compared the whole text as written in all of these, because it took the text between the braces as the key name. The IAM policy simulator reads the default form only in a 2012-10-17 document and under the string and ARN operators (2026-11-03, two samples per row). In a 2008-10-17 document and in one with no Version, a Deny on StringNotEquals aws:PrincipalTag/team "${aws:username, 'x'}" applies to alice tagged team=alice, and an Allow on StringEquals with the same value does not grant until the tag holds the text as written. In a 2012-10-17 document a default does not take part under NumericEquals, DateGreaterThan, Bool or Null, and it does under StringEquals and ArnLike. evaluate_statement_conditions and evaluate_condition_match now take whether the statement's document has Version 2012-10-17. The identity, group, boundary and session policy paths pass their own document's Version, and the trust policy path its own. Where IAM reads the syntax, the default form and the trimmed key are read as in Resource. Elsewhere the text between the braces is the key name as written, as on main, so ${key, 'default'} is compared as written and ${key} expands as it did. The fail-closed check for a variable naming an unresolvable key reads the names the same way, so a defaulted variable outside that scope no longer fails closed. evaluate_condition keeps its signature and reads values as in a 2012-10-17 document. the_default_form_expands_in_a_null_condition becomes the_default_form_is_not_read_in_a_null_condition and repeats the measured Null row. Two rows are not repeated: ExtendDB refuses a document whose IpAddress value is not a CIDR, so the IpAddress row cannot be evaluated, and IAM reads a Bool value written as a variable as false, which ExtendDB does not do for either form. The ${key} form keeps main's behaviour outside a 2012-10-17 document, which #534 tracks, and under the other operators. --- crates/auth/src/policy/condition.rs | 107 +++++- .../src/policy/condition/tests/three_case.rs | 41 ++- .../src/policy/context/request_tag_tests.rs | 26 +- crates/auth/src/policy/evaluator.rs | 42 ++- crates/auth/src/policy/evaluator/tests.rs | 1 + .../tests/condition_variable_scope.rs | 309 ++++++++++++++++++ .../policy/evaluator/tests/variable_values.rs | 30 +- crates/auth/src/policy/variables.rs | 78 +++-- crates/auth/src/policy/variables/tests.rs | 44 ++- crates/server/src/management/assume_role.rs | 107 +++++- 10 files changed, 674 insertions(+), 111 deletions(-) create mode 100644 crates/auth/src/policy/evaluator/tests/condition_variable_scope.rs diff --git a/crates/auth/src/policy/condition.rs b/crates/auth/src/policy/condition.rs index c69e1a124..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,18 +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}` or `${name, 'default'}` policy -/// variable, 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. +/// 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 @@ -207,27 +231,72 @@ fn is_unresolvable(key: &str, context: &impl ConditionContext) -> bool { /// policy variable, a key [`is_unresolvable`] reports. /// /// The names come from [`super::variables::variable_key_names`], which reads -/// the default form the way [`expand_policy_variables`] expands it, so a -/// default written beside such a key does not hide the key from this check. -fn reads_unresolvable_variable(condition: &Condition, context: &impl ConditionContext) -> bool { +/// 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).any(|name| is_unresolvable(name, context)) + super::variables::variable_key_names(value, iam_syntax) + .any(|name| is_unresolvable(name, context)) }) } +/// 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). /// /// `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, @@ -256,9 +325,15 @@ pub fn evaluate_statement_conditions( /// 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`]). -fn expand_policy_variables(value: &str, context: &impl ConditionContext) -> String { - super::variables::expand_condition_value(value, context) +/// ([`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 74d936d8d..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,7 +71,7 @@ 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 ); } @@ -92,7 +101,7 @@ fn variable_naming_unresolvable_key_is_unresolvable_with_or_without_default() { ] { let c = cond(op, "aws:PrincipalTag/team", values.clone()); assert_eq!( - evaluate_condition_match(&c, &ctx), + evaluate_condition_match(&c, true, &ctx), ConditionMatch::Unresolvable, "{values:?}" ); @@ -109,7 +118,7 @@ fn variable_naming_unresolvable_key_is_unresolvable_with_or_without_default() { vec!["${aws:PrincipalOrgID, 'red'}"], ); assert_eq!( - evaluate_condition_match(&c, &present), + evaluate_condition_match(&c, true, &present), ConditionMatch::Match ); // The key absent, not unresolvable: the default is used. @@ -119,7 +128,10 @@ fn variable_naming_unresolvable_key_is_unresolvable_with_or_without_default() { "aws:PrincipalTag/team", vec!["${aws:PrincipalOrgID, 'blue'}"], ); - assert_eq!(evaluate_condition_match(&c, &absent), ConditionMatch::Match); + assert_eq!( + evaluate_condition_match(&c, true, &absent), + ConditionMatch::Match + ); } #[test] @@ -134,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] @@ -157,7 +172,7 @@ fn statement_nomatch_dominates_unresolvable() { ), ]; assert_eq!( - evaluate_statement_conditions(&conds, &ctx), + evaluate_statement_conditions(&conds, true, &ctx), ConditionMatch::NoMatch ); } @@ -180,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 ); } @@ -194,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/request_tag_tests.rs b/crates/auth/src/policy/context/request_tag_tests.rs index e44e87968..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,6 +200,7 @@ 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 @@ -230,22 +232,22 @@ fn ambiguous_request_tag_as_a_defaulted_policy_variable_fails_the_statement_clos [("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 ); } // One spelling expands to its value, and the default is not used. assert_eq!( - evaluate_statement_conditions(&deny, &with_resource_tag(&[("Env", "prod")])), + evaluate_statement_conditions(&deny, true, &with_resource_tag(&[("Env", "prod")])), ConditionMatch::Match ); assert_eq!( - evaluate_statement_conditions(&deny, &with_resource_tag(&[("env", "dev")])), + 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, &with_resource_tag(&[("team", "x")])), + evaluate_statement_conditions(&deny, true, &with_resource_tag(&[("team", "x")])), ConditionMatch::Match ); } @@ -268,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:?}" ); @@ -325,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}" ); @@ -333,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/evaluator.rs b/crates/auth/src/policy/evaluator.rs index f787fffc2..125a01876 100644 --- a/crates/auth/src/policy/evaluator.rs +++ b/crates/auth/src/policy/evaluator.rs @@ -102,7 +102,7 @@ pub fn explicit_deny_matches_arc( stmt.effect == Effect::Deny && action_matches(stmt, action) && resource_matches(policy, stmt, resource_arn, context) - && deny_conditions_apply(stmt, context) + && deny_conditions_apply(policy, stmt, context) }) }) } @@ -131,7 +131,7 @@ where if stmt.effect == Effect::Deny && action_matches(stmt, action) && resource_matches(policy, stmt, resource_arn, context) - && deny_conditions_apply(stmt, context) + && deny_conditions_apply(policy, stmt, context) { return AuthzDecision::Deny; } @@ -144,7 +144,7 @@ where stmt.effect == Effect::Allow && action_matches(stmt, action) && resource_matches(boundary, stmt, resource_arn, context) - && allow_conditions_grant(stmt, context) + && allow_conditions_grant(boundary, stmt, context) }); if !boundary_allows { return AuthzDecision::Deny; @@ -157,7 +157,7 @@ where stmt.effect == Effect::Allow && action_matches(stmt, action) && resource_matches(session, stmt, resource_arn, context) - && allow_conditions_grant(stmt, context) + && allow_conditions_grant(session, stmt, context) }); if !session_allows { return AuthzDecision::Deny; @@ -170,7 +170,7 @@ where if stmt.effect == Effect::Allow && action_matches(stmt, action) && resource_matches(policy, stmt, resource_arn, context) - && allow_conditions_grant(stmt, context) + && allow_conditions_grant(policy, stmt, context) { return AuthzDecision::Allow; } @@ -234,26 +234,44 @@ fn resource_matches( } } -/// 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 afd0e38c0..036c406df 100644 --- a/crates/auth/src/policy/evaluator/tests.rs +++ b/crates/auth/src/policy/evaluator/tests.rs @@ -59,6 +59,7 @@ fn parse(json: &str) -> PolicyDocument { mod case_three; mod combined; +mod condition_variable_scope; mod core; mod resource_variables; mod resource_variables_fields; 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/variable_values.rs b/crates/auth/src/policy/evaluator/tests/variable_values.rs index fb06a4596..7f3ec7099 100644 --- a/crates/auth/src/policy/evaluator/tests/variable_values.rs +++ b/crates/auth/src/policy/evaluator/tests/variable_values.rs @@ -4,14 +4,16 @@ //! 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, `IfExists` and `Null`. +//! multivalued key, under the set operators and `IfExists`, and the default +//! form under `Null`, which does not read it. //! -//! 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 -//! not measured. +//! 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::*; @@ -145,16 +147,16 @@ fn the_default_form_expands_under_set_operators_and_if_exists() { } #[test] -fn the_default_form_expands_in_a_null_condition() { - // With no `flag` tag the value is `false`, so the Allow needs `team`. - let p = allow_get_if( +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/flag, 'false'}", + "${aws:PrincipalTag/f, 'true'}", ); - let with_team = Ctx::empty().with("aws:PrincipalTag/team", vec!["blue"]); - assert_eq!(get(&p, "T", &with_team), AuthzDecision::Allow); - assert_eq!(get(&p, "T", &Ctx::empty()), AuthzDecision::Deny); + assert_eq!(get(&p, "T", &Ctx::empty()), AuthzDecision::Allow); } // --- A multivalued key in a condition value, under every operator form --- diff --git a/crates/auth/src/policy/variables.rs b/crates/auth/src/policy/variables.rs index 0cabed21d..4fab6d4a8 100644 --- a/crates/auth/src/policy/variables.rs +++ b/crates/auth/src/policy/variables.rs @@ -176,7 +176,13 @@ enum Part<'a> { /// Split `value` into plain text and `${...}` references. A `${` with no /// closing `}` is plain text, as before. -fn parts(value: &str) -> Vec> { +/// +/// 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("${") { @@ -185,7 +191,16 @@ fn parts(value: &str) -> Vec> { match after_start.find('}') { Some(end) => { let raw = &rest[start..start + 3 + end]; - out.push(Part::Ref(raw, parse_reference(&after_start[..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 => { @@ -248,7 +263,7 @@ fn resolve<'a>( 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) { + 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))), @@ -307,37 +322,52 @@ pub fn resource_pattern_is_unresolvable( } /// The key names the `${key}` and `${key, 'default'}` variables in `value` -/// read, in the order [`expand_condition_value`] reads them. +/// read, in the order [`expand_condition_value`] reads them with the same +/// `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}`. 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) -> impl Iterator { - parts(value).into_iter().filter_map(|part| match part { - Part::Ref(_, Reference::Key { name, .. }) => Some(name), - Part::Text(_) | Part::Ref(_, Reference::Escape(_)) => None, - }) +/// 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}` and `${key, 'default'}` are replaced by their value; 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`]). +/// `${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, context: &impl ConditionContext) -> String { +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) { + 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 }) => { diff --git a/crates/auth/src/policy/variables/tests.rs b/crates/auth/src/policy/variables/tests.rs index a4f2b8636..f45e0a474 100644 --- a/crates/auth/src/policy/variables/tests.rs +++ b/crates/auth/src/policy/variables/tests.rs @@ -450,15 +450,15 @@ fn unclosed_brace_is_literal_text() { fn condition_value_expands_variable_and_default() { let ctx = team_blue(); assert_eq!( - expand_condition_value("team-${aws:PrincipalTag/team}", &ctx), + expand_condition_value("team-${aws:PrincipalTag/team}", true, &ctx), "team-blue" ); assert_eq!( - expand_condition_value("${aws:username, 'anon'}", &ctx), + expand_condition_value("${aws:username, 'anon'}", true, &ctx), "anon" ); assert_eq!( - expand_condition_value("${aws:PrincipalTag/team, 'none'}", &ctx), + expand_condition_value("${aws:PrincipalTag/team, 'none'}", true, &ctx), "blue" ); } @@ -467,21 +467,22 @@ fn condition_value_expands_variable_and_default() { fn condition_value_leaves_unresolved_and_escapes_literal() { let ctx = Ctx::empty(); assert_eq!( - expand_condition_value("${aws:username}", &ctx), + expand_condition_value("${aws:username}", true, &ctx), "${aws:username}" ); assert_eq!( - expand_condition_value("a${*}b${?}c${$}", &ctx), + expand_condition_value("a${*}b${?}c${$}", true, &ctx), "a${*}b${?}c${$}" ); - assert_eq!(expand_condition_value("plain", &ctx), "plain"); - assert_eq!(expand_condition_value("${open", &ctx), "${open"); + 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!( @@ -494,16 +495,39 @@ fn variable_key_names_strip_the_default_and_skip_escapes() { ); // 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}").collect(); + let names: Vec<&str> = variable_key_names("${a, b}", true).collect(); assert_eq!(names, ["a, b"]); - assert_eq!(variable_key_names("plain").count(), 0); + 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", &ctx), + 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)); + } } From 70243191f8c08b69fd2bc5155f04806bb20322df Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:30:05 +0000 Subject: [PATCH 25/29] test(auth): pin IAM's answers for an empty tag value in a Resource pattern A tag whose value is the empty string adds no characters to a Resource or NotResource pattern, so table/${aws:PrincipalTag/team}* reads as table/* for a caller tagged team="", while with no team tag the pattern matches nothing. The IAM policy simulator gives the same answers (2026-11-03, two samples per row): with team="" an Allow-only table/${team}* grants red and blue, a Deny on it denies red, a Deny NotResource on it spares red, and an Allow-only *${suffix} with suffix="" grants red. With the team tag absent the same Allow gives implicitDeny. table/${team}x and x${team} match table/x, and x${team}y matches table/xy. A StringLike condition "?*" on aws:PrincipalTag/team beside table/${team}* refuses the empty value and grants team=blue on blue-x. The behaviour does not change. evaluator/tests/empty_values.rs repeats the rows. --- crates/auth/src/policy/evaluator/tests.rs | 1 + .../policy/evaluator/tests/empty_values.rs | 110 ++++++++++++++++++ 2 files changed, 111 insertions(+) create mode 100644 crates/auth/src/policy/evaluator/tests/empty_values.rs diff --git a/crates/auth/src/policy/evaluator/tests.rs b/crates/auth/src/policy/evaluator/tests.rs index 036c406df..ecb25aab5 100644 --- a/crates/auth/src/policy/evaluator/tests.rs +++ b/crates/auth/src/policy/evaluator/tests.rs @@ -61,6 +61,7 @@ mod case_three; mod combined; mod condition_variable_scope; mod core; +mod empty_values; mod resource_variables; mod resource_variables_fields; mod variable_spelling; 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" + ); +} From f4148a4ba7720e50eb787d0a108084b5b5295d66 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:30:05 +0000 Subject: [PATCH 26/29] test(auth): say that the region and account field rows contradict IAM's page resource_variables_fields.rs pins, from two simulator rows, that variables in the region and account fields of a Resource ARN expand. IAM's policy variables page says a variable cannot replace parts of the ARN before the fifth colon. The comment now says that the tests follow the measurement, and that a variable in the service field goes through the same code without a measurement, so neither rule is changed without measuring both. --- .../src/policy/evaluator/tests/resource_variables_fields.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs b/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs index 20ffdf87f..04e98107d 100644 --- a/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs +++ b/crates/auth/src/policy/evaluator/tests/resource_variables_fields.rs @@ -133,6 +133,12 @@ fn a_condition_value_naming_a_multivalued_key_with_a_default_uses_it_only_when_a // --- 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() { From ada402c87de182bf623e8908f421434e7aa00108 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:31:36 +0000 Subject: [PATCH 27/29] docs(auth): describe the trimmed key, the default form's scope and empty values The security model says where a condition value reads the default form (a 2012-10-17 document and a string or ARN operator, a trust policy by its own Version), that code points up to U+0020 around a key name are ignored, and that a tag whose value is the empty string expands to no characters, so table/${aws:PrincipalTag/team}* matches every table for a caller tagged team="". It names the StringLike "?*" condition on the tag that refuses such a caller, which the IAM policy simulator confirms. The v0.1.15 upgrade entry limits item 3 to the same scope and adds item 4 for the trimmed key name. Its Version paragraph now says that a stored document with another Version is refused when it is sent again unchanged, that a stored Deny with a Resource variable in such a document keeps denying what the pattern as written matches until its Version is changed to 2012-10-17, and how to find such documents before upgrading. The auth design note gives the default form's scope. --- docs/design/05-component-auth.md | 2 +- docs/manuals/07-upgrade-manual.md | 37 ++++++++++++++++++++++--------- docs/manuals/10-security-model.md | 4 ++-- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/docs/design/05-component-auth.md b/docs/design/05-component-auth.md index df7ce422a..b9e6bc65d 100755 --- a/docs/design/05-component-auth.md +++ b/docs/design/05-component-auth.md @@ -501,7 +501,7 @@ pub enum ConditionOperator { } ``` -> **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 (`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. +> **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:** diff --git a/docs/manuals/07-upgrade-manual.md b/docs/manuals/07-upgrade-manual.md index 24dc78694..c3827b407 100755 --- a/docs/manuals/07-upgrade-manual.md +++ b/docs/manuals/07-upgrade-manual.md @@ -341,7 +341,7 @@ 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). Three decisions change for stored policies: +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 @@ -358,13 +358,24 @@ issue #533). Three decisions change for stored policies: `${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. A condition value written as `${key, 'default'}` 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. +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, IP address, `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 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. `Deny table/${ aws:PrincipalTag/team }` now + denies a caller tagged `team=blue` the table `blue`. Before, the spaces were + part of the key name, so the variable named a key no request carries and the + `Deny` denied 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 @@ -373,8 +384,14 @@ 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. A session policy passed to `AssumeRole` is checked -only for its `Version` and `Statement` members. +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 diff --git a/docs/manuals/10-security-model.md b/docs/manuals/10-security-model.md index c1c7c52cb..b225765b9 100755 --- a/docs/manuals/10-security-model.md +++ b/docs/manuals/10-security-model.md @@ -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. The `${key, 'default'}` form expands to `default` when the request does not carry `key`. A multivalued key such as `dynamodb:LeadingKeys` is not expanded. +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. 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. +- `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 statement (measured with the IAM policy simulator). - `Condition`: Full condition block support (see operators and keys above) ## Transport Security From a6826763bf6c76f8811650a38da5eb679119c162 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:45:15 +0000 Subject: [PATCH 28/29] test: add wire tests for Resource variables and the Version write check No wire test put a variable in a Resource or NotResource pattern or sent a refused Version, so a change to how the server builds the request context for the resource check, or to the console's validation, would pass CI. tests/test_resource_policy_variables.py adds two classes. TestResourceVariable gives a user tagged team=blue an Allow dynamodb:* on * and a Deny dynamodb:GetItem on table/${aws:PrincipalTag/team}, written as Resource and as NotResource, and reads the tables blue and red: under Resource the read of blue is denied and red is allowed, and under NotResource the reverse, as the IAM policy simulator rows in the PR description give. TestVersionRefused sends documents with Version "2012-10-18", "2012-10-17 " and 20121017 through the user, group and role policy writes and the user and role boundary writes of the management API, and through the console's user, group and role policy forms. Each is refused, nothing is stored, and the same document with Version 2008-10-17 is stored. Both classes provision IAM principals through the management API, so they run against ExtendDB only; the module docstring says so. On a SQLite server built from this branch the 26 tests pass. --- tests/test_resource_policy_variables.py | 256 ++++++++++++++++++++++++ 1 file changed, 256 insertions(+) create mode 100644 tests/test_resource_policy_variables.py 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 From 054c262d3c00e87c66cc413b9f52d7155b39c304 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Tue, 3 Nov 2026 12:46:14 +0000 Subject: [PATCH 29/29] docs(auth): correct the upgrade and security notes for policy variables The upgrade entry's example for the trimmed key name used a Resource pattern, but a Resource variable was not expanded at all before this change, so the old answer came from the literal pattern, not from the spaces. The example is now a StringEquals condition value, where a released build read the spaces as part of the key name. The entry also listed IpAddress among the operators that compare the text as written; a policy or boundary write refuses an IpAddress value written as a variable as a malformed CIDR, so the value never reaches evaluation. The "Who is affected" paragraph now counts the condition value changes in items 2 to 4. The security model's guard for an empty tag value applies to an Allow statement. A Deny NotResource statement with such a pattern spares every table for that caller, as the IAM policy simulator shows, and the same condition on it changes nothing. --- docs/manuals/07-upgrade-manual.md | 23 +++++++++++++---------- docs/manuals/10-security-model.md | 2 +- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/docs/manuals/07-upgrade-manual.md b/docs/manuals/07-upgrade-manual.md index c3827b407..3fe989dfc 100755 --- a/docs/manuals/07-upgrade-manual.md +++ b/docs/manuals/07-upgrade-manual.md @@ -366,16 +366,19 @@ issue #533). Four decisions change for stored policies: 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, IP address, `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 role - trust policy follows its own `Version`. + `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. `Deny table/${ aws:PrincipalTag/team }` now - denies a caller tagged `team=blue` the table `blue`. Before, the spaces were - part of the key name, so the variable named a key no request carries and the - `Deny` denied nothing. + 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 @@ -395,8 +398,8 @@ 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 two condition -value changes, role trust policies. Search the stored policies for `${` +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 diff --git a/docs/manuals/10-security-model.md b/docs/manuals/10-security-model.md index b225765b9..0c1ee414f 100755 --- a/docs/manuals/10-security-model.md +++ b/docs/manuals/10-security-model.md @@ -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. 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 statement (measured with the IAM policy simulator). +- `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