fix(s3): add x-amz-grant-* headers to policy condition values (#2031)

This commit is contained in:
安正超
2026-03-01 19:00:04 +08:00
committed by GitHub
parent e5e1010c31
commit 798e620088
4 changed files with 62 additions and 10 deletions
+38
View File
@@ -508,6 +508,25 @@ pub fn get_condition_values(
clone_header.remove(*obj_lock);
}
// S3 policy condition keys use "x-amz-grant-*" (policy key s3:x-amz-grant-* -> name() returns x-amz-grant-*)
for grant_header in &[
"x-amz-grant-full-control",
"x-amz-grant-read",
"x-amz-grant-write",
"x-amz-grant-read-acp",
"x-amz-grant-write-acp",
] {
let values = clone_header
.get_all(*grant_header)
.iter()
.map(|v| v.to_str().unwrap_or("").to_string())
.collect::<Vec<String>>();
if !values.is_empty() {
args.insert((*grant_header).to_string(), values);
}
clone_header.remove(*grant_header);
}
for (key, _values) in clone_header.iter() {
if key.as_str().eq_ignore_ascii_case("x-amz-tagging") {
continue;
@@ -1038,6 +1057,25 @@ mod tests {
);
}
#[test]
fn test_get_condition_values_with_grant_headers() {
let cred = create_test_credentials();
let mut headers = HeaderMap::new();
headers.insert("x-amz-grant-full-control", HeaderValue::from_static("id=owner-123"));
headers.insert(
"x-amz-grant-read",
HeaderValue::from_static("uri=http://acs.amazonaws.com/groups/global/AllUsers"),
);
let conditions = get_condition_values(&headers, &cred, None, None, None);
assert_eq!(conditions.get("x-amz-grant-full-control"), Some(&vec!["id=owner-123".to_string()]));
assert_eq!(
conditions.get("x-amz-grant-read"),
Some(&vec!["uri=http://acs.amazonaws.com/groups/global/AllUsers".to_string()])
);
}
#[test]
fn test_get_condition_values_with_signature_age() {
let cred = create_test_credentials();
+21 -7
View File
@@ -166,14 +166,20 @@ async fn load_object_acl(bucket: &str, object: &str, version_id: Option<&str>, h
}
async fn check_acl_access<T>(req: &S3Request<T>, req_info: &ReqInfo, action: &Action, policy_allowed: bool) -> S3Result<bool> {
if req_info.is_owner || policy_allowed {
return Ok(true);
}
let Some((target, permission)) = acl_permission_for_action(action) else {
return Ok(true);
};
// For object-level operations, do not bypass on account-level is_owner.
// The bucket/account owner must have explicit ACL grant to access objects owned by others.
let bypass_owner = match target {
AclTarget::Bucket => req_info.is_owner,
AclTarget::Object => false,
};
if bypass_owner || policy_allowed {
return Ok(true);
}
let bucket = req_info.bucket.as_deref().unwrap_or("");
if bucket.is_empty() {
return Ok(true);
@@ -364,7 +370,7 @@ pub async fn authorize_request<T>(req: &mut S3Request<T>, action: Action) -> S3R
return Err(s3_error!(AccessDenied, "Access Denied"));
}
if PolicySys::is_allowed(&BucketPolicyArgs {
let policy_allowed_fallback = PolicySys::is_allowed(&BucketPolicyArgs {
bucket: req_info.bucket.as_deref().unwrap_or(""),
action,
is_owner: req_info.is_owner,
@@ -373,8 +379,16 @@ pub async fn authorize_request<T>(req: &mut S3Request<T>, action: Action) -> S3R
conditions: &conditions,
object: req_info.object.as_deref().unwrap_or(""),
})
.await
{
.await;
if policy_allowed_fallback {
// For object-level operations, bucket owner (is_owner) must still pass ACL check.
// Policy may have allowed due to is_owner, but object owner is authoritative.
if let Some((AclTarget::Object, _)) = acl_permission_for_action(&action)
&& !check_acl_access(req, req_info, &action, false).await?
{
return Err(s3_error!(AccessDenied, "Access Denied"));
}
return Ok(());
}
+3 -2
View File
@@ -21,7 +21,7 @@
# - Object ownership: Bucket ownership controls
#
# - SSE-KMS: KMS-related edge cases
# - Bucket Policy: Multipart upload authorization, SSE condition keys
# - Bucket Policy: Multipart upload authorization, SSE condition keys, grant header conditions
# - Versioning: Concurrent multi-object delete
# - ACL: Bucket and Object ACL
# - Object Copy: Same/cross bucket, metadata, versioning
@@ -36,7 +36,7 @@
# - DeleteObject: Proper NoSuchBucket for deleted buckets
# - Multipart Copy: InvalidRange when CopySourceRange exceeds source size
#
# Total: 394 tests
# Total: 395 tests
test_basic_key_count
test_bucket_create_naming_bad_short_one
@@ -227,6 +227,7 @@ test_bucket_policy_another_bucket
test_bucket_policy_allow_notprincipal
test_bucket_policy_multipart
test_bucket_policy_put_obj_acl
test_bucket_policy_put_obj_grant
test_bucket_policy_put_obj_kms_s3
test_bucket_policy_put_obj_s3_kms
test_object_presigned_put_object_with_acl
-1
View File
@@ -28,5 +28,4 @@ test_lifecycle_transition_encrypted
# Tests with known issues (need further investigation)
test_bucket_policy_different_tenant
test_bucket_policy_put_obj_grant
test_bucket_policy_tenanted_bucket