From 230e5fc31a17dad7f56e526957db08f9ba8be2e6 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sat, 18 Jul 2026 10:44:53 +0800 Subject: [PATCH] fix(auth): POST-object lock fields and signed metadata=true listing 403s (#4959) * fix(sse): surface unconfigured managed SSE as 400, fix POST SSE-S3 e2e Managed SSE (SSE-S3 / bucket-default) on a server without KMS and without RUSTFS_SSE_S3_MASTER_KEY fails closed by design since #3564, but surfaced as 500 InternalError. Map the misconfiguration to InvalidRequest (400) and seed the local SSE master key in the anonymous POST-object SSE e2e tests; align the missing-from-policy test with the MinIO-compatible SSE field exemption (s3s#608). Re-admit the tests to the e2e-full profile (rustfs#4844). * fix(auth): POST-object lock fields and signed metadata=true listing 403s Two authorization surfaces returned 403 for allowed requests (rustfs#4845): the PutObject access hook demanded PutObjectRetention/PutObjectLegalHold IAM actions for POST-object form uploads whose lock fields are governed by the validated POST policy, and the metadata=true listing route never resolved SigV4-verified credentials into ReqInfo so signed requests were evaluated as anonymous. Skip the PUT-header lock actions for POST form uploads and resolve credentials in the metadata route; re-admit the four e2e tests to the e2e-full profile. --- .config/nextest.toml | 4 ---- rustfs/src/app/metadata_route.rs | 16 ++++++++++++++++ rustfs/src/storage/access.rs | 13 +++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/.config/nextest.toml b/.config/nextest.toml index a2a23c5f8..6f4545317 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -288,8 +288,6 @@ path = "junit.xml" # negative-path siblings of each family stay in as regression guards. # * rustfs#4843 — over-limit archive entry paths hard-reject the whole # archive even under ignore-errors semantics. -# * rustfs#4845 — 403 on allowed anonymous POST object-lock fields and on -# the list metadata=true extension. # * rustfs#4846 — distributed-lock quorum tests misclassify as timeout # under parallel load (multi-node in-process clusters; natural home is # ci-7's nightly cluster lane). @@ -301,8 +299,6 @@ default-filter = """ & !test(/^replication_extension_test::/) & !test(/^multipart_auth_test::test_signed_put_object_extract_skips_invalid_entry_when_ignore_errors_enabled$/) & !test(/^snowball_auto_extract_test::tests::snowball_auto_extract_(ignores_invalid_entries_when_requested|supports_standard_headers_with_combined_extract_options)$/) - & !test(/^multipart_auth_test::test_anonymous_post_object_(accepts_object_lock_legal_hold_field|accepts_object_lock_retention_fields)$/) - & !test(/^list_object(s_v2|_versions)_metadata_extension_test::/) & !test(/^reliant::lock::test_distributed_lock_(2_nodes_grpc_read_survives_failed_node|4_nodes_grpc_read_write_quorum_split_with_two_failed_nodes)$/) """ fail-fast = false diff --git a/rustfs/src/app/metadata_route.rs b/rustfs/src/app/metadata_route.rs index f1607982a..d6509f60e 100644 --- a/rustfs/src/app/metadata_route.rs +++ b/rustfs/src/app/metadata_route.rs @@ -15,6 +15,7 @@ //! MinIO-compatible metadata listing extension routes. use super::bucket_usecase::DefaultBucketUsecase; +use crate::auth::{check_key_valid, get_session_token}; use crate::storage::access::{ReqInfo, authorize_request, req_info_mut}; use async_trait::async_trait; use http::header::CONTENT_TYPE; @@ -103,11 +104,26 @@ fn metadata_operation(method: &Method, uri: &Uri, headers: &HeaderMap, host: Opt } async fn check_metadata_access(req: &mut S3Request, target: BucketTarget) -> S3Result<()> { + // Custom routes bypass the S3Access::check hook that normally resolves the + // SigV4-verified credentials into ReqInfo, so resolve them here the same + // way; otherwise a signed request is evaluated as anonymous and denied + // (rustfs#4845). Anonymous requests keep cred=None and stay subject to the + // bucket-policy-only evaluation in authorize_request. + let (cred, is_owner) = if let Some(input_cred) = &req.credentials { + let (cred, is_owner) = + check_key_valid(get_session_token(&req.uri, &req.headers).unwrap_or_default(), &input_cred.access_key).await?; + (Some(cred), is_owner) + } else { + (None, false) + }; + { if req_info_mut(req).is_err() { req.extensions.insert(ReqInfo::default()); } let req_info = req_info_mut(req)?; + req_info.cred = cred; + req_info.is_owner = is_owner; req_info.bucket = Some(target.bucket); } diff --git a/rustfs/src/storage/access.rs b/rustfs/src/storage/access.rs index a9c777368..4693cecfe 100644 --- a/rustfs/src/storage/access.rs +++ b/rustfs/src/storage/access.rs @@ -1901,6 +1901,19 @@ impl S3Access for FS { authorize_request(req, Action::S3Action(S3Action::PutObjectAction)).await?; + // POST-object form uploads (s3s routes them through this hook with the + // original POST method before the dedicated post_object hook) are + // governed by the POST policy document instead of the retention / + // legal-hold IAM actions: s3s validates every x-amz-object-lock-* form + // field against the policy conditions before dispatch, and signed POSTs + // sign the policy itself. Anonymous POSTs author their own policy, so + // requiring these PUT-header IAM actions here only breaks the + // policy-covered accept path (rustfs#4845) without adding a boundary — + // the same MinIO handler applies no per-field lock permission either. + if req.method == http::Method::POST { + return Ok(()); + } + if legal_hold_write_requested(req.input.object_lock_legal_hold_status.as_ref()) { authorize_request(req, Action::S3Action(S3Action::PutObjectLegalHoldAction)).await?; }