fix(e2e): send SSE-C key MD5 in Base64 per S3 spec by andybrown668 · Pull Request #4 · wasabi/rustfs

andybrown668 · 2026-03-12T11:09:12Z

Type of Change

Related Issues

N/A

Summary of Changes

E2E tests were sending the SSE-C key MD5 header in hex format (format!("{:x}", md5::compute(key))), while the server and S3 API expect Base64-encoded 128-bit MD5 digest of the encryption key. This caused test_local_kms_key_isolation and any SSE-C flow to fail with "The calculated MD5 hash of the key did not match the hash that was provided."

Updated all e2e tests that use SSE-C to send the key MD5 in Base64:

base64::Engine::encode(..., md5::compute(key).0) (or equivalent with existing compute import).

Files touched: kms_local_test.rs, multipart_encryption_test.rs, kms_vault_test.rs, kms_edge_cases_test.rs, kms_comprehensive_test.rs, common.rs.

Checklist

I have read and followed the CONTRIBUTING.md guidelines
Passed make pre-commit
Added/updated necessary tests (existing tests fixed; no new tests)
Documentation updated (if needed)
CI/CD passed (if applicable)

Impact

Breaking change (compatibility)
Requires doc/config/deployment update
Other impact:

Additional Notes

Verification: cargo test -p e2e_test kms::kms_local_test::test_local_kms_key_isolation passes. Requesting review from Copilot.

Copilot

Pull request overview

Fixes SSE-C E2E test failures by generating the x-amz-server-side-encryption-customer-key-MD5 header as Base64 of the raw 16-byte MD5 digest (per S3 spec), instead of hex encoding.

Changes:

Replace hex-formatted MD5 (format!("{:x}", ...)) with Base64-encoded digest bytes (...encode(..., md5::compute(key).0) / compute(key).0) across SSE-C test flows.
Add/adjust inline comments clarifying the S3 SSE-C MD5 header requirements.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
crates/e2e_test/src/kms/multipart_encryption_test.rs	Updates SSE-C multipart helper to send Base64 MD5 digest bytes.
crates/e2e_test/src/kms/kms_vault_test.rs	Fixes Vault SSE-C key isolation test to send Base64 MD5 digest bytes.
crates/e2e_test/src/kms/kms_local_test.rs	Fixes Local KMS SSE-C MD5 headers and adds clarifying comments.
crates/e2e_test/src/kms/kms_edge_cases_test.rs	Fixes multiple SSE-C edge-case scenarios to use Base64 MD5 digest bytes.
crates/e2e_test/src/kms/kms_comprehensive_test.rs	Fixes comprehensive SSE-C isolation/wrong-key flows to use Base64 MD5 digest bytes.
crates/e2e_test/src/kms/common.rs	Fixes shared SSE-C helpers/config to use Base64 MD5 digest bytes.

You can also share your feedback on Copilot code review. Take the survey.

E2E tests sent x-amz-server-side-encryption-customer-key-MD5 in hex while server expects Base64-encoded 128-bit MD5. Fix all SSE-C usages across kms_local_test, kms_vault_test, kms_edge_cases_test, kms_comprehensive_test, multipart_encryption_test, and common. Made-with: Cursor

- In common.rs: when CARGO_BIN_EXE_rustfs is unset, always build rustfs once per process via Once (remove 'use existing binary if present' path) so the e2e harness uses a single, consistent binary. - Document in tests.mak that e2e setup builds rustfs once and only once. - Add e2e-test make target; RUSTFS_BUILD_FEATURES=ftps for protocol tests. Made-with: Cursor

Extract async test bodies into run_test_* functions callable from both #[tokio::test] and the unified test runner. Fixes E0277 'not a future' and '() is not a future' by having the runner await real async futures instead of #[tokio::test] sync wrappers. - kms_local_test: run_test_local_kms_key_isolation, run_test_local_kms_multipart_upload - multipart_encryption_test: run_test_step1..step5 - kms_edge_cases_test: run_test_kms_* (6 tests) - kms_fault_recovery_test: run_test_kms_* (4 tests) - kms_comprehensive_test: run_test_comprehensive_* (5 tests) - test_runner: dispatch to all run_test_* functions Made-with: Cursor

- Add check-awscurl, install-awscurl, ensure-awscurl in .config/make/check.mak - Prefer pipx install to avoid externally-managed-environment; fallback to pip --user - Use ${AWSCURL_PATH:-} so unset var is safe with set -u - Make e2e-server and probe-e2e depend on ensure-awscurl in tests.mak - In common.rs, surface clearer errors when rustfs binary or awscurl is missing Made-with: Cursor

…tion Errors if stream ends before expected bytes to avoid IncompleteBody when decryption produces fewer bytes than Content-Length. Made-with: Cursor

- Add zoned_now_utc() and ZonedUtcCompatible for consistent serialization - Pass encryption_context into decrypt_sse_dek for envelope verification - Add decrypt_data_key_with_context; trim base64 in headers_to_metadata - SSE-S3: do not store KMS key ID in metadata; key_id fallback to 'default' - Skip DEK cache when encryption_context is set (context-bound DEKs) Made-with: Cursor

…lter - Use ExactLengthReader in decryption path to avoid IncompleteBody - Persist S3 SSE type (AES256/aws:kms) and bucket/object_key in context - decrypt_sse_dek takes optional encryption_context for envelope verification - Add filter_object_metadata_with_options to include x-rustfs-encryption-* for Head/Get - Only store x-amz-server-side-encryption-aws-kms-key-id for aws:kms - Add in-process SSE-S3 roundtrip test with global KMS Made-with: Cursor

…ypted GET - Do not expose ssekms_key_id for SSE-S3 (AES256) in Put/Get/Multipart responses - Use filter_object_metadata_with_options(..., true) for Get/Head to include encryption metadata - PutObject: return VersionId when versioning enabled or suspended; use 'null' for null version - GetObject: set decryption part_number=1 for single-part multipart encrypted objects - Preload small encrypted GET bodies (<=8MB) to validate decryption before sending Made-with: Cursor

Made-with: Cursor

- Add start_rustfs_server_with_env for extra env (e.g. RUSTFS_SCANNER_SPEED) - Run server in own process group to avoid signal propagation to test runner - Local KMS: store encrypted_key_material as base64 in key JSON - Vault: ensure KV v2 at secret, seed default key in KV for backend Made-with: Cursor

- Data usage: poll for scanner, RUSTFS_SCANNER_SPEED=fastest, 200 objects - Group delete: un-ignore, retry on connection reset for set-policy - Policy: spawn RustFS via RustFSTestEnvironment, use dynamic address - Reliant/ftps: use common env and dynamic endpoint, un-ignore Made-with: Cursor

Compute build matrix in build-check; upstream keeps full matrix, forks only build x86_64-unknown-linux-gnu. Format matrix JSON multiline for readability. Made-with: Cursor

Avoid GitHub API timeouts and Node.js 20 deprecation by downloading protoc 33.1 from protocolbuffers/protobuf releases for Linux, macOS, and Windows. Made-with: Cursor

Check for cargo-nextest 0.2.30 before installing to avoid redundant reinstall when cached. Made-with: Cursor

Made-with: Cursor

Copilot

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

rustfs/src/app/object_usecase.rs:3041

HeadObjectOutput.metadata is meant for user-defined metadata (x-amz-meta-). Including x-rustfs-encryption- entries here (include_managed_encryption_metadata=true) will expose internal encryption IV/DEK/context to clients and may be serialized as x-amz-meta-x-rustfs-encryption-*, which is not compatible with S3 behavior. Suggest keeping these internal keys filtered out unless explicitly requested via a privileged/debug mechanism.

Copilot · 2026-04-06T15:30:34Z

rustfs/src/app/object_usecase.rs

            accept_ranges: Some("bytes".to_string()),
            content_range,
            e_tag: info.etag.map(|etag| to_s3s_etag(&etag)),
-            metadata: filter_object_metadata(&info.user_defined),
+            metadata: filter_object_metadata_with_options(&info.user_defined, true),
            server_side_encryption,
            sse_customer_algorithm,
            sse_customer_key_md5,
-            ssekms_key_id,
+            ssekms_key_id: ssekms_key_id_for_response,


GetObjectOutput.metadata is the S3 user-defined metadata map (x-amz-meta-). Passing include_managed_encryption_metadata=true causes internal headers like x-rustfs-encryption-key/iv/context to be exposed as user metadata (and likely emitted as x-amz-meta-x-rustfs-encryption-), which violates S3 semantics and leaks sensitive internal encryption material. Consider keeping managed encryption metadata filtered out by default and only returning it via dedicated response headers or an explicit privileged/debug option.

Copilot · 2026-04-06T15:30:35Z

rustfs/src/storage/options.rs

@@ -451,8 +461,8 @@ pub(crate) fn filter_object_metadata(metadata: &HashMap<String, String>) -> Opti
            continue;
        }

-        // Skip internal encryption metadata
-        if lower_key.starts_with(RUSTFS_ENCRYPTION_LOWER) {
+        // Skip internal encryption metadata unless explicitly included (e.g. for Head/Get response)
+        if !include_managed_encryption_metadata && lower_key.starts_with(RUSTFS_ENCRYPTION_LOWER) {
            continue;


filter_object_metadata_with_options can include internal managed-encryption keys (x-rustfs-encryption-). Returning those via the S3 Metadata map will surface them as user metadata (x-amz-meta-) and can leak internal encryption material (encrypted DEK, IV, context). Consider removing the inclusion option or restricting it to an admin-only code path that returns these values as dedicated headers rather than in Metadata.

Copilot · 2026-04-06T15:30:35Z

rustfs/src/storage/sse.rs

            (decrypt_reader, plain_size)
        };

        // Add hard limit reader to prevent over-reading
-        // final_stream is already Box<dyn Reader>, no need to wrap with WarpReader
        let limit_reader = HardLimitReader::new(final_stream, response_content_length);
-        final_stream = Box::new(limit_reader);
+        // Error if stream ends before expected bytes (avoids IncompleteBody for decrypted GET)
+        let exact_reader = ExactLengthReader::new(Box::new(limit_reader), response_content_length);
+        final_stream = Box::new(exact_reader);



The PR title/description focuses on fixing e2e SSE-C key MD5 encoding, but this diff also changes production behavior in multiple areas (SSE streaming readers, encryption context handling, KMS cache semantics, versioning VersionId behavior, policy resource parsing, CI behavior, etc.). This scope mismatch makes review/rollback harder; consider splitting unrelated changes into separate PRs or expanding the PR description to justify and validate each production change.

Copilot · 2026-04-06T15:30:35Z

crates/rio/src/exact_length_reader.rs

+impl AsyncRead for ExactLengthReader {
+    fn poll_read(mut self: Pin<&mut Self>, cx: &mut Context<'_>, buf: &mut ReadBuf<'_>) -> Poll<Result<()>> {
+        let this = self.as_mut().project();
+        let before = buf.filled().len();
+
+        let poll = this.inner.poll_read(cx, buf);
+
+        if let Poll::Ready(Ok(())) = &poll {
+            let after = buf.filled().len();
+            let n = (after - before) as i64;
+            *this.read_so_far += n;
+            // EOF: inner returned Ok(()) but no new bytes
+            if n == 0 && *this.read_so_far < *this.expected {
+                return Poll::Ready(Err(Error::other(format!(
+                    "decryption stream ended early: expected {} bytes, got {}",
+                    this.expected, this.read_so_far
+                ))));
+            }
+        }
+        poll
+    }


ExactLengthReader is newly introduced but currently has no unit tests. Since similar readers (e.g., HardLimitReader) have coverage, it would be good to add tests for (1) exact-length success, (2) early EOF returns error, and (3) expected=0 behavior, to prevent regressions in the GET/decryption path that now relies on this reader.

Copilot · 2026-04-06T15:30:35Z

rustfs/src/storage/sse.rs

+    // Extract KMS key ID from metadata (optional, used for provider context).
+    // SSE-S3 uses x-rustfs-encryption-key-id only; SSE-KMS may use either.
    let kms_key_id = metadata
-        .get("x-amz-server-side-encryption-aws-kms-key-id")
+        .get("x-rustfs-encryption-key-id")
+        .or_else(|| metadata.get("x-amz-server-side-encryption-aws-kms-key-id"))
        .cloned()
        .unwrap_or_else(|| "default".to_string());


This comment says "SSE-S3 uses x-rustfs-encryption-key-id only", but the metadata written by ObjectEncryptionService::metadata_to_headers does not set x-rustfs-encryption-key-id (and AES256/SSE-S3 falls back to "default" when x-amz-server-side-encryption-aws-kms-key-id is absent). Consider updating the comment (or actually persisting x-rustfs-encryption-key-id) so it matches the headers currently produced/consumed.

andybrown668 requested a review from Copilot March 12, 2026 11:09

Copilot started reviewing on behalf of andybrown668 March 12, 2026 11:10 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

andybrown668 added 12 commits March 16, 2026 13:48

feat(rio): add ExactLengthReader for decrypted GET body length valida…

e0178cd

…tion Errors if stream ends before expected bytes to avoid IncompleteBody when decryption produces fewer bytes than Content-Length. Made-with: Cursor

fix(ecstore): invalidate file cache after write_all

b44a4a7

Made-with: Cursor

feat(policy): allow Resource "*" as IAM shorthand for all resources

bd2e9d1

Made-with: Cursor

andybrown668 force-pushed the fix/ssec-key-md5-base64-e2e branch 2 times, most recently from 409838f to 7470016 Compare March 16, 2026 13:59

ci(build): restrict fork builds to x86_64-unknown-linux-gnu

277a584

Compute build matrix in build-check; upstream keeps full matrix, forks only build x86_64-unknown-linux-gnu. Format matrix JSON multiline for readability. Made-with: Cursor

andybrown668 force-pushed the fix/ssec-key-md5-base64-e2e branch from 7470016 to 277a584 Compare March 16, 2026 14:08

ci: install protoc via direct download instead of arduino/setup-protoc

a10b5e1

Avoid GitHub API timeouts and Node.js 20 deprecation by downloading protoc 33.1 from protocolbuffers/protobuf releases for Linux, macOS, and Windows. Made-with: Cursor

andybrown668 force-pushed the fix/ssec-key-md5-base64-e2e branch 2 times, most recently from 99a837e to 2ca6d5e Compare March 16, 2026 15:55

ci: skip cargo-nextest install when already at pinned version

d80eeb5

Check for cargo-nextest 0.2.30 before installing to avoid redundant reinstall when cached. Made-with: Cursor

andybrown668 force-pushed the fix/ssec-key-md5-base64-e2e branch from 2ca6d5e to d80eeb5 Compare March 17, 2026 12:26

ci: set CARGO_BUILD_JOBS from runner (labs-large → 40)

f006ab6

Made-with: Cursor

Copilot AI review requested due to automatic review settings April 6, 2026 15:22

Copilot started reviewing on behalf of andybrown668 April 6, 2026 15:23 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(e2e): send SSE-C key MD5 in Base64 per S3 spec#4

fix(e2e): send SSE-C key MD5 in Base64 per S3 spec#4
andybrown668 wants to merge 16 commits intomainfrom
fix/ssec-key-md5-base64-e2e

andybrown668 commented Mar 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

andybrown668 commented Mar 12, 2026

Type of Change

Related Issues

Summary of Changes

Checklist

Impact

Additional Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants