diff --git a/.config/e2e-full-selection.txt b/.config/e2e-full-selection.txt index a4d6c6cae..177c91103 100644 --- a/.config/e2e-full-selection.txt +++ b/.config/e2e-full-selection.txt @@ -1,2 +1,2 @@ -sha256-darwin=53b05ac745905809d3828c6994bdd8ecf9d20b2b61a8a9d80fe15eb62f932193 +sha256-darwin=dd14f49a7b0e2c156b4457fdd836499d837890d3e439689a4eff9e8875ee2f5b sha256-linux=7c892afa4b9d1591b46bd79c976b647109a277284fddb3b98edced4b0297eda2 diff --git a/.config/e2e-smoke-selection.txt b/.config/e2e-smoke-selection.txt index 6ebb3fcc0..aa3804f21 100644 --- a/.config/e2e-smoke-selection.txt +++ b/.config/e2e-smoke-selection.txt @@ -1 +1 @@ -sha256=5db88c6fec94d4f269c7d9cfc128bd2adc27b3d7021127e2fa0b1daccc5f900f +sha256=6d18f9cce820c51d5589de944e8cc185f73eeca0ea9a9916651943e3759169d0 diff --git a/.config/security-smoke-floor.txt b/.config/security-smoke-floor.txt index b9d3f31fc..2ee0d4190 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. -18 +26 diff --git a/CHANGELOG.md b/CHANGELOG.md index e6c2e7ed9..2ffd8896f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security +- **Presigned URLs honour only signed headers** (GHSA-g8w9-qw9q-fghr): a SigV4 presigned request that carries an `x-amz-*` request header not listed in `X-Amz-SignedHeaders` is now rejected with `403 AccessDenied` ("There were headers present in the request which were not signed"), matching AWS S3. Previously the holder of a presigned `PutObject` URL could add unsigned `x-amz-tagging`, `x-amz-storage-class`, `x-amz-website-redirect-location`, ACL, metadata, Object Lock or SSE headers and have them applied. Presigners that intend a property must set it before signing so the SDK lists the header in `SignedHeaders`; `x-amz-cf-id` (CloudFront) remains tolerated unsigned. Header-signed SigV4 and SigV2 requests are unchanged. + ### Fixed - **Multipart admission queue**: an `UploadPart` waiting for a foreground write permit now waits at most 10 s by default (`RUSTFS_PUT_MULTIPART_FOREGROUND_ADMISSION_WAIT_TIMEOUT_MS`, previously 30 s), so a queued part returns S3 `SlowDown` before the client's socket write timeout drops the connection. Separately, the API listener no longer forces a 4 MiB `SO_RCVBUF` on every accepted socket (kernel autotuning applies; `RUSTFS_HTTP_SOCKET_RECV_BUFFER_BYTES` restores a fixed size), so a queued part no longer lets up to 8 MiB of unread body accumulate in kernel memory per connection, which is what throttled whole nodes under SDK-default multipart concurrency. Fixes #7385. - **Helm Ingress**: `customAnnotations` are now merged with class-specific annotations (nginx/traefik) instead of being ignored when `ingress.className` is set. diff --git a/crates/e2e_test/src/presigned_negative_test.rs b/crates/e2e_test/src/presigned_negative_test.rs index 04874e0c7..ec8928f72 100644 --- a/crates/e2e_test/src/presigned_negative_test.rs +++ b/crates/e2e_test/src/presigned_negative_test.rs @@ -356,3 +356,238 @@ async fn tampered_presigned_put_returns_signature_does_not_match() -> Result<(), ); Ok(()) } + +/// GHSA-g8w9-qw9q-fghr: a presigned PUT signed with `SignedHeaders=host` must +/// not honour `x-amz-*` headers the uploader adds afterwards. The presign +/// authorised one plain upload; the extra headers would set tags, storage +/// class and a website redirect the presigner never covered. AWS S3 rejects +/// this with 403 `AccessDenied`, and so must RustFS — and the object must not +/// be stored at all, not merely stored without the properties. +#[tokio::test] +async fn ghsa_g8w9_presigned_put_rejects_unsigned_x_amz_headers() -> Result<(), Box> { + init_logging(); + let mut env = RustFSTestEnvironment::new().await?; + setup(&mut env).await?; + + let key = "presigned-put-unsigned-amz-headers.txt"; + let pr = env + .create_s3_client() + .put_object() + .bucket(BUCKET) + .key(key) + .presigned(valid_config()) + .await?; + assert!( + !pr.headers().any(|(name, _)| name.eq_ignore_ascii_case("x-amz-tagging")), + "fixture must presign a plain PutObject without tagging so the header below is unsigned" + ); + + let unsigned: Vec<(&str, &str)> = vec![ + ("x-amz-tagging", "owner=attacker&classification=public"), + ("x-amz-website-redirect-location", "https://attacker.example/phish"), + ("x-amz-storage-class", "REDUCED_REDUNDANCY"), + ]; + let headers = pr.headers().chain(unsigned.iter().copied()); + let resp = send_raw(pr.method(), pr.uri(), headers, Some(b"should-not-be-stored".to_vec())).await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!( + status.as_u16(), + 403, + "presigned PUT with unsigned x-amz-* headers must be 403, body:\n{body}" + ); + assert_error_code(&body, "AccessDenied"); + assert!( + body.contains("were not signed"), + "rejection must name unsigned headers as the cause, got:\n{body}" + ); + + let error = env + .create_s3_client() + .head_object() + .bucket(BUCKET) + .key(key) + .send() + .await + .expect_err("presigned PUT with unsigned x-amz-* headers must not store the object"); + assert_eq!( + error.raw_response().map(|response| response.status().as_u16()), + Some(404), + "absence probe after the rejected upload must return HTTP 404, got {error:?}" + ); + Ok(()) +} + +/// GHSA-g8w9-qw9q-fghr positive control: when the presigner itself sets the +/// property, the SDK lists `x-amz-tagging` in `SignedHeaders`, the uploader +/// replays it, and the upload succeeds with the tags applied. Without this the +/// negative test above could pass because the server rejects every tagged +/// presigned upload. +#[tokio::test] +async fn ghsa_g8w9_presigned_put_accepts_signed_x_amz_headers() -> Result<(), Box> { + init_logging(); + let mut env = RustFSTestEnvironment::new().await?; + setup(&mut env).await?; + + let key = "presigned-put-signed-tagging.txt"; + let pr = env + .create_s3_client() + .put_object() + .bucket(BUCKET) + .key(key) + .tagging("owner=app") + .presigned(valid_config()) + .await?; + assert!( + pr.headers().any(|(name, _)| name.eq_ignore_ascii_case("x-amz-tagging")), + "fixture must carry x-amz-tagging as a signed header" + ); + assert!( + pr.uri().contains("x-amz-tagging"), + "X-Amz-SignedHeaders must list x-amz-tagging, uri: {}", + pr.uri() + ); + + let resp = send_presigned(&pr, Some(b"stored-with-signed-tagging".to_vec())).await?; + let status = resp.status(); + let body = resp.text().await?; + assert!( + status.is_success(), + "presigned PUT with signed x-amz-tagging must succeed, got {status}, body:\n{body}" + ); + + let tags = env + .create_s3_client() + .get_object_tagging() + .bucket(BUCKET) + .key(key) + .send() + .await?; + let tag_set: Vec<(String, String)> = tags + .tag_set() + .iter() + .map(|tag| (tag.key().to_string(), tag.value().to_string())) + .collect(); + assert_eq!(tag_set, vec![("owner".to_string(), "app".to_string())], "signed tagging must be applied"); + info!("signed presigned tagging control passed"); + Ok(()) +} + +/// GHSA-g8w9-qw9q-fghr on the read side: a presigned GET signed with +/// `SignedHeaders=host` must not accept an unsigned SSE-C header. The header +/// would otherwise select a decryption path the presigner never authorised. +#[tokio::test] +async fn ghsa_g8w9_presigned_get_rejects_unsigned_x_amz_headers() -> Result<(), Box> { + init_logging(); + let mut env = RustFSTestEnvironment::new().await?; + setup(&mut env).await?; + + let pr = env + .create_s3_client() + .get_object() + .bucket(BUCKET) + .key(CANONICAL_KEY) + .presigned(valid_config()) + .await?; + + let unsigned: Vec<(&str, &str)> = vec![("x-amz-server-side-encryption-customer-algorithm", "AES256")]; + let headers = pr.headers().chain(unsigned.iter().copied()); + let resp = send_raw(pr.method(), pr.uri(), headers, None).await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!( + status.as_u16(), + 403, + "presigned GET with an unsigned x-amz-* header must be 403, body:\n{body}" + ); + assert_error_code(&body, "AccessDenied"); + assert!( + !body.contains(std::str::from_utf8(CANONICAL_BODY)?), + "rejected GET must not leak the object body" + ); + Ok(()) +} + +/// GHSA-g8w9-qw9q-fghr: an unsigned `x-amz-copy-source` would turn a presigned +/// PutObject into a CopyObject of an arbitrary readable key, since operation +/// routing happens before authorization. The presigned upload must fail and +/// leave nothing behind. +#[tokio::test] +async fn ghsa_g8w9_presigned_put_rejects_unsigned_copy_source() -> Result<(), Box> { + init_logging(); + let mut env = RustFSTestEnvironment::new().await?; + setup(&mut env).await?; + + let key = "presigned-put-unsigned-copy-source.txt"; + let pr = env + .create_s3_client() + .put_object() + .bucket(BUCKET) + .key(key) + .presigned(valid_config()) + .await?; + + let copy_source = format!("/{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?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!( + status.as_u16(), + 403, + "presigned PUT with an unsigned copy source must be 403, body:\n{body}" + ); + assert_error_code(&body, "AccessDenied"); + + let error = env + .create_s3_client() + .head_object() + .bucket(BUCKET) + .key(key) + .send() + .await + .expect_err("rejected copy must not create the destination object"); + assert_eq!(error.raw_response().map(|response| response.status().as_u16()), Some(404)); + Ok(()) +} + +/// GHSA-g8w9-qw9q-fghr boundary control: the rule covers `x-amz-*` only. A +/// plain `Content-Type` on a `SignedHeaders=host` presigned PUT is outside +/// SigV4's signed-header requirement (AWS S3 accepts it too) and must keep +/// working, so the negative tests above cannot pass by rejecting every +/// unsigned header. +#[tokio::test] +async fn ghsa_g8w9_presigned_put_still_accepts_unsigned_non_amz_headers() -> Result<(), Box> +{ + init_logging(); + let mut env = RustFSTestEnvironment::new().await?; + setup(&mut env).await?; + + let key = "presigned-put-unsigned-content-type.txt"; + let pr = env + .create_s3_client() + .put_object() + .bucket(BUCKET) + .key(key) + .presigned(valid_config()) + .await?; + + let unsigned: Vec<(&str, &str)> = vec![("content-type", "text/x-rustfs-test")]; + let headers = pr.headers().chain(unsigned.iter().copied()); + let resp = send_raw(pr.method(), pr.uri(), headers, Some(b"plain-header-upload".to_vec())).await?; + let status = resp.status(); + let body = resp.text().await?; + assert!( + status.is_success(), + "presigned PUT with an unsigned Content-Type must succeed, got {status}, body:\n{body}" + ); + + let head = env.create_s3_client().head_object().bucket(BUCKET).key(key).send().await?; + assert_eq!( + head.content_type(), + Some("text/x-rustfs-test"), + "unsigned Content-Type must still be applied" + ); + Ok(()) +} diff --git a/docs/testing/security-regressions.md b/docs/testing/security-regressions.md index 4abaa342d..3419d2649 100644 --- a/docs/testing/security-regressions.md +++ b/docs/testing/security-regressions.md @@ -17,13 +17,14 @@ Every fixed RustFS GitHub Security Advisory maps to at least one named regressio | [GHSA-v9cp-qfw9-9pfp](https://github.com/rustfs/rustfs/security/advisories/GHSA-v9cp-qfw9-9pfp) | `ForAllValues:`/`ForAnyValue:` negated string operators applied negation to the aggregate instead of the per-value predicate | fixed, GHSA private-fork merge | `ghsa_v9cp_for_all_values_not_equals_partial_overlap`, `ghsa_v9cp_for_any_value_not_equals_partial_overlap` and the absent-key/positive-quantifier cases beside them (`crates/policy/tests/quantified_negation.rs`); the value set must partially overlap the policy set, since contained or disjoint sets cannot tell the quantifiers apart | crate test | | [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-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 | Layer | Command | Lane | Guard | | --- | --- | --- | --- | -| Unit and crate tests (`ghsa_r5qv_*`, the m77q pins, `ghsa_5354_*`, `ghsa_3ppv_*`, `ghsa_6r96_*`, `ghsa_v9cp_*`, `ghsa_g3vq_*`) | `cargo nextest run --profile ci --all --exclude e2e_test` | every PR, `Test and Lint` (required) | none needed; the workspace pass runs every unit and crate test | +| Unit and crate tests (`ghsa_r5qv_*`, the m77q pins, `ghsa_5354_*`, `ghsa_3ppv_*`, `ghsa_6r96_*`, `ghsa_v9cp_*`, `ghsa_g3vq_*`, `ghsa_g8w9_*`) | `cargo nextest run --profile ci --all --exclude e2e_test` | every PR, `Test and Lint` (required) | none needed; the workspace pass runs every unit and crate test | | S3-API negative-auth e2e (`negative_sigv4_test`, `presigned_negative_test`, `admin_auth_test`) | `cargo nextest run --profile e2e-smoke -p e2e_test` | every PR, `End-to-End Tests` (report-only) | `scripts/check_security_smoke_count.sh` with the floor in `.config/security-smoke-floor.txt`, run in the `e2e-tests` job; fails when a rename drops one of these modules out of the smoke filter | | Other S3 e2e guards (`anonymous_access_test`) | `cargo nextest run --profile e2e-smoke -p e2e_test` | every PR, `End-to-End Tests` (report-only) | `scripts/check_test_wiring.py --check-profile e2e-smoke` digest | | Protocol e2e (`protocols::test_protocol_core_suite`, GHSA-3p3x) | `RUSTFS_BUILD_FEATURES=ftps,webdav,sftp cargo nextest run -j 1 --profile e2e-protocols -p e2e_test` | nightly, `e2e-replication-nightly.yml` job `protocols-nightly`; not PR-gated | `scripts/check_test_wiring.py --check-profile e2e-protocols` digest | diff --git a/rustfs/src/admin/router.rs b/rustfs/src/admin/router.rs index 1945acc1f..0c34ff810 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}; +use crate::auth::{check_key_valid, constant_time_eq, get_session_token, reject_unsigned_amz_headers_on_presigned_request}; use crate::error::ApiError; use crate::license::license_check; use crate::server::{ @@ -3269,6 +3269,11 @@ 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())?; + if let Some(server_ctx) = &self.server_ctx { req.extensions.insert(server_ctx.clone()); if !is_public_health_path(req.uri.path()) && server_ctx.installed_app_context().is_none() { @@ -5611,6 +5616,38 @@ mod tests { assert_eq!(err.code(), &S3ErrorCode::AccessDenied); } + /// GHSA-g8w9-qw9q-fghr: custom routes must apply the presigned + /// signed-header rule too, since they never reach `S3Access::check`. + #[tokio::test] + async fn ghsa_g8w9_check_access_rejects_unsigned_amz_header_on_presigned_custom_route() { + let router: S3Router = S3Router::new(false); + let mut headers = HeaderMap::new(); + 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&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Date=20260827T000000Z&X-Amz-Expires=900&X-Amz-SignedHeaders=host&X-Amz-Credential=test%2F20260827%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Signature=signature" + .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("presigned 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 6ac7c5122..dbc9689aa 100644 --- a/rustfs/src/auth.rs +++ b/rustfs/src/auth.rs @@ -50,6 +50,7 @@ const EVENT_KEYSTONE_CREDENTIALS_DETECTED: &str = "keystone_credentials_detected const EVENT_KEYSTONE_CREDENTIALS_VALIDATED: &str = "keystone_credentials_validated"; 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"; /// RustFS-specific query capability for a single presigned PutObject request. pub(crate) const RUSTFS_MAX_CONTENT_LENGTH_QUERY: &str = "x-rustfs-max-content-length"; @@ -1031,6 +1032,102 @@ pub fn get_query_param<'a>(query: &'a str, param_name: &str) -> Option<&'a str> None } +/// `x-amz-*` request headers a SigV4 presigned request may carry without +/// listing them in `X-Amz-SignedHeaders`. +/// +/// CloudFront stamps `x-amz-cf-id` on every origin request it forwards, so a +/// presigned URL served through a CDN could never be honoured if that header +/// had to be signed; nothing in RustFS reads it, so it cannot change what the +/// request does. +const PRESIGNED_UNSIGNED_AMZ_HEADER_ALLOWLIST: &[&str] = &["x-amz-cf-id"]; + +pub(crate) const UNSIGNED_HEADERS_MESSAGE: &str = "There were headers present in the request which were not signed"; + +/// GHSA-g8w9-qw9q-fghr: reject `x-amz-*` request headers that a SigV4 presigned +/// URL did not sign. +/// +/// A presigned URL is a bounded capability: the presigner authorises one +/// method, key, expiry and the header set named in `X-Amz-SignedHeaders`. The +/// upstream verifier only proves that the signed headers match; any other +/// `x-amz-*` header (tagging, storage class, ACL, metadata, website redirect, +/// Object Lock, SSE selection) would still reach the handlers and take effect, +/// so the untrusted holder of an upload URL could set object properties the +/// presign never covered. AWS S3 rejects such a request with `AccessDenied` +/// ("There were headers present in the request which were not signed"); this +/// check mirrors that at the access boundary, before any handler reads a +/// header. +/// +/// Only query-string SigV4 requests are checked. 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. +/// +/// Detection keys on the query, not on the derived [`AuthType`], because the +/// upstream verifier dispatches to the presigned path whenever the query +/// carries `X-Amz-Signature`, even if an `Authorization` header is present too. +/// The rule relies on the verifier signing every query parameter except the +/// signature itself, so neither `X-Amz-SignedHeaders` nor a property-carrying +/// query parameter can be added after presigning. +pub(crate) fn reject_unsigned_amz_headers_on_presigned_request(header: &HeaderMap, query: Option<&str>) -> S3Result<()> { + let Some(query) = query else { + return Ok(()); + }; + + // Presence detection is case-insensitive so a query the upstream verifier + // would not treat as presigned still fails closed here; the signed list is + // read with the exact key the verifier uses (`X-Amz-SignedHeaders`, unique), + // so both sides always see the same list. A duplicate or missing key + // yields an empty list, which signs nothing. + let mut is_presigned_v4 = false; + let mut signed_headers: Option = None; + let mut duplicate_signed_headers = false; + for (name, value) in form_urlencoded::parse(query.as_bytes()) { + if name.eq_ignore_ascii_case("x-amz-signature") { + is_presigned_v4 = true; + } else if name == "X-Amz-SignedHeaders" { + if signed_headers.is_some() { + duplicate_signed_headers = true; + } + signed_headers = Some(value.into_owned()); + } + } + if !is_presigned_v4 { + return Ok(()); + } + if duplicate_signed_headers { + signed_headers = None; + } + + let signed: Vec = signed_headers + .as_deref() + .unwrap_or_default() + .split(';') + .map(|name| name.trim().to_ascii_lowercase()) + .filter(|name| !name.is_empty()) + .collect(); + + for name in header.keys() { + // `HeaderName` is already lowercase. + let name = name.as_str(); + if !name.starts_with("x-amz-") || PRESIGNED_UNSIGNED_AMZ_HEADER_ALLOWLIST.contains(&name) { + continue; + } + if !signed.iter().any(|signed_name| signed_name == name) { + warn!( + event = EVENT_PRESIGNED_UNSIGNED_AMZ_HEADER, + component = LOG_COMPONENT_AUTH, + subsystem = LOG_SUBSYSTEM_REQUEST, + reason = "unsigned_amz_header", + header = name, + "Presigned request rejected" + ); + return Err(S3Error::with_message(S3ErrorCode::AccessDenied, UNSIGNED_HEADERS_MESSAGE.to_string())); + } + } + + Ok(()) +} + /// Parse the RustFS presigned PutObject size capability after authentication. /// /// The query value is covered by SigV4 when it is present before presigning, but @@ -1914,6 +2011,126 @@ mod tests { ); } + /// GHSA-g8w9-qw9q-fghr: `x-amz-*` request headers that are not listed in + /// `X-Amz-SignedHeaders` must not survive the presigned access boundary. + #[test] + fn ghsa_g8w9_presigned_request_rejects_unsigned_x_amz_headers() { + let presigned_host_only = "X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Date=20260827T000000Z&X-Amz-Expires=900&X-Amz-SignedHeaders=host&X-Amz-Credential=test/20260827/us-east-1/s3/aws4_request&X-Amz-Signature=signature"; + + for header in [ + "x-amz-tagging", + "x-amz-website-redirect-location", + "x-amz-storage-class", + "x-amz-acl", + "x-amz-meta-owner", + "x-amz-object-lock-mode", + "x-amz-server-side-encryption", + ] { + let mut headers = HeaderMap::new(); + headers.insert("content-type", HeaderValue::from_static("text/plain")); + headers.insert(header, HeaderValue::from_static("attacker-controlled")); + let error = reject_unsigned_amz_headers_on_presigned_request(&headers, Some(presigned_host_only)).unwrap_err(); + assert_eq!(error.code(), &S3ErrorCode::AccessDenied, "{header} must be rejected when unsigned"); + assert_eq!(error.message(), Some(UNSIGNED_HEADERS_MESSAGE)); + } + + // Non-`x-amz-*` headers are outside the SigV4 rule and stay allowed. + let mut headers = HeaderMap::new(); + headers.insert("content-type", HeaderValue::from_static("text/plain")); + headers.insert("cache-control", HeaderValue::from_static("no-store")); + reject_unsigned_amz_headers_on_presigned_request(&headers, Some(presigned_host_only)).unwrap(); + + // A missing SignedHeaders list signs nothing and still fails closed. + let missing_signed_headers = presigned_host_only.replace("&X-Amz-SignedHeaders=host", ""); + let mut headers = HeaderMap::new(); + headers.insert("x-amz-tagging", HeaderValue::from_static("a=b")); + assert_eq!( + reject_unsigned_amz_headers_on_presigned_request(&headers, Some(&missing_signed_headers)) + .unwrap_err() + .code(), + &S3ErrorCode::AccessDenied + ); + + // Detection follows the upstream dispatch: any query carrying the + // signature is a presigned request, whatever the key's case. + let lowercase_query = presigned_host_only.to_ascii_lowercase(); + assert_eq!( + reject_unsigned_amz_headers_on_presigned_request(&headers, Some(&lowercase_query)) + .unwrap_err() + .code(), + &S3ErrorCode::AccessDenied + ); + + // Only the exact key the upstream verifier reads counts; a second + // (or differently cased) list must not widen the signed set, and a + // duplicate exact key signs nothing at all. + let widened_by_case = format!("{presigned_host_only}&x-amz-signedheaders=host%3Bx-amz-tagging"); + assert_eq!( + reject_unsigned_amz_headers_on_presigned_request(&headers, Some(&widened_by_case)) + .unwrap_err() + .code(), + &S3ErrorCode::AccessDenied + ); + let duplicated = format!("{presigned_host_only}&X-Amz-SignedHeaders=host%3Bx-amz-tagging"); + assert_eq!( + reject_unsigned_amz_headers_on_presigned_request(&headers, Some(&duplicated)) + .unwrap_err() + .code(), + &S3ErrorCode::AccessDenied + ); + } + + #[test] + fn ghsa_g8w9_presigned_request_accepts_signed_or_exempt_x_amz_headers() { + let signed_tagging = "X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Date=20260827T000000Z&X-Amz-Expires=900&X-Amz-SignedHeaders=host%3Bx-amz-tagging%3Bx-amz-meta-owner&X-Amz-Credential=test/20260827/us-east-1/s3/aws4_request&X-Amz-Signature=signature"; + + let mut headers = HeaderMap::new(); + headers.insert("x-amz-tagging", HeaderValue::from_static("owner=app")); + headers.insert("X-Amz-Meta-Owner", HeaderValue::from_static("app")); + reject_unsigned_amz_headers_on_presigned_request(&headers, Some(signed_tagging)).unwrap(); + + // Case differences in the signed list do not matter; header names are + // canonicalised to lowercase on both sides. + let uppercase_list = signed_tagging.replace("x-amz-tagging", "X-Amz-Tagging"); + reject_unsigned_amz_headers_on_presigned_request(&headers, Some(&uppercase_list)).unwrap(); + + // The CDN request id is the only unsigned `x-amz-*` header tolerated. + headers.insert("x-amz-cf-id", HeaderValue::from_static("cloudfront-request-id")); + reject_unsigned_amz_headers_on_presigned_request(&headers, Some(signed_tagging)).unwrap(); + + // Adding one more unsigned header on top of signed ones still fails. + headers.insert("x-amz-storage-class", HeaderValue::from_static("REDUCED_REDUNDANCY")); + assert_eq!( + reject_unsigned_amz_headers_on_presigned_request(&headers, Some(signed_tagging)) + .unwrap_err() + .code(), + &S3ErrorCode::AccessDenied + ); + } + + #[test] + fn ghsa_g8w9_check_ignores_header_signed_sigv2_and_anonymous_requests() { + let mut headers = HeaderMap::new(); + headers.insert("x-amz-tagging", HeaderValue::from_static("owner=app")); + headers.insert("x-amz-storage-class", HeaderValue::from_static("STANDARD")); + + // No query at all: nothing to bind against. + reject_unsigned_amz_headers_on_presigned_request(&headers, None).unwrap(); + + // Header-signed SigV4 and SigV2 carry no `X-Amz-Signature` query. + headers.insert( + "authorization", + HeaderValue::from_static( + "AWS4-HMAC-SHA256 Credential=test/20260827/us-east-1/s3/aws4_request, SignedHeaders=host, Signature=abc", + ), + ); + reject_unsigned_amz_headers_on_presigned_request(&headers, Some("versioning=")).unwrap(); + + // SigV2 presigned URLs sign every `x-amz-*` header in the string to sign. + let sigv2_query = "AWSAccessKeyId=test&Expires=1893456000&Signature=abc"; + reject_unsigned_amz_headers_on_presigned_request(&headers, Some(sigv2_query)).unwrap(); + } + #[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 223a73383..a2a95b017 100644 --- a/rustfs/src/storage/access.rs +++ b/rustfs/src/storage/access.rs @@ -20,6 +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, }; use crate::error::ApiError; use crate::license::license_check; @@ -1792,6 +1793,11 @@ 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 + // 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())?; + // Upper layer has verified ak/sk // info!( // "s3 check uri: {:?}, method: {:?} path: {:?}, s3_op: {:?}, cred: {:?}, headers:{:?}", @@ -1836,11 +1842,11 @@ impl S3Access for FS { ..Default::default() }; - // Publish this server's context slot so downstream data-plane handlers - // resolve the same store (backlog#1052 S6). let auth_type = get_request_auth_type_with_query(cx.headers(), cx.uri().query()); let verified_presigned = matches!(auth_type, AuthType::Presigned); let verified_sigv4 = matches!(auth_type, AuthType::Presigned | AuthType::Signed); + // Publish this server's context slot so downstream data-plane handlers + // resolve the same store (backlog#1052 S6). { let ext = cx.extensions_mut(); ext.insert(self.server_ctx().clone());