From 7b2cc1f42715225aa2172d7ba559cdfc84355cb7 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Fri, 17 Jul 2026 19:02:21 +0800 Subject: [PATCH] fix(sse): surface unconfigured managed SSE as 400 and fix anonymous POST SSE-S3 e2e (#4958) 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). --- .config/nextest.toml | 3 -- crates/e2e_test/src/multipart_auth_test.rs | 52 +++++++++++++++++----- rustfs/src/storage/sse.rs | 34 +++++++++++--- 3 files changed, 68 insertions(+), 21 deletions(-) diff --git a/.config/nextest.toml b/.config/nextest.toml index 8f51a1fdb..a2a23c5f8 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#4844 — anonymous POST-object with SSE-S3 / bucket-default SSE -# returns 500. # * 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 @@ -303,7 +301,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_sse_s3|rejects_sse_s3_missing_from_policy_conditions|uses_bucket_default_sse_kms|uses_bucket_default_sse_s3)$/) & !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)$/) diff --git a/crates/e2e_test/src/multipart_auth_test.rs b/crates/e2e_test/src/multipart_auth_test.rs index 0cc4eb326..feabb0ead 100644 --- a/crates/e2e_test/src/multipart_auth_test.rs +++ b/crates/e2e_test/src/multipart_auth_test.rs @@ -53,6 +53,17 @@ fn sse_customer_key_md5_base64(key: &str) -> String { base64::engine::general_purpose::STANDARD.encode(md5::compute(key).0) } +/// Env var consumed by the local SSE-S3 DEK provider when KMS is not configured. +/// +/// Since rustfs#3564 the server fails closed on managed SSE (SSE-S3 or +/// bucket-default encryption) unless KMS is configured or this master key is +/// provided, so tests exercising managed SSE on a bare server must seed it. +const LOCAL_SSE_MASTER_KEY_ENV: &str = "RUSTFS_SSE_S3_MASTER_KEY"; + +fn local_sse_master_key_value() -> String { + base64::engine::general_purpose::STANDARD.encode([0x42u8; 32]) +} + async fn make_tar(files: &[(&str, &[u8])], dirs: &[&str]) -> Vec { let buf = Cursor::new(Vec::new()); let mut builder = tokio_tar::Builder::new(buf); @@ -930,7 +941,9 @@ async fn test_anonymous_post_object_accepts_sse_s3() -> Result<(), Box Result<(), B init_logging(); let mut env = RustFSTestEnvironment::new().await?; - env.start_rustfs_server(vec![]).await?; + let master_key = local_sse_master_key_value(); + env.start_rustfs_server_with_env(vec![], &[(LOCAL_SSE_MASTER_KEY_ENV, master_key.as_str())]) + .await?; let bucket = "anon-post-default-sse-s3"; let object_key = "post-default-sse-s3-object.txt"; @@ -1057,7 +1072,9 @@ async fn test_anonymous_post_object_uses_bucket_default_sse_kms() -> Result<(), init_logging(); let mut env = RustFSTestEnvironment::new().await?; - env.start_rustfs_server(vec![]).await?; + let master_key = local_sse_master_key_value(); + env.start_rustfs_server_with_env(vec![], &[(LOCAL_SSE_MASTER_KEY_ENV, master_key.as_str())]) + .await?; let bucket = "anon-post-default-sse-kms"; let object_key = "post-default-sse-kms-object.txt"; @@ -1176,15 +1193,24 @@ async fn test_anonymous_post_object_rejects_sse_s3_policy_mismatch() -> Result<( #[tokio::test] #[serial] -async fn test_anonymous_post_object_rejects_sse_s3_missing_from_policy_conditions() +async fn test_anonymous_post_object_accepts_sse_s3_missing_from_policy_conditions() -> Result<(), Box> { init_logging(); + // MinIO-compatible POST-policy validation (s3s-project/s3s#608) exempts the + // x-amz-server-side-encryption* form fields from the "every form field must + // appear in the policy conditions" rule, so an SSE-S3 field that the policy + // does not mention is accepted and encryption is applied. When the policy + // does cover the field, a value mismatch is still rejected — see + // test_anonymous_post_object_rejects_sse_s3_policy_mismatch. let mut env = RustFSTestEnvironment::new().await?; - env.start_rustfs_server(vec![]).await?; + let master_key = local_sse_master_key_value(); + env.start_rustfs_server_with_env(vec![], &[(LOCAL_SSE_MASTER_KEY_ENV, master_key.as_str())]) + .await?; let bucket = "anon-post-sse-s3-missing"; let object_key = "post-sse-s3-missing-object.txt"; + let expected_body = b"post-sse-s3-missing".to_vec(); let admin_client = env.create_s3_client(); admin_client.create_bucket().bucket(bucket).send().await?; @@ -1202,7 +1228,7 @@ async fn test_anonymous_post_object_rejects_sse_s3_missing_from_policy_condition .text("x-amz-server-side-encryption", "AES256") .part( "file", - reqwest::multipart::Part::bytes(b"post-sse-s3-missing".to_vec()) + reqwest::multipart::Part::bytes(expected_body.clone()) .file_name("upload.txt") .mime_str("text/plain")?, ); @@ -1216,11 +1242,15 @@ async fn test_anonymous_post_object_rejects_sse_s3_missing_from_policy_condition let status = post_resp.status(); let response_body = post_resp.text().await?; - assert_eq!(status, reqwest::StatusCode::FORBIDDEN); - assert!( - response_body.contains("AccessDenied"), - "response should contain AccessDenied code, got: {response_body}" - ); + assert_eq!(status, reqwest::StatusCode::NO_CONTENT); + assert!(response_body.is_empty(), "204 response should not contain a body, got: {response_body}"); + + let head = admin_client.head_object().bucket(bucket).key(object_key).send().await?; + assert_eq!(head.server_side_encryption().map(|value| value.as_str()), Some("AES256")); + + let uploaded = admin_client.get_object().bucket(bucket).key(object_key).send().await?; + let uploaded = uploaded.body.collect().await?.into_bytes(); + assert_eq!(uploaded.as_ref(), expected_body.as_slice()); Ok(()) } diff --git a/rustfs/src/storage/sse.rs b/rustfs/src/storage/sse.rs index dc4739dc9..3c438a672 100644 --- a/rustfs/src/storage/sse.rs +++ b/rustfs/src/storage/sse.rs @@ -2019,22 +2019,32 @@ impl TestSseDekProvider { /// Create a local SSE DEK provider for SSE-S3 when KMS is not configured. /// Requires RUSTFS_SSE_S3_MASTER_KEY to be a valid base64-encoded 32-byte key. + /// + /// The failures here are server configuration problems, not internal + /// faults: surface them as `InvalidRequest` (HTTP 400) so a managed-SSE + /// request against an unconfigured server does not report 500 (rustfs#4844). pub fn new_for_local_sse() -> Result { + fn sse_not_configured(message: impl Into) -> ApiError { + ApiError { + code: S3ErrorCode::InvalidRequest, + message: message.into(), + source: None, + } + } + let Some(raw_value) = get_env_opt_str("RUSTFS_SSE_S3_MASTER_KEY").filter(|value| !value.trim().is_empty()) else { - return Err(ApiError::from(StorageError::other( + return Err(sse_not_configured( "SSE-S3 requires RUSTFS_SSE_S3_MASTER_KEY to be set to a base64-encoded 32-byte key when KMS is not configured", - ))); + )); }; let decoded = BASE64_STANDARD.decode(raw_value.trim()).map_err(|err| { - ApiError::from(StorageError::other(format!( + sse_not_configured(format!( "RUSTFS_SSE_S3_MASTER_KEY must be valid base64 for SSE-S3 when KMS is not configured: {err}" - ))) + )) })?; let master_key: [u8; 32] = decoded.try_into().map_err(|_| { - ApiError::from(StorageError::other( - "RUSTFS_SSE_S3_MASTER_KEY must decode to exactly 32 bytes for SSE-S3 when KMS is not configured", - )) + sse_not_configured("RUSTFS_SSE_S3_MASTER_KEY must decode to exactly 32 bytes for SSE-S3 when KMS is not configured") })?; tracing::info!("Using RUSTFS_SSE_S3_MASTER_KEY for SSE-S3 (KMS not configured)"); @@ -3916,6 +3926,11 @@ mod tests { .expect_err("SSE-S3 should fail closed without a configured local master key"); assert!(err.message.contains("RUSTFS_SSE_S3_MASTER_KEY")); + assert_eq!( + err.code, + S3ErrorCode::InvalidRequest, + "missing local SSE master key is a configuration error, not a 500 (rustfs#4844)" + ); }, ) .await; @@ -3947,6 +3962,11 @@ mod tests { .expect_err("SSE-S3 should fail closed with an invalid local master key"); assert!(err.message.contains("valid base64")); + assert_eq!( + err.code, + S3ErrorCode::InvalidRequest, + "invalid local SSE master key is a configuration error, not a 500 (rustfs#4844)" + ); }, ) .await;