From 1872bdcedd37bec58cbf6c8bacdba94f8a649f28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E6=AD=A3=E8=B6=85?= Date: Sat, 28 Feb 2026 22:56:35 +0800 Subject: [PATCH] fix(s3): reject SSE-C with partial headers per S3 spec (#2007) --- rustfs/src/app/object_usecase.rs | 16 +++ rustfs/src/storage/mod.rs | 5 +- rustfs/src/storage/sse.rs | 129 +++++++++++++++++++++++ scripts/s3-tests/implemented_tests.txt | 5 +- scripts/s3-tests/unimplemented_tests.txt | 3 - 5 files changed, 152 insertions(+), 6 deletions(-) diff --git a/rustfs/src/app/object_usecase.rs b/rustfs/src/app/object_usecase.rs index 973406551..dfb0485a0 100644 --- a/rustfs/src/app/object_usecase.rs +++ b/rustfs/src/app/object_usecase.rs @@ -306,6 +306,12 @@ impl DefaultObjectUsecase { .. } = input; + // Merge SSE-C params from headers (fallback when S3 layer does not populate input) + let (h_algo, h_key, h_md5) = extract_ssec_params_from_headers(&req.headers)?; + let sse_customer_algorithm = sse_customer_algorithm.or(h_algo); + let sse_customer_key = sse_customer_key.or(h_key); + let sse_customer_key_md5 = sse_customer_key_md5.or(h_md5); + // Validate object key validate_object_key(&key, "PUT")?; @@ -387,6 +393,16 @@ impl DefaultObjectUsecase { }) }); + // Validate SSE-C headers early: reject partial/invalid combinations per S3 spec + validate_sse_headers_for_write( + effective_sse.as_ref(), + effective_kms_key_id.as_ref(), + sse_customer_algorithm.as_ref(), + sse_customer_key.as_ref(), + sse_customer_key_md5.as_ref(), + true, // PutObject requires all three: algorithm, key, key_md5 + )?; + let mut metadata = metadata.unwrap_or_default(); let owner = req .extensions diff --git a/rustfs/src/storage/mod.rs b/rustfs/src/storage/mod.rs index 49f9db5cb..63a45a007 100644 --- a/rustfs/src/storage/mod.rs +++ b/rustfs/src/storage/mod.rs @@ -35,6 +35,7 @@ mod sse_test; pub(crate) use ecfs_extend::*; pub(crate) use sse::{ - DecryptionRequest, EncryptionRequest, PrepareEncryptionRequest, sse_decryption, sse_encryption, sse_prepare_encryption, - strip_managed_encryption_metadata, validate_sse_headers_for_read, validate_ssec_for_read, + DecryptionRequest, EncryptionRequest, PrepareEncryptionRequest, extract_ssec_params_from_headers, sse_decryption, + sse_encryption, sse_prepare_encryption, strip_managed_encryption_metadata, validate_sse_headers_for_read, + validate_sse_headers_for_write, validate_ssec_for_read, }; diff --git a/rustfs/src/storage/sse.rs b/rustfs/src/storage/sse.rs index ee8887594..83590cf80 100644 --- a/rustfs/src/storage/sse.rs +++ b/rustfs/src/storage/sse.rs @@ -343,6 +343,42 @@ fn sse_invalid_argument(message: &str) -> ApiError { } } +/// SSE-C parameters extracted from headers (algorithm, key, key MD5). +pub(crate) type SsecParamsFromHeaders = (Option, Option, Option); + +/// Extract SSE-C parameters from request headers. +/// Used as fallback when the S3 layer does not populate them in the input struct. +/// +/// Returns an error if an SSE-C header is present but cannot be parsed as valid UTF-8, +/// ensuring malformed headers do not bypass validation. +pub(crate) fn extract_ssec_params_from_headers(headers: &HeaderMap) -> Result { + let algorithm = match headers.get("x-amz-server-side-encryption-customer-algorithm") { + None => Ok(None), + Some(v) => v + .to_str() + .map(|s| Some(SSECustomerAlgorithm::from(s.to_string()))) + .map_err(|_| sse_invalid_argument("The x-amz-server-side-encryption-customer-algorithm header must be valid UTF-8.")), + }?; + + let key = match headers.get("x-amz-server-side-encryption-customer-key") { + None => Ok(None), + Some(v) => v + .to_str() + .map(|s| Some(SSECustomerKey::from(s.to_string()))) + .map_err(|_| sse_invalid_argument("The x-amz-server-side-encryption-customer-key header must be valid UTF-8.")), + }?; + + let key_md5 = match headers.get("x-amz-server-side-encryption-customer-key-md5") { + None => Ok(None), + Some(v) => v + .to_str() + .map(|s| Some(SSECustomerKeyMD5::from(s.to_string()))) + .map_err(|_| sse_invalid_argument("The x-amz-server-side-encryption-customer-key-md5 header must be valid UTF-8.")), + }?; + + Ok((algorithm, key, key_md5)) +} + #[inline] pub(crate) fn validate_sse_headers_for_write( server_side_encryption: Option<&ServerSideEncryption>, @@ -1674,6 +1710,99 @@ fn ssec_invalid_request(message: &str) -> ApiError { #[cfg(test)] mod tests { use super::*; + use http::HeaderValue; + + #[test] + fn test_extract_ssec_params_from_headers() { + let mut headers = http::HeaderMap::new(); + let (algo, key, md5) = extract_ssec_params_from_headers(&headers).unwrap(); + assert!(algo.is_none()); + assert!(key.is_none()); + assert!(md5.is_none()); + + headers.insert("x-amz-server-side-encryption-customer-algorithm", HeaderValue::from_static("AES256")); + let (algo, key, md5) = extract_ssec_params_from_headers(&headers).unwrap(); + assert_eq!(algo.as_deref(), Some("AES256")); + assert!(key.is_none()); + assert!(md5.is_none()); + + headers.insert( + "x-amz-server-side-encryption-customer-key", + HeaderValue::from_static("pO3upElrwuEXSoFwCfnZPdSsmt/xWeFa0N9KgDijwVs="), + ); + headers.insert( + "x-amz-server-side-encryption-customer-key-md5", + HeaderValue::from_static("DWygnHRtgiJ77HCm+1rvHw=="), + ); + let (algo, key, md5) = extract_ssec_params_from_headers(&headers).unwrap(); + assert_eq!(algo.as_deref(), Some("AES256")); + assert!(key.is_some()); + assert!(md5.is_some()); + } + + #[test] + fn test_extract_ssec_params_from_headers_rejects_invalid_utf8() { + let mut headers = http::HeaderMap::new(); + // Header value with invalid UTF-8; to_str() will fail + let invalid_utf8 = HeaderValue::from_bytes(b"invalid-\x80-utf8").unwrap(); + headers.insert("x-amz-server-side-encryption-customer-algorithm", invalid_utf8); + let result = extract_ssec_params_from_headers(&headers); + assert!(result.is_err()); + let err = result.unwrap_err(); + assert_eq!(err.code, S3ErrorCode::InvalidArgument); + } + + #[test] + fn test_validate_sse_headers_for_write_rejects_algorithm_without_key() { + let algorithm = SSECustomerAlgorithm::from("AES256".to_string()); + let result = validate_sse_headers_for_write( + None, + None, + Some(&algorithm), + None, + None, + true, // PutObject requires all three + ); + assert!(result.is_err()); + let err = result.unwrap_err(); + assert_eq!(err.code, S3ErrorCode::InvalidRequest); + } + + #[test] + fn test_validate_sse_headers_for_write_rejects_algorithm_and_key_without_md5() { + let algorithm = SSECustomerAlgorithm::from("AES256".to_string()); + let key = SSECustomerKey::from("pO3upElrwuEXSoFwCfnZPdSsmt/xWeFa0N9KgDijwVs=".to_string()); + let result = validate_sse_headers_for_write( + None, + None, + Some(&algorithm), + Some(&key), + None, + true, // PutObject requires all three + ); + assert!(result.is_err()); + let err = result.unwrap_err(); + assert_eq!(err.code, S3ErrorCode::InvalidRequest); + } + + #[test] + fn test_validate_sse_headers_for_write_rejects_ssec_with_managed_sse() { + let algorithm = SSECustomerAlgorithm::from("AES256".to_string()); + let key = SSECustomerKey::from("pO3upElrwuEXSoFwCfnZPdSsmt/xWeFa0N9KgDijwVs=".to_string()); + let key_md5 = SSECustomerKeyMD5::from("DWygnHRtgiJ77HCm+1rvHw==".to_string()); + let server_side_encryption = ServerSideEncryption::from_static(ServerSideEncryption::AES256); + let result = validate_sse_headers_for_write( + Some(&server_side_encryption), + None, + Some(&algorithm), + Some(&key), + Some(&key_md5), + true, + ); + assert!(result.is_err()); + let err = result.unwrap_err(); + assert_eq!(err.code, S3ErrorCode::InvalidArgument); + } #[test] fn test_is_managed_sse() { diff --git a/scripts/s3-tests/implemented_tests.txt b/scripts/s3-tests/implemented_tests.txt index 574fae670..7e147d0fd 100644 --- a/scripts/s3-tests/implemented_tests.txt +++ b/scripts/s3-tests/implemented_tests.txt @@ -20,7 +20,7 @@ # - SSE-C: Server-side encryption with customer-provided keys # - Object ownership: Bucket ownership controls # -# Total: 174 tests +# Total: 177 tests test_basic_key_count test_bucket_create_naming_bad_short_one @@ -160,6 +160,9 @@ test_get_checksum_object_attributes test_encryption_sse_c_method_head test_encryption_sse_c_present test_encryption_sse_c_other_key +test_encryption_sse_c_no_key +test_encryption_sse_c_no_md5 +test_put_obj_enc_conflict_c_s3 # ListObjectsV2 delimiter and encoding tests test_bucket_list_encoding_basic diff --git a/scripts/s3-tests/unimplemented_tests.txt b/scripts/s3-tests/unimplemented_tests.txt index 7efc145eb..8982eba62 100644 --- a/scripts/s3-tests/unimplemented_tests.txt +++ b/scripts/s3-tests/unimplemented_tests.txt @@ -28,8 +28,6 @@ test_copy_part_enc test_encryption_key_no_sse_c test_encryption_sse_c_invalid_md5 test_encryption_sse_c_multipart_bad_download -test_encryption_sse_c_no_key -test_encryption_sse_c_no_md5 test_get_versioned_object_attributes test_lifecycle_delete test_lifecycle_expiration_header_put @@ -66,7 +64,6 @@ test_put_bucket_logging_permissions test_put_bucket_logging_policy_wildcard test_put_obj_enc_conflict_bad_enc_kms test_put_obj_enc_conflict_c_kms -test_put_obj_enc_conflict_c_s3 test_put_obj_enc_conflict_s3_kms test_rm_bucket_logging test_sse_kms_no_key