diff --git a/crates/fakecloud-e2e/tests/iam_enforcement.rs b/crates/fakecloud-e2e/tests/iam_enforcement.rs index 22d6da708..511a31216 100644 --- a/crates/fakecloud-e2e/tests/iam_enforcement.rs +++ b/crates/fakecloud-e2e/tests/iam_enforcement.rs @@ -411,6 +411,240 @@ async fn sns_publish_allowed_on_specific_topic() { // S3 tests // ====================================================================== +/// A create that configures the bucket needs the permission for what it +/// configures. This matters because those settings now PERSIST: a `public-read` +/// bucket created by a principal with no `s3:PutBucketAcl` used to lose the +/// grant on restart and now keeps it. +#[tokio::test] +async fn s3_create_bucket_with_acl_needs_put_bucket_acl() { + let server = start_strict().await; + let (akid, secret) = bootstrap_user(&server, "s3aclcreate").await; + attach_inline_policy( + &server, + "s3aclcreate", + "create-only", + r#"{"Version":"2012-10-17","Statement":[ + {"Effect":"Allow","Action":"s3:CreateBucket","Resource":"*"} + ]}"#, + ) + .await; + + let cfg = sdk_config_with(&server, &akid, &secret).await; + let s3 = aws_sdk_s3::Client::new(&cfg); + + // A plain create is still allowed by s3:CreateBucket alone. + s3.create_bucket() + .bucket("aclperm-plain") + .send() + .await + .unwrap(); + + // So is `--acl private`: the model'"'"'s CreateBucket permissions exempt it + // and a no-ACL create from s3:PutBucketAcl, and clients that always send a + // canned ACL (Terraform'"'"'s legacy `acl` attribute, SDK wrappers) would + // otherwise be refused where real S3 succeeds. + s3.create_bucket() + .bucket("aclperm-private") + .acl(aws_sdk_s3::types::BucketCannedAcl::Private) + .send() + .await + .expect("a private canned ACL needs only s3:CreateBucket"); + + // Asking for an ACL is not. + let err = s3 + .create_bucket() + .bucket("aclperm-denied") + .acl(aws_sdk_s3::types::BucketCannedAcl::PublicRead) + .send() + .await + .expect_err("a canned ACL needs s3:PutBucketAcl"); + assert!( + format!("{err:?}").contains("AccessDenied"), + "unexpected error: {err:?}" + ); + + // Granting it unblocks the create, and the bucket really is public-read. + attach_inline_policy( + &server, + "s3aclcreate", + "create-and-acl", + r#"{"Version":"2012-10-17","Statement":[ + {"Effect":"Allow","Action":["s3:CreateBucket","s3:PutBucketAcl","s3:GetBucketAcl"],"Resource":"*"} + ]}"#, + ) + .await; + s3.create_bucket() + .bucket("aclperm-allowed") + .acl(aws_sdk_s3::types::BucketCannedAcl::PublicRead) + .send() + .await + .unwrap(); + let acl = s3 + .get_bucket_acl() + .bucket("aclperm-allowed") + .send() + .await + .unwrap(); + assert!(acl.grants().iter().any(|g| { + g.grantee() + .and_then(|gr| gr.uri()) + .is_some_and(|u| u.contains("AllUsers")) + })); +} + +/// The `s3:x-amz-acl` condition key is what a policy uses to pin down WHICH ACL +/// a create may ask for. It was never populated, so such a guardrail silently +/// authorized exactly what it was written to block. +#[tokio::test] +async fn s3_create_bucket_acl_condition_key_gates_the_canned_value() { + let server = start_strict().await; + let (akid, secret) = bootstrap_user(&server, "s3aclcond").await; + attach_inline_policy( + &server, + "s3aclcond", + "private-acls-only", + r#"{"Version":"2012-10-17","Statement":[ + {"Effect":"Allow","Action":["s3:CreateBucket","s3:PutBucketAcl"],"Resource":"*", + "Condition":{"StringEquals":{"s3:x-amz-acl":"private"}}} + ]}"#, + ) + .await; + + let cfg = sdk_config_with(&server, &akid, &secret).await; + let s3 = aws_sdk_s3::Client::new(&cfg); + + s3.create_bucket() + .bucket("aclcond-private") + .acl(aws_sdk_s3::types::BucketCannedAcl::Private) + .send() + .await + .expect("the permitted canned value must be allowed"); + + let err = s3 + .create_bucket() + .bucket("aclcond-public") + .acl(aws_sdk_s3::types::BucketCannedAcl::PublicRead) + .send() + .await + .expect_err("public-read must be denied by the condition"); + assert!( + format!("{err:?}").contains("AccessDenied"), + "unexpected error: {err:?}" + ); +} + +/// Object lock enables versioning, so AWS wants both permissions. +#[tokio::test] +async fn s3_create_bucket_with_object_lock_needs_its_permissions() { + let server = start_strict().await; + let (akid, secret) = bootstrap_user(&server, "s3lockcreate").await; + attach_inline_policy( + &server, + "s3lockcreate", + "create-only", + r#"{"Version":"2012-10-17","Statement":[ + {"Effect":"Allow","Action":"s3:CreateBucket","Resource":"*"} + ]}"#, + ) + .await; + + let cfg = sdk_config_with(&server, &akid, &secret).await; + let s3 = aws_sdk_s3::Client::new(&cfg); + let err = s3 + .create_bucket() + .bucket("lockperm-denied") + .object_lock_enabled_for_bucket(true) + .send() + .await + .expect_err("object lock needs its own permissions"); + assert!( + format!("{err:?}").contains("AccessDenied"), + "unexpected error: {err:?}" + ); + + attach_inline_policy( + &server, + "s3lockcreate", + "create-and-lock", + r#"{"Version":"2012-10-17","Statement":[ + {"Effect":"Allow","Action":["s3:CreateBucket","s3:PutBucketObjectLockConfiguration","s3:PutBucketVersioning"],"Resource":"*"} + ]}"#, + ) + .await; + s3.create_bucket() + .bucket("lockperm-allowed") + .object_lock_enabled_for_bucket(true) + .send() + .await + .unwrap(); +} + +/// A create carrying both tags and an ACL is several authorizations, and AWS +/// evaluates them all against ONE request context. A tag-scoped guardrail that +/// permits the create must therefore permit the `s3:PutBucketAcl` it implies: +/// building the context per action left that one seeing no `aws:RequestTag/*` +/// at all, so the condition matched and denied a create AWS allows. +/// +/// The Deny is written with `Null`, deliberately. A `StringNotEquals` Deny cannot +/// catch this: an unpopulated key safe-fails every non-`IfExists` operator to +/// false, so such a Deny would not apply on the broken code either and the test +/// would pass against the bug. `Null` is the one operator that reads an +/// unpopulated key AS null, which is exactly the state being asserted against. +#[tokio::test] +async fn s3_create_bucket_tag_condition_applies_to_the_implied_acl_action() { + let server = start_strict().await; + let (akid, secret) = bootstrap_user(&server, "s3ctxuser").await; + attach_inline_policy( + &server, + "s3ctxuser", + "tag-scoped", + r#"{"Version":"2012-10-17","Statement":[ + {"Effect":"Allow","Action":"s3:*","Resource":"*"}, + {"Effect":"Deny", + "Action":["s3:CreateBucket","s3:TagResource","s3:PutBucketAcl"], + "Resource":"*", + "Condition":{"Null":{"aws:RequestTag/CostCenter":"true"}}} + ]}"#, + ) + .await; + + let cfg = sdk_config_with(&server, &akid, &secret).await; + let s3 = aws_sdk_s3::Client::new(&cfg); + + // Tagged with the required value AND carrying an ACL: allowed, because + // every action in the request sees `aws:RequestTag/CostCenter=123`. + s3.create_bucket() + .bucket("ctx-tagged") + .acl(aws_sdk_s3::types::BucketCannedAcl::PublicRead) + .create_bucket_configuration( + aws_sdk_s3::types::CreateBucketConfiguration::builder() + .tags( + aws_sdk_s3::types::Tag::builder() + .key("CostCenter") + .value("123") + .build() + .unwrap(), + ) + .build(), + ) + .send() + .await + .expect("a create whose tags satisfy the guardrail must not be denied by its own ACL"); + + // The guardrail still bites: a create carrying no such tag is denied. + let err = s3 + .create_bucket() + .bucket("ctx-untagged") + .acl(aws_sdk_s3::types::BucketCannedAcl::PublicRead) + .send() + .await + .expect_err("a create with no CostCenter tag must be denied"); + assert!( + format!("{err:?}").contains("AccessDenied"), + "unexpected error: {err:?}" + ); +} + /// Real S3 requires `s3:TagResource` on top of `s3:CreateBucket` to create a /// bucket carrying `CreateBucketConfiguration.Tags` (issue #2553). A grant of /// `s3:CreateBucket` alone still creates untagged buckets, but a tagged create diff --git a/crates/fakecloud-s3/src/service/buckets.rs b/crates/fakecloud-s3/src/service/buckets.rs index 0ad38519f..7c8bcc3c8 100644 --- a/crates/fakecloud-s3/src/service/buckets.rs +++ b/crates/fakecloud-s3/src/service/buckets.rs @@ -299,21 +299,10 @@ impl S3Service { // cased value here would refuse an ACL at create that the very next // PutBucketAcl would then happily set. let ownership_enforced = ownership_header == Some("BucketOwnerEnforced"); - let owner_only = |grants: &[crate::state::AclGrant]| { - grants.iter().all(|g| { - g.permission == "FULL_CONTROL" - && g.grantee_type == "CanonicalUser" - && g.grantee_id.as_deref() == Some(req.account_id.as_str()) - }) - }; + // Shared with the `s3:PutBucketAcl` authorization in `iam_actions_for`, + // which asks the same question and must not answer it differently. let acl_requests_grants = grant_headers_present - || acl_header.is_some_and(|a| { - // `aws-exec-read` grants READ to the EC2 service's canonical - // user. That grantee is not modeled, so the resolved grants - // look owner-only -- but the request still asks for an ACL - // reaching outside the owner, which is what conflicts. - a == "aws-exec-read" || !owner_only(&canned_acl_grants(a, &req.account_id)) - }); + || acl_header.is_some_and(|a| super::acl_reaches_past_owner(a, &req.account_id)); if ownership_enforced && acl_requests_grants { return Err(AwsServiceError::aws_error( StatusCode::BAD_REQUEST, diff --git a/crates/fakecloud-s3/src/service/mod.rs b/crates/fakecloud-s3/src/service/mod.rs index 8017a6e70..061dba086 100644 --- a/crates/fakecloud-s3/src/service/mod.rs +++ b/crates/fakecloud-s3/src/service/mod.rs @@ -1692,14 +1692,75 @@ impl AwsService for S3Service { return Vec::new(); }; if action.service == "s3" && action.action == "CreateBucket" { + // A create that also configures the bucket needs the permission for + // each thing it configures, as on AWS: dispatch requires every + // returned action to be allowed, so a principal holding only + // `s3:CreateBucket` can still create a plain bucket. This matters + // more now that these settings PERSIST -- a bucket created + // `public-read` by a caller with no `s3:PutBucketAcl` used to lose + // the grant on restart, and now keeps it. + let mut extra = Vec::new(); let body = std::str::from_utf8(&request.body).unwrap_or(""); if !create_bucket_configuration_tags(body).is_empty() { - let tag_resource = fakecloud_core::auth::IamAction { - service: "s3", - action: "TagResource", - resource: action.resource.clone(), - }; - return vec![action, tag_resource]; + extra.push("TagResource"); + } + // Per the CreateBucket permissions in the vendored model: an ACL set + // to public-read, public-read-write, authenticated-read "or any + // other custom ACLs" needs s3:PutBucketAcl, while "if you set the + // ACL to private, or if you don't specify any ACLs, only the + // s3:CreateBucket permission is required". Judged by the grants the + // request resolves to, so the two spellings of an owner-only ACL + // agree -- and so a least-privilege caller that always sends + // `--acl private` is not refused. + let reaches_past_owner = request + .headers + .get("x-amz-acl") + .and_then(|v| v.to_str().ok()) + // Blank counts as absent here too. It resolves to owner-only today + // only because `canned_acl_grants` falls through to the owner for + // any unrecognized value -- so without this, an arm that ever made + // an unknown value resolve past the owner would silently start + // demanding `s3:PutBucketAcl` for an empty header. + .filter(|acl| !acl.trim().is_empty()) + .is_some_and(|acl| acl_reaches_past_owner(acl, &request.account_id)); + if reaches_past_owner || has_grant_headers(&request.headers) { + extra.push("PutBucketAcl"); + } + // Presence, but blank counts as absent -- the same rule the ACL + // headers and the condition keys follow. Otherwise a blank value + // demands a permission whose condition key is deliberately not + // emitted, so a `StringEquals`-gated Allow 403s a request the handler + // would have answered with a 400. + if request + .headers + .get("x-amz-object-ownership") + .and_then(|v| v.to_str().ok()) + .is_some_and(|v| !v.trim().is_empty()) + { + extra.push("PutBucketOwnershipControls"); + } + if request + .headers + .get("x-amz-bucket-object-lock-enabled") + .and_then(|v| v.to_str().ok()) + .is_some_and(|v| v.eq_ignore_ascii_case("true")) + { + // Object lock forces versioning on, so AWS wants both. + extra.push("PutBucketObjectLockConfiguration"); + extra.push("PutBucketVersioning"); + } + if !extra.is_empty() { + let mut actions = Vec::with_capacity(extra.len() + 1); + let resource = action.resource.clone(); + actions.push(action); + for name in extra { + actions.push(fakecloud_core::auth::IamAction { + service: "s3", + action: name, + resource: resource.clone(), + }); + } + return actions; } } vec![action] @@ -1710,7 +1771,7 @@ impl AwsService for S3Service { request: &AwsRequest, action: &fakecloud_core::auth::IamAction, ) -> std::collections::BTreeMap> { - s3_condition_keys(action.action, &request.query_params) + s3_condition_keys(action.action, &request.query_params, &request.headers) } fn resource_tags_for( @@ -1733,15 +1794,77 @@ impl AwsService for S3Service { /// Extract service-specific IAM condition keys from an S3 request. /// -/// Today only `ListObjects` / `ListObjectsV2` expose keys (`s3:prefix`, -/// `s3:delimiter`, `s3:max-keys`) via their query params. Other actions -/// return an empty map so the evaluator's safe-fail semantics treat any -/// policy condition referencing an unknown key as "doesn't apply". +/// `ListObjects` / `ListObjectsV2` expose `s3:prefix`, `s3:delimiter` and +/// `s3:max-keys` from their query params, and any request carrying the ACL, +/// object-ownership or object-lock headers exposes the matching `s3:x-amz-*` +/// keys. An action with nothing to expose returns an empty map, so the +/// evaluator's safe-fail semantics treat a policy condition on an unknown key +/// as "doesn't apply". fn s3_condition_keys( action: &str, query: &std::collections::HashMap, + headers: &HeaderMap, ) -> std::collections::BTreeMap> { let mut out = std::collections::BTreeMap::new(); + // `s3:x-amz-acl` and the `s3:x-amz-grant-*` family are how a policy pins + // down which ACL a write may ask for -- a guardrail like + // `Deny s3:CreateBucket when s3:x-amz-acl != private` is useless while they + // are never populated. AWS exposes them on every ACL-accepting write, so + // they are emitted whenever the request carries them rather than being + // gated on an action list that would drift. + if let Some(acl) = headers + .get("x-amz-acl") + .and_then(|v| v.to_str().ok()) + // Blank is skipped for consistency with every other key family here (the + // grant headers, object ownership): a present-but-empty header names no + // value, and a key emitted as `""` matches nothing a policy can sensibly + // write. Note this does NOT mean the request succeeds -- three of the four + // canned-ACL paths answer 400 for a blank value; it means the policy + // decision is not made on an empty string. + .filter(|v| !v.trim().is_empty()) + { + out.insert("s3:x-amz-acl".to_string(), vec![acl.to_string()]); + } + for (header, _) in &GRANT_HEADER_PERMISSIONS { + // Blank values are skipped for the same reason `has_grant_headers` + // treats them as absent: a present-but-empty header asks for no grant, + // and emitting the key would make a `Null`-based guardrail read it as + // present and deny the request. + let values: Vec = headers + .get_all(*header) + .iter() + .filter_map(|v| v.to_str().ok()) + .filter(|v| !v.trim().is_empty()) + .map(|v| v.to_string()) + .collect(); + if !values.is_empty() { + out.insert(format!("s3:{header}"), values); + } + } + // The ACL is not the only setting `iam_actions_for` gates: object ownership is + // too, and `Deny CreateBucket unless s3:x-amz-object-ownership == + // BucketOwnerEnforced` is just as useless while its key is never populated. + // + // Object lock is deliberately NOT here. AWS defines no condition key for the + // `x-amz-bucket-object-lock-enabled` header -- its object-lock keys are + // `s3:object-lock-mode`, `-legal-hold`, `-remaining-retention-days` and + // `-retain-until-date`, all about an object's retention rather than a bucket's + // creation. Emitting `s3:x-amz-bucket-object-lock-enabled` would invert the + // point of populating keys at all: a guardrail written against it would work + // here and be a permanent no-op against AWS. The permission the lock header + // requires is unaffected -- that comes from the model's Permissions text. + if let Some(value) = headers + .get("x-amz-object-ownership") + .and_then(|v| v.to_str().ok()) + // Blank is skipped like the grant family: present but empty asks for + // nothing, and emitting the key would make a `Null` check read it as set. + .filter(|v| !v.trim().is_empty()) + { + out.insert( + "s3:x-amz-object-ownership".to_string(), + vec![value.to_string()], + ); + } if matches!(action, "ListObjects" | "ListObjectsV2") { // Both list variants share the same query param shape. if let Some(prefix) = query.get("prefix") { @@ -1824,11 +1947,20 @@ fn s3_request_tags( let tags = parse_tagging_xml(body); Some(tags.into_iter().collect()) } - // CreateBucket carries its tag set inside CreateBucketConfiguration, - // and the `s3:TagResource` authorization it requires alongside - // `s3:CreateBucket` is evaluated against the same tags — so both see - // `aws:RequestTag/*` and `aws:TagKeys`. - "CreateBucket" | "TagResource" => { + // CreateBucket carries its tag set inside CreateBucketConfiguration, and + // every authorization it requires alongside `s3:CreateBucket` is + // evaluated against the same tags -- AWS builds one request context for + // the whole request, so an `aws:RequestTag/*` guardrail that permits the + // create must not then deny the ACL or object-lock action it implies. + // The extra names are harmless on their own operations: a real + // PutBucketAcl body is not a `CreateBucketConfiguration`, so it yields + // the same empty map the catch-all arm would. + "CreateBucket" + | "TagResource" + | "PutBucketAcl" + | "PutBucketOwnershipControls" + | "PutBucketObjectLockConfiguration" + | "PutBucketVersioning" => { let body = std::str::from_utf8(&request.body).unwrap_or(""); let tags = create_bucket_configuration_tags(body); Some(tags.into_iter().collect()) @@ -2568,9 +2700,10 @@ pub(crate) const OBJECT_OWNERSHIP_VALUES: [&str; 3] = [ /// `AWS::S3::Bucket` `AccessControl: AwsExecRead` maps to it), and AWS answers /// 200, so refusing it would turn a working call into a hard failure. Its grant /// is a READ to the EC2 service's canonical user, which this build cannot -/// reproduce, so it resolves to owner-only -- `create_bucket` special-cases it -/// when checking the BucketOwnerEnforced conflict, since the request does ask -/// for an ACL reaching past the owner. +/// reproduce, so it resolves to owner-only -- [`acl_reaches_past_owner`] special- +/// cases it, since the request does ask for an ACL reaching past the owner. That +/// drives both the BucketOwnerEnforced conflict check and the `s3:PutBucketAcl` +/// authorization a create carrying it requires. /// /// `bucket-owner-read` and `bucket-owner-full-control` are object-scoped, and /// the canned-ACL table says S3 IGNORES them when they are given on a bucket @@ -2590,6 +2723,38 @@ pub(crate) const BUCKET_CANNED_ACLS: [&str; 8] = [ "bucket-owner-full-control", ]; +/// Whether the ACL a request asks for reaches past the bucket owner. +/// +/// Both the `BucketOwnerEnforced` conflict check and the `s3:PutBucketAcl` +/// authorization turn on this one question, so they share the answer rather +/// than each re-deriving it -- they disagreed once already, and the IAM side +/// silently authorized an ACL the conflict check treats as reaching outside the +/// owner. +/// +/// `aws-exec-read` is the case that needs saying out loud: its READ grant to +/// the EC2 service's canonical user is not modeled, so [`canned_acl_grants`] +/// resolves it to owner-only -- but the request does ask for an ACL reaching +/// outside the owner. +/// +/// NOT the same question as the object-write check in +/// [`S3Service::resolve_write_acl_headers`], which asks whether the request names +/// bucket-owner-full-control specifically. The two look alike and must not be +/// unified: a BucketOwnerEnforced BUCKET rejects an ACL that grants another +/// account anything, so `private` is fine there, while a BucketOwnerEnforced +/// bucket accepts an object PUT only when it specifies no ACL at all or +/// bucket-owner-full-control -- a value that is object-scoped in the model +/// (`com.amazonaws.s3#BucketCannedACL` omits it), so the bucket-side question +/// never has this answer. The header is still accepted on a bucket, resolving to +/// the owner's FULL_CONTROL, which is what S3 ignoring it produces. +pub(crate) fn acl_reaches_past_owner(acl: &str, owner_id: &str) -> bool { + acl == "aws-exec-read" + || !canned_acl_grants(acl, owner_id).iter().all(|g| { + g.permission == "FULL_CONTROL" + && g.grantee_type == "CanonicalUser" + && g.grantee_id.as_deref() == Some(owner_id) + }) +} + pub(crate) fn canned_acl_grants(acl: &str, owner_id: &str) -> Vec { let owner_grant = AclGrant { grantee_type: "CanonicalUser".to_string(), diff --git a/crates/fakecloud-s3/src/service/tests.rs b/crates/fakecloud-s3/src/service/tests.rs index d64cc1f8d..f85b88584 100644 --- a/crates/fakecloud-s3/src/service/tests.rs +++ b/crates/fakecloud-s3/src/service/tests.rs @@ -6,7 +6,7 @@ fn s3_condition_keys_emits_list_params() { q.insert("prefix".to_string(), "logs/".to_string()); q.insert("delimiter".to_string(), "/".to_string()); q.insert("max-keys".to_string(), "100".to_string()); - let keys = s3_condition_keys("ListObjectsV2", &q); + let keys = s3_condition_keys("ListObjectsV2", &q, &HeaderMap::new()); assert_eq!(keys.get("s3:prefix"), Some(&vec!["logs/".to_string()])); assert_eq!(keys.get("s3:delimiter"), Some(&vec!["/".to_string()])); assert_eq!(keys.get("s3:max-keys"), Some(&vec!["100".to_string()])); @@ -15,7 +15,7 @@ fn s3_condition_keys_emits_list_params() { #[test] fn s3_condition_keys_omits_absent_params() { let q = std::collections::HashMap::new(); - let keys = s3_condition_keys("ListObjectsV2", &q); + let keys = s3_condition_keys("ListObjectsV2", &q, &HeaderMap::new()); assert!(keys.is_empty()); } @@ -23,7 +23,7 @@ fn s3_condition_keys_omits_absent_params() { fn s3_condition_keys_partial_params() { let mut q = std::collections::HashMap::new(); q.insert("prefix".to_string(), "archive/".to_string()); - let keys = s3_condition_keys("ListObjects", &q); + let keys = s3_condition_keys("ListObjects", &q, &HeaderMap::new()); assert_eq!(keys.len(), 1); assert_eq!(keys.get("s3:prefix"), Some(&vec!["archive/".to_string()])); } @@ -32,9 +32,9 @@ fn s3_condition_keys_partial_params() { fn s3_condition_keys_empty_for_non_list_actions() { let mut q = std::collections::HashMap::new(); q.insert("prefix".to_string(), "logs/".to_string()); - assert!(s3_condition_keys("GetObject", &q).is_empty()); - assert!(s3_condition_keys("PutObject", &q).is_empty()); - assert!(s3_condition_keys("ListBuckets", &q).is_empty()); + assert!(s3_condition_keys("GetObject", &q, &HeaderMap::new()).is_empty()); + assert!(s3_condition_keys("PutObject", &q, &HeaderMap::new()).is_empty()); + assert!(s3_condition_keys("ListBuckets", &q, &HeaderMap::new()).is_empty()); } #[test] @@ -4704,6 +4704,183 @@ fn create_bucket_tags_decode_xml_entities() { ); } +#[test] +fn create_bucket_requires_a_permission_per_setting_it_configures() { + use fakecloud_core::service::AwsService as _; + + let svc = make_service(); + let names = |req: &AwsRequest| -> Vec<&'static str> { + svc.iam_actions_for(req).iter().map(|a| a.action).collect() + }; + + // A plain create needs only s3:CreateBucket. + let plain = make_request(Method::PUT, "/perm-plain", &[], b""); + assert_eq!(names(&plain), vec!["CreateBucket"]); + + // An ACL that reaches past the owner needs PutBucketAcl -- the settings + // persist now, so a create that configures them is a configuration call as + // much as a create. + let mut canned = make_request(Method::PUT, "/perm-acl", &[], b""); + canned + .headers + .insert("x-amz-acl", "public-read".parse().unwrap()); + assert_eq!(names(&canned), vec!["CreateBucket", "PutBucketAcl"]); + + // ...but `private` does not. The model'"'"'s CreateBucket permissions are + // explicit: "if you set the ACL to private, or if you don'"'"'t specify any + // ACLs, only the s3:CreateBucket permission is required". A least-privilege + // caller that always sends `--acl private` must not be refused. + let mut private = make_request(Method::PUT, "/perm-private", &[], b""); + private + .headers + .insert("x-amz-acl", "private".parse().unwrap()); + assert_eq!(names(&private), vec!["CreateBucket"]); + + let mut granted = make_request(Method::PUT, "/perm-grant", &[], b""); + granted.headers.insert( + "x-amz-grant-read", + "uri=http://acs.amazonaws.com/groups/global/AllUsers" + .parse() + .unwrap(), + ); + assert_eq!(names(&granted), vec!["CreateBucket", "PutBucketAcl"]); + + let mut owned = make_request(Method::PUT, "/perm-own", &[], b""); + owned.headers.insert( + "x-amz-object-ownership", + "BucketOwnerEnforced".parse().unwrap(), + ); + assert_eq!( + names(&owned), + vec!["CreateBucket", "PutBucketOwnershipControls"] + ); + + // Object lock turns versioning on, so both permissions are required. + let mut locked = make_request(Method::PUT, "/perm-lock", &[], b""); + locked + .headers + .insert("x-amz-bucket-object-lock-enabled", "true".parse().unwrap()); + assert_eq!( + names(&locked), + vec![ + "CreateBucket", + "PutBucketObjectLockConfiguration", + "PutBucketVersioning" + ] + ); + + // `aws-exec-read` resolves to owner-only grants (its READ to the EC2 + // service's canonical user is not modeled), so judging by grants alone let + // it slip past PutBucketAcl -- while the BucketOwnerEnforced conflict check + // has always treated it as an ACL reaching outside the owner. CloudFormation + // sends it for `AccessControl: AwsExecRead`, so the gap was reachable. + let mut exec_read = make_request(Method::PUT, "/perm-exec", &[], b""); + exec_read + .headers + .insert("x-amz-acl", "aws-exec-read".parse().unwrap()); + assert_eq!(names(&exec_read), vec!["CreateBucket", "PutBucketAcl"]); + + // A blank value asks for nothing, so it configures nothing either -- the + // condition keys skip blanks, and demanding a permission whose key is absent + // turns a `StringEquals`-gated Allow into a 403 on a request the handler + // answers with a 400. + let mut blank = make_request(Method::PUT, "/perm-blank", &[], b""); + blank.headers.insert("x-amz-acl", "".parse().unwrap()); + blank + .headers + .insert("x-amz-object-ownership", "".parse().unwrap()); + assert_eq!(names(&blank), vec!["CreateBucket"]); + + // A false object-lock header configures nothing. + let mut unlocked = make_request(Method::PUT, "/perm-unlocked", &[], b""); + unlocked + .headers + .insert("x-amz-bucket-object-lock-enabled", "false".parse().unwrap()); + assert_eq!(names(&unlocked), vec!["CreateBucket"]); + + // Everything at once, tags included, in one authorization set. + let mut everything = make_request( + Method::PUT, + "/perm-all", + &[], + b"teama", + ); + everything + .headers + .insert("x-amz-acl", "public-read".parse().unwrap()); + everything + .headers + .insert("x-amz-object-ownership", "ObjectWriter".parse().unwrap()); + assert_eq!( + names(&everything), + vec![ + "CreateBucket", + "TagResource", + "PutBucketAcl", + "PutBucketOwnershipControls" + ] + ); +} + +#[test] +fn acl_condition_keys_are_populated_from_the_request_headers() { + // A guardrail like `Deny s3:CreateBucket when s3:x-amz-acl != private` is + // useless while the key is never emitted, which is what made the + // over-permission above invisible to policy. + let mut headers = HeaderMap::new(); + headers.insert("x-amz-acl", "public-read".parse().unwrap()); + let keys = s3_condition_keys("CreateBucket", &HashMap::new(), &headers); + assert_eq!( + keys.get("s3:x-amz-acl"), + Some(&vec!["public-read".to_string()]) + ); + + let mut grants = HeaderMap::new(); + grants.insert("x-amz-grant-full-control", "id=abc123".parse().unwrap()); + let keys = s3_condition_keys("PutObject", &HashMap::new(), &grants); + assert_eq!( + keys.get("s3:x-amz-grant-full-control"), + Some(&vec!["id=abc123".to_string()]) + ); + + // Object ownership and object lock are gated by `iam_actions_for` too, so + // their keys are emitted as well -- `Deny CreateBucket unless + // s3:x-amz-object-ownership == BucketOwnerEnforced` is a guardrail people + // actually write. + let mut settings = HeaderMap::new(); + settings.insert( + "x-amz-object-ownership", + "BucketOwnerEnforced".parse().unwrap(), + ); + settings.insert("x-amz-bucket-object-lock-enabled", "true".parse().unwrap()); + let keys = s3_condition_keys("CreateBucket", &HashMap::new(), &settings); + assert_eq!( + keys.get("s3:x-amz-object-ownership"), + Some(&vec!["BucketOwnerEnforced".to_string()]) + ); + // AWS defines no condition key for the object-lock header, so emitting one + // would give policy authors a guardrail that works here and silently does + // nothing on AWS. + assert_eq!(keys.get("s3:x-amz-bucket-object-lock-enabled"), None); + + // Absent headers emit nothing, so a policy condition on them safe-fails to + // "does not apply" rather than matching an empty value. + assert!(s3_condition_keys("CreateBucket", &HashMap::new(), &HeaderMap::new()).is_empty()); + + // A present-but-empty value is skipped for the same reason, so a `Null` + // check does not read it as set. + let mut blank = HeaderMap::new(); + blank.insert("x-amz-object-ownership", "".parse().unwrap()); + blank.insert("x-amz-bucket-object-lock-enabled", "".parse().unwrap()); + blank.insert("x-amz-acl", "".parse().unwrap()); + blank.insert("x-amz-grant-read", "".parse().unwrap()); + assert!( + s3_condition_keys("CreateBucket", &HashMap::new(), &blank).is_empty(), + "blank values must be skipped on EVERY header, not most of them: {:?}", + s3_condition_keys("CreateBucket", &HashMap::new(), &blank) + ); +} + #[test] fn create_bucket_with_tags_also_requires_tag_resource() { use fakecloud_core::service::AwsService as _; @@ -4754,11 +4931,35 @@ fn create_bucket_request_tags_feed_condition_keys() { &[], b"teama", ); - for action in ["CreateBucket", "TagResource"] { + // Every action the create implies, not just the create and its TagResource: + // dispatch rebuilds the context per action, and AWS evaluates one context + // for the whole request. An `aws:RequestTag/*` guardrail that permits the + // create must not then deny the ACL or object-lock action it implies. + for action in [ + "CreateBucket", + "TagResource", + "PutBucketAcl", + "PutBucketOwnershipControls", + "PutBucketObjectLockConfiguration", + "PutBucketVersioning", + ] { let tags = s3_request_tags(&req, action).expect("tags extracted"); assert_eq!(tags.get("team").map(String::as_str), Some("a"), "{action}"); } + // On their own operations those actions carry no create-time tag set: the + // body is not a `CreateBucketConfiguration`, so the map is empty rather + // than picking up a foreign body's tags. + let acl_body = make_request( + Method::PUT, + "/req-acl?acl", + &[("acl", "")], + b"o", + ); + assert!(s3_request_tags(&acl_body, "PutBucketAcl") + .expect("tags extracted") + .is_empty()); + // A body with no tag set yields an empty map, not a miss. let plain = make_request(Method::PUT, "/req-plain", &[], b""); assert!(s3_request_tags(&plain, "CreateBucket") diff --git a/website/content/docs/reference/security.md b/website/content/docs/reference/security.md index cb7284877..9e0d4967f 100644 --- a/website/content/docs/reference/security.md +++ b/website/content/docs/reference/security.md @@ -130,6 +130,9 @@ Every operator supports the `...IfExists` suffix (missing key evaluates to `true | `s3:prefix` | `s3:ListObjects`, `s3:ListObjectsV2` | `?prefix=` query param | | `s3:delimiter` | `s3:ListObjects`, `s3:ListObjectsV2` | `?delimiter=` query param | | `s3:max-keys` | `s3:ListObjects`, `s3:ListObjectsV2` | `?max-keys=` query param | +| `s3:x-amz-acl` | any request carrying the header (`CreateBucket`, `PutObject`, `CopyObject`, `CreateMultipartUpload`, `PutBucketAcl`, `PutObjectAcl`) | `x-amz-acl` header | +| `s3:x-amz-grant-read`, `-write`, `-read-acp`, `-write-acp`, `-full-control` | any request carrying the header | matching `x-amz-grant-*` header | +| `s3:x-amz-object-ownership` | any request carrying the header, which in practice is `CreateBucket` | `x-amz-object-ownership` header. AWS defines this key for `CreateBucket` only; `PutBucketOwnershipControls` carries the value in its XML body rather than a header, so no key is populated there | | `sns:Protocol` | `sns:Subscribe` | `Protocol` request parameter | | `sns:Endpoint` | `sns:Subscribe` | `Endpoint` request parameter | | `lambda:FunctionArn` | `lambda:AddPermission` | Target function ARN resolved from the path | diff --git a/website/content/docs/services/s3.md b/website/content/docs/services/s3.md index 4e9a171e8..18943ff47 100644 --- a/website/content/docs/services/s3.md +++ b/website/content/docs/services/s3.md @@ -16,6 +16,7 @@ fakecloud implements **107 of 107** S3 operations at 100% Smithy conformance. - **Versioning** — enable/suspend, list object versions, delete specific versions. Suspended buckets follow the AWS null-version rules: a write takes over the `null` version (replacing whatever held it), a delete stacks a `null`-id delete marker, earlier versions are kept either way, and an object written before versioning was enabled survives later writes as the `null` version. Suspending is rejected with `InvalidBucketState` on an Object Lock bucket, as on AWS. - **Encryption** — SSE-S3, SSE-KMS (real envelope encryption through the KMS hook with the `aws:s3:arn` encryption context), SSE-C - **Bucket subresources** — policy, CORS, lifecycle, logging, website, public access block, object lock, replication, ownership, inventory, encryption, accelerate, request payment, tagging +- **Permissions a configuring create needs** — `CreateBucket` is authorized as `s3:CreateBucket` plus one action per setting the request configures, as on AWS: `s3:TagResource` for a `CreateBucketConfiguration` tag set, `s3:PutBucketAcl` for an ACL that grants anything beyond the bucket owner (`private`, and a create with no ACL, need only `s3:CreateBucket`), `s3:PutBucketOwnershipControls` for `x-amz-object-ownership`, and `s3:PutBucketObjectLockConfiguration` plus `s3:PutBucketVersioning` for `x-amz-bucket-object-lock-enabled: true`. A principal holding only `s3:CreateBucket` can still create a plain bucket. The `s3:x-amz-acl` and `s3:x-amz-grant-*` condition keys are populated from those headers, so a policy can pin down which ACL a create may ask for. - **Bucket tags at create time** — `CreateBucket` honors the `Tags` tag set inside `CreateBucketConfiguration`, so a bucket comes out of `CreateBucket` already tagged and `GetBucketTagging` returns the set without a follow-up `PutBucketTagging`. This is the path the AWS Terraform provider (6.x) uses for `aws_s3_bucket`'s `tags`. Duplicate keys and `aws:`-prefixed keys are rejected with `InvalidTag` and no bucket is created, and the tags persist across a restart like any other bucket subresource. Under `--iam`, a tagged create is authorized as `s3:CreateBucket` **and** `s3:TagResource`, as on AWS — a grant of `s3:CreateBucket` alone still creates untagged buckets — and the tag set populates `aws:RequestTag/` / `aws:TagKeys` so policies can condition on it. - **CORS** — `PutBucketCors` rules drive real browser CORS: the `OPTIONS` preflight is matched on origin, `Access-Control-Request-Method` and every header in `Access-Control-Request-Headers` before returning `Access-Control-Allow-Origin/-Methods/-Headers` and `Access-Control-Max-Age`, and an actual request carrying `Origin` gets `Access-Control-Allow-Origin` plus `Access-Control-Expose-Headers`. Actual requests are matched on method as well as origin, so a rule allowing only `GET` hands no allow-origin to a `DELETE`. Multipart operations are CORS-evaluated like any other request, so browser multipart upload works. Errors from S3 itself are evaluated too, so a 404 `NoSuchKey` fetched by an allowed origin carries `Access-Control-Allow-Origin` and reaches the caller as a real 404 rather than an opaque CORS failure. Denials raised before the request reaches S3 (SigV4 or IAM, under `--verify-sigv4` / `--iam`) are not CORS-decorated, so under auth enforcement a rejected browser request still surfaces as an opaque failure. `Vary: Origin, Access-Control-Request-Headers, Access-Control-Request-Method` keeps a cache from reusing one origin's response for a different origin. **Every** preflight response carries it, since a preflight's outcome always turns on `Origin`: `200` when the rules allow it, `403 AccessForbidden` when they do not, and `400 BadRequest` ("Insufficient information. Origin request header needed.") when the request omits `Origin` altogether. On the actual-request path it goes on every response evaluated against a bucket's CORS config, successes and errors alike, while a request without `Origin` is not CORS-evaluated and is served normally with neither header, as on S3. A concrete allowed origin also gets `Access-Control-Allow-Credentials: true`, so `credentials: 'include'` works. Note that an actual request declares no headers, so it can match an earlier, broader rule than the preflight did: if a `*` rule precedes the concrete one, the actual response carries `Access-Control-Allow-Origin: *` and no credentials, and the browser blocks a credentialed request that just passed preflight. That is S3's behavior too — order the concrete rule first. `AllowedOrigin` and `AllowedHeader` each take one `*` anywhere in the value (`https://*.example.com`, `x-amz-*`). `ExposeHeader` takes no wildcard at all, as on S3. A rule missing `AllowedMethods` or `AllowedOrigins`, carrying more than one wildcard in a value, or setting a non-numeric `MaxAgeSeconds`, is rejected at write time rather than silently matching nothing. CloudFormation's `CorsConfiguration` goes through the same validation, so a template cannot deploy a bucket whose preflights all fail. - **Bucket and object ACLs** — a canned `x-amz-acl`, the `x-amz-grant-*` headers and an `AccessControlPolicy` body are mutually exclusive, as on S3: naming an ACL two ways is `InvalidRequest` rather than one of them silently winning. Canned values are checked against the set S3 accepts for the target (bucket ACLs take `private`, `public-read`, `public-read-write`, `authenticated-read`, `aws-exec-read` and `log-delivery-write`; `bucket-owner-read` and `bucket-owner-full-control` are object-only), and a grant naming no resolvable grantee is rejected instead of being stored as an entry that matches nobody. `emailAddress=` grantees are kept as `AmazonCustomerByEmail` so they round-trip, but they are not resolved to a canonical user and so convey no access. A bucket whose `ObjectOwnership` is `BucketOwnerEnforced` rejects every ACL-setting call, `CreateMultipartUpload` and `CopyObject` included. The object-write paths (`PutObject`, `CopyObject`, `CreateMultipartUpload`) make one exception, as AWS does: a bucket-owner-full-control ACL is accepted, whether spelled `x-amz-acl: bucket-owner-full-control` or as the equivalent explicit grant (`x-amz-grant-full-control` naming the owner and nobody else). The exception is that ACL specifically, not any ACL that happens to resolve to owner-only grants: `--acl private` is still `AccessControlListNotSupported` there, since AWS accepts only a request that specifies no ACL at all or one that specifies bucket-owner-full-control. `PutObjectAcl` and `PutBucketAcl` reject every ACL on such a bucket. `CopyObject` honors `x-amz-acl` and the `x-amz-grant-*` headers for the destination object under the same rules; a copy is a new object, so an unspecified ACL is the default owner grant rather than whatever the source carried.