From da6fc5314df34f7e9b3b8e93ff3c1b1adb99250d Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sun, 2 Aug 2026 05:31:30 +0800 Subject: [PATCH] docs(kms): correct the static backend's MinIO compatibility claim (#5596) * docs(kms): correct the static backend's MinIO compatibility claim `StaticConfig` claimed the backend derives DEKs via "HMAC-SHA256 + AES-256-GCM, matching the MinIO builtin/static KMS wire format". Neither half holds: the configured key is used directly as the AES-256-GCM key with no derivation step, and wrapped DEKs are serialized as RustFS's own `DataKeyEnvelope` JSON. MinIO's KMS ciphertext uses a different shape, which `is_data_key_envelope` explicitly classifies as foreign (see the `minio_legacy` case in encryption/dek.rs). The claim as written tells a migrating operator that MinIO-written ciphertext will open here, which it will not. Restate what the backend actually does and point at rustfs/backlog#1638 for the real interop work. Also refresh the neighbouring ciphertext-format note in static_kms.rs, which still described a raw `ciphertext || nonce` layout that the JSON envelope replaced. * ci(minio-interop): fix the dead test selector and guard against empty runs The job selected its tests with `-p rustfs-ecstore -E 'binary(minio_generated_read_test)'`. #5435 moved those reader tests from crates/ecstore/tests/minio_generated_read_test.rs into the `rustfs` crate as a `#[cfg(test)] mod`, which removed that test binary; the selector has selected zero interop tests since. Point it at the tests where they now live, verified locally: cargo nextest list --run-ignored all -p rustfs --features rio-v2 \ -E 'test(minio_generated_read_test::)' # 4 tests, was 0 Add a guard step in front of the run. The old `binary(...)` form happened to fail loudly once its binary disappeared, but the name-based form that replaces it is a valid filterset even when it matches nothing, so a later rename would silently reduce this job to a pass that asserts nothing. The guard counts the selection and fails with an explicit reason; the count comes from `filter-match.status`, since the JSON's top-level `test-count` is the package total and ignores `-E`. `--no-tests=fail` on the run step covers the same case if the guard is ever dropped. Also record in the header what this job does and does not prove: MinIO wrapped-DEK envelopes are still rejected by both envelope parsers, and that work is tracked in rustfs/backlog#1638. --- .github/workflows/minio-interop.yml | 49 ++++++++++++++++++++++++--- crates/kms/src/backends/static_kms.rs | 5 ++- crates/kms/src/config.rs | 11 ++++-- 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/.github/workflows/minio-interop.yml b/.github/workflows/minio-interop.yml index ad801f842..22d30ee95 100644 --- a/.github/workflows/minio-interop.yml +++ b/.github/workflows/minio-interop.yml @@ -18,7 +18,14 @@ # This is NOT a PR gate. The fixtures are real MinIO backend trees generated on # the fly (they are gitignored, never committed), so the job regenerates them # each run with Docker and then runs the `#[ignore]` reader tests in -# crates/ecstore/tests/minio_generated_read_test.rs. +# rustfs/src/storage/minio_generated_read_test.rs. +# +# Scope: end-to-end MinIO-to-RustFS SSE interop is NOT implemented yet. Both +# envelope parsers reject MinIO's own wrapped-DEK shape — see +# `is_data_key_envelope` in crates/kms/src/encryption/dek.rs and the +# `deny_unknown_fields` `LocalSseDekEnvelope` in rustfs/src/storage/sse.rs — and +# closing that gap is tracked in rustfs/backlog#1638. Treat this job as the +# harness for #1638, not as standing evidence that a MinIO migration reads back. # # Runner: GitHub-hosted `ubuntu-latest`. It reliably ships Docker + Python, # unlike the self-hosted fleet, whose pods drift in Docker/pip availability @@ -55,6 +62,19 @@ jobs: env: # Fixed 32-byte test KMS key baked into the fixture lab; not a secret. RUSTFS_MINIO_STATIC_KMS_KEY_B64: IyqsU3kMFloCNup4BsZtf/rmfHVcTgznO2F25CkEH1g= + # Single definition of "the interop tests", shared by the guard step and + # the run step so the two cannot drift apart. + # + # These used to live in crates/ecstore/tests/minio_generated_read_test.rs + # and were selected with `-p rustfs-ecstore -E + # 'binary(minio_generated_read_test)'`. #5435 moved them into the `rustfs` + # crate as a `#[cfg(test)] mod`, which deleted that test binary; the + # selector was never updated and has selected zero interop tests ever + # since (cargo-nextest 0.9.140 now rejects it outright: "operator didn't + # match any binary names", exit 94). + INTEROP_PACKAGE: rustfs + INTEROP_FEATURES: rio-v2 + INTEROP_FILTER: "test(minio_generated_read_test::)" steps: - name: Checkout repository uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 @@ -71,8 +91,29 @@ jobs: - name: Generate real MinIO fixtures via Docker run: bash crates/rio-v2/tests/minio_fixture_lab/capture_via_docker.sh + # `binary(...)` at least dies loudly when nothing matches, but `test(...)` + # is a perfectly valid filterset that matches zero tests, so the next + # rename or module move would leave this job selecting nothing and + # reporting success without executing a single interop assertion. Count + # the selection and fail with a reason instead. + # + # Count only `filter-match.status == "matches"`: the top-level + # `test-count` in the JSON is the package total and ignores `-E` entirely. + - name: Assert the interop selector still matches tests + run: | + set -euo pipefail + count="$(cargo nextest list --run-ignored all \ + -p "$INTEROP_PACKAGE" --features "$INTEROP_FEATURES" \ + -E "$INTEROP_FILTER" --message-format json \ + | python3 -c 'import json,sys; d=json.load(sys.stdin); print(sum(1 for s in d.get("rust-suites", {}).values() for t in s.get("testcases", {}).values() if t.get("filter-match", {}).get("status") == "matches"))')" + echo "interop tests selected: ${count}" + if [ "${count}" -eq 0 ]; then + echo "::error::Selector '${INTEROP_FILTER}' in package '${INTEROP_PACKAGE}' matched 0 tests. The MinIO interop reader tests have moved or been renamed again; fix the selector instead of letting this job pass without running them. Context: rustfs/backlog#1638." + exit 1 + fi + - name: Run MinIO interop reader tests run: | - cargo nextest run --run-ignored ignored-only \ - -p rustfs-ecstore --features rio-v2 \ - -E 'binary(minio_generated_read_test)' + cargo nextest run --run-ignored ignored-only --no-tests=fail \ + -p "$INTEROP_PACKAGE" --features "$INTEROP_FEATURES" \ + -E "$INTEROP_FILTER" diff --git a/crates/kms/src/backends/static_kms.rs b/crates/kms/src/backends/static_kms.rs index 20574b5bb..e9ba4027a 100644 --- a/crates/kms/src/backends/static_kms.rs +++ b/crates/kms/src/backends/static_kms.rs @@ -19,7 +19,10 @@ //! //! ## Ciphertext format //! -//! encrypted_data(plaintext_len+16) || nonce (12 bytes) +//! A JSON-serialized `DataKeyEnvelope` carrying the AES-256-GCM ciphertext, the +//! 12-byte nonce and the authenticated encryption context. This is a +//! RustFS-internal format; it is not interchangeable with MinIO's KMS +//! ciphertext (see the note on `StaticConfig`). use crate::backends::{BackendCapabilities, KmsBackend, empty_key_page, list_keys_page_size}; use crate::config::{BackendConfig, KmsConfig}; diff --git a/crates/kms/src/config.rs b/crates/kms/src/config.rs index 5c2d221e3..97fce0772 100644 --- a/crates/kms/src/config.rs +++ b/crates/kms/src/config.rs @@ -297,8 +297,15 @@ impl Default for LocalConfig { /// Static single-key KMS backend configuration /// -/// Uses a pre-configured AES-256 key to derive data encryption keys via -/// HMAC-SHA256 + AES-256-GCM, matching the MinIO builtin/static KMS wire format. +/// The configured 32-byte key is used directly as the AES-256-GCM key that +/// wraps data encryption keys — there is no HMAC-SHA256 derivation step — and +/// each wrapped DEK is serialized as a RustFS `DataKeyEnvelope` JSON blob. +/// +/// This mirrors the *concept* of MinIO's builtin/static single-key KMS, but is +/// not wire-compatible with it: MinIO wraps DEKs in a different (`{"aead": ...}`) +/// blob that this backend neither produces nor accepts, so KMS ciphertext +/// written by MinIO cannot be opened here. Reading MinIO-written SSE objects is +/// tracked separately in rustfs/backlog#1638. #[derive(Clone, Default, Serialize, Deserialize)] pub struct StaticConfig { /// Key identifier (name) for the single configured key