From 5eee2aaeadbee3e3d98698e782f7a332af2ff0d4 Mon Sep 17 00:00:00 2001 From: Chris Date: Mon, 14 Sep 2026 21:08:45 +0800 Subject: [PATCH] fix(auth): reject unsigned x-amz headers on header-signed SigV4 requests (#7796) * fix(auth): reject unsigned x-amz headers on header-signed SigV4 requests A SigV4 request authenticated with an Authorization header only binds the headers named in its SignedHeaders list, but RustFS acted on every x-amz-* header that arrived, so a replayed header-signed PutObject carrying an unsigned x-amz-copy-source became a CopyObject run as the signer that could copy any object the signer can read (GHSA-xm99-m3gq-83g8). The presigned form was already closed by GHSA-g8w9-qw9q-fghr. reject_unsigned_amz_headers_on_sigv4_request now guards S3Access::check and S3Router::check_access: every SigV4 signed-header list the request carries must cover every x-amz-* header, the Authorization header is parsed with the verifier's own s3s-sigv4 parser and compared case-insensitively, the algorithm token is pinned to AWS4-HMAC-SHA256 because the upstream header path accepts any token, and the exempt set mirrors the upstream s3s fix (x-amz-content-sha256, x-amz-decoded-content-length, x-amz-trailer, x-amz-checksum-algorithm) plus x-amz-cf-id. Adds ghsa_xm99 unit, router and e2e regressions, raises the security smoke floor to 28, and records the advisory in docs/testing/security-regressions.md and CHANGELOG.md. * chore(deps): switch s3s and s3s-sigv4 to crates.io 0.16.0 * fix(server): enforce the SigV4 header guard ahead of s3s dispatch s3s 0.16 verifies the claimed algorithm as the first step of its own signature flow, so a request whose Authorization header swaps the AWS4-HMAC-SHA256 token was answered with 501 NotImplemented before RustFS's access layer could rule on the unsigned x-amz-copy-source (GHSA-xm99-m3gq-83g8). Add the SigV4HeaderGuardLayer as the innermost external stack layer, running reject_unsigned_amz_headers_on_sigv4_request in front of s3s and serializing its rejections as the same AccessDenied S3 error document the access layer produces. --------- Co-authored-by: Hauser --- .config/e2e-full-selection.txt | 2 +- .config/e2e-smoke-selection.txt | 2 +- .config/security-smoke-floor.txt | 2 +- CHANGELOG.md | 4 + Cargo.lock | 49 ++-- Cargo.toml | 3 +- crates/e2e_test/src/negative_sigv4_test.rs | 112 ++++++++ .../e2e_test/src/presigned_negative_test.rs | 11 +- deny.toml | 4 +- docs/testing/security-regressions.md | 1 + rustfs/Cargo.toml | 1 + rustfs/src/admin/router.rs | 44 +++- rustfs/src/auth.rs | 241 +++++++++++++++++- rustfs/src/server/http.rs | 4 +- rustfs/src/server/layer.rs | 205 +++++++++++++++ rustfs/src/storage/access.rs | 6 +- 16 files changed, 651 insertions(+), 40 deletions(-) diff --git a/.config/e2e-full-selection.txt b/.config/e2e-full-selection.txt index eb32bf5c2..b349dc0a9 100644 --- a/.config/e2e-full-selection.txt +++ b/.config/e2e-full-selection.txt @@ -1,2 +1,2 @@ -sha256-darwin=d9049d700ef5d38ac55d15fa5bde12355593e500c1879bd832e3b32629d88cba +sha256-darwin=2c5fea45fa251e1d6d1f56798478246ff3ef0d03e06a24640a384a10b28c70fa sha256-linux=6dce56a0e4395bcf5fad4821b661cc9fee355da36b6459afedac3a3805bc4956 diff --git a/.config/e2e-smoke-selection.txt b/.config/e2e-smoke-selection.txt index aa3804f21..db2bbcb2c 100644 --- a/.config/e2e-smoke-selection.txt +++ b/.config/e2e-smoke-selection.txt @@ -1 +1 @@ -sha256=6d18f9cce820c51d5589de944e8cc185f73eeca0ea9a9916651943e3759169d0 +sha256=0ceb56c0041199ec9fde4e5d871a1f78e66031826312e9f6a767dcc532bb8021 diff --git a/.config/security-smoke-floor.txt b/.config/security-smoke-floor.txt index 2ee0d4190..91359daa1 100644 --- a/.config/security-smoke-floor.txt +++ b/.config/security-smoke-floor.txt @@ -9,4 +9,4 @@ # if the selected count drops below this number, so a rename or removal that # thins the security smoke gate must update this file in the same PR. # Adding tests does not require a bump, but bumping keeps the guard tight. -26 +28 diff --git a/CHANGELOG.md b/CHANGELOG.md index 209dac7ab..7a9b7f292 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security + +- **Header-signed SigV4 requests honour only signed headers** (GHSA-xm99-m3gq-83g8): a request authenticated with a SigV4 `Authorization` header that carries an `x-amz-*` request header not listed in its `SignedHeaders` is now rejected with `403 AccessDenied` ("There were headers present in the request which were not signed"), matching AWS S3 and the presigned rule from GHSA-g8w9-qw9q-fghr. Previously anyone holding one header-signed `PutObject` request could add an unsigned `x-amz-copy-source` and turn it into a `CopyObject` that ran with the signer's permissions, copying any object the signer could read into the target. An `Authorization` header whose algorithm token is not `AWS4-HMAC-SHA256` is now rejected instead of being verified as SigV4. The request-envelope headers `x-amz-content-sha256`, `x-amz-decoded-content-length`, `x-amz-trailer` and `x-amz-checksum-algorithm` (the same set the upstream `s3s` fix exempts) and `x-amz-cf-id` (CloudFront) remain tolerated unsigned; AWS SDKs and RustFS's own signers already sign every other `x-amz-*` header. SigV2, JWT and anonymous requests are unchanged. + ### Replication - Object Lock replication PUTs now carry a required integrity header, fixing target rejection introduced by the plain-payload default ([#7097](https://github.com/rustfs/rustfs/pull/7097)). This changes the default outbound request for locked objects but adds no persisted format. diff --git a/Cargo.lock b/Cargo.lock index 6fe1cbf72..0bea7560e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -271,7 +271,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -282,7 +282,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -1906,7 +1906,7 @@ dependencies = [ "maybe-owned", "rustix", "rustix-linux-procfs", - "windows-sys 0.60.2", + "windows-sys 0.61.2", "winx", ] @@ -3942,7 +3942,7 @@ dependencies = [ "libc", "option-ext", "redox_users", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -4287,7 +4287,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -6963,7 +6963,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -8712,7 +8712,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -9593,6 +9593,7 @@ dependencies = [ "rustls", "rustls-pki-types", "s3s", + "s3s-sigv4", "serde", "serde_json", "serde_urlencoded", @@ -11067,7 +11068,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -11150,7 +11151,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -11206,8 +11207,9 @@ checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" [[package]] name = "s3s" -version = "0.15.0" -source = "git+https://github.com/s3s-project/s3s.git?rev=f3e17541f366696bf0cbaf380fcbd8b44c17eba4#f3e17541f366696bf0cbaf380fcbd8b44c17eba4" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d29a4db6a51f1cd3269a36354ce4aa7db2be8816cee42a642b9c85cd3769577" dependencies = [ "arc-swap", "arrayvec", @@ -11264,8 +11266,9 @@ dependencies = [ [[package]] name = "s3s-rfc2047" -version = "0.16.0-alpha.1" -source = "git+https://github.com/s3s-project/s3s.git?rev=f3e17541f366696bf0cbaf380fcbd8b44c17eba4#f3e17541f366696bf0cbaf380fcbd8b44c17eba4" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9370a2f353a7929b79ae7b5f692b4758c59aa19526b18feff6a977e8ff17da5a" dependencies = [ "base64-simd", "thiserror 2.0.20", @@ -11273,8 +11276,9 @@ dependencies = [ [[package]] name = "s3s-sigv2" -version = "0.16.0-alpha.1" -source = "git+https://github.com/s3s-project/s3s.git?rev=f3e17541f366696bf0cbaf380fcbd8b44c17eba4#f3e17541f366696bf0cbaf380fcbd8b44c17eba4" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f277b63ffc9a2135c49d52726199b7436c2f86746d625496984f5bb5cabef050" dependencies = [ "base64-simd", "hmac 0.13.0", @@ -11286,8 +11290,9 @@ dependencies = [ [[package]] name = "s3s-sigv4" -version = "0.16.0-alpha.1" -source = "git+https://github.com/s3s-project/s3s.git?rev=f3e17541f366696bf0cbaf380fcbd8b44c17eba4#f3e17541f366696bf0cbaf380fcbd8b44c17eba4" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a979a401f6ae4035bc22e4ac0f7a9a5923ba93841c931e8e28652fe211d991bc" dependencies = [ "arrayvec", "base64-simd", @@ -11951,7 +11956,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c3d1e2c7f27f8d4cb10542a02c49005dbd6e93095799d6f3be745fae9f8fedd4" dependencies = [ "libc", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -12102,7 +12107,7 @@ dependencies = [ "cfg-if", "libc", "psm", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -12407,10 +12412,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.3.4", + "getrandom 0.4.3", "once_cell", "rustix", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -13512,7 +13517,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 3ff377396..ebee59671 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -313,7 +313,8 @@ rustify = { version = "0.7", default-features = false } rustix = { version = "1.1.4" } rust-embed = { version = "8.12.0" } rustc-hash = { version = "2.1.3" } -s3s = { git = "https://github.com/s3s-project/s3s.git", rev = "f3e17541f366696bf0cbaf380fcbd8b44c17eba4", version = "0.15.0", features = ["minio"] } +s3s = { version = "0.16.0", features = ["minio"] } +s3s-sigv4 = { version = "0.16.0" } serial_test = "4.0.1" shadow-rs = { default-features = false, version = "2.0.0" } siphasher = "1.0.3" diff --git a/crates/e2e_test/src/negative_sigv4_test.rs b/crates/e2e_test/src/negative_sigv4_test.rs index b88587e27..d0b52a4e9 100644 --- a/crates/e2e_test/src/negative_sigv4_test.rs +++ b/crates/e2e_test/src/negative_sigv4_test.rs @@ -261,6 +261,118 @@ async fn valid_header_sigv4_request_succeeds() -> Result<(), Box Result<(String, String), Box> { + env.create_test_bucket(XM99_SOURCE_BUCKET).await?; + env.create_s3_client() + .put_object() + .bucket(XM99_SOURCE_BUCKET) + .key("secret") + .body(ByteStream::from_static(XM99_SOURCE_BODY)) + .send() + .await?; + + let path = format!("/{BUCKET}/xm99-target"); + let signed = signer.sign("PUT", &path, "", UNSIGNED_PAYLOAD); + let resp = send_signed(env, reqwest::Method::PUT, &path, &signed, Some(XM99_TARGET_BODY.to_vec())).await?; + assert_eq!(resp.status().as_u16(), 200, "plain header-signed upload must succeed"); + Ok((path, format!("/{XM99_SOURCE_BUCKET}/secret"))) +} + +async fn xm99_target_body(env: &RustFSTestEnvironment) -> Result, Box> { + let object = env + .create_s3_client() + .get_object() + .bucket(BUCKET) + .key("xm99-target") + .send() + .await?; + Ok(object.body.collect().await?.into_bytes().to_vec()) +} + +/// GHSA-xm99-m3gq-83g8: replaying a header-signed PutObject with an unsigned +/// `x-amz-copy-source` must not become a CopyObject that reads another bucket +/// with the signer's permissions, and swapping the algorithm token must not +/// route the request around the check. +#[tokio::test] +async fn ghsa_xm99_header_sigv4_rejects_unsigned_copy_source() -> Result<(), Box> { + init_logging(); + let mut env = RustFSTestEnvironment::new().await?; + setup(&mut env).await?; + let signer = SigV4::new(&env); + let (path, copy_source) = xm99_seed_target(&env, &signer).await?; + + // The PutObject signature covers host, payload hash and date only. + let signed = signer.sign("PUT", &path, "", UNSIGNED_PAYLOAD); + let variants = [ + ( + signed.authorization.clone(), + "There were headers present in the request which were not signed", + ), + ( + signed.authorization.replacen(SIGN_V4_ALGORITHM, "OTHER", 1), + "Unsupported SigV4 authorization algorithm", + ), + ]; + for (authorization, message) in variants { + let resp = local_http_client() + .put(format!("{}{path}", env.url)) + .header("authorization", &authorization) + .header("x-amz-date", &signed.amz_date) + .header("x-amz-content-sha256", &signed.content_sha256) + .header("x-amz-copy-source", ©_source) + .send() + .await?; + let status = resp.status().as_u16(); + let body = resp.text().await?; + assert_eq!(status, 403, "unsigned x-amz-copy-source must be denied, got body:\n{body}"); + assert_error_code(&body, "AccessDenied"); + assert!(body.contains(message), "expected {message:?} in response body, got:\n{body}"); + } + + assert_eq!( + xm99_target_body(&env).await?, + XM99_TARGET_BODY, + "a rejected copy must leave the destination object unchanged" + ); + Ok(()) +} + +/// Positive control for GHSA-xm99-m3gq-83g8: the same CopyObject succeeds when +/// the credential holder signs `x-amz-copy-source`. +#[tokio::test] +async fn ghsa_xm99_header_sigv4_accepts_signed_copy_source() -> Result<(), Box> { + init_logging(); + let mut env = RustFSTestEnvironment::new().await?; + setup(&mut env).await?; + let signer = SigV4::new(&env); + let (path, copy_source) = xm99_seed_target(&env, &signer).await?; + + let signed = signer.sign_with_extra_headers("PUT", &path, "", UNSIGNED_PAYLOAD, &[("x-amz-copy-source", ©_source)]); + let resp = local_http_client() + .put(format!("{}{path}", env.url)) + .header("authorization", &signed.authorization) + .header("x-amz-date", &signed.amz_date) + .header("x-amz-content-sha256", &signed.content_sha256) + .header("x-amz-copy-source", ©_source) + .send() + .await?; + let status = resp.status().as_u16(); + let body = resp.text().await?; + assert_eq!(status, 200, "signed copy must succeed, got body:\n{body}"); + assert!(body.contains("CopyObjectResult"), "expected CopyObjectResult, got:\n{body}"); + assert_eq!(xm99_target_body(&env).await?, XM99_SOURCE_BODY, "signed copy must replace the target"); + Ok(()) +} + /// (a) Tampering the `Signature=` component must be rejected with /// SignatureDoesNotMatch / 403. #[tokio::test] diff --git a/crates/e2e_test/src/presigned_negative_test.rs b/crates/e2e_test/src/presigned_negative_test.rs index ec8928f72..3ab5e7a9b 100644 --- a/crates/e2e_test/src/presigned_negative_test.rs +++ b/crates/e2e_test/src/presigned_negative_test.rs @@ -527,7 +527,16 @@ async fn ghsa_g8w9_presigned_put_rejects_unsigned_copy_source() -> Result<(), Bo .presigned(valid_config()) .await?; - let copy_source = format!("/{BUCKET}/{CANONICAL_KEY}"); + let source_bucket = "presigned-copy-source"; + env.create_test_bucket(source_bucket).await?; + env.create_s3_client() + .put_object() + .bucket(source_bucket) + .key(CANONICAL_KEY) + .body(ByteStream::from_static(CANONICAL_BODY)) + .send() + .await?; + let copy_source = format!("/{source_bucket}/{CANONICAL_KEY}"); let unsigned: Vec<(&str, &str)> = vec![("x-amz-copy-source", copy_source.as_str())]; let headers = pr.headers().chain(unsigned.iter().copied()); let resp = send_raw(pr.method(), pr.uri(), headers, None).await?; diff --git a/deny.toml b/deny.toml index cfecab82f..952a5d77e 100644 --- a/deny.toml +++ b/deny.toml @@ -44,10 +44,10 @@ allow-git = [ # Official s3s repository. Temporarily pinned to the merged generic REST # SigV4 payload-checksum fix until it is available in a crates.io release. # owner: marshawcoco review: 2026-10 - "https://github.com/s3s-project/s3s.git", + # "https://github.com/s3s-project/s3s.git", # RustFS fork carrying presigned expiry and constant-time authentication fixes. # owner: rustfs-maintainers review: 2026-10 - "https://github.com/rustfs/s3s.git", + # "https://github.com/rustfs/s3s.git", ] [bans] diff --git a/docs/testing/security-regressions.md b/docs/testing/security-regressions.md index 2852361aa..7b3a2588b 100644 --- a/docs/testing/security-regressions.md +++ b/docs/testing/security-regressions.md @@ -18,6 +18,7 @@ Every fixed RustFS GitHub Security Advisory maps to at least one named regressio | [GHSA-6r96-hmgc-726c](https://github.com/rustfs/rustfs/security/advisories/GHSA-6r96-hmgc-726c) | Request headers must not populate server-derived IAM condition keys (`userid`, `groups`, `jwt:`/`ldap:` claims) | fixed, GHSA private-fork merge | `ghsa_6r96_identity_condition_keys_ignore_spoofed_headers`, `ghsa_6r96_claim_condition_keys_ignore_spoofed_headers`, and `test_request_headers_still_reach_conditions`, which keeps the reserved set from growing too broad (`rustfs/src/auth.rs`) | unit | | [GHSA-x298-9x87-fvjq](https://github.com/rustfs/rustfs/security/advisories/GHSA-x298-9x87-fvjq) | Anonymous ListObjectVersions -> `s3:ListBucket` fallback must reach the same public-access gates as a direct grant | fixed, GHSA private-fork merge | `ghsa_x298_anonymous_list_object_versions_denied_when_restrict_public_buckets_enabled` (`crates/e2e_test/src/anonymous_access_test.rs`); asserts 200 before the public-access block is applied so it proves the gate, not a broken fallback | e2e (`e2e-smoke`) | | [GHSA-g8w9-qw9q-fghr](https://github.com/rustfs/rustfs/security/advisories/GHSA-g8w9-qw9q-fghr) | A SigV4 presigned request must reject `x-amz-*` headers missing from `X-Amz-SignedHeaders` (tags, storage class, ACL, metadata, redirect, Object Lock, SSE) instead of applying them | this fix | `ghsa_g8w9_presigned_request_rejects_unsigned_x_amz_headers`, `ghsa_g8w9_presigned_request_accepts_signed_or_exempt_x_amz_headers`, `ghsa_g8w9_check_ignores_header_signed_sigv2_and_anonymous_requests` (`rustfs/src/auth.rs`); `ghsa_g8w9_check_access_rejects_unsigned_amz_header_on_presigned_custom_route` for routes that bypass `S3Access::check` (`rustfs/src/admin/router.rs`); `ghsa_g8w9_presigned_put_rejects_unsigned_x_amz_headers`, `ghsa_g8w9_presigned_get_rejects_unsigned_x_amz_headers`, `ghsa_g8w9_presigned_put_rejects_unsigned_copy_source`, plus the signed-tagging control `ghsa_g8w9_presigned_put_accepts_signed_x_amz_headers` and the unsigned-`Content-Type` boundary control `ghsa_g8w9_presigned_put_still_accepts_unsigned_non_amz_headers` (`crates/e2e_test/src/presigned_negative_test.rs`) | unit; e2e (`e2e-smoke`) | +| GHSA-xm99-m3gq-83g8 | A header-signed SigV4 request must reject `x-amz-*` headers missing from its `SignedHeaders` (an unsigned `x-amz-copy-source` turned a replayed PutObject into a cross-bucket CopyObject), and a non-`AWS4-HMAC-SHA256` algorithm token must not route around that check | this fix | `ghsa_xm99_header_sigv4_rejects_unsigned_x_amz_headers`, `ghsa_xm99_header_sigv4_accepts_signed_or_exempt_x_amz_headers`, `ghsa_xm99_header_sigv4_rejects_unsupported_algorithm_token`, `ghsa_xm99_header_and_query_signatures_cannot_widen_each_other`, `ghsa_xm99_check_ignores_sigv2_jwt_and_anonymous_requests` (`rustfs/src/auth.rs`); `ghsa_xm99_check_access_rejects_unsigned_amz_header_on_header_signed_custom_route` for routes that bypass `S3Access::check` (`rustfs/src/admin/router.rs`); `ghsa_xm99_header_sigv4_rejects_unsigned_copy_source`, which also asserts the destination bytes survive, plus the signed-copy control `ghsa_xm99_header_sigv4_accepts_signed_copy_source` (`crates/e2e_test/src/negative_sigv4_test.rs`) | unit; e2e (`e2e-smoke`) | | [GHSA-g3vq-vv42-f647](https://github.com/rustfs/rustfs/security/advisories/GHSA-g3vq-vv42-f647) | FTPS `MKD` must clear the `s3:CreateBucket` authorization boundary before reaching the backend | fixed, GHSA private-fork merge | `ghsa_g3vq_mkd_denied_before_reaching_backend` (`crates/protocols/src/ftps/driver.rs`); primes `create_bucket` to succeed so the assertion distinguishes "denied at authorization" from "backend refused" | unit (`ftps` feature) | ## Where these run diff --git a/rustfs/Cargo.toml b/rustfs/Cargo.toml index abfee7bde..b8debf1dd 100644 --- a/rustfs/Cargo.toml +++ b/rustfs/Cargo.toml @@ -349,6 +349,7 @@ pin-project-lite.workspace = true parking_lot = { workspace = true } rust-embed = { workspace = true, features = ["interpolate-folder-path"] } s3s = { workspace = true, features = ["minio"] } +s3s-sigv4 = { workspace = true } shadow-rs = { workspace = true, default-features = false, features = ["build", "metadata"] } sysinfo = { workspace = true, features = ["multithread"] } thiserror = { workspace = true } diff --git a/rustfs/src/admin/router.rs b/rustfs/src/admin/router.rs index 0c34ff810..328395f17 100644 --- a/rustfs/src/admin/router.rs +++ b/rustfs/src/admin/router.rs @@ -34,7 +34,7 @@ use crate::admin::runtime_sources::{ }; use crate::admin::storage_api::access::{ReqInfo, authorize_request, spawn_traced}; use crate::admin::storage_api::contract::bucket::{BucketOperations, BucketOptions}; -use crate::auth::{check_key_valid, constant_time_eq, get_session_token, reject_unsigned_amz_headers_on_presigned_request}; +use crate::auth::{check_key_valid, constant_time_eq, get_session_token, reject_unsigned_amz_headers_on_sigv4_request}; use crate::error::ApiError; use crate::license::license_check; use crate::server::{ @@ -3270,9 +3270,8 @@ where // check_access before call async fn check_access(&self, req: &mut S3Request) -> S3Result<()> { // GHSA-g8w9-qw9q-fghr: custom routes bypass `S3Access::check`, so the - // presigned signed-header rule is enforced here as well. A request - // without a presigned signature passes through untouched. - reject_unsigned_amz_headers_on_presigned_request(&req.headers, req.uri.query())?; + // SigV4 signed-header rule is enforced here as well. + reject_unsigned_amz_headers_on_sigv4_request(&req.headers, req.uri.query())?; if let Some(server_ctx) = &self.server_ctx { req.extensions.insert(server_ctx.clone()); @@ -5648,6 +5647,43 @@ mod tests { assert_eq!(err.message(), Some(crate::auth::UNSIGNED_HEADERS_MESSAGE)); } + /// GHSA-xm99-m3gq-83g8: custom routes must apply the header-signed SigV4 + /// signed-header rule too, since they never reach `S3Access::check`. + #[tokio::test] + async fn ghsa_xm99_check_access_rejects_unsigned_amz_header_on_header_signed_custom_route() { + let router: S3Router = S3Router::new(false); + let mut headers = HeaderMap::new(); + let authorization = format!( + "AWS4-HMAC-SHA256 Credential=test/20260827/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature={}", + "0".repeat(64) + ); + headers.insert("authorization", authorization.parse().expect("authorization")); + headers.insert("x-amz-date", HeaderValue::from_static("20260827T000000Z")); + headers.insert("x-amz-content-sha256", HeaderValue::from_static("UNSIGNED-PAYLOAD")); + headers.insert("x-amz-tagging", HeaderValue::from_static("owner=attacker")); + let mut req = S3Request { + input: Body::from(String::new()), + method: Method::GET, + uri: "/demo-bucket?replication-metrics".parse().expect("uri should parse"), + headers, + extensions: http::Extensions::new(), + credentials: Some(s3s::auth::Credentials { + access_key: "test".into(), + secret_key: s3s::auth::SecretKey::from("secret".to_string()), + }), + region: None, + service: None, + trailing_headers: None, + }; + + let err = router + .check_access(&mut req) + .await + .expect_err("header-signed custom-route request with an unsigned x-amz header must be denied"); + assert_eq!(err.code(), &S3ErrorCode::AccessDenied); + assert_eq!(err.message(), Some(crate::auth::UNSIGNED_HEADERS_MESSAGE)); + } + // backlog#1052 S2: the router hands its server's context slot to every // dispatched request via extensions, so the static admin operations can // resolve their server's store instead of the process default. diff --git a/rustfs/src/auth.rs b/rustfs/src/auth.rs index dbc9689aa..4f1fbc36a 100644 --- a/rustfs/src/auth.rs +++ b/rustfs/src/auth.rs @@ -51,6 +51,8 @@ const EVENT_KEYSTONE_CREDENTIALS_VALIDATED: &str = "keystone_credentials_validat const EVENT_KEYSTONE_CONTEXT_MISSING: &str = "keystone_context_missing"; const EVENT_SESSION_TOKEN_EXTRACTION: &str = "session_token_extraction"; const EVENT_PRESIGNED_UNSIGNED_AMZ_HEADER: &str = "presigned_unsigned_amz_header"; +const EVENT_SIGV4_UNSIGNED_AMZ_HEADER: &str = "sigv4_unsigned_amz_header"; +const EVENT_SIGV4_UNSUPPORTED_ALGORITHM: &str = "sigv4_unsupported_algorithm"; /// RustFS-specific query capability for a single presigned PutObject request. pub(crate) const RUSTFS_MAX_CONTENT_LENGTH_QUERY: &str = "x-rustfs-max-content-length"; @@ -1032,6 +1034,99 @@ pub fn get_query_param<'a>(query: &'a str, param_name: &str) -> Option<&'a str> None } +pub(crate) const UNSUPPORTED_SIGV4_ALGORITHM_MESSAGE: &str = "Unsupported SigV4 authorization algorithm"; + +/// Request-envelope `x-amz-*` headers a header-signed SigV4 request may carry +/// without listing them in `SignedHeaders`, matching the upstream verifier. +/// +/// `x-amz-content-sha256` is bound through the canonical request's payload +/// hash. The aws-chunked framing headers are added around an already signed +/// request and are only read after verification to decode the body; none of +/// them selects a different operation. +const SIGV4_UNSIGNED_ENVELOPE_HEADERS: &[&str] = &[ + "x-amz-content-sha256", + "x-amz-decoded-content-length", + "x-amz-trailer", + "x-amz-checksum-algorithm", +]; + +/// GHSA-xm99-m3gq-83g8: reject `x-amz-*` request headers that the request's +/// SigV4 signature does not cover, for both SigV4 authentication forms. +/// +/// The upstream verifier only proves that the headers named in `SignedHeaders` +/// match; every other `x-amz-*` header still reaches the handlers. Anyone who +/// captures one header-signed `PutObject` could replay it with an unsigned +/// `x-amz-copy-source` and turn it into a `CopyObject` that runs with the +/// signer's permissions, reading any object the signer can read. AWS S3 rejects +/// unsigned `x-amz-*` headers on header-signed requests as well as presigned +/// ones. +/// +/// Every signed-header list the request carries must cover every `x-amz-*` +/// header, so an `Authorization` header can never widen a presigned URL and a +/// query can never widen a header signature. The header is parsed with the +/// verifier's own parser so both sides read the same `SignedHeaders` list, and +/// the algorithm token is pinned because the verifier accepts any token there. +/// A header the parser rejects never authenticates as SigV4 upstream, but one +/// that claims the SigV4 algorithm still fails closed here. SigV2 signs every +/// `x-amz-*` header itself; JWT and anonymous requests carry no SigV4 list. +pub(crate) fn reject_unsigned_amz_headers_on_sigv4_request(header: &HeaderMap, query: Option<&str>) -> S3Result<()> { + reject_unsigned_amz_headers_on_presigned_request(header, query)?; + + for value in header.get_all(http::header::AUTHORIZATION) { + let Ok(value) = value.to_str() else { + continue; + }; + let authorization = match s3s_sigv4::AuthorizationV4::parse(value) { + Ok(authorization) => authorization, + Err(_) if value.starts_with(SIGN_V4_ALGORITHM) => { + return Err(S3Error::with_message( + S3ErrorCode::AccessDenied, + "Invalid SigV4 authorization header".to_owned(), + )); + } + Err(_) => continue, + }; + if authorization.algorithm != SIGN_V4_ALGORITHM { + warn!( + event = EVENT_SIGV4_UNSUPPORTED_ALGORITHM, + component = LOG_COMPONENT_AUTH, + subsystem = LOG_SUBSYSTEM_REQUEST, + reason = "unsupported_algorithm", + "SigV4 request rejected" + ); + return Err(S3Error::with_message( + S3ErrorCode::AccessDenied, + UNSUPPORTED_SIGV4_ALGORITHM_MESSAGE.to_owned(), + )); + } + for name in header.keys() { + // `HeaderName` is already lowercase; the verifier looks signed + // names up case-insensitively, so compare them the same way. + let name = name.as_str(); + if !name.starts_with("x-amz-") + || SIGV4_UNSIGNED_ENVELOPE_HEADERS.contains(&name) + || PRESIGNED_UNSIGNED_AMZ_HEADER_ALLOWLIST.contains(&name) + || authorization + .signed_headers + .iter() + .any(|signed_name| signed_name.eq_ignore_ascii_case(name)) + { + continue; + } + warn!( + event = EVENT_SIGV4_UNSIGNED_AMZ_HEADER, + component = LOG_COMPONENT_AUTH, + subsystem = LOG_SUBSYSTEM_REQUEST, + reason = "unsigned_amz_header", + header = name, + "SigV4 request rejected" + ); + return Err(S3Error::with_message(S3ErrorCode::AccessDenied, UNSIGNED_HEADERS_MESSAGE.to_string())); + } + } + Ok(()) +} + /// `x-amz-*` request headers a SigV4 presigned request may carry without /// listing them in `X-Amz-SignedHeaders`. /// @@ -1057,10 +1152,9 @@ pub(crate) const UNSIGNED_HEADERS_MESSAGE: &str = "There were headers present in /// check mirrors that at the access boundary, before any handler reads a /// header. /// -/// Only query-string SigV4 requests are checked. SigV2 canonicalises every +/// This helper checks query-string SigV4 requests. SigV2 canonicalises every /// `x-amz-*` header into the string to sign, so adding one there already breaks -/// the signature, and a header-signed SigV4 request is sent by the credential -/// holder itself, so an unsigned header there is not a delegation bypass. +/// the signature. The outer guard checks header-signed SigV4 requests. /// /// Detection keys on the query, not on the derived [`AuthType`], because the /// upstream verifier dispatches to the presigned path whenever the query @@ -2131,6 +2225,147 @@ mod tests { reject_unsigned_amz_headers_on_presigned_request(&headers, Some(sigv2_query)).unwrap(); } + fn header_sigv4_authorization(algorithm: &str, signed_headers: &str) -> HeaderValue { + format!( + "{algorithm} Credential=test/20260827/us-east-1/s3/aws4_request, SignedHeaders={signed_headers}, Signature={}", + "0".repeat(64) + ) + .parse() + .expect("authorization header") + } + + fn header_sigv4_headers(signed_headers: &str) -> HeaderMap { + let mut headers = HeaderMap::new(); + headers.insert("authorization", header_sigv4_authorization(SIGN_V4_ALGORITHM, signed_headers)); + headers.insert("x-amz-date", HeaderValue::from_static("20260827T000000Z")); + headers.insert("x-amz-content-sha256", HeaderValue::from_static("UNSIGNED-PAYLOAD")); + headers + } + + fn assert_unsigned_headers_denied(result: S3Result<()>) { + let error = result.expect_err("unsigned x-amz header must be denied"); + assert_eq!(error.code(), &S3ErrorCode::AccessDenied); + assert_eq!(error.message(), Some(UNSIGNED_HEADERS_MESSAGE)); + } + + /// GHSA-xm99-m3gq-83g8: a header-signed SigV4 request must not carry an + /// `x-amz-*` header its `SignedHeaders` list leaves out. An unsigned + /// `x-amz-copy-source` turned a replayed PutObject into a CopyObject. + #[test] + fn ghsa_xm99_header_sigv4_rejects_unsigned_x_amz_headers() { + for name in [ + "x-amz-copy-source", + "x-amz-copy-source-range", + "x-amz-tagging", + "x-amz-meta-owner", + "x-amz-metadata-directive", + "x-amz-security-token", + "x-amz-server-side-encryption", + ] { + let mut headers = header_sigv4_headers("host;x-amz-content-sha256;x-amz-date"); + headers.insert(name, HeaderValue::from_static("injected")); + assert_unsigned_headers_denied(reject_unsigned_amz_headers_on_sigv4_request(&headers, None)); + assert_unsigned_headers_denied(reject_unsigned_amz_headers_on_sigv4_request(&headers, Some("tagging"))); + } + + // `x-amz-date` is read by the verifier, but it is not exempt here. + let headers = header_sigv4_headers("host;x-amz-content-sha256"); + assert_unsigned_headers_denied(reject_unsigned_amz_headers_on_sigv4_request(&headers, None)); + } + + #[test] + fn ghsa_xm99_header_sigv4_accepts_signed_or_exempt_x_amz_headers() { + // The payload hash is bound through the canonical request's payload + // field, the aws-chunked framing headers wrap an already signed + // request, and CloudFront stamps `x-amz-cf-id` after the client signs. + let mut headers = header_sigv4_headers("host;x-amz-date"); + headers.insert("x-amz-cf-id", HeaderValue::from_static("cdn-request")); + headers.insert("x-amz-decoded-content-length", HeaderValue::from_static("1024")); + headers.insert("x-amz-trailer", HeaderValue::from_static("x-amz-checksum-crc32")); + headers.insert("x-amz-checksum-algorithm", HeaderValue::from_static("CRC32")); + reject_unsigned_amz_headers_on_sigv4_request(&headers, None).expect("envelope headers and CDN id are exempt"); + + // The exemption is by exact name, not by prefix. + headers.insert("x-amz-sdk-checksum-algorithm", HeaderValue::from_static("CRC32")); + assert_unsigned_headers_denied(reject_unsigned_amz_headers_on_sigv4_request(&headers, None)); + + let mut headers = header_sigv4_headers("host;x-amz-content-sha256;x-amz-copy-source;x-amz-date"); + headers.insert("x-amz-copy-source", HeaderValue::from_static("/source/secret")); + headers.insert("content-type", HeaderValue::from_static("text/plain")); + reject_unsigned_amz_headers_on_sigv4_request(&headers, None).expect("signed copy source is allowed"); + + // The verifier looks signed names up case-insensitively. + let headers = header_sigv4_headers("Host;X-Amz-Content-Sha256;X-Amz-Date"); + reject_unsigned_amz_headers_on_sigv4_request(&headers, None).expect("mixed-case signed names are allowed"); + } + + /// The verifier accepts any algorithm token in the Authorization header, so + /// swapping it must not move the request out of this check. + #[test] + fn ghsa_xm99_header_sigv4_rejects_unsupported_algorithm_token() { + for algorithm in ["OTHER", "aws4-hmac-sha256", "AWS4-ECDSA-P256-SHA256"] { + let mut headers = header_sigv4_headers("host;x-amz-content-sha256;x-amz-date"); + headers.insert( + "authorization", + header_sigv4_authorization(algorithm, "host;x-amz-content-sha256;x-amz-date"), + ); + let error = reject_unsigned_amz_headers_on_sigv4_request(&headers, None) + .expect_err("non-SigV4 algorithm token must be denied"); + assert_eq!(error.code(), &S3ErrorCode::AccessDenied); + assert_eq!(error.message(), Some(UNSUPPORTED_SIGV4_ALGORITHM_MESSAGE)); + + headers.insert("x-amz-copy-source", HeaderValue::from_static("/source/secret")); + reject_unsigned_amz_headers_on_sigv4_request(&headers, None) + .expect_err("algorithm token cannot smuggle an unsigned copy source"); + } + + let mut headers = header_sigv4_headers("host"); + headers.insert("authorization", HeaderValue::from_static("AWS4-HMAC-SHA256 invalid")); + let error = + reject_unsigned_amz_headers_on_sigv4_request(&headers, None).expect_err("malformed SigV4 header must fail closed"); + assert_eq!(error.code(), &S3ErrorCode::AccessDenied); + } + + /// Every SigV4 signed-header list present must cover every `x-amz-*` + /// header: neither auth form can widen the other. + #[test] + fn ghsa_xm99_header_and_query_signatures_cannot_widen_each_other() { + let mut headers = header_sigv4_headers("host;x-amz-content-sha256;x-amz-copy-source;x-amz-date"); + headers.insert("x-amz-copy-source", HeaderValue::from_static("/source/secret")); + let presigned = "X-Amz-Signature=test&X-Amz-SignedHeaders=host"; + assert_unsigned_headers_denied(reject_unsigned_amz_headers_on_sigv4_request(&headers, Some(presigned))); + + let headers_signing_less = { + let mut headers = header_sigv4_headers("host;x-amz-content-sha256;x-amz-date"); + headers.insert("x-amz-copy-source", HeaderValue::from_static("/source/secret")); + headers + }; + let presigned_copy = "X-Amz-Signature=test&X-Amz-SignedHeaders=host%3Bx-amz-copy-source%3Bx-amz-date"; + assert_unsigned_headers_denied(reject_unsigned_amz_headers_on_sigv4_request(&headers_signing_less, Some(presigned_copy))); + + // A duplicate Authorization header is checked entry by entry. + let mut headers = header_sigv4_headers("host;x-amz-content-sha256;x-amz-copy-source;x-amz-date"); + headers.insert("x-amz-copy-source", HeaderValue::from_static("/source/secret")); + headers.append( + "authorization", + header_sigv4_authorization(SIGN_V4_ALGORITHM, "host;x-amz-content-sha256;x-amz-date"), + ); + assert_unsigned_headers_denied(reject_unsigned_amz_headers_on_sigv4_request(&headers, None)); + } + + #[test] + fn ghsa_xm99_check_ignores_sigv2_jwt_and_anonymous_requests() { + let mut headers = HeaderMap::new(); + headers.insert("x-amz-copy-source", HeaderValue::from_static("/source/secret")); + reject_unsigned_amz_headers_on_sigv4_request(&headers, None).expect("anonymous auth is handled downstream"); + + for authorization in ["AWS key:signature", "Bearer token"] { + headers.insert("authorization", HeaderValue::from_static(authorization)); + reject_unsigned_amz_headers_on_sigv4_request(&headers, None) + .expect("non-SigV4 authorization carries no signed-header list"); + } + } + #[test] fn presigned_put_max_content_length_rejects_unsigned_or_invalid_values() { let headers = HeaderMap::new(); diff --git a/rustfs/src/server/http.rs b/rustfs/src/server/http.rs index 6caf84e2b..8e1addadd 100644 --- a/rustfs/src/server/http.rs +++ b/rustfs/src/server/http.rs @@ -26,7 +26,7 @@ use crate::server::{ BodylessStatusFixLayer, ConditionalCorsLayer, DoubleSlashListBucketsCompatLayer, EmptyBodyContentLengthCompatLayer, ExternalRequestContextLayer, HeadRequestBodyFixLayer, IcebergRestErrorCompatLayer, ObjectAttributesEtagFixLayer, PublicHealthEndpointLayer, RedirectLayer, RequestContextLayer, RequestLoggingLayer, S3ErrorMessageCompatLayer, - StsQueryApiCompatLayer, VirtualHostStyleHintLayer, redact_sensitive_uri_query, + SigV4HeaderGuardLayer, StsQueryApiCompatLayer, VirtualHostStyleHintLayer, redact_sensitive_uri_query, }, rate_limit::{RateLimitLayer, api_rate_limit_layer_from_env}, ssec_transport::SsecTransportLayer, @@ -1942,6 +1942,7 @@ fn process_connection( // 22. PublicHealthEndpointLayer — handles public health before s3s host parsing // 23. VirtualHostStyleHintLayer — actionable error for unroutable virtual-hosted-style (conditional) // 24. DoubleSlashListBucketsCompatLayer — rewrites `GET //` to `GET /` for ListBuckets (MinIO browser compat) + // 25. SigV4HeaderGuardLayer — GHSA-xm99/-g8w9 unsigned x-amz-* rules, ahead of s3s signature dispatch // The internode lane below intentionally keeps only the shared // transport/auth/observability subset needed by `/rustfs/rpc/...`. // ───────────────────────────────────────────────────────────── @@ -2061,6 +2062,7 @@ fn process_connection( )) .option_layer((!server_domains_configured && !is_console).then_some(VirtualHostStyleHintLayer)) .layer(DoubleSlashListBucketsCompatLayer) + .layer(SigV4HeaderGuardLayer) .service(service) }; let build_internode_stack = |service| { diff --git a/rustfs/src/server/layer.rs b/rustfs/src/server/layer.rs index 2b4819f87..f0e9f484e 100644 --- a/rustfs/src/server/layer.rs +++ b/rustfs/src/server/layer.rs @@ -48,6 +48,7 @@ use rustfs_protocols::swift::SwiftRouter; use rustfs_trusted_proxies::ClientInfo; use rustfs_utils::get_env_opt_str; use rustfs_utils::http::headers::{AMZ_REQUEST_ID, REQUEST_ID_HEADER}; +use s3s::S3Error; use s3s::S3ErrorCode; use serde::{Deserialize, Serialize}; use std::borrow::Cow; @@ -1606,6 +1607,94 @@ where .expect("failed to build virtual-host hint response") } +/// GHSA-xm99-m3gq-83g8 / GHSA-g8w9-qw9q-fghr: enforce the SigV4 unsigned +/// `x-amz-*` header rules ahead of s3s dispatch. +/// +/// s3s verifies the claimed algorithm as the first step of its own signature +/// flow and answers a swapped algorithm token with `501 NotImplemented` before +/// RustFS's access layer (`S3Access::check`) ever runs, so the `AccessDenied` +/// rulings of [`crate::auth::reject_unsigned_amz_headers_on_sigv4_request`] +/// must be applied here, in front of s3s. Rejections carry the same S3 error +/// document the access layer would have produced. +#[derive(Clone, Default)] +pub struct SigV4HeaderGuardLayer; + +impl Layer for SigV4HeaderGuardLayer { + type Service = SigV4HeaderGuardService; + + fn layer(&self, inner: S) -> Self::Service { + SigV4HeaderGuardService { inner } + } +} + +#[derive(Clone)] +pub struct SigV4HeaderGuardService { + inner: S, +} + +impl Service> for SigV4HeaderGuardService +where + S: Service, Response = Response>> + Clone + Send + 'static, + S::Future: Send + 'static, + ReqBody: Send + 'static, + RestBody: From + Send + 'static, + GrpcBody: Send + 'static, +{ + type Response = Response>; + type Error = S::Error; + type Future = Pin> + Send>>; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx) + } + + fn call(&mut self, req: HttpRequest) -> Self::Future { + match crate::auth::reject_unsigned_amz_headers_on_sigv4_request(req.headers(), req.uri().query()) { + Ok(()) => {} + Err(error) => { + let version = req.version(); + return Box::pin(async move { Ok(sigv4_header_guard_rejection(version, error)) }); + } + } + let mut inner = self.inner.clone(); + Box::pin(async move { inner.call(req).await }) + } +} + +/// Serialize a header-guard rejection as the S3 error document the access +/// layer would have produced for the same rule violation. +fn sigv4_header_guard_rejection( + version: http::Version, + error: S3Error, +) -> Response> +where + RestBody: From, +{ + let status = error.status_code().unwrap_or(StatusCode::FORBIDDEN); + let message = error.message().unwrap_or_default().to_owned(); + let body = format!( + "\ + {code}{message}", + code = xml_escape(error.code().as_str()), + message = xml_escape(&message), + ); + + let mut builder = Response::builder() + .status(status) + .header(http::header::CONTENT_TYPE, "application/xml"); + // This short-circuit path does not drain the request body. For HTTP/1.x, signal + // connection close so an undrained body cannot disrupt keep-alive reuse. `Connection` + // is a forbidden header in HTTP/2+, so it is only set for HTTP/1.x. + if !matches!(version, http::Version::HTTP_2 | http::Version::HTTP_3) { + builder = builder.header(http::header::CONNECTION, "close"); + } + builder + .body(HybridBody::Rest { + rest_body: RestBody::from(Bytes::from(body)), + }) + .expect("failed to build SigV4 header guard rejection response") +} + /// Returns an actionable error for virtual-hosted-style S3 requests that cannot be /// routed because `RUSTFS_SERVER_DOMAINS` is not configured. See /// [`unroutable_virtual_host_target`]. The layer is only installed when no server @@ -3142,6 +3231,122 @@ mod tests { assert!(h2_response.headers().get(http::header::CONNECTION).is_none()); } + #[tokio::test] + async fn sigv4_header_guard_layer_rejects_swapped_algorithm_token_with_access_denied() { + let inner = CountingHybridService::default(); + let calls = inner.calls(); + let mut service = SigV4HeaderGuardLayer.layer(inner); + + let response = service + .call( + Request::builder() + .method(Method::PUT) + .uri("/xm99-private-source/target") + .header( + "authorization", + "OTHER Credential=rustfsadmin/20260914/us-east-1/s3/aws4_request, \ + SignedHeaders=host;x-amz-content-sha256;x-amz-date, \ + Signature=00e997a1db4d3b6ee6c26d3f7d3f3fb3b6ee6c26d3f7d3f3fb3b6ee6c26d3f7d", + ) + .header("x-amz-date", "20260914T000000Z") + .header("x-amz-content-sha256", "UNSIGNED-PAYLOAD") + .header("x-amz-copy-source", "/negative-sigv4-bucket/source") + .body(Full::::from(Bytes::new())) + .expect("request"), + ) + .await + .expect("guard response"); + + // The swapped token must be answered with the access-layer ruling + // instead of s3s's algorithm 501. + assert_eq!(response.status(), StatusCode::FORBIDDEN); + assert_eq!(calls.load(Ordering::SeqCst), 0); + let body = BodyExt::collect(response.into_body()).await.expect("body").to_bytes(); + let body = String::from_utf8(body.to_vec()).expect("utf8 body"); + assert!(body.contains("AccessDenied"), "body: {body}"); + assert!(body.contains("Unsupported SigV4 authorization algorithm"), "body: {body}"); + } + + #[tokio::test] + async fn sigv4_header_guard_layer_rejects_unsigned_copy_source_header() { + let inner = CountingHybridService::default(); + let calls = inner.calls(); + let mut service = SigV4HeaderGuardLayer.layer(inner); + + let response = service + .call( + Request::builder() + .method(Method::PUT) + .uri("/xm99-private-source/target") + .header( + "authorization", + "AWS4-HMAC-SHA256 Credential=rustfsadmin/20260914/us-east-1/s3/aws4_request, \ + SignedHeaders=host;x-amz-content-sha256;x-amz-date, \ + Signature=00e997a1db4d3b6ee6c26d3f7d3f3fb3b6ee6c26d3f7d3f3fb3b6ee6c26d3f7d", + ) + .header("x-amz-date", "20260914T000000Z") + .header("x-amz-content-sha256", "UNSIGNED-PAYLOAD") + .header("x-amz-copy-source", "/negative-sigv4-bucket/source") + .body(Full::::from(Bytes::new())) + .expect("request"), + ) + .await + .expect("guard response"); + + assert_eq!(response.status(), StatusCode::FORBIDDEN); + assert_eq!(calls.load(Ordering::SeqCst), 0); + let body = BodyExt::collect(response.into_body()).await.expect("body").to_bytes(); + let body = String::from_utf8(body.to_vec()).expect("utf8 body"); + assert!(body.contains("AccessDenied"), "body: {body}"); + assert!( + body.contains("There were headers present in the request which were not signed"), + "body: {body}" + ); + } + + #[tokio::test] + async fn sigv4_header_guard_layer_passes_unsigned_and_signed_envelope_requests_through() { + let inner = CountingHybridService::default(); + let calls = inner.calls(); + let mut service = SigV4HeaderGuardLayer.layer(inner); + + // Anonymous request: no Authorization header, no presigned query. + let response = service + .call( + Request::builder() + .method(Method::GET) + .uri("/bucket/key") + .body(Full::::from(Bytes::new())) + .expect("request"), + ) + .await + .expect("inner response"); + assert_eq!(response.status(), StatusCode::IM_A_TEAPOT); + assert_eq!(calls.load(Ordering::SeqCst), 1); + + // Header-signed request whose only x-amz-* headers are the signed envelope. + let response = service + .call( + Request::builder() + .method(Method::PUT) + .uri("/bucket/key") + .header( + "authorization", + "AWS4-HMAC-SHA256 Credential=rustfsadmin/20260914/us-east-1/s3/aws4_request, \ + SignedHeaders=host;x-amz-content-sha256;x-amz-date, \ + Signature=00e997a1db4d3b6ee6c26d3f7d3f3fb3b6ee6c26d3f7d3f3fb3b6ee6c26d3f7d", + ) + .header("x-amz-date", "20260914T000000Z") + .header("x-amz-content-sha256", "UNSIGNED-PAYLOAD") + .body(Full::::from(Bytes::new())) + .expect("request"), + ) + .await + .expect("inner response"); + assert_eq!(response.status(), StatusCode::IM_A_TEAPOT); + assert_eq!(calls.load(Ordering::SeqCst), 2); + } + #[tokio::test] async fn virtual_host_style_hint_layer_short_circuits_unroutable_put() { let inner = CountingHybridService::default(); diff --git a/rustfs/src/storage/access.rs b/rustfs/src/storage/access.rs index 8da1a97ad..67c0e134f 100644 --- a/rustfs/src/storage/access.rs +++ b/rustfs/src/storage/access.rs @@ -20,7 +20,7 @@ use crate::auth::{ VerifiedSigV4Request, check_key_valid_with_context, get_condition_values_with_client_info, get_condition_values_with_query_and_client_info, get_request_auth_type_with_query, get_session_token, parse_presigned_multipart_max_total_object_size, parse_presigned_put_max_content_length, - reject_unsigned_amz_headers_on_presigned_request, + reject_unsigned_amz_headers_on_sigv4_request, }; use crate::error::ApiError; use crate::license::license_check; @@ -1696,10 +1696,10 @@ fn validate_post_object_success_controls(input: &PostObjectInput) -> S3Result<() #[async_trait::async_trait] impl S3Access for FS { async fn check(&self, cx: &mut S3AccessContext<'_>) -> S3Result<()> { - // GHSA-g8w9-qw9q-fghr: a presigned URL only authorises the headers it + // SigV4 only authorises the request properties covered by headers it // signed. Reject unsigned `x-amz-*` headers first, before the session // token lookup below or any handler reads a request header. - reject_unsigned_amz_headers_on_presigned_request(cx.headers(), cx.uri().query())?; + reject_unsigned_amz_headers_on_sigv4_request(cx.headers(), cx.uri().query())?; // Upper layer has verified ak/sk // info!(