fix(ecstore): preserve checksums through write transforms (#5765)

* fix: preserve checksums through write transforms

* test(e2e): cover SSE-KMS multipart CRC32

---------

Co-authored-by: Anthony Martin <949506+anthonymartin@users.noreply.github.com>
This commit is contained in:
anthonymartin
2026-08-06 02:02:08 -07:00
committed by GitHub
parent efd5481b35
commit e26b869259
2 changed files with 150 additions and 30 deletions
@@ -21,9 +21,12 @@
use super::common::LocalKMSTestEnvironment;
use crate::common::{TEST_BUCKET, init_logging};
use aws_sdk_s3::primitives::ByteStream;
use aws_sdk_s3::types::{
ServerSideEncryption, ServerSideEncryptionByDefault, ServerSideEncryptionConfiguration, ServerSideEncryptionRule,
ChecksumAlgorithm, ChecksumMode, CompletedMultipartUpload, CompletedPart, ServerSideEncryption,
ServerSideEncryptionByDefault, ServerSideEncryptionConfiguration, ServerSideEncryptionRule,
};
use rustfs_rio::{Checksum, ChecksumType};
use serial_test::serial;
use tracing::{debug, info, warn};
@@ -273,7 +276,7 @@ async fn test_bucket_default_sse_kms_put_object() -> Result<(), Box<dyn std::err
/// Test 3: When bucket is configured with default encryption, create_multipart_upload should inherit the configuration
#[tokio::test]
#[serial]
async fn test_bucket_default_encryption_multipart_upload() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
async fn test_bucket_default_sse_kms_multipart_crc32() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
init_logging();
info!("Testing bucket default encryption impact on create_multipart_upload");
@@ -309,15 +312,16 @@ async fn test_bucket_default_encryption_multipart_upload() -> Result<(), Box<dyn
.await
.expect("Failed to set bucket encryption");
// Step 2: Create multipart upload (without specifying encryption parameters)
info!("Creating multipart upload (without specifying encryption parameters, should use bucket default configuration)");
let test_key = "test-multipart-bucket-default.txt";
// Step 2: Declare CRC32 without specifying encryption parameters. The AWS SDK
// calculates each UploadPart checksum and sends it as a flexible checksum.
info!("Creating CRC32 multipart upload that should use bucket default encryption");
let test_key = "test-multipart-bucket-default-crc32.bin";
let create_multipart_response = s3_client
.create_multipart_upload()
.bucket(TEST_BUCKET)
.key(test_key)
// Note: No encryption parameters specified here, should use bucket default configuration
.checksum_algorithm(ChecksumAlgorithm::Crc32)
.send()
.await
.expect("Failed to create multipart upload");
@@ -343,28 +347,61 @@ async fn test_bucket_default_encryption_multipart_upload() -> Result<(), Box<dyn
"create_multipart_upload response should contain correct KMS key ID"
);
// Step 3: Upload a part and complete multipart upload
info!("Uploading part and completing multipart upload");
let test_data = b"test-multipart-bucket-default-encryption-data";
// Step 3: Upload two parts. The first is exactly the S3 minimum size so this
// follows the same managed SSE-KMS multipart path as issue #5756.
const PART_SIZE: usize = 5 * 1024 * 1024;
let part1: Vec<u8> = (0..PART_SIZE).map(|i| (i % 251) as u8).collect();
let part2: Vec<u8> = (0..1024 * 1024).map(|i| ((i + 17) % 251) as u8).collect();
let expected_body: Vec<u8> = part1.iter().chain(&part2).copied().collect();
// Upload part 1
let upload_part_response = s3_client
.upload_part()
.bucket(TEST_BUCKET)
.key(test_key)
.upload_id(upload_id)
.part_number(1)
.body(test_data.to_vec().into())
.send()
.await
.expect("Failed to upload part");
let upload_part = |part_number: i32, body: Vec<u8>| {
s3_client
.upload_part()
.bucket(TEST_BUCKET)
.key(test_key)
.upload_id(upload_id)
.part_number(part_number)
.checksum_algorithm(ChecksumAlgorithm::Crc32)
.body(ByteStream::from(body))
.send()
};
let etag = upload_part_response.e_tag().unwrap().to_string();
let expected_part1_crc32 = Checksum::new_from_data(ChecksumType::CRC32, &part1)
.expect("calculate part 1 CRC32")
.encoded;
let upload1 = upload_part(1, part1).await.expect("Failed to upload part 1 with CRC32");
assert_eq!(
upload1.checksum_crc32(),
Some(expected_part1_crc32.as_str()),
"UploadPart must return the CRC32 calculated over plaintext"
);
let expected_part2_crc32 = Checksum::new_from_data(ChecksumType::CRC32, &part2)
.expect("calculate part 2 CRC32")
.encoded;
let upload2 = upload_part(2, part2).await.expect("Failed to upload part 2 with CRC32");
assert_eq!(
upload2.checksum_crc32(),
Some(expected_part2_crc32.as_str()),
"UploadPart must return the CRC32 calculated over plaintext"
);
// Complete multipart upload
let completed_part = aws_sdk_s3::types::CompletedPart::builder()
.part_number(1)
.e_tag(&etag)
let completed_upload = CompletedMultipartUpload::builder()
.parts(
CompletedPart::builder()
.part_number(1)
.e_tag(upload1.e_tag().expect("No ETag for part 1"))
.checksum_crc32(upload1.checksum_crc32().expect("No CRC32 for part 1"))
.build(),
)
.parts(
CompletedPart::builder()
.part_number(2)
.e_tag(upload2.e_tag().expect("No ETag for part 2"))
.checksum_crc32(upload2.checksum_crc32().expect("No CRC32 for part 2"))
.build(),
)
.build();
let complete_multipart_response = s3_client
@@ -372,11 +409,7 @@ async fn test_bucket_default_encryption_multipart_upload() -> Result<(), Box<dyn
.bucket(TEST_BUCKET)
.key(test_key)
.upload_id(upload_id)
.multipart_upload(
aws_sdk_s3::types::CompletedMultipartUpload::builder()
.parts(completed_part)
.build(),
)
.multipart_upload(completed_upload)
.send()
.await
.expect("Failed to complete multipart upload");
@@ -400,6 +433,7 @@ async fn test_bucket_default_encryption_multipart_upload() -> Result<(), Box<dyn
.get_object()
.bucket(TEST_BUCKET)
.key(test_key)
.checksum_mode(ChecksumMode::Enabled)
.send()
.await
.expect("Failed to get object");
@@ -410,6 +444,13 @@ async fn test_bucket_default_encryption_multipart_upload() -> Result<(), Box<dyn
Some(&ServerSideEncryption::AwsKms),
"Final object should contain SSE-KMS encryption information"
);
if let Some(completed_crc32) = complete_multipart_response.checksum_crc32() {
assert_eq!(
get_response.checksum_crc32(),
Some(completed_crc32),
"GetObject should return the persisted composite CRC32 when completion reports it"
);
}
// Verify data integrity
let downloaded_data = get_response
@@ -418,7 +459,11 @@ async fn test_bucket_default_encryption_multipart_upload() -> Result<(), Box<dyn
.await
.expect("Failed to collect body")
.into_bytes();
assert_eq!(&downloaded_data[..], test_data, "Downloaded data should match original data");
assert_eq!(
downloaded_data.as_ref(),
expected_body.as_slice(),
"Downloaded data should match the uploaded multipart body"
);
// Cleanup is handled automatically when the test environment is dropped
info!("Test passed: bucket default encryption correctly applied to multipart upload");
+75
View File
@@ -380,6 +380,12 @@ impl WritePlan {
}
pub fn apply(self, mut reader: HashReader, actual_size: i64) -> std::io::Result<HashReader> {
// Transformations create new HashReaders around the plaintext reader. Keep
// the request checksum metadata on the final reader for multipart/single
// PUT persistence, but leave verification to the plaintext reader.
let checksum = reader.content_hash().clone();
let trailer = reader.get_trailer().cloned();
let encrypted = self.encryption.is_some();
if let Some(algorithm) = self.compression {
reader = HashReader::from_reader(
@@ -438,6 +444,12 @@ impl WritePlan {
};
}
// `ignore_value` deliberately avoids a second hasher over compressed or
// encrypted bytes. The inner reader still validates the plaintext request
// checksum while this outer reader exposes the request checksum context.
reader.add_non_trailing_checksum(checksum, true)?;
reader.set_trailer(trailer);
Ok(reader)
}
}
@@ -445,10 +457,73 @@ impl WritePlan {
#[cfg(test)]
mod tests {
use super::*;
use http::{HeaderMap, HeaderValue};
use rustfs_rio::{Checksum, ChecksumType};
use rustfs_utils::CompressionAlgorithm;
use std::io::Cursor;
use tokio::io::AsyncReadExt;
async fn assert_non_trailing_checksum_survives(plan: WritePlan) {
let plaintext = b"checksum-context-through-write-plan".repeat(256);
let actual_size = plaintext.len() as i64;
let checksum = Checksum::new_from_data(ChecksumType::CRC32, &plaintext).expect("create CRC32 checksum");
let mut reader = HashReader::from_stream(Cursor::new(plaintext), actual_size, actual_size, None, None, false)
.expect("create hash reader");
reader
.add_non_trailing_checksum(Some(checksum.clone()), false)
.expect("attach plaintext checksum");
let mut transformed = plan.apply(reader, actual_size).expect("apply write plan");
assert_eq!(transformed.content_crc_type(), Some(ChecksumType::CRC32));
let mut transformed_bytes = Vec::new();
transformed
.read_to_end(&mut transformed_bytes)
.await
.expect("stream transformed data without rehashing ciphertext");
assert!(!transformed_bytes.is_empty());
assert_eq!(transformed.content_crc().get("CRC32"), Some(&checksum.encoded));
}
#[tokio::test]
async fn write_plan_preserves_non_trailing_checksum_context_across_transforms() {
assert_non_trailing_checksum_survives(WritePlan::new().with_compression(CompressionAlgorithm::default())).await;
assert_non_trailing_checksum_survives(
WritePlan::new().with_encryption(WriteEncryption::singlepart([0x5Au8; 32], [0xA5u8; 12])),
)
.await;
assert_non_trailing_checksum_survives(
WritePlan::new()
.with_compression(CompressionAlgorithm::default())
.with_encryption(WriteEncryption::singlepart([0x5Au8; 32], [0xA5u8; 12])),
)
.await;
}
#[tokio::test]
async fn write_plan_preserves_trailing_checksum_type_across_transforms() {
let plaintext = b"trailing-checksum-context".to_vec();
let actual_size = plaintext.len() as i64;
let mut reader = HashReader::from_stream(Cursor::new(plaintext), actual_size, actual_size, None, None, false)
.expect("create hash reader");
let mut headers = HeaderMap::new();
headers.insert("x-amz-trailer", HeaderValue::from_static("x-amz-checksum-crc32"));
reader
.add_checksum_from_s3s(&headers, None, false)
.expect("attach trailing checksum metadata");
let transformed = WritePlan::new()
.with_encryption(WriteEncryption::singlepart([0x5Au8; 32], [0xA5u8; 12]))
.apply(reader, actual_size)
.expect("apply encryption plan");
assert_eq!(
transformed.content_crc_type(),
Some(ChecksumType(ChecksumType::CRC32.0 | ChecksumType::TRAILING.0))
);
}
#[cfg(feature = "rio-v2")]
fn s2_chunk_types(stream: &[u8]) -> Vec<u8> {
let mut chunk_types = Vec::new();