fix(iam): derive bucket location conditions from XML (#8744)

* fix(iam): derive bucket location conditions from XML

Capture the parsed CreateBucket LocationConstraint at the access boundary
and move it into shared authorization conditions independently of signing
and server regions. Cover conditional allow/deny across user, session and
anonymous requests, including absent and spoofed values.

Co-Authored-By: heihutu <heihutu@gmail.com>
Co-Authored-By: zhi22915 <qiuzgang@gmail.com>

* refactor(iam): pass bucket location directly to authorization

Replace temporary request-extension storage with an explicit owned XML
argument to the single authorization core, keeping the public entry point
compatible. Remove the marker allocation and typemap insert/remove path.

Cover Object Lock and Versioning dependent authorizations with an absent
location condition and verify their enabled state through real HTTP.

Co-Authored-By: heihutu <heihutu@gmail.com>
Co-Authored-By: zhi22915 <qiuzgang@gmail.com>

* test(connect): isolate bounded object probe storage

Use an independent temporary volume on Linux shared memory when available
so the real endpoint/CLI fixture does not depend on contended CI disk IO.
Await server shutdown before dropping the caller-owned storage directory.
Retain the production cleanup deadline, measurement budget, assertions,
and existing nextest capacity reservation without adding retries.

Co-Authored-By: heihutu <heihutu@gmail.com>
Co-Authored-By: zhi22915 <qiuzgang@gmail.com>

---------

Co-authored-by: heihutu <heihutu@gmail.com>
Co-authored-by: zhi22915 <qiuzgang@gmail.com>
This commit is contained in:
Hauser
2026-10-21 13:35:35 +08:00
committed by GitHub
parent 001e92226e
commit 2eeaca4cd0
5 changed files with 441 additions and 19 deletions
+2 -2
View File
@@ -1,2 +1,2 @@
sha256-darwin=4582837585bf82ad073d7be7f572755e39d7933f015170f5b58872919b63a528
sha256-linux=0ffd624e8a62a8818f0062eb85855f7dbc39ad07254f7146585148c121d86aba
sha256-darwin=f801018455712d8767ff6d467f3883036f762936f75435a22de74317274bf90d
sha256-linux=1621c50e25d63f124b411babbf613c04474c5e68864d2e810e50ec892379e91e
+1 -1
View File
@@ -1 +1 @@
sha256=84db060be346066398f0bcd580122d5e0059176bec4044af46d947b31f876606
sha256=eaa140bb7c7fff74c92a0cd191805f7fdee3174eaa6a649a1b807e1e6b497a31
@@ -23,7 +23,10 @@
#[cfg(test)]
mod tests {
use crate::common::{RustFSTestEnvironment, init_logging};
use crate::common::{
AdminTransport, RustFSTestEnvironment, admin_add_canned_policy_via, admin_attach_user_policy_via, admin_create_user,
build_test_s3_config, build_test_sts_client, init_logging, local_http_client, signed_s3_request_with_headers,
};
use aws_sdk_s3::Client;
use aws_sdk_s3::config::Region;
use aws_sdk_s3::error::ProvideErrorMetadata;
@@ -194,4 +197,315 @@ mod tests {
}
Ok(())
}
async fn location_policy_client(
env: &RustFSTestEnvironment,
user: &str,
statements: serde_json::Value,
) -> Result<Client, Box<dyn Error + Send + Sync>> {
let secret = "location-policy-test-secret";
admin_create_user(env, user, secret).await?;
let policy = serde_json::json!({"Version":"2012-10-17", "Statement":statements});
admin_add_canned_policy_via(
AdminTransport::Signed,
&env.url,
&env.access_key,
&env.secret_key,
user,
&policy.to_string(),
)
.await?;
admin_attach_user_policy_via(AdminTransport::Signed, &env.url, &env.access_key, &env.secret_key, user, user).await?;
Ok(Client::from_conf(
build_test_s3_config(&env.url, user, secret, None, "location-policy")
.to_builder()
.region(Region::new("eu-west-1"))
.build(),
))
}
#[tokio::test]
async fn test_create_bucket_location_constraint_policy_allow() -> Result<(), Box<dyn Error + Send + Sync>> {
init_logging();
let mut env = RustFSTestEnvironment::new().await?;
env.start_rustfs_server(vec!["--region", "eu-west-1"]).await?;
let root = Client::from_conf(
env.create_s3_client()
.config()
.to_builder()
.region(Region::new("eu-west-1"))
.build(),
);
let configuration = |region: &str| {
CreateBucketConfiguration::builder()
.location_constraint(BucketLocationConstraint::from(region))
.build()
};
for (user, condition, accepts_region, accepts_alias, accepts_missing) in [
(
"location-allow",
serde_json::json!({"StringEquals":{"s3:LocationConstraint":"eu-west-1"}}),
true,
false,
false,
),
(
"location-alias",
serde_json::json!({"StringEquals":{"s3:LocationConstraint":"EU"}}),
false,
true,
false,
),
(
"location-null",
serde_json::json!({"Null":{"s3:LocationConstraint":"true"}}),
false,
false,
true,
),
] {
let client = location_policy_client(
&env,
user,
serde_json::json!([{
"Effect":"Allow", "Action":"s3:CreateBucket", "Resource":"arn:aws:s3:::location-*", "Condition":condition,
}]),
)
.await?;
for (index, (config, allowed)) in [
(Some(configuration("eu-west-1")), accepts_region),
(Some(configuration("EU")), accepts_alias),
(None, accepts_missing),
(Some(CreateBucketConfiguration::builder().build()), accepts_missing),
(Some(configuration("")), accepts_missing),
]
.into_iter()
.enumerate()
{
let bucket = format!("{user}-{index}");
let result = client
.create_bucket()
.bucket(&bucket)
.set_create_bucket_configuration(config)
.send()
.await;
if allowed {
result.unwrap_or_else(|error| panic!("{bucket} should be allowed: {error:?}"));
root.head_bucket().bucket(&bucket).send().await?;
} else {
let denied = result.expect_err("the requested XML constraint must satisfy the conditional Allow");
assert_eq!(denied.raw_response().map(|response| response.status().as_u16()), Some(403), "{bucket}");
assert_eq!(
denied.as_service_error().and_then(ProvideErrorMetadata::code),
Some("AccessDenied"),
"{bucket}"
);
let absent = root
.head_bucket()
.bucket(&bucket)
.send()
.await
.expect_err("denied bucket must not be created");
assert_eq!(absent.raw_response().map(|response| response.status().as_u16()), Some(404));
}
}
}
let object_lock_client = location_policy_client(
&env,
"location-lock",
serde_json::json!([
{"Effect":"Allow", "Action":"s3:CreateBucket", "Resource":"arn:aws:s3:::location-*",
"Condition":{"StringEquals":{"s3:LocationConstraint":"eu-west-1"}}},
{"Effect":"Allow", "Action":["s3:PutBucketObjectLockConfiguration", "s3:PutBucketVersioning"],
"Resource":"arn:aws:s3:::location-*", "Condition":{"Null":{"s3:LocationConstraint":"true"}}}
]),
)
.await?;
object_lock_client
.create_bucket()
.bucket("location-with-lock")
.create_bucket_configuration(configuration("eu-west-1"))
.object_lock_enabled_for_bucket(true)
.send()
.await
.expect("only CreateBucket receives the XML condition; dependent actions see it absent");
let object_lock = root
.get_object_lock_configuration()
.bucket("location-with-lock")
.send()
.await?;
assert_eq!(
object_lock
.object_lock_configuration()
.and_then(|configuration| configuration.object_lock_enabled())
.map(|value| value.as_str()),
Some("Enabled")
);
let versioning = root.get_bucket_versioning().bucket("location-with-lock").send().await?;
assert_eq!(versioning.status().map(|value| value.as_str()), Some("Enabled"));
let user = "location-session";
location_policy_client(
&env,
user,
serde_json::json!([{
"Effect":"Allow", "Action":"s3:CreateBucket", "Resource":"arn:aws:s3:::location-*"
}]),
)
.await?;
let session_policy = serde_json::json!({"Version":"2012-10-17", "Statement":[
{"Effect":"Allow", "Action":"s3:CreateBucket", "Resource":"arn:aws:s3:::location-*"},
{"Effect":"Deny", "Action":"s3:CreateBucket", "Resource":"arn:aws:s3:::location-*",
"Condition":{"StringEquals":{"s3:LocationConstraint":"eu-west-1"}}}
]});
let assumed = build_test_sts_client(&env.url, user, "location-policy-test-secret", None, "location-policy-sts")
.assume_role()
.role_arn("arn:aws:iam::123456789012:role/location-policy")
.role_session_name("location-policy-session")
.policy(session_policy.to_string())
.send()
.await?;
let credentials = assumed.credentials().expect("AssumeRole returns temporary credentials");
let session = Client::from_conf(
build_test_s3_config(
&env.url,
credentials.access_key_id(),
credentials.secret_access_key(),
Some(credentials.session_token()),
"location-policy-session",
)
.to_builder()
.region(Region::new("eu-west-1"))
.build(),
);
let denied = session
.create_bucket()
.bucket("location-session-denied")
.create_bucket_configuration(configuration("eu-west-1"))
.send()
.await
.expect_err("the STS session's conditional Deny must be enforced");
assert_eq!(denied.as_service_error().and_then(ProvideErrorMetadata::code), Some("AccessDenied"));
let absent = root
.head_bucket()
.bucket("location-session-denied")
.send()
.await
.expect_err("session denial must not create a bucket");
assert_eq!(absent.raw_response().map(|response| response.status().as_u16()), Some(404));
session.create_bucket().bucket("location-session-allowed").send().await?;
Ok(())
}
#[tokio::test]
async fn test_create_bucket_location_constraint_policy_deny() -> Result<(), Box<dyn Error + Send + Sync>> {
init_logging();
let mut env = RustFSTestEnvironment::new().await?;
env.start_rustfs_server(vec!["--region", "eu-west-1"]).await?;
let root = Client::from_conf(
env.create_s3_client()
.config()
.to_builder()
.region(Region::new("eu-west-1"))
.build(),
);
let user = "location-deny";
let client = location_policy_client(
&env,
user,
serde_json::json!([
{"Effect":"Allow", "Action":"s3:CreateBucket", "Resource":"arn:aws:s3:::location-*"},
{"Effect":"Deny", "Action":"s3:CreateBucket", "Resource":"arn:aws:s3:::location-*",
"Condition":{"StringEquals":{"s3:LocationConstraint":"eu-west-1"}}},
]),
)
.await?;
let denied = client
.create_bucket()
.bucket("location-deny-matching")
.create_bucket_configuration(
CreateBucketConfiguration::builder()
.location_constraint(BucketLocationConstraint::from("eu-west-1"))
.build(),
)
.send()
.await
.expect_err("a matching explicit Deny must override the unconditional Allow");
assert_eq!(denied.as_service_error().and_then(ProvideErrorMetadata::code), Some("AccessDenied"));
assert_eq!(denied.raw_response().map(|response| response.status().as_u16()), Some(403));
let absent = root
.head_bucket()
.bucket("location-deny-matching")
.send()
.await
.expect_err("denied bucket must not exist");
assert_eq!(absent.raw_response().map(|response| response.status().as_u16()), Some(404));
client.create_bucket().bucket("location-deny-missing").send().await?;
let xml = r#"<CreateBucketConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><LocationConstraint>eu-west-1</LocationConstraint></CreateBucketConfiguration>"#;
let mut headers = http::HeaderMap::new();
headers.insert("locationconstraint", http::HeaderValue::from_static("us-east-1"));
for (index, body) in [xml.to_owned(), xml.replace("eu-west-1", "eu-west-&#49;")]
.into_iter()
.enumerate()
{
let bucket = format!("location-deny-spoofed-{index}");
let spoofed = signed_s3_request_with_headers(
http::Method::PUT,
&format!("{}/{bucket}?LocationConstraint=us-east-1", env.url),
Some(body),
Some("application/xml"),
user,
"location-policy-test-secret",
&headers,
)
.await?;
assert_eq!(spoofed.status(), reqwest::StatusCode::FORBIDDEN);
assert!(spoofed.text().await?.contains("<Code>AccessDenied</Code>"));
let absent = root
.head_bucket()
.bucket(&bucket)
.send()
.await
.expect_err("spoofed conditions must not create a bucket");
assert_eq!(absent.raw_response().map(|response| response.status().as_u16()), Some(404));
}
let bucket = "location-anonymous";
root.create_bucket().bucket(bucket).send().await?;
for (effect, match_status, missing_status) in [
("Allow", reqwest::StatusCode::CONFLICT, reqwest::StatusCode::FORBIDDEN),
("Deny", reqwest::StatusCode::FORBIDDEN, reqwest::StatusCode::CONFLICT),
] {
let mut statements = vec![];
if effect == "Deny" {
statements.push(serde_json::json!({"Effect":"Allow", "Principal":"*", "Action":"s3:CreateBucket", "Resource":format!("arn:aws:s3:::{bucket}")}));
}
statements.push(serde_json::json!({"Effect":effect, "Principal":"*", "Action":"s3:CreateBucket", "Resource":format!("arn:aws:s3:::{bucket}"),
"Condition":{"StringEquals":{"s3:LocationConstraint":"eu-west-1"}}}));
root.put_bucket_policy()
.bucket(bucket)
.policy(serde_json::json!({"Version":"2012-10-17", "Statement":statements}).to_string())
.send()
.await?;
let unsigned = local_http_client();
for (body, expected) in [(xml, match_status), ("", missing_status)] {
let response = unsigned
.put(format!("{}/{bucket}", env.url))
.header("content-type", "application/xml")
.body(body)
.send()
.await?;
assert_eq!(response.status(), expected, "anonymous {effect}, XML present={}", !body.is_empty());
let response = response.text().await?;
assert!(response.contains(if expected == reqwest::StatusCode::FORBIDDEN {
"<Code>AccessDenied</Code>"
} else {
"<Code>BucketAlreadyExists</Code>"
}));
}
}
Ok(())
}
}
+112 -15
View File
@@ -884,7 +884,7 @@ fn authorization_conditions<T>(
req: &S3Request<T>,
cred: &rustfs_credentials::Credentials,
version_id: Option<&str>,
region: Option<s3s::region::Region>,
location_constraint: Option<String>,
remote_addr: Option<std::net::SocketAddr>,
client_info: Option<&ClientInfo>,
action: Action,
@@ -893,7 +893,7 @@ fn authorization_conditions<T>(
&req.headers,
cred,
version_id,
region,
None,
remote_addr,
req.uri.query(),
client_info,
@@ -909,6 +909,11 @@ fn authorization_conditions<T>(
if internal_object_authorization {
return Ok(conditions);
}
if action == Action::S3Action(S3Action::CreateBucketAction)
&& let Some(location) = location_constraint
{
conditions.insert("LocationConstraint".to_string(), vec![location]);
}
merge_list_bucket_query_conditions(action, req.uri.query(), &mut conditions);
merge_request_object_tag_conditions(action, &req.headers, &mut conditions)?;
if let Some(RequestedObjectLockRetention(retention)) = req.extensions.get::<RequestedObjectLockRetention>() {
@@ -949,6 +954,17 @@ pub(crate) fn set_requested_object_lock_retain_until<T>(req: &mut S3Request<T>,
}
}
/// Keep the XML value literal for IAM; endpoint aliases are resolved later.
fn requested_bucket_location_constraint(input: &CreateBucketInput) -> Option<String> {
input
.create_bucket_configuration
.as_ref()
.and_then(|configuration| configuration.location_constraint.as_ref())
.map(BucketLocationConstraint::as_str)
.filter(|location| !location.is_empty())
.map(str::to_owned)
}
fn set_requested_object_lock_retention(req: &mut S3Request<PutObjectRetentionInput>) {
let retention = req.input.retention.clone().unwrap_or_default();
set_requested_object_lock_retain_until(req, retention.retain_until_date.as_ref());
@@ -1305,6 +1321,14 @@ impl DenialContext<'_> {
/// Authorizes the request based on the action and credentials.
pub async fn authorize_request<T>(req: &mut S3Request<T>, action: Action) -> S3Result<()> {
authorize_request_with_location_constraint(req, action, None).await
}
async fn authorize_request_with_location_constraint<T>(
req: &mut S3Request<T>,
action: Action,
location_constraint: Option<String>,
) -> S3Result<()> {
let internal_object_authorization = req.extensions.get::<InternalObjectAuthorization>().is_some();
let remote_addr = req.extensions.get::<Option<RemoteAddr>>().and_then(|opt| opt.map(|a| a.0));
let req_info = req_info_ref(req)?;
@@ -1330,7 +1354,8 @@ pub async fn authorize_request<T>(req: &mut S3Request<T>, action: Action) -> S3R
let default_claims = HashMap::new();
let claims = cred.claims.as_ref().unwrap_or(&default_claims);
let client_info = req.extensions.get::<ClientInfo>();
let mut conditions = authorization_conditions(req, cred, version_id.as_deref(), None, remote_addr, client_info, action)?;
let mut conditions =
authorization_conditions(req, cred, version_id.as_deref(), location_constraint, remote_addr, client_info, action)?;
let action_args = Args {
account: &cred.access_key,
@@ -1506,7 +1531,7 @@ pub async fn authorize_request<T>(req: &mut S3Request<T>, action: Action) -> S3R
req,
&default_cred,
version_id.as_deref(),
req.region.clone(),
location_constraint,
remote_addr,
client_info,
action,
@@ -1930,15 +1955,8 @@ async fn prepare_table_data_plane_list_access<T>(req: &mut S3Request<T>, bucket:
let client_info = req.extensions.get::<ClientInfo>();
let default_cred = rustfs_credentials::Credentials::default();
let condition_cred = cred.as_ref().unwrap_or(&default_cred);
let conditions = authorization_conditions(
req,
condition_cred,
None,
req.region.clone(),
remote_addr,
client_info,
Action::S3Action(action),
)?;
let conditions =
authorization_conditions(req, condition_cred, None, None, remote_addr, client_info, Action::S3Action(action))?;
let iam_store = cred.as_ref().map(|_| request_iam_store(req)).transpose()?;
let store = table_catalog_store_for_data_plane(req)?;
let tables = crate::table_catalog::TableCatalogStore::list_all_tables(&store, bucket)
@@ -2292,7 +2310,9 @@ impl S3Access for FS {
let req_info = ext_req_info_mut(&mut req.extensions)?;
req_info.bucket = Some(req.input.bucket.clone());
authorize_request(req, Action::S3Action(S3Action::CreateBucketAction)).await?;
let location_constraint = requested_bucket_location_constraint(&req.input);
authorize_request_with_location_constraint(req, Action::S3Action(S3Action::CreateBucketAction), location_constraint)
.await?;
if req.input.object_lock_enabled_for_bucket.is_some_and(|v| v) {
authorize_request(req, Action::S3Action(S3Action::PutBucketObjectLockConfigurationAction)).await?;
@@ -3554,7 +3574,10 @@ mod tests {
table_data_plane_content_mutation, table_data_plane_resource_for_request, table_publication_guard_error,
validate_post_object_success_controls, versioned_read_action, versioned_tagging_action,
};
use super::{remaining_retention_days, set_requested_object_lock_retain_until, set_requested_object_lock_retention};
use super::{
remaining_retention_days, requested_bucket_location_constraint, set_requested_object_lock_retain_until,
set_requested_object_lock_retention,
};
use crate::auth::OBJECT_LOCK_REMAINING_RETENTION_DAYS_CONDITION;
use crate::error::ApiError;
use crate::storage::storage_api::contract::bucket::{BucketOperations as _, DeleteBucketOptions, MakeBucketOptions};
@@ -4595,6 +4618,80 @@ mod tests {
}
}
#[test]
fn create_bucket_location_constraint_conditions_use_only_xml() {
let configuration = |region: &str| CreateBucketConfiguration {
location_constraint: Some(BucketLocationConstraint::from(region.to_owned())),
..Default::default()
};
for (configuration, expected) in [
(None, None),
(Some(CreateBucketConfiguration::default()), None),
(Some(configuration("")), None),
(Some(configuration("eu-west-1")), Some("eu-west-1")),
(Some(configuration("EU")), Some("EU")),
(Some(configuration("tenant-region-1")), Some("tenant-region-1")),
] {
let mut req = build_request(
CreateBucketInput {
bucket: "location-policy-bucket".to_string(),
create_bucket_configuration: configuration,
..Default::default()
},
Method::PUT,
);
req.region = Some("us-east-1".parse().expect("signing region"));
req.uri = Uri::from_static("/location-policy-bucket?LocationConstraint=forged&s3%3ALocationConstraint=forged");
req.headers.insert("locationconstraint", HeaderValue::from_static("forged"));
let credentials = rustfs_credentials::Credentials {
claims: Some(
[
("LocationConstraint".to_string(), serde_json::json!("forged")),
("ldapLocationConstraint".to_string(), serde_json::json!("forged")),
]
.into_iter()
.collect(),
),
..Default::default()
};
let location = requested_bucket_location_constraint(&req.input);
let conditions = authorization_conditions(
&req,
&credentials,
None,
location,
None,
None,
Action::S3Action(S3Action::CreateBucketAction),
)
.expect("CreateBucket conditions");
assert_eq!(
conditions
.get("LocationConstraint")
.and_then(|values| values.first())
.map(String::as_str),
expected,
);
for action in [
S3Action::PutBucketObjectLockConfigurationAction,
S3Action::PutBucketVersioningAction,
S3Action::GetObjectAction,
] {
let conditions = authorization_conditions(
&req,
&credentials,
None,
expected.map(str::to_owned),
None,
None,
Action::S3Action(action),
)
.expect("non-CreateBucket conditions");
assert!(!conditions.contains_key("LocationConstraint"), "{action:?}");
}
}
}
fn remaining_retention_days_conditions<T>(req: &S3Request<T>, action: Action) -> HashMap<String, Vec<String>> {
let credentials = rustfs_credentials::Credentials::default();
authorization_conditions(req, &credentials, None, None, None, None, action).expect("conditions should build")
+11
View File
@@ -831,8 +831,17 @@ async fn real_rustfs_endpoint_and_production_cli_support_bounded_get_and_put_bod
Err(err) if err.kind() == std::io::ErrorKind::PermissionDenied => return,
Err(err) => panic!("find free port: {err}"),
};
// This functional probe keeps production deadlines while avoiding the
// shared CI disk's fsync latency when Linux shared memory is available.
let storage = if cfg!(target_os = "linux") && Path::new("/dev/shm").is_dir() {
tempfile::tempdir_in("/dev/shm")
} else {
tempfile::tempdir()
}
.expect("isolated object fixture storage");
let server = RustFSServerBuilder::new()
.address(format!("127.0.0.1:{port}"))
.volume(storage.path().to_str().expect("object fixture storage path"))
.access_key("object-perf-access")
.secret_key("object-perf-secret")
.build()
@@ -968,6 +977,8 @@ async fn real_rustfs_endpoint_and_production_cli_support_bounded_get_and_put_bod
.expect("public key")
.verify(&signed, &signature)
.expect("valid ES256 signature");
server.shutdown().await;
}
fn write_credential(path: &Path, value: &str) {