diff --git a/.config/e2e-full-selection.txt b/.config/e2e-full-selection.txt index 8f0642faf..cdb358bed 100644 --- a/.config/e2e-full-selection.txt +++ b/.config/e2e-full-selection.txt @@ -1,2 +1,2 @@ -sha256-darwin=4582837585bf82ad073d7be7f572755e39d7933f015170f5b58872919b63a528 -sha256-linux=0ffd624e8a62a8818f0062eb85855f7dbc39ad07254f7146585148c121d86aba +sha256-darwin=f801018455712d8767ff6d467f3883036f762936f75435a22de74317274bf90d +sha256-linux=1621c50e25d63f124b411babbf613c04474c5e68864d2e810e50ec892379e91e diff --git a/.config/e2e-smoke-selection.txt b/.config/e2e-smoke-selection.txt index 2692eeb26..70834f420 100644 --- a/.config/e2e-smoke-selection.txt +++ b/.config/e2e-smoke-selection.txt @@ -1 +1 @@ -sha256=84db060be346066398f0bcd580122d5e0059176bec4044af46d947b31f876606 +sha256=eaa140bb7c7fff74c92a0cd191805f7fdee3174eaa6a649a1b807e1e6b497a31 diff --git a/crates/e2e_test/src/create_bucket_region_test.rs b/crates/e2e_test/src/create_bucket_region_test.rs index 8c1a75055..0514b9052 100644 --- a/crates/e2e_test/src/create_bucket_region_test.rs +++ b/crates/e2e_test/src/create_bucket_region_test.rs @@ -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> { + 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> { + 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> { + 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#"eu-west-1"#; + 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-1")] + .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("AccessDenied")); + 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 { + "AccessDenied" + } else { + "BucketAlreadyExists" + })); + } + } + Ok(()) + } } diff --git a/rustfs/src/storage/access.rs b/rustfs/src/storage/access.rs index c38b44c47..270c49635 100644 --- a/rustfs/src/storage/access.rs +++ b/rustfs/src/storage/access.rs @@ -884,7 +884,7 @@ fn authorization_conditions( req: &S3Request, cred: &rustfs_credentials::Credentials, version_id: Option<&str>, - region: Option, + location_constraint: Option, remote_addr: Option, client_info: Option<&ClientInfo>, action: Action, @@ -893,7 +893,7 @@ fn authorization_conditions( &req.headers, cred, version_id, - region, + None, remote_addr, req.uri.query(), client_info, @@ -909,6 +909,11 @@ fn authorization_conditions( 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::() { @@ -949,6 +954,17 @@ pub(crate) fn set_requested_object_lock_retain_until(req: &mut S3Request, } } +/// Keep the XML value literal for IAM; endpoint aliases are resolved later. +fn requested_bucket_location_constraint(input: &CreateBucketInput) -> Option { + 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) { 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(req: &mut S3Request, action: Action) -> S3Result<()> { + authorize_request_with_location_constraint(req, action, None).await +} + +async fn authorize_request_with_location_constraint( + req: &mut S3Request, + action: Action, + location_constraint: Option, +) -> S3Result<()> { let internal_object_authorization = req.extensions.get::().is_some(); let remote_addr = req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)); let req_info = req_info_ref(req)?; @@ -1330,7 +1354,8 @@ pub async fn authorize_request(req: &mut S3Request, action: Action) -> S3R let default_claims = HashMap::new(); let claims = cred.claims.as_ref().unwrap_or(&default_claims); let client_info = req.extensions.get::(); - 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(req: &mut S3Request, 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(req: &mut S3Request, bucket: let client_info = req.extensions.get::(); 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(req: &S3Request, action: Action) -> HashMap> { let credentials = rustfs_credentials::Credentials::default(); authorization_conditions(req, &credentials, None, None, None, None, action).expect("conditions should build") diff --git a/rustfs/tests/connect_perf_object.rs b/rustfs/tests/connect_perf_object.rs index 292d74634..895df2d63 100644 --- a/rustfs/tests/connect_perf_object.rs +++ b/rustfs/tests/connect_perf_object.rs @@ -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) {