diff --git a/crates/e2e_test/src/kms/copy_object_self_copy_sse_test.rs b/crates/e2e_test/src/kms/copy_object_self_copy_sse_test.rs new file mode 100644 index 000000000..85abf316e --- /dev/null +++ b/crates/e2e_test/src/kms/copy_object_self_copy_sse_test.rs @@ -0,0 +1,351 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Regression test: a same-key CopyObject that only rewrites metadata must never re-key a +//! managed-SSE (SSE-S3 / SSE-KMS) object. +//! +//! On an **unversioned** bucket the handler marks a same-name copy `metadata_only`, and the +//! store layer then updates `xl.meta` in place without touching the data blocks. The handler +//! nevertheless strips the source encryption metadata and generates a *fresh* DEK for the +//! destination. Combining the two writes "new DEK + old ciphertext": the object is permanently +//! undecryptable. The fix forces a full data rewrite whenever the copy re-derives managed +//! encryption material, so the stored bytes always match the key metadata beside them. +//! +//! Companion to `copy_object_version_restore_sse_test` (issue #4238), which pins the same +//! invariant for the versioned historical-restore path. + +use super::common::{LocalKMSTestEnvironment, create_key_with_specific_id}; +use crate::common::init_logging; +use aws_sdk_s3::primitives::ByteStream; +use aws_sdk_s3::types::{ + MetadataDirective, ServerSideEncryption, ServerSideEncryptionByDefault, ServerSideEncryptionConfiguration, + ServerSideEncryptionRule, +}; +use serial_test::serial; +use tracing::info; + +#[tokio::test] +#[serial] +async fn test_metadata_replace_self_copy_of_sse_object_stays_decryptable() { + init_logging(); + info!("same-key CopyObject with REPLACE metadata must not re-key an SSE-S3 object"); + + let mut kms_env = LocalKMSTestEnvironment::new().await.expect("failed to create local KMS env"); + let default_key_id = "rustfs-e2e-test-default-key"; + let keys_dir = kms_env.kms_keys_dir.clone(); + create_key_with_specific_id(&keys_dir, default_key_id) + .await + .expect("failed to create local KMS key"); + kms_env + .base_env + .start_rustfs_server_with_env( + vec![ + "--kms-enable", + "--kms-backend", + "local", + "--kms-key-dir", + &keys_dir, + "--kms-default-key-id", + default_key_id, + ], + &[("RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS", "true")], + ) + .await + .expect("failed to start RustFS with local KMS"); + tokio::time::sleep(tokio::time::Duration::from_secs(3)).await; + + let client = kms_env.base_env.create_s3_client(); + // Deliberately an UNVERSIONED bucket: that is the branch where the store layer can service + // the self-copy as a pure metadata update. + let bucket = "copy-object-self-copy-sse-test"; + let key = "secrets/report.txt"; + + client + .create_bucket() + .bucket(bucket) + .send() + .await + .expect("failed to create bucket"); + + // Content long enough that a truncated/garbled decrypt cannot coincidentally match. + let content = b"encrypted payload that must survive a metadata-only self copy -- 0123456789"; + let put = client + .put_object() + .bucket(bucket) + .key(key) + .content_type("text/plain; charset=utf-8") + .metadata("stage", "before") + .server_side_encryption(ServerSideEncryption::Aes256) + .body(ByteStream::from_static(content)) + .send() + .await + .expect("PUT failed"); + assert_eq!(put.server_side_encryption(), Some(&ServerSideEncryption::Aes256)); + + // Copy the object onto itself, replacing user metadata. This is the `mc cp --attr` / + // "edit metadata in place" shape that AWS supports on an existing object. + let copy_out = client + .copy_object() + .bucket(bucket) + .key(key) + .copy_source(format!("{bucket}/{key}")) + .metadata_directive(MetadataDirective::Replace) + .content_type("text/plain; charset=utf-8") + .metadata("stage", "after") + .server_side_encryption(ServerSideEncryption::Aes256) + .send() + .await + .expect("same-key CopyObject with REPLACE metadata must succeed"); + assert_eq!(copy_out.server_side_encryption(), Some(&ServerSideEncryption::Aes256)); + + // The object must still decrypt to the original plaintext. Before the fix the stored + // ciphertext was left untouched while the metadata carried a brand-new DEK, so this GET + // either failed outright or returned garbage. + let get = client + .get_object() + .bucket(bucket) + .key(key) + .send() + .await + .expect("GET after self-copy failed: the object was re-keyed without rewriting the ciphertext"); + assert_eq!(get.server_side_encryption(), Some(&ServerSideEncryption::Aes256)); + assert_eq!( + get.metadata().and_then(|m| m.get("stage")), + Some(&"after".to_string()), + "REPLACE metadata must take effect" + ); + let body = get.body.collect().await.expect("collect body").into_bytes(); + assert_eq!( + body.as_ref(), + content, + "object must still decrypt to the original plaintext after a metadata-only self copy" + ); + + kms_env.base_env.stop_server(); +} + +#[tokio::test] +#[serial] +async fn test_metadata_replace_self_copy_dropping_sse_rewrites_plaintext() { + init_logging(); + info!("same-key CopyObject that drops SSE must rewrite the data, not orphan the ciphertext"); + + let mut kms_env = LocalKMSTestEnvironment::new().await.expect("failed to create local KMS env"); + let default_key_id = "rustfs-e2e-test-default-key"; + let keys_dir = kms_env.kms_keys_dir.clone(); + create_key_with_specific_id(&keys_dir, default_key_id) + .await + .expect("failed to create local KMS key"); + kms_env + .base_env + .start_rustfs_server_with_env( + vec![ + "--kms-enable", + "--kms-backend", + "local", + "--kms-key-dir", + &keys_dir, + "--kms-default-key-id", + default_key_id, + ], + &[("RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS", "true")], + ) + .await + .expect("failed to start RustFS with local KMS"); + tokio::time::sleep(tokio::time::Duration::from_secs(3)).await; + + let client = kms_env.base_env.create_s3_client(); + // Unversioned, and deliberately WITHOUT a bucket default-encryption rule, so the copy below + // resolves to "no destination encryption". + let bucket = "copy-object-self-copy-drop-sse-test"; + let key = "secrets/report.txt"; + + client + .create_bucket() + .bucket(bucket) + .send() + .await + .expect("failed to create bucket"); + + let content = b"encrypted payload whose ciphertext must not survive as bogus plaintext -- 0123456789"; + client + .put_object() + .bucket(bucket) + .key(key) + .metadata("stage", "before") + .server_side_encryption(ServerSideEncryption::Aes256) + .body(ByteStream::from_static(content)) + .send() + .await + .expect("PUT failed"); + + // Self-copy with REPLACE and no SSE header. Per AWS semantics the destination ends up + // unencrypted. The dangerous outcome is the silent one: the handler strips the source key + // metadata while a metadata-only copy leaves the ciphertext in place, so a later GET would + // hand back raw ciphertext as if it were plaintext — corruption with no error anywhere. + client + .copy_object() + .bucket(bucket) + .key(key) + .copy_source(format!("{bucket}/{key}")) + .metadata_directive(MetadataDirective::Replace) + .metadata("stage", "after") + .send() + .await + .expect("same-key CopyObject dropping SSE must succeed"); + + let get = client + .get_object() + .bucket(bucket) + .key(key) + .send() + .await + .expect("GET after self-copy failed"); + assert_eq!( + get.server_side_encryption(), + None, + "destination must be unencrypted once the copy drops SSE" + ); + assert_eq!( + get.metadata().and_then(|m| m.get("stage")), + Some(&"after".to_string()), + "REPLACE metadata must take effect" + ); + let body = get.body.collect().await.expect("collect body").into_bytes(); + assert_eq!( + body.as_ref(), + content, + "object must read back as the original plaintext, not the orphaned ciphertext" + ); + + kms_env.base_env.stop_server(); +} + +#[tokio::test] +#[serial] +async fn test_metadata_replace_self_copy_under_bucket_default_sse_stays_decryptable() { + init_logging(); + info!("bucket default encryption must also keep a same-key copy off the metadata-only path"); + + let mut kms_env = LocalKMSTestEnvironment::new().await.expect("failed to create local KMS env"); + let default_key_id = "rustfs-e2e-test-default-key"; + let keys_dir = kms_env.kms_keys_dir.clone(); + create_key_with_specific_id(&keys_dir, default_key_id) + .await + .expect("failed to create local KMS key"); + kms_env + .base_env + .start_rustfs_server_with_env( + vec![ + "--kms-enable", + "--kms-backend", + "local", + "--kms-key-dir", + &keys_dir, + "--kms-default-key-id", + default_key_id, + ], + &[("RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS", "true")], + ) + .await + .expect("failed to start RustFS with local KMS"); + tokio::time::sleep(tokio::time::Duration::from_secs(3)).await; + + let client = kms_env.base_env.create_s3_client(); + let bucket = "copy-object-self-copy-bucket-default-sse-test"; + let key = "secrets/report.txt"; + + client + .create_bucket() + .bucket(bucket) + .send() + .await + .expect("failed to create bucket"); + + // Store the object as PLAINTEXT first: no SSE header and no bucket default rule yet. This is + // what makes the case sharp — at copy time the source metadata carries no encryption markers, + // so the source-side half of the guard cannot fire. + let content = b"plaintext payload that must not be orphaned under a new DEK -- 0123456789"; + let put = client + .put_object() + .bucket(bucket) + .key(key) + .metadata("stage", "before") + .body(ByteStream::from_static(content)) + .send() + .await + .expect("PUT failed"); + assert_eq!(put.server_side_encryption(), None, "the object must start out unencrypted"); + + // Only NOW enable bucket default encryption. The destination's encryption therefore comes + // from the bucket rule and from nowhere else: the source is unencrypted and the copy request + // carries no SSE header. A guard that only inspects request headers (MinIO decides + // `isTargetEncrypted` from `crypto.S3.IsRequested(r.Header)`) would let this through, yet + // `sse_encryption` still mints a fresh DEK from the resolved bucket default — which is why + // the guard keys off the *effective* encryption rather than the requested one. + let encryption_config = ServerSideEncryptionConfiguration::builder() + .rules( + ServerSideEncryptionRule::builder() + .apply_server_side_encryption_by_default( + ServerSideEncryptionByDefault::builder() + .sse_algorithm(ServerSideEncryption::Aes256) + .build() + .unwrap(), + ) + .build(), + ) + .build() + .unwrap(); + client + .put_bucket_encryption() + .bucket(bucket) + .server_side_encryption_configuration(encryption_config) + .send() + .await + .expect("failed to set bucket default encryption"); + + // No SSE header on the copy — the bucket default alone drives the destination encryption. + client + .copy_object() + .bucket(bucket) + .key(key) + .copy_source(format!("{bucket}/{key}")) + .metadata_directive(MetadataDirective::Replace) + .metadata("stage", "after") + .send() + .await + .expect("same-key CopyObject under bucket default encryption must succeed"); + + let get = client + .get_object() + .bucket(bucket) + .key(key) + .send() + .await + .expect("GET after self-copy failed: the object was re-keyed without rewriting the ciphertext"); + assert_eq!(get.server_side_encryption(), Some(&ServerSideEncryption::Aes256)); + assert_eq!( + get.metadata().and_then(|m| m.get("stage")), + Some(&"after".to_string()), + "REPLACE metadata must take effect" + ); + let body = get.body.collect().await.expect("collect body").into_bytes(); + assert_eq!( + body.as_ref(), + content, + "object must still decrypt to the original plaintext after a metadata-only self copy" + ); + + kms_env.base_env.stop_server(); +} diff --git a/crates/e2e_test/src/kms/mod.rs b/crates/e2e_test/src/kms/mod.rs index dd371480d..5b0cad360 100644 --- a/crates/e2e_test/src/kms/mod.rs +++ b/crates/e2e_test/src/kms/mod.rs @@ -48,6 +48,9 @@ mod bucket_default_encryption_test; #[cfg(test)] mod encryption_metadata_test; +#[cfg(test)] +mod copy_object_self_copy_sse_test; + #[cfg(test)] mod copy_object_version_restore_sse_test; diff --git a/crates/ecstore/src/store/object.rs b/crates/ecstore/src/store/object.rs index 2cacb4465..6ffed22b7 100644 --- a/crates/ecstore/src/store/object.rs +++ b/crates/ecstore/src/store/object.rs @@ -969,6 +969,15 @@ impl ECStore { if !dst_opts.versioned && src_opts.version_id.is_none() { if src_info.metadata_only { + // Zero-copy update: only xl.meta is rewritten, the data blocks stay as they + // are. The caller must therefore guarantee that the destination metadata + // still describes the stored bytes. In particular a copy that re-derives + // encryption material may NOT set metadata_only — that would leave a fresh + // DEK beside ciphertext sealed under the old one, permanently destroying the + // object. The S3 handler enforces this before calling in (see the + // metadata_only decision in rustfs/src/app/object_usecase.rs); the sibling + // versioned branch below resolves the same risk by rewriting through + // put_object (issue #4238). return self.pools[pool_idx] .copy_object(src_bucket, &src_object, dst_bucket, &dst_object, src_info, src_opts, &dst_opts) .await; diff --git a/rustfs/src/app/object_usecase.rs b/rustfs/src/app/object_usecase.rs index 3a2be7dee..265fe3afc 100644 --- a/rustfs/src/app/object_usecase.rs +++ b/rustfs/src/app/object_usecase.rs @@ -6344,7 +6344,35 @@ impl DefaultObjectUsecase { return Err(s3_error!(PreconditionFailed)); } - if cp_src_dst_same && src_info.transitioned_object.tier.is_empty() { + // A same-name copy is normally serviced as a metadata-only update: the store layer + // rewrites xl.meta in place and leaves the data blocks alone. That shortcut is only sound + // when the destination's physical bytes are identical to the source's, and encryption + // breaks exactly that. The destination metadata is rebuilt from scratch below — + // `strip_managed_encryption_metadata` drops the source DEK and `sse_encryption` mints a + // fresh one — so reusing the stored ciphertext would leave a new DEK sitting beside bytes + // it cannot decrypt, permanently destroying the object (GET fails with an AEAD tag + // mismatch). The mirror case is worse because it is silent: an encrypted source copied + // without any destination SSE keeps its ciphertext while losing the key metadata, so GET + // hands back raw ciphertext as if it were plaintext. So whenever either side is + // encrypted, leave metadata_only = false and let the store layer do a full read/write + // rewrite through put_object, the same resolution the versioned historical-restore path + // uses (issue #4238, crates/ecstore/src/store/object.rs). + // + // This mirrors MinIO's `isSourceEncrypted || isTargetEncrypted -> metadataOnly = false` + // in CopyObjectHandler, with one deliberate difference: MinIO decides "target encrypted" + // from request headers alone, while `effective_sse` here also resolves the bucket default + // encryption rule. `sse_encryption` mints a DEK from that resolved value, so a + // header-only check would miss a self-copy under a bucket default rule. The source half + // deliberately reuses `ObjectInfo::is_encrypted` rather than naming individual headers, + // so a future encryption flavour is covered here the moment it is recognised there. + // + // The zero-copy shortcut is only recoverable for encrypted objects by *preserving* the + // DEK that sealed the bytes and re-wrapping it under a new master key (MinIO's + // `rotateKey` + `keyRotation` flag, which is why it may keep metadataOnly = true). RustFS + // has no such rewrap primitive today; adding one is backlog#1637, and it would enter here + // as an explicit exception rather than by relaxing this guard. + let copy_changes_encryption = src_info.is_encrypted() || effective_sse.is_some() || has_explicit_ssec; + if cp_src_dst_same && src_info.transitioned_object.tier.is_empty() && !copy_changes_encryption { src_info.metadata_only = true; } diff --git a/rustfs/src/storage/sse.rs b/rustfs/src/storage/sse.rs index 83b540147..7f1ada523 100644 --- a/rustfs/src/storage/sse.rs +++ b/rustfs/src/storage/sse.rs @@ -3430,6 +3430,7 @@ mod tests { DARE_CIPHER_AES_256_GCM, DARE_CIPHER_CHACHA20_POLY1305, MINIO_INTERNAL_ENCRYPTION_SEAL_ALGORITHM, SEALED_KEY_IV_SIZE, SEALED_KEY_SIZE, is_legacy_rustfs_managed_metadata, is_supported_sealed_object_key_cipher, }; + use rustfs_utils::http::headers::SSEC_ALGORITHM_HEADER; #[test] fn parse_simple_sse_cmk_rejects_bad_keys_without_crashing() { @@ -4583,6 +4584,89 @@ mod tests { assert!(metadata.contains_key("content-type")); } + /// A same-name CopyObject is normally serviced as a metadata-only update that reuses the + /// stored ciphertext, while the handler rebuilds the destination metadata around a *fresh* + /// DEK. `ObjectInfo::is_encrypted` — i.e. `is_object_encryption_marker` — is the guard that + /// keeps that shortcut away from encrypted objects (see the `metadata_only` decision in + /// `rustfs/src/app/object_usecase.rs`). Any encryption flavour whose headers + /// `strip_managed_encryption_metadata` drops must therefore also be *detected* there — a + /// flavour that is stripped but not detected would resurrect "new DEK + old ciphertext", + /// which leaves the object permanently undecryptable. + #[test] + fn every_strippable_encryption_shape_is_detected_as_encrypted() { + let shapes: [(&str, Vec<(&str, &str)>); 4] = [ + ( + "sse-s3", + vec![ + ("x-amz-server-side-encryption", "AES256"), + (MINIO_INTERNAL_ENCRYPTION_S3_SEALED_KEY_HEADER, "sealed"), + (MINIO_INTERNAL_ENCRYPTION_IV_HEADER, "iv"), + ], + ), + ( + "sse-kms", + vec![ + ("x-amz-server-side-encryption", "aws:kms"), + (MINIO_INTERNAL_ENCRYPTION_KMS_SEALED_KEY_HEADER, "sealed"), + (MINIO_INTERNAL_ENCRYPTION_KMS_KEY_ID_HEADER, "key-1"), + ], + ), + ( + "legacy rustfs managed", + vec![ + ("x-amz-server-side-encryption", "AES256"), + (INTERNAL_ENCRYPTION_KEY_HEADER, "wrapped"), + (INTERNAL_ENCRYPTION_IV_HEADER, "iv"), + ], + ), + ( + "sse-c", + vec![ + (SSEC_ALGORITHM_HEADER, "AES256"), + (MINIO_INTERNAL_ENCRYPTION_SSEC_SEALED_KEY_HEADER, "sealed"), + ], + ), + ]; + + for (label, entries) in shapes { + let metadata: HashMap = entries + .iter() + .map(|(key, value)| ((*key).to_string(), (*value).to_string())) + .collect(); + + assert!( + metadata + .keys() + .any(|key| rustfs_utils::http::is_object_encryption_marker(key)), + "{label}: an encrypted object must be detected, otherwise a same-name copy would re-key it \ + without rewriting the ciphertext" + ); + + let mut stripped = metadata.clone(); + strip_managed_encryption_metadata(&mut stripped); + assert_ne!( + stripped, metadata, + "{label}: the copy path strips this shape's encryption metadata, so the destination cannot \ + reuse the source ciphertext" + ); + } + } + + #[test] + fn plain_object_metadata_is_not_flagged_as_encrypted() { + // The metadata-only self-copy shortcut must stay available for unencrypted objects. + let metadata: HashMap = [("content-type", "text/plain"), ("x-amz-meta-stage", "before")] + .iter() + .map(|(key, value)| ((*key).to_string(), (*value).to_string())) + .collect(); + + assert!( + !metadata + .keys() + .any(|key| rustfs_utils::http::is_object_encryption_marker(key)) + ); + } + #[cfg(feature = "rio-v2")] #[test] fn test_legacy_managed_metadata_excludes_sealed_keys() {