mirror of
https://github.com/rustfs/rustfs.git
synced 2026-10-27 06:18:43 +00:00
feat(iam): support wildcard in IAM policy resources (#8555)
Allow wildcard in IAM policy
This commit is contained in:
@@ -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<Self, Self::Error> {
|
||||
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")]
|
||||
|
||||
@@ -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() {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user