From 63027cd833bb9965990c0df761f3486071336d20 Mon Sep 17 00:00:00 2001 From: overtrue Date: Wed, 19 Aug 2026 09:19:19 +0800 Subject: [PATCH] refactor(app): resolve the bucket default SSE in one place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PUT, COPY, and extract each walked the bucket encryption configuration themselves to fill in an unrequested SSE algorithm and KMS key. That duplication is what let the unknown-algorithm fallback diverge in the first place (#6022 fixed the divergence; this removes the room for it). `resolve_bucket_default_sse` now returns the `(sse, kms_key_id)` pair for all three. A request-level value still wins, the unknown-algorithm fallback stays in `bucket_default_write_sse`, and PUT keeps its `ciphertext_passthrough` override at the call site because that clears both values after resolution rather than participating in it. `has_explicit_ssec` is passed per caller — `true` from COPY, `false` from PUT and extract — because that is what each does today. The resulting divergence is real: PUT hands the resolved default to `validate_sse_headers_for_write`, which rejects SSE-C and managed headers together, so an SSE-C PUT into a bucket with default encryption fails where S3 would let the request win. That is a behaviour change on a P0 encryption path, so it is recorded on backlog#1826 for its own PR rather than folded into this refactor. Refs backlog#1826 --- rustfs/src/app/object_usecase.rs | 192 +++++++++++++++++++------------ 1 file changed, 121 insertions(+), 71 deletions(-) diff --git a/rustfs/src/app/object_usecase.rs b/rustfs/src/app/object_usecase.rs index b63110d39..55d863588 100644 --- a/rustfs/src/app/object_usecase.rs +++ b/rustfs/src/app/object_usecase.rs @@ -169,8 +169,8 @@ use s3s::dto::{ ObjectLockLegalHoldStatus, ObjectLockMode, ObjectLockRetention, ObjectLockRetentionMode, ObjectPart, PutObjectInput, PutObjectOutput, Range, RequestCharged, RestoreObjectInput, RestoreObjectOutput, RestoreStatus, SSECustomerAlgorithm, SSECustomerKeyMD5, SSEKMSKeyId, SelectObjectContentInput, SelectObjectContentOutput, ServerSideEncryption, - ServerSideEncryptionByDefault, StorageClass, StreamingBlob, TaggingDirective, TaggingHeader, Timestamp, TimestampFormat, - WebsiteRedirectLocation, + ServerSideEncryptionByDefault, ServerSideEncryptionConfiguration, StorageClass, StreamingBlob, TaggingDirective, + TaggingHeader, Timestamp, TimestampFormat, WebsiteRedirectLocation, }; use s3s::header::{X_AMZ_RESTORE, X_AMZ_RESTORE_OUTPUT_PATH}; use s3s::stream::{ByteStream, DynByteStream, RemainingLength}; @@ -2695,6 +2695,39 @@ fn bucket_default_write_sse(sse: &ServerSideEncryptionByDefault) -> ServerSideEn } } +/// Resolve the effective server-side encryption for a write against the bucket's +/// default encryption configuration. +/// +/// A request-level value always wins; the bucket default only fills a gap, and +/// the unknown-algorithm fallback lives once in [`bucket_default_write_sse`]. +/// +/// `has_explicit_ssec` suppresses the default entirely. Only COPY passes `true` +/// today: its destination may carry SSE-C, which must not also be given managed +/// encryption. PUT and extract pass `false`, matching their current behaviour — +/// see backlog#1826 for the divergence that leaves. +/// +/// Callers layering further overrides (PUT's `ciphertext_passthrough`) apply +/// them to the returned pair. +fn resolve_bucket_default_sse( + bucket_sse_config: Option<&ServerSideEncryptionConfiguration>, + requested_sse: Option, + requested_kms_key_id: Option, + has_explicit_ssec: bool, +) -> (Option, Option) { + let bucket_default = || { + if has_explicit_ssec { + return None; + } + bucket_sse_config + .and_then(|config| config.rules.first()) + .and_then(|rule| rule.apply_server_side_encryption_by_default.as_ref()) + }; + + let effective_sse = requested_sse.or_else(|| bucket_default().map(bucket_default_write_sse)); + let effective_kms_key_id = requested_kms_key_id.or_else(|| bucket_default().and_then(|sse| sse.kms_master_key_id.clone())); + (effective_sse, effective_kms_key_id) +} + fn should_use_small_eager_put_path( size: i64, headers: &HeaderMap, @@ -5823,19 +5856,12 @@ impl DefaultObjectUsecase { ); let original_sse = server_side_encryption.clone(); - let mut effective_sse = server_side_encryption.or_else(|| { - bucket_sse_config.as_ref().and_then(|(config, _timestamp)| { - config.rules.first().and_then(|rule| { - rule.apply_server_side_encryption_by_default.as_ref().map(|sse| { - match sse.sse_algorithm.as_str() { - "AES256" => ServerSideEncryption::from_static(ServerSideEncryption::AES256), - "aws:kms" => ServerSideEncryption::from_static(ServerSideEncryption::AWS_KMS), - _ => ServerSideEncryption::from_static(ServerSideEncryption::AES256), // fallback to AES256 - } - }) - }) - }) - }); + let (mut effective_sse, mut effective_kms_key_id) = resolve_bucket_default_sse( + bucket_sse_config.as_ref().map(|(config, _timestamp)| config), + server_side_encryption, + ssekms_key_id, + false, + ); debug!( target: "rustfs::app::object_usecase", component = "app", @@ -5847,16 +5873,6 @@ impl DefaultObjectUsecase { "Resolved effective SSE configuration" ); - let mut effective_kms_key_id = ssekms_key_id.or_else(|| { - bucket_sse_config.as_ref().and_then(|(config, _timestamp)| { - config.rules.first().and_then(|rule| { - rule.apply_server_side_encryption_by_default - .as_ref() - .and_then(|sse| sse.kms_master_key_id.clone()) - }) - }) - }); - if ciphertext_passthrough { // The replica keeps the source's SSE-C metadata; the bucket // default must not claim managed encryption on it. @@ -7658,30 +7674,12 @@ impl DefaultObjectUsecase { } }; - let mut effective_sse = requested_sse.or_else(|| { - if has_explicit_ssec { - return None; - } - bucket_sse_config.as_ref().and_then(|(config, _)| { - config.rules.first().and_then(|rule| { - rule.apply_server_side_encryption_by_default - .as_ref() - .map(bucket_default_write_sse) - }) - }) - }); - let mut effective_kms_key_id = requested_kms_key_id.or_else(|| { - if has_explicit_ssec { - return None; - } - bucket_sse_config.as_ref().and_then(|(config, _)| { - config.rules.first().and_then(|rule| { - rule.apply_server_side_encryption_by_default - .as_ref() - .and_then(|sse| sse.kms_master_key_id.clone()) - }) - }) - }); + let (mut effective_sse, mut effective_kms_key_id) = resolve_bucket_default_sse( + bucket_sse_config.as_ref().map(|(config, _)| config), + requested_sse, + requested_kms_key_id, + has_explicit_ssec, + ); let h = build_ssec_read_headers( copy_source_sse_customer_algorithm.as_ref(), @@ -9587,28 +9585,12 @@ impl DefaultObjectUsecase { let original_sse = server_side_encryption.or(extract_server_side_encryption_from_headers(&req.headers)?); let bucket_sse_config = metadata_sys::get_sse_config(&bucket).await.ok(); - let mut effective_sse = original_sse.or_else(|| { - bucket_sse_config.as_ref().and_then(|(config, _timestamp)| { - config.rules.first().and_then(|rule| { - rule.apply_server_side_encryption_by_default - .as_ref() - .map(|sse| match sse.sse_algorithm.as_str() { - "AES256" => ServerSideEncryption::from_static(ServerSideEncryption::AES256), - "aws:kms" => ServerSideEncryption::from_static(ServerSideEncryption::AWS_KMS), - _ => ServerSideEncryption::from_static(ServerSideEncryption::AES256), - }) - }) - }) - }); - let mut effective_kms_key_id = ssekms_key_id.or_else(|| { - bucket_sse_config.as_ref().and_then(|(config, _timestamp)| { - config.rules.first().and_then(|rule| { - rule.apply_server_side_encryption_by_default - .as_ref() - .and_then(|sse| sse.kms_master_key_id.clone()) - }) - }) - }); + let (mut effective_sse, mut effective_kms_key_id) = resolve_bucket_default_sse( + bucket_sse_config.as_ref().map(|(config, _timestamp)| config), + original_sse, + ssekms_key_id, + false, + ); if effective_sse .as_ref() .is_some_and(|sse| sse.as_str().eq_ignore_ascii_case(ServerSideEncryption::AWS_KMS)) @@ -10397,6 +10379,74 @@ mod tests { } } + fn bucket_sse_config_with(algorithm: &str, kms_key_id: Option<&str>) -> ServerSideEncryptionConfiguration { + ServerSideEncryptionConfiguration { + rules: vec![ServerSideEncryptionRule { + apply_server_side_encryption_by_default: Some(ServerSideEncryptionByDefault { + sse_algorithm: ServerSideEncryption::from(String::from(algorithm)), + kms_master_key_id: kms_key_id.map(|id| SSEKMSKeyId::from(id.to_string())), + }), + bucket_key_enabled: None, + }], + } + } + + #[test] + fn resolve_bucket_default_sse_prefers_the_request_over_the_bucket_default() { + let config = bucket_sse_config_with(ServerSideEncryption::AWS_KMS, Some("bucket-key")); + + let (sse, kms_key_id) = resolve_bucket_default_sse( + Some(&config), + Some(ServerSideEncryption::from_static(ServerSideEncryption::AES256)), + Some(SSEKMSKeyId::from("request-key".to_string())), + false, + ); + + assert_eq!(sse.as_ref().map(|sse| sse.as_str()), Some(ServerSideEncryption::AES256)); + assert_eq!(kms_key_id.as_deref(), Some("request-key")); + } + + #[test] + fn resolve_bucket_default_sse_fills_gaps_from_the_bucket_default() { + let config = bucket_sse_config_with(ServerSideEncryption::AWS_KMS, Some("bucket-key")); + + let (sse, kms_key_id) = resolve_bucket_default_sse(Some(&config), None, None, false); + + assert_eq!(sse.as_ref().map(|sse| sse.as_str()), Some(ServerSideEncryption::AWS_KMS)); + assert_eq!(kms_key_id.as_deref(), Some("bucket-key")); + } + + #[test] + fn resolve_bucket_default_sse_falls_back_to_aes256_for_an_unknown_algorithm() { + // Reachable only through corrupt or hand-edited bucket metadata; + // PutBucketEncryption rejects unknown algorithms. All three call sites + // now share this single decision (backlog#1826). + let config = bucket_sse_config_with("garbage", None); + + let (sse, kms_key_id) = resolve_bucket_default_sse(Some(&config), None, None, false); + + assert_eq!(sse.as_ref().map(|sse| sse.as_str()), Some(ServerSideEncryption::AES256)); + assert!(kms_key_id.is_none()); + } + + #[test] + fn resolve_bucket_default_sse_suppresses_the_default_for_explicit_ssec() { + let config = bucket_sse_config_with(ServerSideEncryption::AES256, Some("bucket-key")); + + let (sse, kms_key_id) = resolve_bucket_default_sse(Some(&config), None, None, true); + + assert!(sse.is_none(), "an SSE-C destination must not also get managed encryption"); + assert!(kms_key_id.is_none()); + } + + #[test] + fn resolve_bucket_default_sse_returns_nothing_without_a_bucket_default() { + let (sse, kms_key_id) = resolve_bucket_default_sse(None, None, None, false); + + assert!(sse.is_none()); + assert!(kms_key_id.is_none()); + } + #[test] fn put_request_user_metadata_cannot_suppress_bucket_default_retention() { let mut metadata =