From 65ea7db6a91a10d4e38e90f83ef51ef7908a70c8 Mon Sep 17 00:00:00 2001 From: alpbeysir <94457197+alpbeysir@users.noreply.github.com> Date: Wed, 14 Oct 2026 23:58:27 +0100 Subject: [PATCH] feat(iam): support wildcard in IAM policy resources (#8555) Allow wildcard in IAM policy --- crates/policy/src/policy/resource.rs | 44 ++++++-- crates/policy/src/policy/statement.rs | 13 ++- crates/policy/tests/policy_is_allowed.rs | 125 +++++++++++++++++++++++ 3 files changed, 174 insertions(+), 8 deletions(-) diff --git a/crates/policy/src/policy/resource.rs b/crates/policy/src/policy/resource.rs index 262b13f4e..6d203accc 100644 --- a/crates/policy/src/policy/resource.rs +++ b/crates/policy/src/policy/resource.rs @@ -38,11 +38,7 @@ impl Serialize for ResourceSet { let mut seq = serializer.serialize_seq(Some(self.0.len()))?; for resource in &self.0 { - let resource_str = match resource { - Resource::S3(value) => format!("{}{}", Resource::S3_PREFIX, value), - Resource::Kms(value) => format!("{}{}", Resource::KMS_PREFIX, value), - }; - seq.serialize_element(&resource_str)?; + seq.serialize_element(resource)?; } seq.end() } @@ -165,6 +161,7 @@ impl<'de> Deserialize<'de> for ResourceSet { #[derive(Hash, Eq, PartialEq, Clone, Debug)] pub enum Resource { + All, S3(String), Kms(String), } @@ -197,6 +194,7 @@ impl Resource { resolver: Option<&dyn PolicyVariableResolver>, ) -> bool { let pattern = match self { + Resource::All => return true, Resource::S3(s) => s.to_owned(), Resource::Kms(s) => s.to_owned(), }; @@ -242,7 +240,9 @@ impl Resource { impl TryFrom<&str> for Resource { type Error = Error; fn try_from(value: &str) -> std::result::Result { - let resource = if let Some(suffix) = value.strip_prefix(Self::S3_PREFIX) { + let resource = if value == "*" { + Resource::All + } else if let Some(suffix) = value.strip_prefix(Self::S3_PREFIX) { Resource::S3(suffix.into()) } else if let Some(suffix) = value.strip_prefix(Self::KMS_PREFIX) { Resource::Kms(suffix.into()) @@ -259,6 +259,7 @@ impl Validator for Resource { type Error = Error; fn is_valid(&self) -> std::result::Result<(), Error> { match self { + Self::All => {} Self::S3(pattern) => { if pattern.is_empty() || pattern.starts_with('/') { return Err(IamError::InvalidResource("s3".into(), pattern.into()).into()); @@ -290,6 +291,7 @@ impl Serialize for Resource { S: serde::Serializer, { match self { + Resource::All => serializer.serialize_str("*"), Resource::S3(s) => serializer.serialize_str(&format!("{}{}", Self::S3_PREFIX, s)), Resource::Kms(s) => serializer.serialize_str(&format!("{}{}", Self::KMS_PREFIX, s)), } @@ -312,6 +314,10 @@ mod tests { use std::collections::HashMap; use test_case::test_case; + #[test_case("*", "mybucket" => true; "all buckets")] + #[test_case("*", "mybucket/myobject" => true; "all objects")] + #[test_case("*", "key/mykey" => true; "all keys")] + #[test_case("*", "" => true; "unscoped action")] #[test_case("arn:aws:s3:::*","mybucket" => true; "1")] #[test_case("arn:aws:s3:::*","mybucket/myobject" => true; "2")] #[test_case("arn:aws:s3:::mybucket*","mybucket" => true; "3")] @@ -346,6 +352,32 @@ mod tests { pollster::block_on(resource.is_match(object, &HashMap::new())) } + #[test] + fn test_wildcard_resource_serialization_round_trip() { + use crate::policy::resource::ResourceSet; + + let resource: Resource = serde_json::from_str(r#""*""#).expect("wildcard resource should parse"); + assert_eq!(serde_json::to_string(&resource).expect("resource should serialize"), r#""*""#); + + for json in [r#""*""#, r#"["*"]"#, r#"["*","*"]"#] { + let set: ResourceSet = serde_json::from_str(json).expect("wildcard resource set should parse"); + assert_eq!(set.len(), 1, "duplicate wildcard resources should be removed"); + let serialized = serde_json::to_string(&set).expect("resource set should serialize"); + assert_eq!(serialized, r#"["*"]"#); + let round_trip: ResourceSet = serde_json::from_str(&serialized).expect("resource set should parse again"); + assert_eq!(round_trip, set); + } + } + + #[test_case(""; "empty resource")] + #[test_case("**"; "multiple wildcards")] + #[test_case("*/*"; "unqualified path")] + #[test_case(" *"; "leading whitespace")] + #[test_case("arn:aws:unknown:::*"; "unknown service")] + fn test_invalid_resource_is_rejected(resource: &str) { + assert!(Resource::try_from(resource).is_err(), "invalid resource must not become a wildcard"); + } + #[test_case("arn:aws:kms:::key/mykey" => true; "key id parses")] #[test_case("arn:aws:kms:::key/*" => true; "key wildcard parses")] #[test_case("arn:aws:kms:::key/app-key.v2" => true; "key id with dot parses")] diff --git a/crates/policy/src/policy/statement.rs b/crates/policy/src/policy/statement.rs index 5b03aca02..960fb0a63 100644 --- a/crates/policy/src/policy/statement.rs +++ b/crates/policy/src/policy/statement.rs @@ -182,15 +182,24 @@ impl Statement { /// left empty. An empty `args.object` means the caller did not scope the request to a /// key, which preserves the legacy match-every-key behaviour. async fn kms_key_scope_matches(&self, args: &Args<'_>, resolver: &VariableResolver) -> bool { + if self.not_resources.iter().any(|resource| matches!(resource, Resource::All)) { + return false; + } + if matches!(args.action, Action::KmsAction(KmsAction::BackupAction | KmsAction::RestoreAction)) - && (!self.resources.is_empty() || !self.not_resources.is_empty()) + && ((!self.resources.is_empty() && !self.resources.iter().any(|resource| matches!(resource, Resource::All))) + || !self.not_resources.is_empty()) { // Global bundle operations require an unscoped Allow, while a Deny // covering any key must still block an operation covering every key. return matches!(self.effect, Effect::Deny); } - let kms_resources: Vec<&Resource> = self.resources.iter().filter(|resource| resource.is_kms()).collect(); + let kms_resources: Vec<&Resource> = self + .resources + .iter() + .filter(|resource| matches!(resource, Resource::All | Resource::Kms(_))) + .collect(); let kms_not_resources: Vec<&Resource> = self.not_resources.iter().filter(|resource| resource.is_kms()).collect(); if kms_resources.len() != self.resources.len() || kms_not_resources.len() != self.not_resources.len() { diff --git a/crates/policy/tests/policy_is_allowed.rs b/crates/policy/tests/policy_is_allowed.rs index 868f25072..732ddfccd 100644 --- a/crates/policy/tests/policy_is_allowed.rs +++ b/crates/policy/tests/policy_is_allowed.rs @@ -808,3 +808,128 @@ fn merge_keeps_allow_statements_differing_only_in_not_resource() { "both Allow statements must survive the merge; neither may be dropped as a duplicate" ); } + +#[test] +fn wildcard_deny_only_session_grants_no_storage_access() { + let policy = Policy::parse_config( + br#"{"Statement":[{"Action":["s3:GetObject"],"Effect":"Deny","Resource":"*","Sid":"1"}],"Version":"2012-10-17"}"#, + ) + .expect("deny-only session policy should parse"); + let encoded = serde_json::to_vec(&policy).expect("session policy should serialize into claims"); + let decoded = Policy::parse_config(&encoded).expect("session policy should parse from claims"); + for action in ["s3:GetObject", "s3:PutObject", "s3:DeleteObject", "s3:ListBucket"] { + for bucket in ["bucket", "other-bucket"] { + assert!( + !policy_is_allowed( + decoded.clone(), + ArgsBuilder { + action: action.into(), + bucket: bucket.into(), + object: "data/object".into(), + ..Default::default() + } + ), + "deny-only session must not grant {action} on {bucket}" + ); + } + } +} + +#[test_case("s3:GetObject", "bucket", "object" => true; "matching object action")] +#[test_case("s3:PutObject", "bucket", "object" => false; "ungranted action")] +#[test_case("s3:GetObject", "other-bucket", "object" => true; "other bucket")] +fn wildcard_resource_allow_preserves_action_scope(action: &str, bucket: &str, object: &str) -> bool { + let policy = Policy::parse_config( + br#"{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:GetObject","Resource":["*"]}]}"#, + ) + .expect("wildcard allow policy should parse"); + policy_is_allowed( + policy, + ArgsBuilder { + action: action.into(), + bucket: bucket.into(), + object: object.into(), + ..Default::default() + }, + ) +} + +#[test_case("s3:GetObject", "bucket", "object"; "object")] +#[test_case("s3:ListBucket", "bucket", ""; "bucket")] +#[test_case("s3:ListAllMyBuckets", "", ""; "all buckets")] +#[test_case("kms:Decrypt", "", "mykey"; "kms key")] +#[test_case("kms:Backup", "", ""; "kms backup")] +#[test_case("kms:Restore", "", ""; "kms restore")] +fn wildcard_resource_allow_deny_and_exclusion(action: &str, bucket: &str, object: &str) { + let other_resource = if action.starts_with("kms:") { + "arn:aws:kms:::key/other-key" + } else { + "arn:aws:s3:::other-bucket/*" + }; + for (field, resource) in [ + ("Resource", serde_json::json!("*")), + ("Resource", serde_json::json!(["*"])), + ("Resource", serde_json::json!([other_resource, "*"])), + ("NotResource", serde_json::json!("*")), + ("NotResource", serde_json::json!([other_resource, "*"])), + ] { + let mut json = serde_json::json!({ + "Version": "2012-10-17", + "Statement": [{"Effect": "Allow", "Action": action, field: resource}] + }); + let allow = Policy::parse_config(&serde_json::to_vec(&json).expect("test policy should serialize")) + .expect("wildcard policy should parse"); + let args = ArgsBuilder { + action: action.into(), + bucket: bucket.into(), + object: object.into(), + ..Default::default() + }; + assert_eq!(policy_is_allowed(allow, args), field == "Resource", "{action} with {field}"); + + json["Statement"][0]["Effect"] = serde_json::json!("Deny"); + json["Statement"] + .as_array_mut() + .expect("statements are an array") + .push(serde_json::json!({ + "Effect": "Allow", "Action": action, "Resource": "*" + })); + let deny = Policy::parse_config(&serde_json::to_vec(&json).expect("test policy should serialize")) + .expect("wildcard deny policy should parse"); + assert_eq!( + policy_is_allowed( + deny, + ArgsBuilder { + action: action.into(), + bucket: bucket.into(), + object: object.into(), + ..Default::default() + } + ), + field == "NotResource", + "explicit deny must win unless {field} excludes the resource for {action}" + ); + } +} + +#[test_case(Some("allowed/data/") => true; "matching prefix")] +#[test_case(Some("other/data/") => false; "outside allowed prefix")] +#[test_case(None => false; "missing prefix")] +fn wildcard_resource_preserves_conditions(prefix: Option<&str>) -> bool { + let policy = Policy::parse_config( + br#"{"Version":"2012-10-17","Statement":[{ + "Effect":"Allow","Action":"s3:ListBucket","Resource":"*", + "Condition":{"StringLike":{"s3:prefix":"allowed/*"}} + }]}"#, + ) + .expect("conditional wildcard policy should parse"); + let mut args = ArgsBuilder { + action: "s3:ListBucket".into(), + bucket: "bucket".into(), + ..Default::default() + }; + if let Some(prefix) = prefix { + args.conditions.insert("prefix".into(), vec![prefix.into()]); + } + policy_is_allowed(policy, args) +}