From 703b1a5c9c4bab984066c03a047a30e22a4229c0 Mon Sep 17 00:00:00 2001 From: Chris Date: Mon, 14 Sep 2026 11:48:09 +0800 Subject: [PATCH] fix(auth): reject unsigned x-amz headers on header-signed SigV4 requests (release) (#7813) --- .config/e2e-full-selection.txt | 2 +- .config/e2e-smoke-selection.txt | 2 +- .config/security-smoke-floor.txt | 2 +- CHANGELOG.md | 4 + Cargo.lock | 1 + Cargo.toml | 1 + crates/e2e_test/src/negative_sigv4_test.rs | 112 ++++++++ .../e2e_test/src/presigned_negative_test.rs | 11 +- docs/testing/security-regressions.md | 1 + rustfs/Cargo.toml | 1 + rustfs/src/admin/router.rs | 44 +++- rustfs/src/auth.rs | 241 +++++++++++++++++- rustfs/src/storage/access.rs | 6 +- 13 files changed, 414 insertions(+), 14 deletions(-) diff --git a/.config/e2e-full-selection.txt b/.config/e2e-full-selection.txt index a64d5796b..611d1816a 100644 --- a/.config/e2e-full-selection.txt +++ b/.config/e2e-full-selection.txt @@ -1,2 +1,2 @@ -sha256-darwin=5886d44ada0efc6a0a57f1391baa044cdb4a0e14bbc10c8b1207f0fa79cfa870 +sha256-darwin=9e9687ded961aa4e619a15def6e1ee0a5153be05423d009ee7a58126624c83e7 sha256-linux=7633342a1d5bfc265bdeba59f3c373c937031dc446c3f6db31360bb8c666de12 diff --git a/.config/e2e-smoke-selection.txt b/.config/e2e-smoke-selection.txt index f3b450db0..78a1d1484 100644 --- a/.config/e2e-smoke-selection.txt +++ b/.config/e2e-smoke-selection.txt @@ -1 +1 @@ -sha256=5fbb230b89212b7c3d7229d6cef3e7e2d16f0ecfec62237ebc770785706f67d9 +sha256=548a58d74f3cb3b5a1ab9dcd4d3f8625b9ddebe847a2e14de1d08c3b2e6292eb 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 8b9c0c4dc..3a7d9ccff 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 e6248d9ba..81d440dad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9588,6 +9588,7 @@ dependencies = [ "rustls", "rustls-pki-types", "s3s", + "s3s-sigv4", "serde", "serde_json", "serde_urlencoded", diff --git a/Cargo.toml b/Cargo.toml index 531f031e9..f9a4d52bd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -313,6 +313,7 @@ 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-sigv4 = { git = "https://github.com/s3s-project/s3s.git", rev = "f3e17541f366696bf0cbaf380fcbd8b44c17eba4", version = "0.16.0-alpha.1" } 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/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 edf9de206..3230d47bd 100644 --- a/rustfs/Cargo.toml +++ b/rustfs/Cargo.toml @@ -346,6 +346,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/storage/access.rs b/rustfs/src/storage/access.rs index 5835839be..1ea3973c7 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; @@ -1710,10 +1710,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!(