From 99fa86b33aa13418146dd94358b7656f9524a822 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Thu, 16 Jul 2026 04:49:51 -0300 Subject: [PATCH] fix(s3): validate x-amz-content-sha256 against received body (bug-hunt 5.4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A plain (non-chunked) SigV4 PutObject carries the payload's real SHA-256 in the x-amz-content-sha256 header. fakecloud spooled the body but never compared this header against the bytes received, so a corrupt/tampered upload whose header disagreed with its body stored silently. AWS rejects the divergence with XAmzContentSHA256Mismatch (400). This lives at the S3 service layer, NOT in sigv4 verification: the signed payload is not reliably re-derivable there (aws-chunked / streaming empties the buffered body), which is why the earlier sigv4-layer attempt (#2288) was reverted. The S3 handler holds the fully-decoded object bytes, so the hash is authoritative. - core: SpooledBody gains sha256_hex, computed in the same single streaming pass as md5_hex (no extra IO). Over the DECODED payload for aws-chunked. - s3 write path: when x-amz-content-sha256 is a 64-char hex digest (not UNSIGNED-PAYLOAD, not a STREAMING-... marker, not aws-chunked), compare it to the spool's sha256 and return XAmzContentSHA256Mismatch on divergence. - Skips markers/streaming entirely, so correct clients (SDK, aws-cli, boto3 which default to aws-chunked STREAMING) are unaffected. Tests: core unit tests for spool sha256 (plain + aws-chunked decoded); e2e s3_put_object_rejects_content_sha256_mismatch (bogus hex -> 400, correct hex -> 200, UNSIGNED-PAYLOAD -> bypass). Ran iam_enforcement + iam_enforcement_abac (the verify_sigv4 paths #2288's revert restored), s3 (80), s3_aws_chunked, s3_streaming_body, and s3 conformance (52/52) — all green. No non-code surface change: behavior now matches AWS more faithfully; invisible to correct clients, no new API/flag/field, no SDK change. --- Cargo.lock | 1 + crates/fakecloud-core/Cargo.toml | 1 + crates/fakecloud-core/src/service.rs | 43 +++++++++ .../tests/s3_checksum_integrity.rs | 96 +++++++++++++++++++ .../fakecloud-s3/src/service/objects/write.rs | 33 +++++++ 5 files changed, 174 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 3873d4916..ed5c21271 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5278,6 +5278,7 @@ dependencies = [ "parking_lot", "serde", "serde_json", + "sha2 0.10.9", "tempfile", "thiserror 2.0.18", "tokio", diff --git a/crates/fakecloud-core/Cargo.toml b/crates/fakecloud-core/Cargo.toml index d9378cc69..a64966a5e 100644 --- a/crates/fakecloud-core/Cargo.toml +++ b/crates/fakecloud-core/Cargo.toml @@ -21,6 +21,7 @@ md-5 = { workspace = true } parking_lot = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +sha2 = { workspace = true } tempfile = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } diff --git a/crates/fakecloud-core/src/service.rs b/crates/fakecloud-core/src/service.rs index d56419092..ff002620a 100644 --- a/crates/fakecloud-core/src/service.rs +++ b/crates/fakecloud-core/src/service.rs @@ -160,6 +160,13 @@ pub struct SpooledBody { pub path: PathBuf, pub size: u64, pub md5_hex: String, + /// Lowercase-hex SHA-256 of the decoded payload, computed in the same + /// single streaming pass as the MD5. Lets the S3 layer verify a client's + /// `x-amz-content-sha256` header against the bytes actually received + /// (returning `XAmzContentSHA256Mismatch` on divergence) for plain, + /// non-`aws-chunked` uploads where the header carries the real payload + /// hash rather than a `STREAMING-…`/`UNSIGNED-PAYLOAD` marker. + pub sha256_hex: String, } /// Incremental decoder for the `aws-chunked` content-encoding that modern AWS @@ -360,6 +367,7 @@ pub async fn spool_request_stream( let mut file = tokio::fs::File::from_std(std_file); let mut hasher = Md5::new(); + let mut sha = sha2::Sha256::new(); let mut size: u64 = 0; let mut body = stream; let mut decoder = aws_chunked.then(AwsChunkedDecoder::default); @@ -396,6 +404,7 @@ pub async fn spool_request_stream( }; if !payload.is_empty() { hasher.update(&payload); + sha.update(&payload); size += payload.len() as u64; if let Err(e) = file.write_all(&payload).await { cleanup(file, &path).await; @@ -430,10 +439,12 @@ pub async fn spool_request_stream( drop(file); let md5_hex = hex_lower(&hasher.finalize()); + let sha256_hex = hex_lower(&sha.finalize()); Ok(SpooledBody { path, size, md5_hex, + sha256_hex, }) } @@ -853,6 +864,38 @@ mod tests { assert_eq!(decode_all(&body, 3), Vec::::new()); } + fn sha256_hex(bytes: &[u8]) -> String { + let mut h = sha2::Sha256::new(); + h.update(bytes); + hex_lower(&h.finalize()) + } + + #[tokio::test] + async fn spool_computes_sha256_over_plain_payload() { + let payload = b"hello world".to_vec(); + let spooled = spool_request_stream(axum::body::Body::from(payload.clone()), None, false) + .await + .expect("spool ok"); + assert_eq!(spooled.size, payload.len() as u64); + assert_eq!(spooled.sha256_hex, sha256_hex(&payload)); + let _ = std::fs::remove_file(&spooled.path); + } + + #[tokio::test] + async fn spool_sha256_is_over_decoded_aws_chunked_payload() { + // The header the client sends over aws-chunked framing is a STREAMING + // marker, but the spool's sha256 must still describe the DECODED bytes + // (so the S3 layer never compares against the framed wire form). + let payload: Vec = (0..9000u32).map(|i| (i % 251) as u8).collect(); + let body = aws_chunked_body(&payload, 1024, true); + let spooled = spool_request_stream(axum::body::Body::from(body), None, true) + .await + .expect("spool ok"); + assert_eq!(spooled.size, payload.len() as u64); + assert_eq!(spooled.sha256_hex, sha256_hex(&payload)); + let _ = std::fs::remove_file(&spooled.path); + } + #[test] fn aws_chunked_decoder_rejects_bad_size_line() { let mut d = AwsChunkedDecoder::default(); diff --git a/crates/fakecloud-e2e/tests/s3_checksum_integrity.rs b/crates/fakecloud-e2e/tests/s3_checksum_integrity.rs index 2a27ee3ab..8f8433895 100644 --- a/crates/fakecloud-e2e/tests/s3_checksum_integrity.rs +++ b/crates/fakecloud-e2e/tests/s3_checksum_integrity.rs @@ -45,3 +45,99 @@ async fn s3_put_object_rejects_checksum_mismatch() { .await .expect("a correct checksum must be accepted"); } + +fn sha256_hex(bytes: &[u8]) -> String { + use sha2::{Digest, Sha256}; + let mut h = Sha256::new(); + h.update(bytes); + h.finalize().iter().map(|b| format!("{b:02x}")).collect() +} + +/// Bug-hunt 5.4: a plain (non-chunked) SigV4 PutObject carries the payload's +/// real SHA-256 in `x-amz-content-sha256`. fakecloud spooled the body but never +/// compared this header against the bytes received, so a corrupt/tampered +/// upload whose header disagreed with its body stored silently. AWS rejects the +/// divergence with `XAmzContentSHA256Mismatch` (400). This is enforced at the +/// S3 layer (the buffered payload is authoritative) rather than in sigv4 +/// verification, where aws-chunked/streaming empties the body buffer. +#[tokio::test] +async fn s3_put_object_rejects_content_sha256_mismatch() { + let s = TestServer::start().await; + let s3 = s.s3_client().await; + s3.create_bucket() + .bucket("sha-bucket") + .send() + .await + .unwrap(); + + let body = b"hello world".to_vec(); + let bogus = "0".repeat(64); // valid hex, wrong for the body + + // Raw PUT with a SigV4 Authorization header (routes through the streaming + // spool path; signature is not verified — auth is off by default) and a + // mismatched x-amz-content-sha256 hex digest. + let url = format!("{}/sha-bucket/obj", s.endpoint()); + let resp = reqwest::Client::new() + .put(&url) + .header( + "authorization", + "AWS4-HMAC-SHA256 Credential=test/20240101/us-east-1/s3/aws4_request, \ + SignedHeaders=host;x-amz-content-sha256, Signature=0000000000000000000000000000000000000000000000000000000000000000", + ) + .header("x-amz-content-sha256", &bogus) + .header("content-length", body.len().to_string()) + .body(body.clone()) + .send() + .await + .unwrap(); + assert_eq!( + resp.status(), + 400, + "a mismatched x-amz-content-sha256 must be rejected with 400" + ); + let text = resp.text().await.unwrap(); + assert!( + text.contains("XAmzContentSHA256Mismatch"), + "expected XAmzContentSHA256Mismatch, got: {text}" + ); + + // The correct digest for the same body round-trips fine. + let good = sha256_hex(&body); + let ok = reqwest::Client::new() + .put(&url) + .header( + "authorization", + "AWS4-HMAC-SHA256 Credential=test/20240101/us-east-1/s3/aws4_request, \ + SignedHeaders=host;x-amz-content-sha256, Signature=0000000000000000000000000000000000000000000000000000000000000000", + ) + .header("x-amz-content-sha256", &good) + .header("content-length", body.len().to_string()) + .body(body) + .send() + .await + .unwrap(); + assert!( + ok.status().is_success(), + "a correct x-amz-content-sha256 must be accepted, got {}", + ok.status() + ); + + // UNSIGNED-PAYLOAD must skip the check entirely (marker, not a body hash). + let unsigned = reqwest::Client::new() + .put(format!("{}/sha-bucket/obj2", s.endpoint())) + .header( + "authorization", + "AWS4-HMAC-SHA256 Credential=test/20240101/us-east-1/s3/aws4_request, \ + SignedHeaders=host;x-amz-content-sha256, Signature=0000000000000000000000000000000000000000000000000000000000000000", + ) + .header("x-amz-content-sha256", "UNSIGNED-PAYLOAD") + .body(b"anything".to_vec()) + .send() + .await + .unwrap(); + assert!( + unsigned.status().is_success(), + "UNSIGNED-PAYLOAD must bypass the content-sha256 check, got {}", + unsigned.status() + ); +} diff --git a/crates/fakecloud-s3/src/service/objects/write.rs b/crates/fakecloud-s3/src/service/objects/write.rs index 9fbd58356..637d66f28 100644 --- a/crates/fakecloud-s3/src/service/objects/write.rs +++ b/crates/fakecloud-s3/src/service/objects/write.rs @@ -201,6 +201,39 @@ impl S3Service { } } } + + // x-amz-content-sha256 payload-integrity check (bug-hunt 5.4). When a + // client sends the header as a real lowercase-hex SHA-256 digest — the + // default for SigV4-signed, non-chunked PutObject — AWS verifies it + // against the received bytes and rejects a mismatch with + // `XAmzContentSHA256Mismatch` (400). This lives at the S3 layer, NOT in + // sigv4 verification: the signed payload is not reliably re-derivable + // there (aws-chunked / streaming empties the buffered body). Here we + // hold the fully-decoded object bytes, so the hash is authoritative. + // + // Skip markers that are not a plain body hash: `UNSIGNED-PAYLOAD`, any + // `STREAMING-…` value, and aws-chunked uploads (the header then covers + // the chunk framing, and the spool's sha256 is over the decoded + // payload, so they would never match). Only a 64-char hex value is + // treated as an assertion about the payload. + if !fakecloud_core::service::is_aws_chunked(&req.headers) { + if let Some(hdr) = req + .headers + .get("x-amz-content-sha256") + .and_then(|v| v.to_str().ok()) + { + let supplied = hdr.trim(); + let is_hex_digest = + supplied.len() == 64 && supplied.bytes().all(|b| b.is_ascii_hexdigit()); + if is_hex_digest && !supplied.eq_ignore_ascii_case(&spooled.sha256_hex) { + return Err(AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "XAmzContentSHA256Mismatch", + "The provided 'x-amz-content-sha256' header does not match what was computed.", + )); + } + } + } let content_type = req .headers .get("content-type")