feat(policy): add Service principal, ArnLike/IfExists conditions, and logging error ordering (#2018)

This commit is contained in:
安正超
2026-03-01 08:44:42 +08:00
committed by GitHub
parent 2c01b8c49d
commit 7eb136faf0
8 changed files with 246 additions and 76 deletions
+4
View File
@@ -266,6 +266,10 @@ pub enum S3Action {
PutBucketLifecycleAction,
#[strum(serialize = "s3:GetBucketLifecycle")]
GetBucketLifecycleAction,
#[strum(serialize = "s3:PutBucketLogging")]
PutBucketLoggingAction,
#[strum(serialize = "s3:GetBucketLogging")]
GetBucketLoggingAction,
#[strum(serialize = "s3:PutBucketNotification")]
PutBucketNotificationAction,
#[strum(serialize = "s3:PutBucketPolicy")]
+3 -3
View File
@@ -82,17 +82,17 @@ impl Serialize for Functions {
serializer.serialize_map(Some(self.for_any_value.len() + self.for_all_values.len() + self.for_normal.len()))?;
for conditions in self.for_all_values.iter() {
se.serialize_key(format!("ForAllValues:{}", conditions.to_key()).as_str())?;
se.serialize_key(&format!("ForAllValues:{}", conditions.to_key_with_suffix()))?;
conditions.serialize_map(&mut se)?;
}
for conditions in self.for_any_value.iter() {
se.serialize_key(format!("ForAnyValue:{}", conditions.to_key()).as_str())?;
se.serialize_key(&format!("ForAnyValue:{}", conditions.to_key_with_suffix()))?;
conditions.serialize_map(&mut se)?;
}
for conditions in self.for_normal.iter() {
se.serialize_key(conditions.to_key())?;
se.serialize_key(&conditions.to_key_with_suffix())?;
conditions.serialize_map(&mut se)?;
}
+117 -33
View File
@@ -29,6 +29,10 @@ pub enum Condition {
StringNotEqualsIgnoreCase(StringFunc),
StringLike(StringFunc),
StringNotLike(StringFunc),
ArnLike(StringFunc),
ArnNotLike(StringFunc),
ArnEquals(StringFunc),
ArnNotEquals(StringFunc),
BinaryEquals(BinaryFunc),
IpAddress(AddrFunc),
NotIpAddress(AddrFunc),
@@ -47,6 +51,10 @@ pub enum Condition {
DateLessThanEquals(DateFunc),
DateGreaterThan(DateFunc),
DateGreaterThanEquals(DateFunc),
/// Wraps any condition with IfExists semantics: if none of the
/// referenced keys are present in the request context, the
/// condition evaluates to true.
IfExists(Box<Condition>),
}
impl Condition {
@@ -58,6 +66,10 @@ impl Condition {
"StringNotEqualsIgnoreCase" => Self::StringNotEqualsIgnoreCase(d.next_value()?),
"StringLike" => Self::StringLike(d.next_value()?),
"StringNotLike" => Self::StringNotLike(d.next_value()?),
"ArnLike" => Self::ArnLike(d.next_value()?),
"ArnNotLike" => Self::ArnNotLike(d.next_value()?),
"ArnEquals" => Self::ArnEquals(d.next_value()?),
"ArnNotEquals" => Self::ArnNotEquals(d.next_value()?),
"BinaryEquals" => Self::BinaryEquals(d.next_value()?),
"IpAddress" => Self::IpAddress(d.next_value()?),
"NotIpAddress" => Self::NotIpAddress(d.next_value()?),
@@ -74,6 +86,11 @@ impl Condition {
"DateLessThanEquals" => Self::DateLessThanEquals(d.next_value()?),
"DateGreaterThan" => Self::DateGreaterThan(d.next_value()?),
"DateGreaterThanEquals" => Self::DateGreaterThanEquals(d.next_value()?),
_ if key.ends_with("IfExists") => {
let base = key.strip_suffix("IfExists").unwrap_or(key);
let inner = Self::from_deserializer(base, d)?;
Self::IfExists(Box::new(inner))
}
_ => Err(Error::custom(format!("unknown key: {key}")))?,
})
}
@@ -86,6 +103,10 @@ impl Condition {
Condition::StringNotEqualsIgnoreCase(_) => "StringNotEqualsIgnoreCase",
Condition::StringLike(_) => "StringLike",
Condition::StringNotLike(_) => "StringNotLike",
Condition::ArnLike(_) => "ArnLike",
Condition::ArnNotLike(_) => "ArnNotLike",
Condition::ArnEquals(_) => "ArnEquals",
Condition::ArnNotEquals(_) => "ArnNotEquals",
Condition::BinaryEquals(_) => "BinaryEquals",
Condition::IpAddress(_) => "IpAddress",
Condition::NotIpAddress(_) => "NotIpAddress",
@@ -104,45 +125,98 @@ impl Condition {
Condition::DateLessThanEquals(_) => "DateLessThanEquals",
Condition::DateGreaterThan(_) => "DateGreaterThan",
Condition::DateGreaterThanEquals(_) => "DateGreaterThanEquals",
Condition::IfExists(_) => "IfExists",
}
}
pub async fn evaluate_with_resolver(
&self,
for_all: bool,
values: &HashMap<String, Vec<String>>,
resolver: Option<&dyn PolicyVariableResolver>,
) -> bool {
pub fn to_key_with_suffix(&self) -> String {
match self {
Condition::IfExists(inner) => format!("{}IfExists", inner.to_key()),
_ => self.to_key().to_owned(),
}
}
pub fn has_any_key_in(&self, values: &HashMap<String, Vec<String>>) -> bool {
use Condition::*;
match self {
StringEquals(s)
| StringNotEquals(s)
| StringEqualsIgnoreCase(s)
| StringNotEqualsIgnoreCase(s)
| StringLike(s)
| StringNotLike(s)
| ArnLike(s)
| ArnNotLike(s)
| ArnEquals(s)
| ArnNotEquals(s) => s.key_names().any(|k| values.contains_key(k.as_str())),
BinaryEquals(s) => s.key_names().any(|k| values.contains_key(k.as_str())),
IpAddress(s) | NotIpAddress(s) => s.key_names().any(|k| values.contains_key(k.as_str())),
Null(s) | Bool(s) => s.key_names().any(|k| values.contains_key(k.as_str())),
NumericEquals(s)
| NumericNotEquals(s)
| NumericLessThan(s)
| NumericLessThanEquals(s)
| NumericGreaterThan(s)
| NumericGreaterThanIfExists(s)
| NumericGreaterThanEquals(s) => s.key_names().any(|k| values.contains_key(k.as_str())),
DateEquals(s)
| DateNotEquals(s)
| DateLessThan(s)
| DateLessThanEquals(s)
| DateGreaterThan(s)
| DateGreaterThanEquals(s) => s.key_names().any(|k| values.contains_key(k.as_str())),
IfExists(inner) => inner.has_any_key_in(values),
}
}
let r = match self {
StringEquals(s) => s.evaluate_with_resolver(for_all, false, false, false, values, resolver).await,
StringNotEquals(s) => s.evaluate_with_resolver(for_all, false, false, true, values, resolver).await,
StringEqualsIgnoreCase(s) => s.evaluate_with_resolver(for_all, true, false, false, values, resolver).await,
StringNotEqualsIgnoreCase(s) => s.evaluate_with_resolver(for_all, true, false, true, values, resolver).await,
StringLike(s) => s.evaluate_with_resolver(for_all, false, true, false, values, resolver).await,
StringNotLike(s) => s.evaluate_with_resolver(for_all, false, true, true, values, resolver).await,
BinaryEquals(s) => s.evaluate(values),
IpAddress(s) => s.evaluate(values),
NotIpAddress(s) => s.evaluate(values),
Null(s) => s.evaluate_null(values),
Bool(s) => s.evaluate_bool(values),
NumericEquals(s) => s.evaluate(i64::eq, false, values),
NumericNotEquals(s) => s.evaluate(i64::ne, false, values),
NumericLessThan(s) => s.evaluate(i64::lt, false, values),
NumericLessThanEquals(s) => s.evaluate(i64::le, false, values),
NumericGreaterThan(s) => s.evaluate(i64::gt, false, values),
NumericGreaterThanIfExists(s) => s.evaluate(i64::ge, true, values),
NumericGreaterThanEquals(s) => s.evaluate(i64::ge, false, values),
DateEquals(s) => s.evaluate(OffsetDateTime::eq, values),
DateNotEquals(s) => s.evaluate(OffsetDateTime::ne, values),
DateLessThan(s) => s.evaluate(OffsetDateTime::lt, values),
DateLessThanEquals(s) => s.evaluate(OffsetDateTime::le, values),
DateGreaterThan(s) => s.evaluate(OffsetDateTime::gt, values),
DateGreaterThanEquals(s) => s.evaluate(OffsetDateTime::ge, values),
};
pub fn evaluate_with_resolver<'a>(
&'a self,
for_all: bool,
values: &'a HashMap<String, Vec<String>>,
resolver: Option<&'a dyn PolicyVariableResolver>,
) -> std::pin::Pin<Box<dyn std::future::Future<Output = bool> + Send + 'a>> {
Box::pin(async move {
use Condition::*;
if self.is_negate() { !r } else { r }
let r = match self {
StringEquals(s) => s.evaluate_with_resolver(for_all, false, false, false, values, resolver).await,
StringNotEquals(s) => s.evaluate_with_resolver(for_all, false, false, true, values, resolver).await,
StringEqualsIgnoreCase(s) => s.evaluate_with_resolver(for_all, true, false, false, values, resolver).await,
StringNotEqualsIgnoreCase(s) => s.evaluate_with_resolver(for_all, true, false, true, values, resolver).await,
StringLike(s) => s.evaluate_with_resolver(for_all, false, true, false, values, resolver).await,
StringNotLike(s) => s.evaluate_with_resolver(for_all, false, true, true, values, resolver).await,
ArnLike(s) => s.evaluate_with_resolver(for_all, false, true, false, values, resolver).await,
ArnNotLike(s) => s.evaluate_with_resolver(for_all, false, true, true, values, resolver).await,
ArnEquals(s) => s.evaluate_with_resolver(for_all, false, false, false, values, resolver).await,
ArnNotEquals(s) => s.evaluate_with_resolver(for_all, false, false, true, values, resolver).await,
BinaryEquals(s) => s.evaluate(values),
IpAddress(s) => s.evaluate(values),
NotIpAddress(s) => s.evaluate(values),
Null(s) => s.evaluate_null(values),
Bool(s) => s.evaluate_bool(values),
NumericEquals(s) => s.evaluate(i64::eq, false, values),
NumericNotEquals(s) => s.evaluate(i64::ne, false, values),
NumericLessThan(s) => s.evaluate(i64::lt, false, values),
NumericLessThanEquals(s) => s.evaluate(i64::le, false, values),
NumericGreaterThan(s) => s.evaluate(i64::gt, false, values),
NumericGreaterThanIfExists(s) => s.evaluate(i64::ge, true, values),
NumericGreaterThanEquals(s) => s.evaluate(i64::ge, false, values),
DateEquals(s) => s.evaluate(OffsetDateTime::eq, values),
DateNotEquals(s) => s.evaluate(OffsetDateTime::ne, values),
DateLessThan(s) => s.evaluate(OffsetDateTime::lt, values),
DateLessThanEquals(s) => s.evaluate(OffsetDateTime::le, values),
DateGreaterThan(s) => s.evaluate(OffsetDateTime::gt, values),
DateGreaterThanEquals(s) => s.evaluate(OffsetDateTime::ge, values),
IfExists(inner) => {
if !inner.has_any_key_in(values) {
return true;
}
return inner.evaluate_with_resolver(for_all, values, resolver).await;
}
};
if self.is_negate() { !r } else { r }
})
}
#[inline]
@@ -161,6 +235,10 @@ impl Condition {
Condition::StringNotEqualsIgnoreCase(s) => se.serialize_value(s),
Condition::StringLike(s) => se.serialize_value(s),
Condition::StringNotLike(s) => se.serialize_value(s),
Condition::ArnLike(s) => se.serialize_value(s),
Condition::ArnNotLike(s) => se.serialize_value(s),
Condition::ArnEquals(s) => se.serialize_value(s),
Condition::ArnNotEquals(s) => se.serialize_value(s),
Condition::BinaryEquals(s) => se.serialize_value(s),
Condition::IpAddress(s) => se.serialize_value(s),
Condition::NotIpAddress(s) => se.serialize_value(s),
@@ -179,6 +257,7 @@ impl Condition {
Condition::DateLessThanEquals(s) => se.serialize_value(s),
Condition::DateGreaterThan(s) => se.serialize_value(s),
Condition::DateGreaterThanEquals(s) => se.serialize_value(s),
Condition::IfExists(inner) => inner.serialize_map(se),
}
}
}
@@ -210,6 +289,11 @@ impl PartialEq for Condition {
(Self::DateLessThanEquals(l0), Self::DateLessThanEquals(r0)) => l0 == r0,
(Self::DateGreaterThan(l0), Self::DateGreaterThan(r0)) => l0 == r0,
(Self::DateGreaterThanEquals(l0), Self::DateGreaterThanEquals(r0)) => l0 == r0,
(Self::ArnLike(l0), Self::ArnLike(r0)) => l0 == r0,
(Self::ArnNotLike(l0), Self::ArnNotLike(r0)) => l0 == r0,
(Self::ArnEquals(l0), Self::ArnEquals(r0)) => l0 == r0,
(Self::ArnNotEquals(l0), Self::ArnNotEquals(r0)) => l0 == r0,
(Self::IfExists(l0), Self::IfExists(r0)) => l0 == r0,
_ => false,
}
}
@@ -45,6 +45,12 @@ impl<T: Clone> Clone for InnerFunc<T> {
}
}
impl<T> InnerFunc<T> {
pub fn key_names(&self) -> impl Iterator<Item = String> + '_ {
self.0.iter().map(|kv| kv.key.name())
}
}
impl<T: Serialize> Serialize for InnerFunc<T> {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
@@ -336,6 +336,12 @@ pub enum AwsKeyName {
#[strum(serialize = "aws:groups")]
AWSGroups,
#[strum(serialize = "aws:SourceArn")]
AWSSourceArn,
#[strum(serialize = "aws:SourceAccount")]
AWSSourceAccount,
}
#[cfg(test)]
+82 -34
View File
@@ -19,9 +19,11 @@ use std::collections::HashSet;
/// Principal that serializes AWS field as single string when containing only "*",
/// or as an array otherwise (matching AWS S3 API format).
/// Also supports Service principals (e.g., "logging.s3.amazonaws.com").
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct Principal {
aws: HashSet<String>,
service: HashSet<String>,
}
impl Serialize for Principal {
@@ -31,15 +33,27 @@ impl Serialize for Principal {
{
use serde::ser::SerializeMap;
let mut map = serializer.serialize_map(Some(1))?;
let field_count = usize::from(!self.aws.is_empty()) + usize::from(!self.service.is_empty());
let mut map = serializer.serialize_map(Some(field_count))?;
// If single element, serialize as string; otherwise as array
if self.aws.len() == 1 {
if let Some(val) = self.aws.iter().next() {
map.serialize_entry("AWS", val)?;
if !self.aws.is_empty() {
if self.aws.len() == 1 {
if let Some(val) = self.aws.iter().next() {
map.serialize_entry("AWS", val)?;
}
} else {
map.serialize_entry("AWS", &self.aws)?;
}
}
if !self.service.is_empty() {
if self.service.len() == 1 {
if let Some(val) = self.service.iter().next() {
map.serialize_entry("Service", val)?;
}
} else {
map.serialize_entry("Service", &self.service)?;
}
} else {
map.serialize_entry("AWS", &self.aws)?;
}
map.end()
@@ -50,22 +64,33 @@ impl Serialize for Principal {
#[serde(untagged)]
enum PrincipalFormat {
Wildcard(String),
AwsObject(PrincipalAwsObject),
Object(PrincipalObject),
}
#[derive(serde::Deserialize)]
struct PrincipalAwsObject {
#[serde(rename = "AWS")]
aws: AwsValues,
struct PrincipalObject {
#[serde(rename = "AWS", default)]
aws: Option<PrincipalValues>,
#[serde(rename = "Service", default)]
service: Option<PrincipalValues>,
}
#[derive(serde::Deserialize)]
#[serde(untagged)]
enum AwsValues {
enum PrincipalValues {
Single(String),
Multiple(HashSet<String>),
}
impl PrincipalValues {
fn into_set(self) -> HashSet<String> {
match self {
PrincipalValues::Single(s) => HashSet::from([s]),
PrincipalValues::Multiple(set) => set,
}
}
}
impl<'de> serde::Deserialize<'de> for Principal {
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
where
@@ -77,6 +102,7 @@ impl<'de> serde::Deserialize<'de> for Principal {
if s == "*" {
Ok(Principal {
aws: HashSet::from(["*".to_string()]),
service: HashSet::new(),
})
} else {
Err(serde::de::Error::custom(format!(
@@ -85,24 +111,28 @@ impl<'de> serde::Deserialize<'de> for Principal {
)))
}
}
PrincipalFormat::AwsObject(obj) => {
let aws = match obj.aws {
AwsValues::Single(s) => HashSet::from([s]),
AwsValues::Multiple(set) => set,
};
Ok(Principal { aws })
PrincipalFormat::Object(obj) => {
let aws = obj.aws.map(PrincipalValues::into_set).unwrap_or_default();
let service = obj.service.map(PrincipalValues::into_set).unwrap_or_default();
if aws.is_empty() && service.is_empty() {
return Err(serde::de::Error::custom("Principal must have at least one of AWS or Service"));
}
Ok(Principal { aws, service })
}
}
}
}
impl Principal {
pub fn is_match(&self, parincipal: &str) -> bool {
pub fn is_match(&self, principal: &str) -> bool {
for pattern in self.aws.iter() {
if wildcard::is_simple_match(pattern, parincipal) {
if wildcard::is_simple_match(pattern, principal) {
return true;
}
}
// Service principals (e.g., logging.s3.amazonaws.com) allow internal
// AWS services. Treat them as non-matching for user requests — they
// only apply to service-initiated actions.
false
}
}
@@ -110,7 +140,7 @@ impl Principal {
impl Validator for Principal {
type Error = Error;
fn is_valid(&self) -> Result<(), Error> {
if self.aws.is_empty() {
if self.aws.is_empty() && self.service.is_empty() {
return Err(Error::other("Principal is empty"));
}
Ok(())
@@ -126,26 +156,22 @@ mod test {
#[test_case(r#""*""#, true ; "wildcard_string")]
#[test_case(r#"{"AWS": "*"}"#, true ; "aws_object_single_string")]
#[test_case(r#"{"AWS": ["*"]}"#, true ; "aws_object_array")]
#[test_case(r#"{"Service": "logging.s3.amazonaws.com"}"#, true ; "service_single")]
#[test_case(r#"{"Service": ["logging.s3.amazonaws.com"]}"#, true ; "service_array")]
#[test_case(r#"{"AWS": "*", "Service": "logging.s3.amazonaws.com"}"#, true ; "aws_and_service")]
#[test_case(r#""invalid""#, false ; "invalid_string")]
#[test_case(r#""""#, false ; "empty_string")]
#[test_case(r#"{"Other": "*"}"#, false ; "wrong_field")]
#[test_case(r#"{}"#, false ; "empty_object")]
fn test_principal_parsing(json: &str, should_succeed: bool) {
let result = match serde_json::from_str::<Principal>(json) {
Ok(principal) => {
assert!(principal.aws.contains("*"));
should_succeed
}
Err(_) => !should_succeed,
};
assert!(result);
let result = serde_json::from_str::<Principal>(json);
assert_eq!(result.is_ok(), should_succeed, "input: {json}, result: {result:?}");
}
#[test]
fn test_principal_serialize_single_element() {
// Single element should serialize as string (AWS format)
let principal = Principal {
aws: HashSet::from(["*".to_string()]),
service: HashSet::new(),
};
let json = serde_json::to_string(&principal).expect("Should serialize");
@@ -154,16 +180,38 @@ mod test {
#[test]
fn test_principal_serialize_multiple_elements() {
// Multiple elements should serialize as array
let principal = Principal {
aws: HashSet::from(["*".to_string(), "arn:aws:iam::123456789012:root".to_string()]),
service: HashSet::new(),
};
let json = serde_json::to_string(&principal).expect("Should serialize");
let parsed: serde_json::Value = serde_json::from_str(&json).expect("Should parse");
let aws_value = parsed.get("AWS").expect("Should have AWS field");
assert!(aws_value.is_array());
let arr = aws_value.as_array().expect("Should be array");
assert_eq!(arr.len(), 2);
assert_eq!(aws_value.as_array().unwrap().len(), 2);
}
#[test]
fn test_principal_serialize_service() {
let principal = Principal {
aws: HashSet::new(),
service: HashSet::from(["logging.s3.amazonaws.com".to_string()]),
};
let json = serde_json::to_string(&principal).expect("Should serialize");
assert_eq!(json, r#"{"Service":"logging.s3.amazonaws.com"}"#);
}
#[test]
fn test_principal_roundtrip_service() {
let json = r#"{"Service": "logging.s3.amazonaws.com"}"#;
let principal: Principal = serde_json::from_str(json).expect("Should parse");
assert!(principal.service.contains("logging.s3.amazonaws.com"));
assert!(principal.aws.is_empty());
let serialized = serde_json::to_string(&principal).expect("Should serialize");
let reparsed: Principal = serde_json::from_str(&serialized).expect("Should re-parse");
assert_eq!(principal, reparsed);
}
}
+5 -5
View File
@@ -1370,11 +1370,11 @@ impl S3Access for FS {
authorize_request(req, Action::S3Action(S3Action::PutBucketLifecycleAction)).await
}
/// Checks whether the PutBucketLogging request has accesses to the resources.
///
/// This method returns `Ok(())` by default.
async fn put_bucket_logging(&self, _req: &mut S3Request<PutBucketLoggingInput>) -> S3Result<()> {
Ok(())
async fn put_bucket_logging(&self, req: &mut S3Request<PutBucketLoggingInput>) -> S3Result<()> {
let req_info = ext_req_info_mut(&mut req.extensions)?;
req_info.bucket = Some(req.input.bucket.clone());
authorize_request(req, Action::S3Action(S3Action::PutBucketLoggingAction)).await
}
/// Checks whether the PutBucketMetricsConfiguration request has accesses to the resources.
+23 -1
View File
@@ -19,7 +19,7 @@ use rustfs_ecstore::{
bucket::tagging::decode_tags_to_map,
error::{is_err_object_not_found, is_err_version_not_found},
new_object_layer_fn,
store_api::{ObjectOperations, ObjectOptions},
store_api::{BucketOperations, BucketOptions, ObjectOperations, ObjectOptions},
};
use s3s::{S3, S3Error, S3ErrorCode, S3Request, S3Response, S3Result, dto::*, s3_error};
use serde::{Deserialize, Serialize};
@@ -1022,6 +1022,28 @@ impl S3 for FS {
usecase.execute_put_bucket_cors(req).await
}
async fn get_bucket_logging(&self, req: S3Request<GetBucketLoggingInput>) -> S3Result<S3Response<GetBucketLoggingOutput>> {
let Some(store) = new_object_layer_fn() else {
return Err(s3_error!(InternalError, "Not init"));
};
store
.get_bucket_info(&req.input.bucket, &BucketOptions::default())
.await
.map_err(crate::error::ApiError::from)?;
Err(s3_error!(NotImplemented, "GetBucketLogging is not implemented yet"))
}
async fn put_bucket_logging(&self, req: S3Request<PutBucketLoggingInput>) -> S3Result<S3Response<PutBucketLoggingOutput>> {
let Some(store) = new_object_layer_fn() else {
return Err(s3_error!(InternalError, "Not init"));
};
store
.get_bucket_info(&req.input.bucket, &BucketOptions::default())
.await
.map_err(crate::error::ApiError::from)?;
Err(s3_error!(NotImplemented, "PutBucketLogging is not implemented yet"))
}
async fn put_bucket_encryption(
&self,
req: S3Request<PutBucketEncryptionInput>,