From 9979ab12a9206f64b599fb9d7043792569ab6410 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Fri, 25 Sep 2026 04:09:26 -0300 Subject: [PATCH] fix(s3): persist the ACL, object lock and ownership a bucket is created with CreateBucket set acl_grants, object_lock_config and ownership_controls in memory and wrote none of them. None live in BucketMeta, and the loader restores the latter two as None and falls back to the default owner grant when acl.toml is absent, so after a restart a bucket created with `x-amz-acl: public-read` came back owner-only and one created with `x-amz-bucket-object-lock-enabled: true` came back with no lock configuration at all -- which silently stops retention being enforced. Versioning survived, since that one IS in meta, which is what made the gap easy to miss. Each of the three is now written when the create sets it, and the bucket's stored subresources are cleared first so a create cannot inherit a sidecar left by a create or delete that stopped partway, or by a `/_fakecloud/reset` (which clears memory and deliberately leaves the store alone). The meta is written before that clear, so a create that fails has destroyed nothing; `objects/` is never touched, because the loader skips a bucket whose objects it cannot read and re-creating the name must not be what destroys data that is still recoverable. Persisting the ACL made a set of latent ACL bugs durable, so they are fixed here too: - A canned `x-amz-acl`, the `x-amz-grant-*` headers and an AccessControlPolicy body are mutually exclusive, as on S3. Letting one win silently discarded what the others asked for. - Canned values are validated against the set the target accepts. An unrecognized one fell through to owner-only, so a typo wiped every public grant. The object-scoped `bucket-owner-*` values are accepted on a bucket and resolve to owner-only, which is what S3 ignoring them produces. - A grant naming no resolvable grantee is refused rather than stored as an entry that can never match anyone, and grantee values are trimmed so a pretty-printed does not bake whitespace into a canonical id. - `emailAddress=` grantees are kept as AmazonCustomerByEmail instead of being dropped, and logged as unenforced: nothing resolves them to a canonical user, so the grant conveys no access. - PutBucketAcl honors the grant headers; without that arm a `put-bucket-acl --grant-read` fell through to an empty body and wiped the ACL. - PutObjectAcl updates the versioned copy of the object too, so a versionId read no longer answers a different ACL than the unversioned one. - An ACL on a bucket whose ownership disables them is refused by every path that can set one, CreateMultipartUpload included. - PutBucketOwnershipControls validates every ObjectOwnership the document carries -- `bucket_owner_enforced` matches the stored text, so a second rule could turn ACLs off while the API reported otherwise -- and persists before mutating memory. And three storage-integrity fixes in the same paths: - Every writer of a persisted snapshot propagates a serialization failure rather than writing an empty document. An empty acl.toml is worse than none: the loader takes its presence to mean an explicit ACL and drops the owner grant. A sidecar whose grants are all unusable now falls back to that owner grant too. - delete_object removes the sidecar before the body. `load` iterates `*.toml` and treats a sidecar with no body as fatal for the bucket, while an orphan body is never read, so the body is the residue an interrupted delete should leave. A body that cannot be unlinked warns rather than failing a delete that did happen. - The analytics, intelligent-tiering and metrics sidecars neither default silently (losing the configurations, which the next Put then makes permanent) nor fail the bucket over a reporting config: they are skipped with a warning. Fixes the create-time half of the gap found while reviewing #2557. --- crates/fakecloud-e2e/tests/s3_persistence.rs | 427 +++++++++- crates/fakecloud-persistence/src/lib.rs | 2 +- crates/fakecloud-persistence/src/s3.rs | 78 +- crates/fakecloud-s3/src/persistence.rs | 130 ++- crates/fakecloud-s3/src/service/acl.rs | 76 +- crates/fakecloud-s3/src/service/buckets.rs | 239 +++++- .../src/service/config/bucket_inventory.rs | 4 +- crates/fakecloud-s3/src/service/config/mod.rs | 82 +- .../src/service/config/ownership.rs | 45 +- .../src/service/config/tagging.rs | 2 +- crates/fakecloud-s3/src/service/mod.rs | 363 ++++++++- crates/fakecloud-s3/src/service/multipart.rs | 24 +- .../fakecloud-s3/src/service/objects/mod.rs | 4 +- .../fakecloud-s3/src/service/objects/write.rs | 18 +- crates/fakecloud-s3/src/service/tests.rs | 742 ++++++++++++++++++ website/content/docs/services/s3.md | 3 +- 16 files changed, 2124 insertions(+), 115 deletions(-) diff --git a/crates/fakecloud-e2e/tests/s3_persistence.rs b/crates/fakecloud-e2e/tests/s3_persistence.rs index 4ffff317d..fb6330302 100644 --- a/crates/fakecloud-e2e/tests/s3_persistence.rs +++ b/crates/fakecloud-e2e/tests/s3_persistence.rs @@ -4,9 +4,10 @@ use std::time::Duration; use aws_sdk_s3::primitives::ByteStream; use aws_sdk_s3::types::{ - BucketVersioningStatus, CompletedMultipartUpload, CompletedPart, CorsConfiguration, CorsRule, - CreateBucketConfiguration, ObjectLockConfiguration, ObjectLockEnabled, ObjectLockLegalHold, - ObjectLockLegalHoldStatus, ServerSideEncryption, ServerSideEncryptionByDefault, + BucketCannedAcl, BucketVersioningStatus, CompletedMultipartUpload, CompletedPart, + CorsConfiguration, CorsRule, CreateBucketConfiguration, ObjectCannedAcl, + ObjectLockConfiguration, ObjectLockEnabled, ObjectLockLegalHold, ObjectLockLegalHoldStatus, + ObjectOwnership, ServerSideEncryption, ServerSideEncryptionByDefault, ServerSideEncryptionConfiguration, ServerSideEncryptionRule, StorageClass, Tag, Tagging, VersioningConfiguration, }; @@ -1265,6 +1266,206 @@ async fn persistence_body_cache_small_and_large_objects() { } } +#[tokio::test] +async fn persistence_create_time_acl_object_lock_and_ownership_round_trip() { + // The canned ACL, object-lock enablement, and object-ownership rule a + // bucket is CREATED with live in subresources, not in BucketMeta, so + // without an explicit write at create time they were all gone after a + // restart -- object-lock retention silently stopped being enforced, and a + // public-read bucket came back owner-only. + let tmp = tempfile::tempdir().unwrap(); + let mut server = TestServer::start_persistent(tmp.path()).await; + let client = server.s3_client().await; + + client + .create_bucket() + .bucket("create-subres") + .acl(BucketCannedAcl::PublicRead) + .object_lock_enabled_for_bucket(true) + .object_ownership(ObjectOwnership::ObjectWriter) + .send() + .await + .unwrap(); + + server.restart().await; + let client = server.s3_client().await; + + let acl = client + .get_bucket_acl() + .bucket("create-subres") + .send() + .await + .unwrap(); + assert!( + acl.grants().iter().any(|g| { + g.grantee() + .and_then(|gr| gr.uri()) + .is_some_and(|uri| uri.contains("AllUsers")) + }), + "create-time public-read grant lost across restart: {:?}", + acl.grants() + ); + + let lock = client + .get_object_lock_configuration() + .bucket("create-subres") + .send() + .await; + assert!( + lock.is_ok(), + "create-time object lock lost across restart: {:?}", + lock.err() + ); + + let ownership = client + .get_bucket_ownership_controls() + .bucket("create-subres") + .send() + .await + .expect("create-time ownership controls lost across restart"); + assert_eq!( + ownership + .ownership_controls() + .and_then(|oc| oc.rules().first()) + .map(|r| r.object_ownership()), + Some(&ObjectOwnership::ObjectWriter), + ); +} + +#[tokio::test] +async fn persistence_plain_create_clears_orphan_create_time_subresources() { + // A create that failed partway leaves sidecars in a directory the loader + // skips (no meta.toml). A later plain create of that name must not adopt + // them. + let tmp = tempfile::tempdir().unwrap(); + let mut server = TestServer::start_persistent(tmp.path()).await; + let _client = server.s3_client().await; + + let orphan_dir = tmp.path().join("s3").join("buckets").join("orphan-subres"); + std::fs::create_dir_all(&orphan_dir).unwrap(); + std::fs::write( + orphan_dir.join("ownership.toml"), + "BucketOwnerEnforced", + ) + .unwrap(); + std::fs::write( + orphan_dir.join("object_lock.toml"), + "Enabled", + ) + .unwrap(); + // Subresources the create path never writes must be cleared too -- the + // whole directory is stale, not just the three create-time sidecars. + std::fs::write( + orphan_dir.join("policy.toml"), + r#"{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Principal":"*","Action":"s3:GetObject","Resource":"arn:aws:s3:::orphan-subres/*"}]}"#, + ) + .unwrap(); + std::fs::write(orphan_dir.join("versioning.toml"), "Enabled").unwrap(); + + server.restart().await; + let client = server.s3_client().await; + client + .create_bucket() + .bucket("orphan-subres") + .send() + .await + .unwrap(); + + server.restart().await; + let client = server.s3_client().await; + + let ownership = client + .get_bucket_ownership_controls() + .bucket("orphan-subres") + .send() + .await; + assert!( + ownership.is_err(), + "plain create inherited an orphan ownership rule" + ); + let lock = client + .get_object_lock_configuration() + .bucket("orphan-subres") + .send() + .await; + assert!( + lock.is_err(), + "plain create inherited an orphan object-lock configuration" + ); + let policy = client + .get_bucket_policy() + .bucket("orphan-subres") + .send() + .await; + assert!(policy.is_err(), "plain create inherited an orphan policy"); + let versioning = client + .get_bucket_versioning() + .bucket("orphan-subres") + .send() + .await + .unwrap(); + assert!( + versioning.status().is_none(), + "plain create inherited orphan versioning state: {:?}", + versioning.status() + ); +} + +#[tokio::test] +async fn persistence_create_bucket_grant_headers_round_trip() { + // CreateBucket honors the x-amz-grant-* headers as well as the canned + // x-amz-acl, and the resulting ACL is an explicit one, so it gets an + // acl.toml and survives a restart. + let tmp = tempfile::tempdir().unwrap(); + let mut server = TestServer::start_persistent(tmp.path()).await; + let client = server.s3_client().await; + + client + .create_bucket() + .bucket("grant-hdr") + .grant_read("uri=http://acs.amazonaws.com/groups/global/AllUsers") + .send() + .await + .unwrap(); + + let before = client + .get_bucket_acl() + .bucket("grant-hdr") + .send() + .await + .unwrap(); + assert!( + before.grants().iter().any(|g| { + g.permission().map(|p| p.as_str()) == Some("READ") + && g.grantee() + .and_then(|gr| gr.uri()) + .is_some_and(|uri| uri.contains("AllUsers")) + }), + "grant header ignored at create: {:?}", + before.grants() + ); + + server.restart().await; + let client = server.s3_client().await; + + let after = client + .get_bucket_acl() + .bucket("grant-hdr") + .send() + .await + .unwrap(); + assert!( + after.grants().iter().any(|g| { + g.permission().map(|p| p.as_str()) == Some("READ") + && g.grantee() + .and_then(|gr| gr.uri()) + .is_some_and(|uri| uri.contains("AllUsers")) + }), + "create-time grant headers lost across restart: {:?}", + after.grants() + ); +} + #[tokio::test] async fn persistence_create_bucket_tags_survive_restart() { // Tags supplied through CreateBucketConfiguration go to the same Tags @@ -1347,3 +1548,223 @@ async fn persistence_untagged_create_clears_an_orphan_tag_file() { let msg = format!("{err:?}"); assert!(msg.contains("NoSuchTagSet"), "unexpected error: {msg}"); } + +#[tokio::test] +async fn persistence_create_after_reset_reuses_the_name() { + // `/_fakecloud/reset/s3` clears in-memory state and deliberately leaves the + // store alone, so every bucket ever created still has a readable meta.toml + // on disk afterwards. Re-creating those names has to keep working -- gating + // the "unloaded state" refusal on files existing would brick all of them. + let tmp = tempfile::tempdir().unwrap(); + let server = TestServer::start_persistent(tmp.path()).await; + let client = server.s3_client().await; + + client + .create_bucket() + .bucket("reset-reuse") + .send() + .await + .unwrap(); + client + .put_object() + .bucket("reset-reuse") + .key("k.txt") + .body(ByteStream::from_static(b"v")) + .send() + .await + .unwrap(); + // The bucket needs a stored configuration for the assertion below to mean + // anything: without a tag set there is nothing that could be adopted. + client + .put_bucket_tagging() + .bucket("reset-reuse") + .tagging( + Tagging::builder() + .tag_set(Tag::builder().key("era").value("before").build().unwrap()) + .build() + .unwrap(), + ) + .send() + .await + .unwrap(); + + let status = reqwest::Client::new() + .post(format!("{}/_fakecloud/reset/s3", server.endpoint())) + .send() + .await + .expect("reset should respond") + .status(); + assert!(status.is_success(), "reset failed: {status}"); + + // The name is free again as far as the running server is concerned. + client + .create_bucket() + .bucket("reset-reuse") + .send() + .await + .expect("re-creating a bucket after a reset must succeed"); + + // The bucket's stored CONFIGURATION does not carry over: the create clears + // every sidecar for the name, so the tag set from before the reset is gone + // rather than being restored on the next load. + let mut server = server; + server.restart().await; + let client = server.s3_client().await; + let tags = client + .get_bucket_tagging() + .bucket("reset-reuse") + .send() + .await; + assert!( + tags.is_err(), + "the pre-reset tag set was adopted by the re-created bucket" + ); +} + +#[tokio::test] +async fn persistence_recreating_a_load_skipped_bucket_does_not_destroy_its_objects() { + // The loader skips a bucket it cannot fully read (a corrupt object meta, a + // missing part body) and logs a warning -- the data is still on disk and + // recoverable by fixing the one bad file. Such a bucket is absent from + // ListBuckets, so its name looks free: re-creating it must not be what + // destroys the objects, which is why the create clears only the sidecars. + let tmp = tempfile::tempdir().unwrap(); + let mut server = TestServer::start_persistent(tmp.path()).await; + let client = server.s3_client().await; + + client + .create_bucket() + .bucket("skipped") + .send() + .await + .unwrap(); + for key in ["keep.txt", "corrupt.txt"] { + client + .put_object() + .bucket("skipped") + .key(key) + .body(ByteStream::from_static(b"precious")) + .send() + .await + .unwrap(); + } + + let objects_dir = tmp + .path() + .join("s3") + .join("buckets") + .join("skipped") + .join("objects"); + let corrupt_meta = objects_dir.join("corrupt.txt").join("null.toml"); + assert!(corrupt_meta.exists(), "expected {corrupt_meta:?} to exist"); + std::fs::write(&corrupt_meta, "not valid toml = = =").unwrap(); + + server.restart().await; + let client = server.s3_client().await; + + let list = client.list_buckets().send().await.unwrap(); + assert!( + !list.buckets().iter().any(|b| b.name() == Some("skipped")), + "expected the unreadable bucket to be skipped on load" + ); + + client + .create_bucket() + .bucket("skipped") + .send() + .await + .unwrap(); + assert!( + objects_dir.join("keep.txt").join("null.bin").exists(), + "re-creating a load-skipped bucket destroyed its objects" + ); + + // Repairing the one bad file brings the whole bucket back. + std::fs::remove_dir_all(objects_dir.join("corrupt.txt")).unwrap(); + server.restart().await; + let client = server.s3_client().await; + let body = client + .get_object() + .bucket("skipped") + .key("keep.txt") + .send() + .await + .expect("the repaired bucket should load") + .body + .collect() + .await + .unwrap() + .into_bytes(); + assert_eq!(&body[..], b"precious"); +} + +#[tokio::test] +async fn persistence_put_object_acl_survives_restart() { + // PutObjectAcl writes the object's meta sidecar, and that snapshot copies + // acl_grants -- so snapshotting before applying the new grants persists the + // OLD ACL and leaves the new one memory-only, which only a restart reveals. + let tmp = tempfile::tempdir().unwrap(); + let mut server = TestServer::start_persistent(tmp.path()).await; + let client = server.s3_client().await; + + client + .create_bucket() + .bucket("obj-acl-persist") + .send() + .await + .unwrap(); + client + .put_object() + .bucket("obj-acl-persist") + .key("k.txt") + .body(ByteStream::from_static(b"v")) + .send() + .await + .unwrap(); + + client + .put_object_acl() + .bucket("obj-acl-persist") + .key("k.txt") + .acl(ObjectCannedAcl::PublicRead) + .send() + .await + .unwrap(); + + let before = client + .get_object_acl() + .bucket("obj-acl-persist") + .key("k.txt") + .send() + .await + .unwrap(); + assert!( + before.grants().iter().any(|g| { + g.grantee() + .and_then(|gr| gr.uri()) + .is_some_and(|u| u.contains("AllUsers")) + }), + "public-read not applied: {:?}", + before.grants() + ); + + server.restart().await; + let client = server.s3_client().await; + + let after = client + .get_object_acl() + .bucket("obj-acl-persist") + .key("k.txt") + .send() + .await + .unwrap(); + assert!( + after.grants().iter().any(|g| { + g.grantee() + .and_then(|gr| gr.uri()) + .is_some_and(|u| u.contains("AllUsers")) + }), + "object ACL reverted across restart: {:?}", + after.grants() + ); +} diff --git a/crates/fakecloud-persistence/src/lib.rs b/crates/fakecloud-persistence/src/lib.rs index 692de1870..f7d26a43a 100644 --- a/crates/fakecloud-persistence/src/lib.rs +++ b/crates/fakecloud-persistence/src/lib.rs @@ -11,6 +11,6 @@ pub use s3::{ AclGrantSnapshot, AclSnapshot, AnnotationSnapshot, BodyRef, BodySource, BucketMeta, BucketSnapshot, BucketSubresource, InventorySnapshot, LoadedMpu, LoadedObject, LoadedPart, MemoryS3Store, MpuInit, ObjectMeta, S3State as S3StateSnapshot, S3Store, StoreError, - StoreResult, TagsSnapshot, UploadPartMeta, + StoreResult, TagsSnapshot, UploadPartMeta, ALL_SUBRESOURCES, }; pub use snapshot::{DiskSnapshotStore, MemorySnapshotStore, SnapshotHook, SnapshotStore}; diff --git a/crates/fakecloud-persistence/src/s3.rs b/crates/fakecloud-persistence/src/s3.rs index 87b6392b2..01a0d3869 100644 --- a/crates/fakecloud-persistence/src/s3.rs +++ b/crates/fakecloud-persistence/src/s3.rs @@ -982,11 +982,29 @@ impl S3Store for DiskS3Store { fn delete_object(&self, bucket: &str, key: &str, version: Option<&str>) -> StoreResult<()> { let (dir, bin_path, toml_path) = self.object_paths(bucket, key, version); - for p in [&bin_path, &toml_path] { - match std::fs::remove_file(p) { - Ok(_) => {} - Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} - Err(e) => return Err(e.into()), + // Sidecar first, body second. `load` iterates `*.toml` and treats a + // sidecar whose `.bin` is gone as a hard error that costs the whole + // bucket, while an orphan `.bin` is never looked at. So if this is + // interrupted -- a signal, or the second `remove_file` failing -- the + // residue left behind has to be the body, not the sidecar. + // The sidecar is the durable record, so its removal is the one that has + // to succeed. Once it is gone the object is deleted as far as every + // reader is concerned, and failing the call over a body that could not + // be unlinked would report a delete that did happen as an error -- + // while still leaving the caller no way to reclaim the bytes. + match std::fs::remove_file(&toml_path) { + Ok(_) => {} + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(e.into()), + } + if let Err(e) = std::fs::remove_file(&bin_path) { + if e.kind() != std::io::ErrorKind::NotFound { + tracing::warn!( + path = %bin_path.display(), + error = %e, + "deleted object's body could not be removed; it is orphaned until the \ + bucket is deleted", + ); } } Self::cleanup_empty(&dir); @@ -1504,6 +1522,56 @@ mod disk_tests { assert_eq!(obj.meta.tags.get("x").map(String::as_str), Some("y")); } + #[test] + fn delete_object_removes_the_sidecar_before_the_body() { + // `load` iterates `*.toml` and treats a sidecar whose `.bin` is gone as + // a hard error for the whole bucket, while an orphan `.bin` is never + // read. So the sidecar has to be the file the delete attempts first. + // + // Forces the SIDECAR removal to fail, by making that path a non-empty + // directory `remove_file` cannot take. Only a sidecar-first delete + // reports the failure with the body still on disk; a body-first one + // would have removed the body before reaching it. + let tmp = TempDir::new().unwrap(); + let store = new_store(&tmp); + store + .put_bucket_meta( + "b", + &BucketMeta { + name: "b".to_string(), + ..Default::default() + }, + ) + .unwrap(); + store + .put_object( + "b", + "k.txt", + None, + BodySource::Bytes(Bytes::from_static(b"v")), + &ObjectMeta { + key: "k.txt".to_string(), + size: 1, + ..Default::default() + }, + ) + .unwrap(); + + let (_dir, bin, toml) = store.object_paths("b", "k.txt", None); + std::fs::remove_file(&toml).unwrap(); + std::fs::create_dir(&toml).unwrap(); + std::fs::write(toml.join("blocker"), b"x").unwrap(); + + assert!( + store.delete_object("b", "k.txt", None).is_err(), + "a sidecar that cannot be removed must fail the delete: it is the durable record" + ); + assert!( + bin.exists(), + "the body must still be there, which is only true if the sidecar was attempted first" + ); + } + #[test] fn delete_object_cleans_up_files_and_cache() { let tmp = TempDir::new().unwrap(); diff --git a/crates/fakecloud-s3/src/persistence.rs b/crates/fakecloud-s3/src/persistence.rs index f4c023a52..40a4306aa 100644 --- a/crates/fakecloud-s3/src/persistence.rs +++ b/crates/fakecloud-s3/src/persistence.rs @@ -112,6 +112,24 @@ pub fn upload_part_meta_snapshot(p: &UploadPart) -> UploadPartMeta { } } +/// Whether a stored grant names someone it could ever match. +/// +/// An older build read an email grantee's absent `` as an empty string, so +/// a legacy `acl.toml` can hold a grant that matches nobody. Dropping those at +/// load keeps memory, disk and `GetBucketAcl` telling the same story -- hiding +/// them only when rendering would make the response disagree with the ACL the +/// bucket actually has, and a read-modify-write would then persist the omission. +fn acl_grant_names_a_grantee(g: &AclGrantSnapshot) -> bool { + // Trimmed, like the request-side checks: a stored grantee of whitespace + // names nobody just as surely as an empty one. + let named = |v: &Option| v.as_deref().is_some_and(|s| !s.trim().is_empty()); + match g.grantee_type.as_str() { + "Group" => named(&g.grantee_uri), + "AmazonCustomerByEmail" => named(&g.grantee_display_name), + _ => named(&g.grantee_id), + } +} + fn acl_grant_from_snapshot(g: &AclGrantSnapshot) -> AclGrant { AclGrant { grantee_type: g.grantee_type.clone(), @@ -247,7 +265,13 @@ pub fn s3_bucket_from_snapshot( grantee_uri: None, permission: "FULL_CONTROL".to_string(), }; - let has_acl_sidecar = subresources.contains_key("acl.toml"); + // An `acl.toml` written by an older build could be empty (its writer used + // `unwrap_or_default()` on serialization failure). Treat that as no sidecar + // at all, so the bucket keeps the default owner grant instead of loading + // with no grants and silently stripping the owner's FULL_CONTROL. + let has_acl_sidecar = subresources + .get("acl.toml") + .is_some_and(|text| !text.trim().is_empty()); let mut b = S3Bucket { name: name.to_string(), creation_date: meta.creation_date, @@ -330,7 +354,30 @@ pub fn s3_bucket_from_snapshot( if !snap.owner_id.is_empty() { b.acl_owner_id = snap.owner_id; } - b.acl_grants = snap.grants.iter().map(acl_grant_from_snapshot).collect(); + // Drop grants that name nobody, so the loaded ACL is one the + // server would also accept back from a client. + let usable: Vec = snap + .grants + .iter() + .filter(|g| acl_grant_names_a_grantee(g)) + .map(acl_grant_from_snapshot) + .collect(); + // ...but never leave the bucket with no grants at all. A + // sidecar whose every grant is unusable is indistinguishable + // from one that was never written, so fall back to the same + // default owner grant a bucket with no `acl.toml` gets, rather + // than loading a bucket whose owner has lost FULL_CONTROL. + b.acl_grants = if usable.is_empty() { + vec![AclGrant { + grantee_type: "CanonicalUser".to_string(), + grantee_id: Some(b.acl_owner_id.clone()), + grantee_display_name: Some(b.acl_owner_id.clone()), + grantee_uri: None, + permission: "FULL_CONTROL".to_string(), + }] + } else { + usable + }; } "inventory.toml" => { if text.trim().is_empty() { @@ -345,19 +392,58 @@ pub fn s3_bucket_from_snapshot( if text.trim().is_empty() { continue; } - b.analytics_configs = toml::from_str(&text).unwrap_or_default(); + // Neither `unwrap_or_default` nor `?`. Defaulting would load the + // bucket with no configurations and no word, and the next Put + // would overwrite the file and make the loss permanent; failing + // would hide every object in the bucket behind an unparseable + // reporting config. Skip the sidecar, keep the bucket, and say so, + // so the file is still there to look at. + match toml::from_str(&text) { + Ok(parsed) => b.analytics_configs = parsed, + Err(e) => tracing::warn!( + bucket = %name, + error = %e, + "ignoring unreadable analytics.toml; its configurations are not loaded", + ), + } } "intelligent_tiering.toml" => { if text.trim().is_empty() { continue; } - b.intelligent_tiering_configs = toml::from_str(&text).unwrap_or_default(); + // Neither `unwrap_or_default` nor `?`. Defaulting would load the + // bucket with no configurations and no word, and the next Put + // would overwrite the file and make the loss permanent; failing + // would hide every object in the bucket behind an unparseable + // reporting config. Skip the sidecar, keep the bucket, and say so, + // so the file is still there to look at. + match toml::from_str(&text) { + Ok(parsed) => b.intelligent_tiering_configs = parsed, + Err(e) => tracing::warn!( + bucket = %name, + error = %e, + "ignoring unreadable intelligent_tiering.toml; its configurations are not loaded", + ), + } } "metrics.toml" => { if text.trim().is_empty() { continue; } - b.metrics_configs = toml::from_str(&text).unwrap_or_default(); + // Neither `unwrap_or_default` nor `?`. Defaulting would load the + // bucket with no configurations and no word, and the next Put + // would overwrite the file and make the loss permanent; failing + // would hide every object in the bucket behind an unparseable + // reporting config. Skip the sidecar, keep the bucket, and say so, + // so the file is still there to look at. + match toml::from_str(&text) { + Ok(parsed) => b.metrics_configs = parsed, + Err(e) => tracing::warn!( + bucket = %name, + error = %e, + "ignoring unreadable metrics.toml; its configurations are not loaded", + ), + } } "request_payment.toml" => { b.request_payment = Some(text); @@ -381,11 +467,41 @@ pub fn hydrate_s3_state( snapshot: S3StateSnapshot, account_id: &str, region: &str, +) -> Result { + hydrate_s3_state_reporting(snapshot, account_id, region, &mut |_, _| {}) +} + +/// Hydrate a loaded snapshot, reporting each bucket that cannot be used instead +/// of failing the whole load. +/// +/// The store already isolates a bucket whose objects it cannot read: it warns, +/// records the refusal and carries on, so one bad file costs one bucket rather +/// than the server. Sidecars are parsed here, a layer later, and a single +/// malformed `acl.toml` or `tags.toml` used to abort startup -- every other +/// bucket inaccessible because of one file. Such a bucket is skipped and handed +/// to `refused` so it is reported exactly like a store-level refusal: absent +/// from memory, its name refused by CreateBucket, and clearable by DeleteBucket. +pub fn hydrate_s3_state_reporting( + snapshot: S3StateSnapshot, + account_id: &str, + region: &str, + refused: &mut dyn FnMut(&str, &str), ) -> Result { let mut state = S3State::new(account_id, region); for (name, snap) in snapshot.buckets { - let bucket = s3_bucket_from_snapshot(&name, snap, region)?; - state.buckets.insert(name, bucket); + match s3_bucket_from_snapshot(&name, snap, region) { + Ok(bucket) => { + state.buckets.insert(name, bucket); + } + Err(e) => { + tracing::warn!( + bucket = %name, + error = %e, + "skipping S3 bucket whose stored configuration could not be read", + ); + refused(&name, &e); + } + } } Ok(state) } diff --git a/crates/fakecloud-s3/src/service/acl.rs b/crates/fakecloud-s3/src/service/acl.rs index 6e6667979..6d1c5b4c5 100644 --- a/crates/fakecloud-s3/src/service/acl.rs +++ b/crates/fakecloud-s3/src/service/acl.rs @@ -6,8 +6,7 @@ use fakecloud_core::service::{AwsRequest, AwsResponse, AwsServiceError}; use crate::persistence::object_meta_snapshot; use super::{ - build_acl_xml, canned_acl_grants_for_object, no_such_key, parse_acl_xml, parse_grant_headers, - s3_xml, S3Service, + build_acl_xml, canned_acl_grants_for_object, no_such_key, parse_acl_xml, s3_xml, S3Service, }; impl S3Service { @@ -43,6 +42,12 @@ impl S3Service { .get("x-amz-acl") .and_then(|v| v.to_str().ok()) .map(|s| s.to_string()); + // Validated before the key is resolved, as S3 does: a bad canned value + // is an InvalidArgument whether or not the key exists. + if let Some(acl) = canned.as_deref() { + super::validate_object_canned_acl(acl)?; + } + super::reject_conflicting_acl_sources(canned.as_deref(), &req.headers, &req.body)?; if self.bucket_owner_enforced(account_id, bucket) { return Err(AwsServiceError::aws_error( @@ -70,19 +75,35 @@ impl S3Service { let proposed_grants = if let Some(acl) = &canned { canned_acl_grants_for_object(acl, &owner_id) } else { - let has_grant_headers = req.headers.keys().any(|k| { - let name = k.as_str(); - name.starts_with("x-amz-grant-") - }); - if has_grant_headers { - parse_grant_headers(&req.headers) + if super::has_grant_headers(&req.headers) { + super::resolved_grant_headers(&req.headers)? } else { + // No canned header, no grant header, no body names no ACL at + // all. Re-persisting the object's current grants would answer + // 200 for a request that asked for nothing, which the caller + // cannot tell from an applied change -- the same rule + // PutBucketAcl applies. let body_str = std::str::from_utf8(&req.body).unwrap_or(""); - if !body_str.is_empty() { - parse_acl_xml(body_str)? - } else { - obj.acl_grants.clone() + if body_str.trim().is_empty() { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "MalformedACLError", + "The XML you provided was not well-formed or did not validate against our published schema", + )); + } + // A body that is not an AccessControlPolicy at all (JSON, a + // misspelled root) parses to zero grants, and taking that as + // "remove every grant" strips the object's owner FULL_CONTROL + // and writes the empty list into its meta. Same rule the bucket + // path applies. + if !body_str.contains(", + ) -> Result<(), AwsServiceError> { + let Some(text) = payload else { + return Ok(()); + }; + self.store + .put_bucket_subresource(bucket, kind, text) + .map_err(super::persistence_error) + } + pub(super) fn list_buckets( &self, account_id: &str, @@ -222,12 +240,93 @@ impl S3Service { let create_tags = create_bucket_configuration_tags(body_str); validate_tags(&create_tags)?; - // Parse ACL from header - let acl = req + // Parse the ACL the create asks for. Either a canned `x-amz-acl` or the + // `x-amz-grant-*` headers, never both -- S3 rejects the combination + // rather than picking a winner. Whichever is used, the result is an ACL + // the caller chose, so it needs an `acl.toml`; the default private + // grant is what the loader reconstructs on its own and needs no + // sidecar. + let acl_header = req.headers.get("x-amz-acl").and_then(|v| v.to_str().ok()); + let grant_headers_present = has_grant_headers(&req.headers); + if acl_header.is_some() && grant_headers_present { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "InvalidRequest", + "Specifying both Canned ACLs and Header Grants is not allowed", + )); + } + if let Some(acl) = acl_header { + if !BUCKET_CANNED_ACLS.contains(&acl) { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "InvalidArgument", + format!("Invalid x-amz-acl value: {acl}"), + )); + } + } + let header_grants = if grant_headers_present { + resolved_grant_headers(&req.headers)? + } else { + Vec::new() + }; + + // BucketOwnerEnforced turns ACLs off for the bucket, so S3 refuses a + // create that also asks for one. Without this the bucket would come out + // publicly readable via a canned `public-read` while `PutBucketAcl` can + // no longer edit that ACL -- and this create now persists it, so the + // state would survive restarts instead of evaporating. + // + // A canned ACL is judged by the grants it resolves to (`private` grants + // nothing beyond the owner, so S3 accepts it), while ANY `x-amz-grant-*` + // header conflicts on presence alone, as on S3 -- the caller is asking + // for an ACL on a bucket that has none. + let ownership_header = req .headers - .get("x-amz-acl") - .and_then(|v| v.to_str().ok()) - .unwrap_or("private"); + .get("x-amz-object-ownership") + .and_then(|v| v.to_str().ok()); + if let Some(ownership) = ownership_header { + if !OBJECT_OWNERSHIP_VALUES.contains(&ownership) { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "InvalidArgument", + format!("Invalid x-amz-object-ownership value: {ownership}"), + )); + } + } + // Compared exactly, not case-insensitively: `bucket_owner_enforced()` + // matches the stored XML case-sensitively, so accepting a differently + // 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()) + }) + }; + 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)) + }); + if ownership_enforced && acl_requests_grants { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "InvalidBucketAclWithObjectOwnership", + "Bucket cannot have ACLs set with ObjectOwnership's BucketOwnerEnforced setting", + )); + } + // `aws-exec-read` resolves to owner-only here (its READ grant to the EC2 + // service's canonical user is not modeled), which is exactly what the + // loader reconstructs without a sidecar -- so it gets no `acl.toml` + // rather than a stored record claiming an explicit ACL it does not have. + let acl_header_present = + grant_headers_present || acl_header.is_some_and(|a| a != "aws-exec-read"); + let acl = acl_header.unwrap_or("private"); let mut accts = self.state.write(); // Check global uniqueness across all accounts before creating @@ -248,7 +347,7 @@ impl S3Service { if let Some(existing) = state.buckets.get(bucket) { // In us-east-1, re-creating same bucket in same region is idempotent // (returns 200). The re-create is a no-op on the existing bucket: it - // re-applies none of the create-time settings — not the canned ACL, + // re-applies none of the create-time settings -- not the canned ACL, // not object lock, not object ownership, and not the // `CreateBucketConfiguration` tag set. Applying only the tags here // would make them the one create-time setting that mutates a bucket @@ -279,7 +378,11 @@ impl S3Service { .unwrap_or(false); let mut b = S3Bucket::new(bucket, &requested_region, &req.account_id); - b.acl_grants = canned_acl_grants(acl, &req.account_id); + b.acl_grants = if grant_headers_present { + header_grants + } else { + canned_acl_grants(acl, &req.account_id) + }; if object_lock_enabled { b.versioning = Some("Enabled".to_string()); b.object_lock_config = Some( @@ -292,11 +395,7 @@ impl S3Service { } // Handle x-amz-object-ownership header - if let Some(ownership) = req - .headers - .get("x-amz-object-ownership") - .and_then(|v| v.to_str().ok()) - { + if let Some(ownership) = ownership_header { b.ownership_controls = Some(format!( "\ \ @@ -315,36 +414,92 @@ impl S3Service { }; let meta = bucket_meta_snapshot(&b); - // Persist before committing the bucket to memory, tags first: bucket - // tags live in their own subresource rather than in the bucket meta, so - // they need an explicit write to survive a restart, and writing them - // ahead of `meta.toml` keeps a failed create from leaving anything - // usable behind. The loader skips a bucket directory with no - // `meta.toml`, and the in-memory insert is last, so a store error at - // either step surfaces as an error with no half-created bucket — the - // same guarantee the `InvalidTag` path gives. - match tags_snapshot { - Some(snap) => { - let payload = toml::to_string(&snap).unwrap_or_default(); - self.store - .put_bucket_subresource(bucket, BucketSubresource::Tags, &payload) - .map_err(super::persistence_error)?; - } - // An untagged create clears the file rather than leaving it alone: - // a create that failed after its tag write (or died between the two - // writes) leaves a `tags.toml` in a directory with no `meta.toml`, - // which the loader skips — but a later untagged create of the same - // name would adopt that never-committed tag set on the next restart. - // Deleting is a no-op when the file is absent, which is the normal - // case. - None => self - .store - .delete_bucket_subresource(bucket, BucketSubresource::Tags) - .map_err(super::persistence_error)?, - } + // Persist the create-time subresources before `meta.toml` and before the + // in-memory insert. None of these three live in `BucketMeta`, and the + // loader restores `object_lock_config` / `ownership_controls` as `None` + // and falls back to the default owner grant when `acl.toml` is absent -- + // so without an explicit write, a bucket created with `x-amz-acl`, + // object lock, or an ownership rule came back after a restart with none + // of them (object-lock retention silently stopped being enforced). + // + // Each one is written only when the create set it; the clear above is + // what guarantees nothing stale is left for the ones it did not. + let acl_payload = if acl_header_present { + let snap = AclSnapshot { + owner_id: b.acl_owner_id.clone(), + grants: b.acl_grants.iter().map(AclGrantSnapshot::from).collect(), + }; + // Never fall back to an empty document here: the loader takes the + // mere presence of `acl.toml` to mean "this bucket has an explicit + // ACL" and skips the default owner grant, so an empty file would + // restore the bucket with no grants at all. + Some(toml::to_string(&snap).map_err(|e| { + AwsServiceError::aws_error( + StatusCode::INTERNAL_SERVER_ERROR, + "InternalError", + format!("failed to serialize bucket ACL: {e}"), + ) + })?) + } else { + None + }; + let tags_payload = match tags_snapshot { + // Never fall back to an empty document: a blank `tags.toml` is not + // what "no tags" means on disk, and the sweep below is what clears + // a stale one. + Some(snap) => Some(toml::to_string(&snap).map_err(|e| { + AwsServiceError::aws_error( + StatusCode::INTERNAL_SERVER_ERROR, + "InternalError", + format!("failed to serialize bucket tags: {e}"), + ) + })?), + None => None, + }; + // The meta goes first, and nothing is destroyed until it lands: a + // create that fails here has changed nothing on disk, where clearing + // the old sidecars first would have thrown away the configuration of + // whatever bucket this name belonged to for a create that never + // happened. self.store .put_bucket_meta(bucket, &meta) .map_err(super::persistence_error)?; + // Clear every stored subresource for this name before writing this + // bucket's own, now that the create is committed. A create or delete that stopped partway -- or a + // `/_fakecloud/reset`, which clears memory and leaves the store alone -- + // can leave sidecars behind, and a later create would otherwise be + // restored carrying the old bucket's `policy.toml`, `acl.toml` and the + // rest. Each delete tolerates a missing file, which is the normal case. + // + // Scoped to the sidecars on purpose. `objects/` is NOT touched: the + // loader skips a bucket whose objects it cannot read, so that bucket is + // absent from memory while its data sits intact on disk, and clearing + // the directory here would make re-creating the name the thing that + // destroys it. Whether a create should adopt or discard a stale object + // tree is a separate question from this one, and this is not the change + // that answers it. + for kind in fakecloud_persistence::ALL_SUBRESOURCES { + self.store + .delete_bucket_subresource(bucket, *kind) + .map_err(super::persistence_error)?; + } + + self.put_bucket_subresource_if_set( + bucket, + BucketSubresource::Tags, + tags_payload.as_deref(), + )?; + self.put_bucket_subresource_if_set(bucket, BucketSubresource::Acl, acl_payload.as_deref())?; + self.put_bucket_subresource_if_set( + bucket, + BucketSubresource::ObjectLock, + b.object_lock_config.as_deref(), + )?; + self.put_bucket_subresource_if_set( + bucket, + BucketSubresource::Ownership, + b.ownership_controls.as_deref(), + )?; state.buckets.insert(bucket.to_string(), b); let mut headers = HeaderMap::new(); diff --git a/crates/fakecloud-s3/src/service/config/bucket_inventory.rs b/crates/fakecloud-s3/src/service/config/bucket_inventory.rs index a4af4de19..89b5cfdea 100644 --- a/crates/fakecloud-s3/src/service/config/bucket_inventory.rs +++ b/crates/fakecloud-s3/src/service/config/bucket_inventory.rs @@ -25,7 +25,7 @@ impl S3Service { let snap = InventorySnapshot { configs: b.inventory_configs.clone(), }; - toml::to_string(&snap).unwrap_or_default() + crate::service::toml_or_internal_error(&snap)? }; self.store .put_bucket_subresource(bucket, BucketSubresource::Inventory, &payload) @@ -114,7 +114,7 @@ impl S3Service { let snap = InventorySnapshot { configs: b.inventory_configs.clone(), }; - let payload = toml::to_string(&snap).unwrap_or_default(); + let payload = crate::service::toml_or_internal_error(&snap)?; self.store .put_bucket_subresource(bucket, BucketSubresource::Inventory, &payload) .map_err(crate::service::persistence_error)?; diff --git a/crates/fakecloud-s3/src/service/config/mod.rs b/crates/fakecloud-s3/src/service/config/mod.rs index b612af564..368b96e35 100644 --- a/crates/fakecloud-s3/src/service/config/mod.rs +++ b/crates/fakecloud-s3/src/service/config/mod.rs @@ -10,9 +10,10 @@ use crate::inventory; use crate::persistence::{bucket_meta_snapshot, object_meta_snapshot}; use super::{ - build_acl_xml, canned_acl_grants, empty_response, extract_xml_value, no_such_bucket, - normalize_notification_ids, normalize_replication_xml, parse_acl_xml, parse_tagging_xml, - s3_xml, validate_lifecycle_xml, validate_tags, xml_escape, S3Service, + build_acl_xml, canned_acl_grants, empty_response, extract_xml_value, has_grant_headers, + no_such_bucket, normalize_notification_ids, normalize_replication_xml, parse_acl_xml, + parse_tagging_xml, reject_conflicting_acl_sources, resolved_grant_headers, s3_xml, + validate_lifecycle_xml, validate_tags, xml_escape, S3Service, BUCKET_CANNED_ACLS, }; /// Decoded `PublicAccessBlockConfiguration` flags read by the request @@ -240,12 +241,28 @@ impl S3Service { req: &AwsRequest, bucket: &str, ) -> Result { - // Check for canned ACL header + // An unrecognized canned value used to fall through `canned_acl_grants`' + // catch-all to owner-only, silently stripping every public grant -- and + // that wipe is now written to `acl.toml`, so it outlives the process. + // + // Checked before the bucket is resolved, matching PutObjectAcl and S3 + // itself: a bad header is an InvalidArgument whether or not the target + // exists. let canned = req .headers .get("x-amz-acl") .and_then(|v| v.to_str().ok()) .map(|s| s.to_string()); + if let Some(acl) = canned.as_deref() { + if !BUCKET_CANNED_ACLS.contains(&acl) { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "InvalidArgument", + format!("Invalid x-amz-acl value: {acl}"), + )); + } + } + reject_conflicting_acl_sources(canned.as_deref(), &req.headers, &req.body)?; // BucketOwnerEnforced disables ACLs on this bucket entirely; // any ACL-mutating call rejects with @@ -267,12 +284,41 @@ impl S3Service { .get_mut(bucket) .ok_or_else(|| no_such_bucket(bucket))?; + // Canned header, else the `x-amz-grant-*` headers, else the XML body -- + // the precedence PutObjectAcl uses. Without the grant-header arm a + // `put-bucket-acl --grant-read uri=...` request fell through to an + // empty body and wiped the bucket's ACL to nothing. let proposed_grants = if let Some(acl) = &canned { canned_acl_grants(acl, &b.acl_owner_id.clone()) + } else if has_grant_headers(&req.headers) { + resolved_grant_headers(&req.headers)? } else { let body_str = std::str::from_utf8(&req.body).unwrap_or(""); parse_acl_xml(body_str)? }; + // A request that names no ACL at all is malformed rather than "remove + // every grant". An explicit `` IS the documented + // way to drop all grants, so zero grants on its own is not an error -- + // but a body that is not an AccessControlPolicy at all (whitespace, + // JSON, a misspelled root) parses to zero grants too, and taking that + // as "drop everything" wipes the bucket's ACL durably. + if canned.is_none() && !has_grant_headers(&req.headers) { + let body_str = std::str::from_utf8(&req.body).unwrap_or(""); + if body_str.trim().is_empty() { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "MalformedACLError", + "The XML you provided was not well-formed or did not validate against our published schema", + )); + } + if !body_str.contains("") { + let after = &rest[start + "".len()..]; + let Some(end) = after.find("") else { + break; + }; + let value = after[..end].trim(); + if !crate::service::OBJECT_OWNERSHIP_VALUES.contains(&value) { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "InvalidArgument", + format!("Invalid ObjectOwnership value: {value}"), + )); + } + seen += 1; + rest = &after[end..]; + } + let single = seen == 1; + let value = extract_xml_value(&body_str, "ObjectOwnership").unwrap_or_default(); + // `bucket_owner_enforced` looks for the literal anywhere in the stored + // document, so it must appear exactly when it is the rule's value -- + // never in a second rule or a comment. + let enforced_mentions = body_str.matches("BucketOwnerEnforced").count(); + let expected = usize::from(value == "BucketOwnerEnforced"); + if !single || enforced_mentions != expected { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "MalformedXML", + "OwnershipControls must carry exactly one Rule with a valid ObjectOwnership", + )); + } let mut accts = self.state.write(); let state = accts.get_or_create(account_id); let b = state .buckets .get_mut(bucket) .ok_or_else(|| no_such_bucket(bucket))?; - b.ownership_controls = Some(body_str.clone()); + // Persist before mutating memory, as the ACL paths do: a store failure + // would otherwise leave memory ahead of disk, and a restart would + // silently revert what the caller was told had failed. self.store .put_bucket_subresource(bucket, BucketSubresource::Ownership, &body_str) .map_err(crate::service::persistence_error)?; + b.ownership_controls = Some(body_str.clone()); Ok(empty_response(StatusCode::OK)) } diff --git a/crates/fakecloud-s3/src/service/config/tagging.rs b/crates/fakecloud-s3/src/service/config/tagging.rs index 0d89b7f77..2ab61ee28 100644 --- a/crates/fakecloud-s3/src/service/config/tagging.rs +++ b/crates/fakecloud-s3/src/service/config/tagging.rs @@ -62,7 +62,7 @@ impl S3Service { let snap = TagsSnapshot { tags: b.tags.clone(), }; - let payload = toml::to_string(&snap).unwrap_or_default(); + let payload = crate::service::toml_or_internal_error(&snap)?; self.store .put_bucket_subresource(bucket, BucketSubresource::Tags, &payload) .map_err(crate::service::persistence_error)?; diff --git a/crates/fakecloud-s3/src/service/mod.rs b/crates/fakecloud-s3/src/service/mod.rs index 850c2426e..3a8ccbb23 100644 --- a/crates/fakecloud-s3/src/service/mod.rs +++ b/crates/fakecloud-s3/src/service/mod.rs @@ -142,6 +142,72 @@ pub struct S3Service { pub(crate) credential_resolver: Option>, } +/// Serialize a persistence snapshot, turning a failure into a 500 rather than an +/// empty document. +/// +/// An empty file is worse than no file: the loader either skips it (losing the +/// configuration with no word, which the next Put then makes permanent) or, for +/// `acl.toml`, takes its presence to mean an explicit ACL and drops the default +/// owner grant. +pub(crate) fn toml_or_internal_error( + value: &T, +) -> Result { + toml::to_string(value).map_err(|e| { + AwsServiceError::aws_error( + StatusCode::INTERNAL_SERVER_ERROR, + "InternalError", + format!("failed to serialize persisted state: {e}"), + ) + }) +} + +/// Reject a request that names an ACL more than one way. +/// +/// A canned header, the `x-amz-grant-*` headers and an `AccessControlPolicy` +/// body are mutually exclusive on S3, and letting one win silently discards what +/// the others asked for -- durably, since the result is persisted. Shared so the +/// bucket and object paths cannot drift on which pairs they reject. +pub(crate) fn reject_conflicting_acl_sources( + canned: Option<&str>, + headers: &HeaderMap, + body: &[u8], +) -> Result<(), AwsServiceError> { + let grants = has_grant_headers(headers); + let has_body = !body_is_blank(body); + let conflict = match (canned.is_some(), grants, has_body) { + (true, true, _) => Some("Specifying both Canned ACLs and Header Grants is not allowed"), + (true, _, true) => { + Some("Specifying both a Canned ACL and an AccessControlPolicy body is not allowed") + } + (_, true, true) => { + Some("Specifying both Header Grants and an AccessControlPolicy body is not allowed") + } + _ => None, + }; + match conflict { + Some(message) => Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "InvalidRequest", + message, + )), + None => Ok(()), + } +} + +/// Whether a request body carries nothing an ACL could be read from. +/// +/// The mutual-exclusion checks and the "names no ACL at all" check have to agree +/// on this: testing raw emptiness in one and trimmed emptiness in the other made +/// a canned ACL plus a stray newline -- one ACL, named once -- fail as though it +/// named two. +pub(crate) fn body_is_blank(body: &[u8]) -> bool { + // A body that does not decode is one no ACL can be read from, which is + // exactly how the parse sites treat it (`from_utf8(..).unwrap_or("")`). + // Calling it present would reject a canned ACL for conflicting with a body + // the next line discards. + std::str::from_utf8(body).map_or(true, |s| s.trim().is_empty()) +} + /// Map a [`StoreError`] from the persistence layer to a 500 InternalError /// response. Invoked at every mutation site when the write-through persistence /// call fails: the in-memory mutation has already happened, but we surface the @@ -2262,6 +2328,13 @@ pub(crate) fn build_acl_xml(owner_id: &str, grants: &[AclGrant], _account_id: &s {}", xml_escape(uri), ) + } else if g.grantee_type == "AmazonCustomerByEmail" { + let email = g.grantee_display_name.as_deref().unwrap_or(""); + format!( + "\ + {}", + xml_escape(email), + ) } else { let id = g.grantee_id.as_deref().unwrap_or(""); format!( @@ -2286,6 +2359,71 @@ pub(crate) fn build_acl_xml(owner_id: &str, grants: &[AclGrant], _account_id: &s ) } +/// The canned ACLs an object accepts, per `com.amazonaws.s3#ObjectCannedACL`. +/// `log-delivery-write` is bucket-only and is not among them. +pub(crate) const OBJECT_CANNED_ACLS: [&str; 7] = [ + "private", + "public-read", + "public-read-write", + "authenticated-read", + "aws-exec-read", + "bucket-owner-read", + "bucket-owner-full-control", +]; + +/// Reject a canned ACL an object does not accept. An unrecognized value used to +/// fall through `canned_acl_grants`' catch-all to owner-only, silently wiping +/// the object's grants with a 200. +pub(crate) fn validate_object_canned_acl(acl: &str) -> Result<(), AwsServiceError> { + if OBJECT_CANNED_ACLS.contains(&acl) { + return Ok(()); + } + Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "InvalidArgument", + format!("Invalid x-amz-acl value: {acl}"), + )) +} + +/// The values `x-amz-object-ownership` accepts, per +/// `com.amazonaws.s3#ObjectOwnership`. Matched case-sensitively, as S3 does. +pub(crate) const OBJECT_OWNERSHIP_VALUES: [&str; 3] = [ + "BucketOwnerPreferred", + "ObjectWriter", + "BucketOwnerEnforced", +]; + +/// The canned ACLs a bucket accepts. `com.amazonaws.s3#BucketCannedACL` lists +/// only the first four; `log-delivery-write` is bucket-scoped in the S3 +/// canned-ACL table (it is how a server-access-logging target is set up), so +/// the header takes it even though the modeled enum omits it. +/// +/// `aws-exec-read` is bucket-scoped too (CloudFormation's +/// `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. +/// +/// `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 +/// rather than refusing the call -- so they are accepted here and resolve to +/// the owner's FULL_CONTROL, which is what ignoring them produces. Rejecting +/// them would have turned a working call into a hard failure on a claim about +/// live S3 this build cannot check. A value S3 does not define at all is still +/// refused, since that is a typo silently wiping the bucket's grants. +pub(crate) const BUCKET_CANNED_ACLS: [&str; 8] = [ + "private", + "public-read", + "public-read-write", + "authenticated-read", + "aws-exec-read", + "log-delivery-write", + "bucket-owner-read", + "bucket-owner-full-control", +]; + pub(crate) fn canned_acl_grants(acl: &str, owner_id: &str) -> Vec { let owner_grant = AclGrant { grantee_type: "CanonicalUser".to_string(), @@ -2335,6 +2473,27 @@ pub(crate) fn canned_acl_grants(acl: &str, owner_id: &str) -> Vec { permission: "READ".to_string(), }, ], + // Grants the log-delivery group what it needs to write access logs into + // the bucket, which is the entire point of this canned ACL. + "log-delivery-write" => vec![ + owner_grant, + AclGrant { + grantee_type: "Group".to_string(), + grantee_id: None, + grantee_display_name: None, + grantee_uri: Some("http://acs.amazonaws.com/groups/s3/LogDelivery".to_string()), + permission: "WRITE".to_string(), + }, + AclGrant { + grantee_type: "Group".to_string(), + grantee_id: None, + grantee_display_name: None, + grantee_uri: Some("http://acs.amazonaws.com/groups/s3/LogDelivery".to_string()), + permission: "READ_ACP".to_string(), + }, + ], + // `bucket-owner-read` / `bucket-owner-full-control` on an object owned + // by the caller come out as the owner's FULL_CONTROL. "bucket-owner-full-control" => vec![owner_grant], _ => vec![owner_grant], } @@ -2345,24 +2504,110 @@ pub(crate) fn canned_acl_grants_for_object(acl: &str, owner_id: &str) -> Vec usize { + let mut clauses = 0; + for (header, _) in &GRANT_HEADER_PERMISSIONS { + // `get_all`, not `get`: a repeated header carries more than one value, + // and counting only the first would let the extras be dropped silently + // -- the very thing the count exists to catch. + for value in headers.get_all(*header) { + match value.to_str() { + Ok(value) => { + clauses += value + .split(',') + .filter(|part| !part.trim().is_empty()) + .count(); + } + // Header values are opaque bytes. `parse_grant_headers` cannot + // read this one either, so counting it as zero would let the + // counts agree and the grant disappear -- count it as a clause + // that did not resolve. + Err(_) => clauses += 1, + } + } + } + clauses +} + +/// Whether the request carries any of the recognized `x-amz-grant-*` headers. +/// Matched by exact name, not by prefix, so an unknown `x-amz-grant-…` header +/// is ignored rather than turning a valid create into an error. +pub(crate) fn has_grant_headers(headers: &HeaderMap) -> bool { + // A present-but-blank value is what a client sends for an unset config + // field, and S3 treats it as absent. Testing presence alone turned that + // into a hard rejection on every ACL-setting operation. + GRANT_HEADER_PERMISSIONS.iter().any(|(header, _)| { + headers + .get_all(*header) + .iter() + .any(|v| v.to_str().map(|s| !s.trim().is_empty()).unwrap_or(true)) + }) +} + +/// Resolve the `x-amz-grant-*` headers into grants, refusing the request when +/// any grantee clause does not resolve into one. +/// +/// [`parse_grant_headers`] skips a clause it cannot make a grantee of -- a key +/// with an empty value, or one it does not recognize -- so a caller that STORES +/// the result as the authoritative ACL would persist a permission set nobody +/// asked for, and a request where nothing resolves would store an ACL with not +/// even an owner entry. Every ACL-setting path goes through here so they all +/// agree. (`emailAddress=` IS resolved, into an AmazonCustomerByEmail grantee.) +pub(crate) fn resolved_grant_headers( + headers: &HeaderMap, +) -> Result, AwsServiceError> { + let grants = parse_grant_headers(headers); + if grants.is_empty() || grants.len() != grant_header_clause_count(headers) { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "InvalidArgument", + "Argument format not recognized: grantees must be specified as id= or uri=", + )); + } + Ok(grants) +} + pub(crate) fn parse_grant_headers(headers: &HeaderMap) -> Vec { let mut grants = Vec::new(); - let header_permission_map = [ - ("x-amz-grant-read", "READ"), - ("x-amz-grant-write", "WRITE"), - ("x-amz-grant-read-acp", "READ_ACP"), - ("x-amz-grant-write-acp", "WRITE_ACP"), - ("x-amz-grant-full-control", "FULL_CONTROL"), - ]; + let header_permission_map = GRANT_HEADER_PERMISSIONS; for (header, permission) in &header_permission_map { - if let Some(value) = headers.get(*header).and_then(|v| v.to_str().ok()) { + for value in headers.get_all(*header) { + let Ok(value) = value.to_str() else { continue }; // Parse "id=xxx" or "uri=xxx" or "emailAddress=xxx" for part in value.split(',') { let part = part.trim(); if let Some((key, val)) = part.split_once('=') { - let val = val.trim().trim_matches('"'); + // Quotes first, then trim: trimming first leaves `" "` as + // two spaces, which the emptiness checks below would accept + // as a grantee naming somebody. + let val = val.trim().trim_matches('"').trim(); let key = key.trim().to_lowercase(); + // `id=` / `uri=` with nothing after it names no grantee. + // Emitting a grant with an empty id would store an ACL + // entry that can never match anyone; leaving it out lets + // the caller's clause count catch the request instead. + if val.is_empty() { + continue; + } match key.as_str() { "id" => { grants.push(AclGrant { @@ -2373,6 +2618,34 @@ pub(crate) fn parse_grant_headers(headers: &HeaderMap) -> Vec { permission: permission.to_string(), }); } + // S3 resolves an email to the account's canonical id. + // There is no directory to resolve against here, so the + // grant keeps the address as an AmazonCustomerByEmail + // grantee -- a real ACL grantee type -- rather than + // being dropped (which would silently lose a grant) or + // refused (which would fail a call AWS accepts). + // + // It is recorded, not enforced: every authorization path + // matches a Group URI or a canonical id, so this grant + // gives the named address no access. Say so rather than + // letting it look like working authorization. + "emailaddress" => { + tracing::warn!( + target: "fakecloud::s3", + email = %val, + permission = %permission, + "storing an email-addressed ACL grant that is not enforced: \ + email grantees are not resolved to a canonical user here, so \ + this grant conveys no access", + ); + grants.push(AclGrant { + grantee_type: "AmazonCustomerByEmail".to_string(), + grantee_id: None, + grantee_display_name: Some(val.to_string()), + grantee_uri: None, + permission: permission.to_string(), + }); + } "uri" | "url" => { grants.push(AclGrant { grantee_type: "Group".to_string(), @@ -2422,7 +2695,21 @@ pub(crate) fn parse_acl_xml(xml: &str) -> Result, AwsServiceError> // Determine grantee type if grant_body.contains("xsi:type=\"Group\"") || grant_body.contains("") { - let uri = extract_xml_value(grant_body, "URI").unwrap_or_default(); + // Trimmed: `\n http://...\n` is what a formatted + // policy carries, and the padding would otherwise be stored as + // part of the group URI and never match. + let uri = extract_xml_value(grant_body, "URI") + .map(|u| u.trim().to_string()) + .unwrap_or_default(); + // Same rule as an empty : a grant naming no group can never + // match anyone, and it would be stored as the bucket's ACL. + if uri.is_empty() { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "MalformedACLError", + "The XML you provided was not well-formed or did not validate against our published schema", + )); + } grants.push(AclGrant { grantee_type: "Group".to_string(), grantee_id: None, @@ -2430,10 +2717,60 @@ pub(crate) fn parse_acl_xml(xml: &str) -> Result, AwsServiceError> grantee_uri: Some(uri), permission, }); + } else if grant_body.contains("AmazonCustomerByEmail") + || grant_body.contains("") + { + // S3 allows exactly one grantee identifier. A Grantee carrying + // both an ID and an EmailAddress used to silently drop the + // canonical id in favor of the (unenforced) email grant, and the + // wrong one was the persisted one. + if grant_body.contains("") { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "MalformedACLError", + "The XML you provided was not well-formed or did not validate against our published schema", + )); + } + // An email grantee carries , never . Reading it + // as a CanonicalUser would store a grant with an empty id -- + // an entry that can never match anyone, which is exactly what + // the header path refuses. This is reachable by feeding + // GetBucketAcl output straight back into PutBucketAcl. + let email = extract_xml_value(grant_body, "EmailAddress") + .map(|e| e.trim().to_string()) + .unwrap_or_default(); + if email.is_empty() { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "MalformedACLError", + "The XML you provided was not well-formed or did not validate against our published schema", + )); + } + grants.push(AclGrant { + grantee_type: "AmazonCustomerByEmail".to_string(), + grantee_id: None, + grantee_display_name: Some(email), + grantee_uri: None, + permission, + }); } else { - let id = extract_xml_value(grant_body, "ID").unwrap_or_default(); - let display = - extract_xml_value(grant_body, "DisplayName").unwrap_or_else(|| id.clone()); + // Trimmed for the same reason: a pretty-printed canonical id + // would otherwise be stored with its padding and never match + // the owner in any authorization comparison. + let id = extract_xml_value(grant_body, "ID") + .map(|i| i.trim().to_string()) + .unwrap_or_default(); + if id.is_empty() { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "MalformedACLError", + "The XML you provided was not well-formed or did not validate against our published schema", + )); + } + let display = extract_xml_value(grant_body, "DisplayName") + .map(|d| d.trim().to_string()) + .filter(|d| !d.is_empty()) + .unwrap_or_else(|| id.clone()); grants.push(AclGrant { grantee_type: "CanonicalUser".to_string(), grantee_id: Some(id), diff --git a/crates/fakecloud-s3/src/service/multipart.rs b/crates/fakecloud-s3/src/service/multipart.rs index dbdf866ad..364d5fb55 100644 --- a/crates/fakecloud-s3/src/service/multipart.rs +++ b/crates/fakecloud-s3/src/service/multipart.rs @@ -13,8 +13,8 @@ use md5::{Digest, Md5}; use super::{ canned_acl_grants, compute_md5, extract_user_metadata, no_such_bucket, no_such_key, - no_such_upload, parse_complete_multipart_xml, parse_grant_headers, parse_url_encoded_tags, - precondition_failed, resolve_object, s3_xml, xml_escape, S3Service, + no_such_upload, parse_complete_multipart_xml, parse_url_encoded_tags, precondition_failed, + resolve_object, resolved_grant_headers, s3_xml, xml_escape, S3Service, }; /// Build the `CompleteMultipartUploadResult` XML response for an object that @@ -100,10 +100,19 @@ impl S3Service { .get("x-amz-acl") .and_then(|v| v.to_str().ok()) .map(|s| s.to_string()); - let has_grant_headers = req - .headers - .keys() - .any(|k| k.as_str().starts_with("x-amz-grant-")); + let has_grant_headers = super::has_grant_headers(&req.headers); + // Every sibling ACL-setting path rejects an ACL on a bucket whose + // ownership disables them; this one used to accept it and carry the + // grants into the completed object, which now persists them. + if (acl_header.is_some() || has_grant_headers) + && self.bucket_owner_enforced(account_id, bucket) + { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "AccessControlListNotSupported", + "The bucket does not allow ACLs", + )); + } if acl_header.is_some() && has_grant_headers { return Err(AwsServiceError::aws_error( @@ -128,9 +137,10 @@ impl S3Service { .ok_or_else(|| no_such_bucket(bucket))?; let acl_grants = if has_grant_headers { - parse_grant_headers(&req.headers) + resolved_grant_headers(&req.headers)? } else { let acl = acl_header.as_deref().unwrap_or("private"); + super::validate_object_canned_acl(acl)?; canned_acl_grants(acl, &b.acl_owner_id) }; diff --git a/crates/fakecloud-s3/src/service/objects/mod.rs b/crates/fakecloud-s3/src/service/objects/mod.rs index 9a3b57687..7682b2adb 100644 --- a/crates/fakecloud-s3/src/service/objects/mod.rs +++ b/crates/fakecloud-s3/src/service/objects/mod.rs @@ -15,8 +15,8 @@ use super::{ check_object_lock_for_overwrite, compute_checksum, deliver_notifications, etag_matches, extract_user_metadata, extract_xml_value, is_frozen, is_valid_storage_class, make_delete_marker, no_such_bucket, no_such_key, parse_delete_objects_quiet, - parse_delete_objects_xml, parse_grant_headers, parse_range_header, parse_url_encoded_tags, - precondition_failed, replicate_through_store, resolve_object, s3_xml, url_encode_s3_key, + parse_delete_objects_xml, parse_range_header, parse_url_encoded_tags, precondition_failed, + replicate_through_store, resolve_object, resolved_grant_headers, s3_xml, url_encode_s3_key, xml_escape, RangeResult, S3Service, }; diff --git a/crates/fakecloud-s3/src/service/objects/write.rs b/crates/fakecloud-s3/src/service/objects/write.rs index 221328470..94f8dea08 100644 --- a/crates/fakecloud-s3/src/service/objects/write.rs +++ b/crates/fakecloud-s3/src/service/objects/write.rs @@ -48,11 +48,17 @@ impl S3Service { .map(|s| s.to_string()); // Check for grant headers alongside canned ACL - let has_grant_headers = req.headers.keys().any(|k| { - let name = k.as_str(); - name.starts_with("x-amz-grant-") - }); + let has_grant_headers = super::super::has_grant_headers(&req.headers); + // Validated here, before `take_body_stream` spools the payload to disk: + // returning after the spool leaks the file, since nothing unlinks it on + // the error paths. + if let Some(acl) = acl_header.as_deref() { + super::super::validate_object_canned_acl(acl)?; + } + if has_grant_headers { + resolved_grant_headers(&req.headers)?; + } if acl_header.is_some() && has_grant_headers { return Err(AwsServiceError::aws_error( StatusCode::BAD_REQUEST, @@ -301,7 +307,9 @@ impl S3Service { // Build ACL grants for object let acl_grants = if has_grant_headers { - parse_grant_headers(&req.headers) + // Already validated before the body was spooled; this cannot fail + // here, but resolving again keeps one source for the grants. + resolved_grant_headers(&req.headers)? } else if let Some(ref acl) = acl_header { canned_acl_grants_for_object(acl, &acl_owner_id) } else { diff --git a/crates/fakecloud-s3/src/service/tests.rs b/crates/fakecloud-s3/src/service/tests.rs index 4cae69419..6b9f5369d 100644 --- a/crates/fakecloud-s3/src/service/tests.rs +++ b/crates/fakecloud-s3/src/service/tests.rs @@ -3473,6 +3473,748 @@ fn create_bucket_idempotent_same_region_us_east_1() { assert_eq!(resp.status, StatusCode::OK); } +#[test] +fn create_bucket_honors_grant_headers() { + let svc = make_service(); + let mut req = make_request(Method::PUT, "/granted", &[], b""); + req.headers.insert( + "x-amz-grant-read", + "uri=http://acs.amazonaws.com/groups/global/AllUsers" + .parse() + .unwrap(), + ); + svc.create_bucket("123456789012", &req, "granted").unwrap(); + + let get = make_request(Method::GET, "/granted", &[("acl", "")], b""); + let resp = svc.get_bucket_acl("123456789012", &get, "granted").unwrap(); + let body = std::str::from_utf8(resp.body.expect_bytes()).unwrap(); + assert!(body.contains("AllUsers"), "grant header ignored: {body}"); + assert!(body.contains("READ"), "{body}"); +} + +#[test] +fn create_bucket_rejects_canned_acl_and_grant_headers_together() { + let svc = make_service(); + let mut req = make_request(Method::PUT, "/both-acl", &[], b""); + req.headers + .insert("x-amz-acl", "public-read".parse().unwrap()); + req.headers.insert( + "x-amz-grant-read", + "uri=http://acs.amazonaws.com/groups/global/AllUsers" + .parse() + .unwrap(), + ); + assert_aws_err( + svc.create_bucket("123456789012", &req, "both-acl"), + "InvalidRequest", + ); + assert_aws_err(svc.head_bucket("123456789012", "both-acl"), "NotFound"); +} + +#[test] +fn create_bucket_keeps_an_email_grantee_instead_of_dropping_or_refusing_it() { + // S3 resolves `emailAddress=` against its own directory and answers 200. + // There is nothing to resolve against here, so the grant keeps the address + // as an AmazonCustomerByEmail grantee: dropping it would silently lose a + // grant, and refusing would fail a call AWS accepts. + let svc = make_service(); + let mut req = make_request(Method::PUT, "/email-grant", &[], b""); + req.headers.insert( + "x-amz-grant-full-control", + "emailAddress=someone@example.com".parse().unwrap(), + ); + svc.create_bucket("123456789012", &req, "email-grant") + .unwrap(); + + let get = make_request(Method::GET, "/email-grant", &[("acl", "")], b""); + let resp = svc + .get_bucket_acl("123456789012", &get, "email-grant") + .unwrap(); + let body = std::str::from_utf8(resp.body.expect_bytes()).unwrap(); + assert!( + body.contains("AmazonCustomerByEmail") + && body.contains("someone@example.com"), + "email grantee not preserved: {body}" + ); +} + +#[test] +fn create_bucket_rejects_an_unparseable_grantee_rather_than_writing_an_empty_acl() { + // A clause naming no known grantee key parses to nothing. Storing that as + // the bucket's ACL would leave it with no grants at all -- not even the + // owner -- which the loader honors as an explicit empty ACL. + let svc = make_service(); + for value in ["nonsense=whoever", "justtext"] { + let mut req = make_request(Method::PUT, "/bad-grantee", &[], b""); + req.headers + .insert("x-amz-grant-full-control", value.parse().unwrap()); + assert_aws_err( + svc.create_bucket("123456789012", &req, "bad-grantee"), + "InvalidArgument", + ); + } + assert_aws_err(svc.head_bucket("123456789012", "bad-grantee"), "NotFound"); +} + +#[test] +fn create_bucket_rejects_an_acl_alongside_bucket_owner_enforced() { + // Real S3 answers InvalidBucketAclWithObjectOwnership: BucketOwnerEnforced + // disables ACLs, so a create cannot also ask for one. Allowing it would + // leave a publicly readable bucket whose ACL PutBucketAcl refuses to edit. + let svc = make_service(); + let mut req = make_request(Method::PUT, "/owner-enforced", &[], b""); + req.headers + .insert("x-amz-acl", "public-read".parse().unwrap()); + req.headers.insert( + "x-amz-object-ownership", + "BucketOwnerEnforced".parse().unwrap(), + ); + assert_aws_err( + svc.create_bucket("123456789012", &req, "owner-enforced"), + "InvalidBucketAclWithObjectOwnership", + ); + + // `private` grants nothing beyond the owner, so S3 accepts it alongside + // BucketOwnerEnforced -- and so do we. + let mut ok_req = make_request(Method::PUT, "/owner-enforced-ok", &[], b""); + ok_req + .headers + .insert("x-amz-acl", "private".parse().unwrap()); + ok_req.headers.insert( + "x-amz-object-ownership", + "BucketOwnerEnforced".parse().unwrap(), + ); + svc.create_bucket("123456789012", &ok_req, "owner-enforced-ok") + .unwrap(); + + // A grant header conflicts on presence alone, even one naming the owner: + // the caller is asking for an ACL on a bucket that has none. + let mut grant_req = make_request(Method::PUT, "/owner-enforced-grant", &[], b""); + grant_req.headers.insert( + "x-amz-grant-full-control", + "id=123456789012".parse().unwrap(), + ); + grant_req.headers.insert( + "x-amz-object-ownership", + "BucketOwnerEnforced".parse().unwrap(), + ); + assert_aws_err( + svc.create_bucket("123456789012", &grant_req, "owner-enforced-grant"), + "InvalidBucketAclWithObjectOwnership", + ); +} + +#[test] +fn create_bucket_rejects_a_canned_acl_that_is_not_a_bucket_canned_acl() { + // `com.amazonaws.s3#BucketCannedACL` has exactly four members; + // `bucket-owner-full-control` is an OBJECT canned ACL, and a typo is not an + // ACL at all. Both used to fall through to owner-only grants silently, and + // this branch would then persist that as the bucket's explicit ACL. + let svc = make_service(); + for value in ["pubic-read", "not-an-acl"] { + let mut req = make_request(Method::PUT, "/bad-canned", &[], b""); + req.headers.insert("x-amz-acl", value.parse().unwrap()); + assert_aws_err( + svc.create_bucket("123456789012", &req, "bad-canned"), + "InvalidArgument", + ); + } + assert_aws_err(svc.head_bucket("123456789012", "bad-canned"), "NotFound"); + + // The object-scoped canned ACLs are IGNORED on a bucket rather than + // refused, so they are accepted and resolve to the owner's FULL_CONTROL. + let mut owner_acl = make_request(Method::PUT, "/owner-canned", &[], b""); + owner_acl + .headers + .insert("x-amz-acl", "bucket-owner-full-control".parse().unwrap()); + svc.create_bucket("123456789012", &owner_acl, "owner-canned") + .unwrap(); +} + +#[test] +fn create_bucket_accepts_log_delivery_write_with_real_grants() { + // log-delivery-write is bucket-scoped in S3's canned-ACL table even though + // `com.amazonaws.s3#BucketCannedACL` omits it, and it has to actually grant + // the log-delivery group write access -- that is the whole point of using + // it on a logging target. + let svc = make_service(); + let mut req = make_request(Method::PUT, "/canned-logdelivery", &[], b""); + req.headers + .insert("x-amz-acl", "log-delivery-write".parse().unwrap()); + svc.create_bucket("123456789012", &req, "canned-logdelivery") + .unwrap(); + + let get = make_request(Method::GET, "/canned-logdelivery", &[("acl", "")], b""); + let resp = svc + .get_bucket_acl("123456789012", &get, "canned-logdelivery") + .unwrap(); + let body = std::str::from_utf8(resp.body.expect_bytes()).unwrap(); + assert!(body.contains("s3/LogDelivery"), "{body}"); + assert!(body.contains("WRITE"), "{body}"); +} + +#[test] +fn create_bucket_accepts_aws_exec_read_but_treats_it_as_granting_past_the_owner() { + // aws-exec-read is bucket-scoped and AWS answers 200 (CloudFormation's + // AccessControl: AwsExecRead maps to it), so refusing it would break a + // working call. Its READ grant to the EC2 service's canonical user is not + // modeled, so the stored ACL is owner-only -- but the request still asks for + // an ACL reaching past the owner, so it conflicts with BucketOwnerEnforced. + let svc = make_service(); + let mut req = make_request(Method::PUT, "/aws-exec", &[], b""); + req.headers + .insert("x-amz-acl", "aws-exec-read".parse().unwrap()); + svc.create_bucket("123456789012", &req, "aws-exec").unwrap(); + + let mut enforced = make_request(Method::PUT, "/aws-exec-enforced", &[], b""); + enforced + .headers + .insert("x-amz-acl", "aws-exec-read".parse().unwrap()); + enforced.headers.insert( + "x-amz-object-ownership", + "BucketOwnerEnforced".parse().unwrap(), + ); + assert_aws_err( + svc.create_bucket("123456789012", &enforced, "aws-exec-enforced"), + "InvalidBucketAclWithObjectOwnership", + ); +} + +#[test] +fn put_object_acl_rejects_a_body_that_is_not_an_access_control_policy() { + // A non-XML body parses to zero grants; treating that as "remove every + // grant" strips the object's owner FULL_CONTROL and writes the empty list + // into its meta. + let svc = make_service(); + seed_bucket(&svc, "obj-junk"); + seed_object(&svc, "obj-junk", "k.txt", b"body"); + + let json = make_request(Method::PUT, "/obj-junk/k.txt", &[("acl", "")], b"{}"); + assert_aws_err( + svc.put_object_acl("123456789012", &json, "obj-junk", "k.txt"), + "MalformedXML", + ); + + let blank = make_request(Method::PUT, "/obj-junk/k.txt", &[("acl", "")], b" "); + assert_aws_err( + svc.put_object_acl("123456789012", &blank, "obj-junk", "k.txt"), + "MalformedACLError", + ); +} + +#[test] +fn put_object_acl_rejects_a_canned_acl_with_grants_or_a_body() { + // Letting the canned value win discarded the grants the caller asked for, + // and the result is written into the object meta. + let svc = make_service(); + seed_bucket(&svc, "obj-both"); + seed_object(&svc, "obj-both", "k.txt", b"body"); + + let mut with_grant = make_request(Method::PUT, "/obj-both/k.txt", &[("acl", "")], b""); + with_grant + .headers + .insert("x-amz-acl", "private".parse().unwrap()); + with_grant.headers.insert( + "x-amz-grant-read", + "uri=http://acs.amazonaws.com/groups/global/AllUsers" + .parse() + .unwrap(), + ); + assert_aws_err( + svc.put_object_acl("123456789012", &with_grant, "obj-both", "k.txt"), + "InvalidRequest", + ); + + let mut with_body = make_request( + Method::PUT, + "/obj-both/k.txt", + &[("acl", "")], + b"123456789012", + ); + with_body + .headers + .insert("x-amz-acl", "private".parse().unwrap()); + assert_aws_err( + svc.put_object_acl("123456789012", &with_body, "obj-both", "k.txt"), + "InvalidRequest", + ); +} + +#[test] +fn create_multipart_upload_rejects_an_acl_when_ownership_disables_them() { + // Every sibling ACL-setting path refuses this; multipart used to accept it + // and carry the grants into the completed object, which is now persisted. + let svc = make_service(); + seed_bucket(&svc, "mpu-owner"); + { + let mut mas = svc.state.write(); + let state = mas.default_mut(); + let b = state.buckets.get_mut("mpu-owner").unwrap(); + b.ownership_controls = Some( + "BucketOwnerEnforced\ + " + .to_string(), + ); + } + + let mut canned = make_request(Method::POST, "/mpu-owner/k.txt", &[("uploads", "")], b""); + canned + .headers + .insert("x-amz-acl", "public-read".parse().unwrap()); + assert_aws_err( + svc.create_multipart_upload("123456789012", &canned, "mpu-owner", "k.txt"), + "AccessControlListNotSupported", + ); + + let mut granted = make_request(Method::POST, "/mpu-owner/k.txt", &[("uploads", "")], b""); + granted.headers.insert( + "x-amz-grant-read", + "uri=http://acs.amazonaws.com/groups/global/AllUsers" + .parse() + .unwrap(), + ); + assert_aws_err( + svc.create_multipart_upload("123456789012", &granted, "mpu-owner", "k.txt"), + "AccessControlListNotSupported", + ); +} + +#[test] +fn put_bucket_ownership_controls_rejects_an_unknown_value() { + // An unrecognized value used to be stored verbatim and reloaded as an + // ownership rule meaning nothing, silently returning the bucket to + // ACLs-enabled. + let svc = make_service(); + seed_bucket(&svc, "own-bad"); + let req = make_request( + Method::PUT, + "/own-bad", + &[("ownershipControls", "")], + b"Whatever", + ); + assert_aws_err( + svc.put_bucket_ownership_controls("123456789012", &req, "own-bad"), + "InvalidArgument", + ); +} + +#[test] +fn put_object_acl_updates_the_versioned_copy_too() { + // A versioned bucket keeps a second copy of the current version in + // `object_versions`. Leaving it stale let a later delete re-derive the + // current object from the old grants, silently reverting a change that had + // already been persisted. The ids have to be real: with None on both sides + // the version guard matches anything and the test proves nothing. + let svc = make_service(); + seed_bucket(&svc, "ver-acl"); + seed_object(&svc, "ver-acl", "k.txt", b"body"); + { + let mut mas = svc.state.write(); + let state = mas.default_mut(); + let b = state.buckets.get_mut("ver-acl").unwrap(); + b.versioning = Some("Enabled".to_string()); + let mut current = b.objects.get("k.txt").unwrap().clone(); + current.version_id = Some("v2".to_string()); + let mut older = current.clone(); + older.version_id = Some("v1".to_string()); + b.objects.insert("k.txt".to_string(), current.clone()); + b.object_versions + .insert("k.txt".to_string(), vec![older, current]); + } + + let mut req = make_request(Method::PUT, "/ver-acl/k.txt", &[("acl", "")], b""); + req.headers + .insert("x-amz-acl", "public-read".parse().unwrap()); + svc.put_object_acl("123456789012", &req, "ver-acl", "k.txt") + .unwrap(); + + let mas = svc.state.read(); + let state = mas.get("123456789012").unwrap(); + let b = state.buckets.get("ver-acl").unwrap(); + let versions = b.object_versions.get("k.txt").unwrap(); + let public = |o: &crate::state::S3Object| { + o.acl_grants.iter().any(|g| { + g.grantee_uri + .as_deref() + .is_some_and(|u| u.contains("AllUsers")) + }) + }; + let current = versions + .iter() + .find(|v| v.version_id.as_deref() == Some("v2")); + assert!( + public(current.expect("v2 present")), + "the current version's copy kept the old grants" + ); + let older = versions + .iter() + .find(|v| v.version_id.as_deref() == Some("v1")); + assert!( + !public(older.expect("v1 present")), + "an older version must not be touched" + ); +} + +#[test] +fn put_object_acl_rejects_an_unresolvable_grantee() { + // The object paths share parse_grant_headers, which drops what it cannot + // resolve. Without the shared guard they stored an object ACL with not even + // the owner's FULL_CONTROL. + let svc = make_service(); + seed_bucket(&svc, "obj-acl"); + seed_object(&svc, "obj-acl", "k.txt", b"body"); + + let mut req = make_request(Method::PUT, "/obj-acl/k.txt", &[("acl", "")], b""); + req.headers.insert( + "x-amz-grant-full-control", + "nonsense=alice".parse().unwrap(), + ); + assert_aws_err( + svc.put_object_acl("123456789012", &req, "obj-acl", "k.txt"), + "InvalidArgument", + ); +} + +#[test] +fn create_bucket_rejects_a_grant_header_with_an_empty_grantee() { + // `id=` with nothing after it names no grantee. Storing a grant with an + // empty canonical id would put an entry in the bucket's ACL that can never + // match anyone. + let svc = make_service(); + for value in ["id=", "uri=", "id= "] { + let mut req = make_request(Method::PUT, "/empty-grantee", &[], b""); + req.headers + .insert("x-amz-grant-full-control", value.parse().unwrap()); + assert_aws_err( + svc.create_bucket("123456789012", &req, "empty-grantee"), + "InvalidArgument", + ); + } + assert_aws_err(svc.head_bucket("123456789012", "empty-grantee"), "NotFound"); +} + +#[test] +fn put_bucket_acl_rejects_a_body_that_is_not_an_access_control_policy() { + // parse_acl_xml returns no grants for a body with no and no + // 123456789012", + ); + svc.put_bucket_acl("123456789012", &empty_list, "pba-junk") + .unwrap(); +} + +#[test] +fn put_bucket_acl_rejects_a_canned_acl_alongside_a_policy_body() { + // Letting the canned value win would answer 200 while dropping everything + // the body granted, and this branch persists the result. + let svc = make_service(); + let create = make_request(Method::PUT, "/pba-canned-body", &[], b""); + svc.create_bucket("123456789012", &create, "pba-canned-body") + .unwrap(); + + let mut req = make_request( + Method::PUT, + "/pba-canned-body", + &[("acl", "")], + b"123456789012http://acs.amazonaws.com/groups/global/AllUsersREAD", + ); + req.headers.insert("x-amz-acl", "private".parse().unwrap()); + assert_aws_err( + svc.put_bucket_acl("123456789012", &req, "pba-canned-body"), + "InvalidRequest", + ); +} + +#[test] +fn bucket_acl_email_grantee_round_trips_through_the_policy_body() { + // GetBucketAcl now emits AmazonCustomerByEmail, so its output can be fed + // straight back into PutBucketAcl. Reading that grantee as a CanonicalUser + // would store an empty-id grant -- an entry matching nobody, which the + // header path refuses. + let svc = make_service(); + let mut create = make_request(Method::PUT, "/email-rt", &[], b""); + create.headers.insert( + "x-amz-grant-read", + "emailAddress=someone@example.com".parse().unwrap(), + ); + svc.create_bucket("123456789012", &create, "email-rt") + .unwrap(); + + let get = make_request(Method::GET, "/email-rt", &[("acl", "")], b""); + let first = svc + .get_bucket_acl("123456789012", &get, "email-rt") + .unwrap(); + let xml = std::str::from_utf8(first.body.expect_bytes()) + .unwrap() + .to_string(); + + let put = make_request(Method::PUT, "/email-rt", &[("acl", "")], xml.as_bytes()); + svc.put_bucket_acl("123456789012", &put, "email-rt") + .unwrap(); + + let second = svc + .get_bucket_acl("123456789012", &get, "email-rt") + .unwrap(); + let body = std::str::from_utf8(second.body.expect_bytes()).unwrap(); + assert!( + body.contains("someone@example.com"), + "email grantee lost on the body round trip: {body}" + ); + // Assert against the stored ACL, not the rendering: the grant has to BE an + // email grantee, not merely render without an empty . + let mas = svc.state.read(); + let state = mas.get("123456789012").unwrap(); + let stored = &state.buckets.get("email-rt").unwrap().acl_grants; + assert!( + stored.iter().any(|g| { + g.grantee_type == "AmazonCustomerByEmail" + && g.grantee_display_name.as_deref() == Some("someone@example.com") + }), + "the round trip did not store an email grantee: {stored:?}" + ); + assert!( + stored.iter().all(|g| g.grantee_id.as_deref() != Some("")), + "stored a grant naming nobody: {stored:?}" + ); +} + +#[test] +fn put_bucket_acl_rejects_a_grantee_naming_nobody() { + let svc = make_service(); + let create = make_request(Method::PUT, "/pba-empty-id", &[], b""); + svc.create_bucket("123456789012", &create, "pba-empty-id") + .unwrap(); + + let req = make_request( + Method::PUT, + "/pba-empty-id", + &[("acl", "")], + b"123456789012FULL_CONTROL", + ); + assert_aws_err( + svc.put_bucket_acl("123456789012", &req, "pba-empty-id"), + "MalformedACLError", + ); +} + +#[test] +fn put_bucket_acl_rejects_grant_headers_alongside_a_policy_body() { + let svc = make_service(); + let create = make_request(Method::PUT, "/pba-both", &[], b""); + svc.create_bucket("123456789012", &create, "pba-both") + .unwrap(); + + let mut req = make_request( + Method::PUT, + "/pba-both", + &[("acl", "")], + b"123456789012123456789012FULL_CONTROL", + ); + req.headers.insert( + "x-amz-grant-read", + "uri=http://acs.amazonaws.com/groups/global/AllUsers" + .parse() + .unwrap(), + ); + assert_aws_err( + svc.put_bucket_acl("123456789012", &req, "pba-both"), + "InvalidRequest", + ); +} + +#[test] +fn create_bucket_rejects_an_unknown_object_ownership_value() { + // An unrecognized value used to be stored verbatim, and this branch + // persists it -- so the bucket would come back after a restart with an + // OwnershipControls rule that means nothing. + let svc = make_service(); + let mut req = make_request(Method::PUT, "/bad-ownership", &[], b""); + req.headers + .insert("x-amz-object-ownership", "Whatever".parse().unwrap()); + assert_aws_err( + svc.create_bucket("123456789012", &req, "bad-ownership"), + "InvalidArgument", + ); + + // Casing is significant: `bucket_owner_enforced` matches the stored XML + // case-sensitively, so a differently cased value must not be accepted here + // and then read back as "ACLs enabled" by the next call. + let mut cased = make_request(Method::PUT, "/cased-ownership", &[], b""); + cased.headers.insert( + "x-amz-object-ownership", + "bucketownerenforced".parse().unwrap(), + ); + assert_aws_err( + svc.create_bucket("123456789012", &cased, "cased-ownership"), + "InvalidArgument", + ); +} + +#[test] +fn put_bucket_acl_rejects_an_unknown_canned_value_instead_of_wiping_grants() { + // The catch-all in `canned_acl_grants` turned an unrecognized canned ACL + // into owner-only, silently stripping every public grant -- and that wipe + // is now persisted. + let svc = make_service(); + let create = make_request(Method::PUT, "/pba-canned", &[], b""); + svc.create_bucket("123456789012", &create, "pba-canned") + .unwrap(); + + let mut req = make_request(Method::PUT, "/pba-canned", &[("acl", "")], b""); + req.headers + .insert("x-amz-acl", "pubic-read".parse().unwrap()); + assert_aws_err( + svc.put_bucket_acl("123456789012", &req, "pba-canned"), + "InvalidArgument", + ); +} + +#[test] +fn put_bucket_acl_honors_grant_headers() { + let svc = make_service(); + let create = make_request(Method::PUT, "/pba-grant", &[], b""); + svc.create_bucket("123456789012", &create, "pba-grant") + .unwrap(); + + let mut req = make_request(Method::PUT, "/pba-grant", &[("acl", "")], b""); + req.headers.insert( + "x-amz-grant-read", + "uri=http://acs.amazonaws.com/groups/global/AllUsers" + .parse() + .unwrap(), + ); + svc.put_bucket_acl("123456789012", &req, "pba-grant") + .unwrap(); + + let get = make_request(Method::GET, "/pba-grant", &[("acl", "")], b""); + let resp = svc + .get_bucket_acl("123456789012", &get, "pba-grant") + .unwrap(); + let body = std::str::from_utf8(resp.body.expect_bytes()).unwrap(); + assert!(body.contains("AllUsers"), "grant headers ignored: {body}"); +} + +#[test] +fn put_bucket_acl_rejects_a_request_that_names_no_acl() { + // No canned header, no grant header, no body: malformed, not "drop every + // grant". Without this the bucket's ACL was wiped to nothing, persisted. + let svc = make_service(); + let create = make_request(Method::PUT, "/pba-empty", &[], b""); + svc.create_bucket("123456789012", &create, "pba-empty") + .unwrap(); + + let req = make_request(Method::PUT, "/pba-empty", &[("acl", "")], b""); + assert_aws_err( + svc.put_bucket_acl("123456789012", &req, "pba-empty"), + "MalformedACLError", + ); + + // The grants the bucket had are untouched. + let get = make_request(Method::GET, "/pba-empty", &[("acl", "")], b""); + let resp = svc + .get_bucket_acl("123456789012", &get, "pba-empty") + .unwrap(); + let body = std::str::from_utf8(resp.body.expect_bytes()).unwrap(); + assert!(body.contains("FULL_CONTROL"), "{body}"); +} + +#[test] +fn create_bucket_rejects_an_empty_grant_header() { + // An empty or blank header value resolves to no grants at all. Storing that + // would leave the bucket with not even an owner grant, permanently. + // A blank value is what a client sends for an unset config field, and S3 + // treats the header as absent rather than failing the call. + let svc = make_service(); + for (name, value) in [("blank-empty", ""), ("blank-space", " ")] { + let path = format!("/{name}"); + let mut req = make_request(Method::PUT, &path, &[], b""); + req.headers + .insert("x-amz-grant-read", value.parse().unwrap()); + svc.create_bucket("123456789012", &req, name) + .unwrap_or_else(|e| panic!("blank grant header should be ignored: {e:?}")); + } + + // A value that carries something but names no grantee is still refused: a + // bare separator, or a grantee form this build cannot resolve. + for value in [",", "nonsense=x"] { + let mut bad = make_request(Method::PUT, "/blank-grant", &[], b""); + bad.headers + .insert("x-amz-grant-read", value.parse().unwrap()); + assert_aws_err( + svc.create_bucket("123456789012", &bad, "blank-grant"), + "InvalidArgument", + ); + } + assert_aws_err(svc.head_bucket("123456789012", "blank-grant"), "NotFound"); +} + +#[test] +fn create_bucket_rejects_partially_unresolvable_grant_headers() { + // A mixed request is the dangerous case: the resolvable clause would be + // stored as the bucket's authoritative ACL while the unresolvable one is + // dropped, so GetBucketAcl would report a permission set nobody asked for. + let svc = make_service(); + let mut req = make_request(Method::PUT, "/mixed-grant", &[], b""); + req.headers.insert( + "x-amz-grant-full-control", + "nonsense=alice".parse().unwrap(), + ); + req.headers.insert( + "x-amz-grant-read", + "uri=http://acs.amazonaws.com/groups/global/AllUsers" + .parse() + .unwrap(), + ); + assert_aws_err( + svc.create_bucket("123456789012", &req, "mixed-grant"), + "InvalidArgument", + ); + assert_aws_err(svc.head_bucket("123456789012", "mixed-grant"), "NotFound"); +} + +#[test] +fn create_bucket_ignores_an_unrecognized_grant_header() { + // Only the five real `x-amz-grant-*` headers count. A lookalike must not + // turn a valid create into an error, nor collide with a canned ACL. + let svc = make_service(); + let mut req = make_request(Method::PUT, "/odd-grant", &[], b""); + req.headers + .insert("x-amz-acl", "public-read".parse().unwrap()); + req.headers + .insert("x-amz-grant-nonsense", "id=whoever".parse().unwrap()); + svc.create_bucket("123456789012", &req, "odd-grant") + .unwrap(); + + let get = make_request(Method::GET, "/odd-grant", &[("acl", "")], b""); + let resp = svc + .get_bucket_acl("123456789012", &get, "odd-grant") + .unwrap(); + let body = std::str::from_utf8(resp.body.expect_bytes()).unwrap(); + assert!(body.contains("AllUsers"), "canned ACL not applied: {body}"); +} + #[test] fn create_bucket_stores_tags_from_create_bucket_configuration() { let svc = make_service(); diff --git a/website/content/docs/services/s3.md b/website/content/docs/services/s3.md index 10b3a2189..efa446855 100644 --- a/website/content/docs/services/s3.md +++ b/website/content/docs/services/s3.md @@ -18,7 +18,8 @@ fakecloud implements **107 of 107** S3 operations at 100% Smithy conformance. - **Bucket subresources** — policy, CORS, lifecycle, logging, website, public access block, object lock, replication, ownership, inventory, encryption, accelerate, request payment, tagging - **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. -- **Object Lock** — legal hold, retention modes +- **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` included. +- **Object Lock** — legal hold, retention modes. A bucket created with `x-amz-bucket-object-lock-enabled` keeps its lock configuration (and its canned ACL and object-ownership rule) across a restart in persistent mode, so retention stays enforced. - **Website hosting** — index/error documents, redirect rules - **Access Points** — full control plane (`CreateAccessPoint`, `GetAccessPoint`, `DeleteAccessPoint`, `ListAccessPoints`) via the `s3-control` host prefix; data plane traffic to `s3-accesspoint.` resolves the alias to its underlying bucket so standard S3 operations work unchanged. - **S3 Select** — real `SelectObjectContent` over CSV/JSON via EventStream framing (`Records`, `Stats`, `End` messages).