Files
rustfs/crates/kms/tests/behavior_resilience.rs
T
唐小鸭 62cc19e937 fix(kms): repair unopenable ciphertext and cover the Vault backends (#5668)
* Add black-box behavior tests for KMS resilience and serialization

* fix(kms): repair unopenable ciphertext across backends

Black-box testing of the KMS crate surfaced several defects that make
encrypted data permanently unreadable.

Symmetric envelopes. The Local and Vault Transit backends returned raw
cipher output from `encrypt` while `decrypt` parsed a JSON envelope, so
anything sealed through the master-key path could never be opened again.
Local also discarded the AES-GCM nonce. Both now emit the same envelope
`decrypt` consumes, matching the Static backend.

Deterministic AAD. The object layer derived AEAD additional data by
serializing a `HashMap` directly. Iteration order differs per instance,
so a context rebuilt from storage produced different AAD bytes than the
one used to seal and the object stopped opening. Ordering by key removes
that dependency, matching the Static backend's existing `context_aad`.
Objects written with the default single-key context are unaffected,
since a one-entry map has only one serialization.

Cipher in the header projection. `metadata_to_headers` recorded the SSE
mode (`AES256` / `aws:kms`), which cannot represent ChaCha20-Poly1305,
so a ChaCha-sealed object came back claiming `aws:kms` and was opened
with the wrong cipher. The cipher now travels in
`x-rustfs-encryption-algorithm` — the header the storage layer already
reads but nothing ever wrote. Objects without it fall back as before.

Also: the Static backend ignored `key_spec` and always issued 256-bit
data keys; Local `list_keys` hardcoded `truncated: false`, ignored
`marker`, and paginated over unordered `read_dir`, so a paginating
client silently saw a partial key list; and Local and Vault KV2 reported
`key_id: "unknown"` from `decrypt` despite the envelope naming the
master key.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(kms): cover both Vault backends and key rotation

The behavior suite ran only against Local and Static, and its own harness
documented the gap: the Vault backends had no business-capability
coverage at all. Setting `RUSTFS_KMS_VAULT_TOKEN` now adds Vault KV2 and
Vault Transit to every `for_each_backend` spec against a live server.
That lane is what surfaced the Transit envelope defect fixed in the
previous commit.

`rotate` and `versioning` are advertised only by the Vault backends, so
until now every capability-gated branch for them took the
`UnsupportedCapability` side and the working half was never asserted — a
rotation that dropped prior key versions would have gone green. The new
`behavior_rotation.rs` pins that half: material sealed before a rotation
still opens after it, repeated rotations accumulate versions rather than
overwriting a single spare, and the history survives a restart.

Two test defects fixed. `objects_round_trip_across_sizes_and_algorithms`
asserted a 1-byte object differs from its own ciphertext, which collides
once every 256 runs; the assertion now applies only where a collision is
not realistic, and small objects stay covered by the tag check and the
decrypt round-trip. `test_from_env_selects_token_file` depended on
`RUSTFS_KMS_VAULT_TOKEN` being absent from the caller's environment and
now clears it explicitly.

The snapshots directory was also removed from `.gitignore`: insta
snapshots are the assertions themselves, so leaving them untracked gives
CI nothing to compare against. Only `.snap.new` scratch files are
ignored now.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(kms): adapt behavior suite to current key APIs

Rebasing onto main brought four API changes the suite predates.

`DeleteKeyRequest` gained `confirm_key_id`, and immediate deletion is now
gated on the server's `allow_immediate_deletion`. Scheduled deletions pass
`None`; the four specs that destroy a key outright echo the key id back
and opt the harness config in, which is what the gate asks of a real
caller.

`LocalBackupExportRequest` gained `sanitized_config`. These specs cover
the key-material path, so they seal no configuration and pass `None`.

`KmsCacheStats` became a named struct with real hit, miss, and eviction
counters. `cache_stats_returns_an_entry_count_and_no_hit_or_miss_data`
existed to pin the old placeholder behavior — that the second tuple
element was always zero — which main has since fixed, so it is now
`cache_stats_reports_hits_and_misses_separately` and asserts the counters
actually move.

Starting the service provisions the reserved probe key, so it shows up in
listings and backup bundles. Exact-set assertions filter it through a new
`without_probe_key` helper rather than naming it, keeping those specs
about the keys they seeded.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(kms): bind the AAD to the stored context bytes

Review caught that canonicalizing the AAD on decrypt breaks objects sealed
before canonicalization existed, and it was right. The AAD is the
*serialization* of the encryption context, and `x-rustfs-encryption-context`
stores that exact byte sequence: `encrypt_object` fed one `HashMap` to the
AEAD and then moved the same map into the metadata the header is written
from, so the stored string is byte-identical to the AAD the object was
sealed under. Those objects are therefore recoverable — but only while
nothing round-trips the value through a `HashMap` and re-serializes it.

Recomputing sorted AAD on decrypt would have turned a readable object into
a permanently unreadable one. The previous behavior was worse than the
first analysis credited: it did not merely fail intermittently, it made
the failure deterministic.

`EncryptionMetadata` now carries `context_aad`, the bytes the object was
actually sealed with. Encryption records what it fed the AEAD, the header
projection stores those bytes verbatim (and preserves a legacy ordering
across a re-projection rather than rewriting it into sorted form), and
`headers_to_metadata` carries the stored string through untouched. Both
decrypt paths, SSE-KMS and SSE-C, prefer it and fall back to canonical
serialization only when no stored serialization exists. Canonicalization
still applies to everything newly sealed, so the original ordering bug
cannot recur.

Two tests pin this: a legacy record whose sealed bytes are non-canonical
must survive a full header round trip unchanged, and a context header
rewritten to an equivalent-but-reordered serialization must fail
authentication rather than silently re-deriving a working AAD. Both were
mutation-checked against the reinstated bug on each side.

Also from review: the lifecycle churn test asserted only that every
request was accounted for, which holds whether the state gate exists or
not, so both branches are now pinned deterministically after the churn
(asserting `refused > 0` on the concurrent phase would only trade the hole
for a scheduling flake). And the Local and Vault KV2 envelopes compare
`encryption_context` without authenticating it — `DekCrypto` seals only
the plaintext — which is now documented at both sites; closing it needs a
versioned envelope, since existing ciphertext was sealed without AAD.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-03 23:33:08 +08:00

309 lines
12 KiB
Rust

// 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.
//! Black-box behavior: how the service reacts to an unreachable backend.
//!
//! The rule that matters operationally is **a bad configuration must never take
//! down a working KMS**. Applying a config that points at a dead Vault is a
//! routine operator mistake; if it stopped the running service, every encrypted
//! object in the deployment would become unreadable until someone noticed. So
//! the candidate is health-checked *before* the swap, and a failing candidate
//! is discarded with the incumbent still serving.
//!
//! Everything here runs offline: an unreachable backend is a loopback port with
//! nothing listening, which is deterministic and needs no external server.
//!
//! Error *classification* and retry accounting for transport faults live in
//! `tests/vault_fault_injection.rs`, which drives the same public API with a
//! metrics recorder attached. This file deliberately does not duplicate it.
//!
//! Not covered offline: throttling (429) and recoverable 5xx responses. Forcing
//! those needs the crate's scripted Vault responder, which is `pub(crate)` and
//! therefore out of reach from an integration test; they are pinned by the
//! in-crate wiring tests in `backends::vault` instead.
mod common;
use std::sync::Arc;
use std::time::Duration;
use common::{TestKms, assert_configuration_error, ctx};
use rustfs_kms::{
BackendConfig, DecryptRequest, GenerateDataKeyRequest, KeySpec, KmsBackend as KmsBackendKind, KmsConfig, KmsServiceManager,
KmsServiceStatus, VaultAuthMethod, VaultConfig,
};
/// A loopback address with nothing listening on it: reserve a port, then
/// release it so a connection there is refused immediately.
fn dead_address() -> String {
let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("reserve a loopback port");
let address = format!("http://{}", listener.local_addr().expect("reserved port addr"));
drop(listener);
address
}
fn unreachable_vault_config() -> KmsConfig {
KmsConfig {
backend: KmsBackendKind::VaultKv2,
backend_config: BackendConfig::VaultKv2(Box::new(VaultConfig {
address: dead_address(),
auth_method: VaultAuthMethod::Token {
token: "unused-token".to_string(),
},
namespace: None,
mount_path: "transit".to_string(),
kv_mount: "secret".to_string(),
key_path_prefix: "rustfs/kms/resilience".to_string(),
tls: None,
})),
allow_insecure_dev_defaults: true,
// Keep the failure fast: one short attempt is enough to prove the point.
timeout: Duration::from_millis(300),
retry_attempts: 1,
..KmsConfig::default()
}
}
#[tokio::test]
async fn starting_against_an_unreachable_backend_fails_without_publishing_a_service() {
let manager = KmsServiceManager::new();
let config = unreachable_vault_config();
manager
.configure(config.clone())
.await
.expect("configuring an unreachable backend is allowed: validation is not connectivity");
assert_eq!(manager.get_status().await, KmsServiceStatus::Configured);
let error = manager
.start()
.await
.expect_err("starting must fail when the backend is unreachable");
assert!(
format!("{error}").contains("KMS backend"),
"the failure must name the backend, got {error}"
);
match manager.get_status().await {
KmsServiceStatus::Error(message) => assert!(!message.is_empty(), "a failed start must record why"),
other => panic!("a failed start must leave an Error status, got {other:?}"),
}
assert!(
manager.get_encryption_service().await.is_none(),
"a failed start must not publish a half-built service"
);
assert!(manager.get_manager().await.is_none(), "a failed start must not publish a manager either");
assert!(
manager.get_service_version().await.is_none(),
"a failed start must not claim a service version"
);
assert!(
manager.get_config().await.is_some(),
"the configuration survives so an operator can fix and retry"
);
assert!(
!manager
.health_check()
.await
.expect("health check must not error when nothing runs"),
"a service that never started is unhealthy"
);
}
/// The load-bearing case: pointing a *running* KMS at a dead backend must be a
/// rejected reconfigure, not an outage.
#[tokio::test]
async fn a_failing_candidate_never_replaces_a_healthy_service() {
let kms = TestKms::local().await;
let manager = kms.manager().clone();
let key_id = kms.create_key("survives-bad-config").await;
let context = ctx(&[("bucket", "resilience-behavior")]);
let incumbent = manager.get_encryption_service().await.expect("service v1");
let incumbent_manager = manager.get_manager().await.expect("manager v1");
let dek = incumbent_manager
.generate_data_key(GenerateDataKeyRequest {
key_id: key_id.clone(),
key_spec: KeySpec::Aes256,
encryption_context: context.clone(),
})
.await
.expect("the incumbent works before the bad reconfigure");
// Attempt the bad swap. The Local backend's identity is also frozen, so a
// cross-backend move is refused before connectivity is even attempted —
// assert the refusal, then assert nothing moved.
let error = manager
.reconfigure(unreachable_vault_config())
.await
.expect_err("a reconfigure onto an unreachable backend must fail");
assert!(!format!("{error}").is_empty(), "the failure must be reported");
assert_eq!(
manager.get_status().await,
KmsServiceStatus::Running,
"the incumbent must still be Running after a rejected reconfigure"
);
assert_eq!(
manager.get_service_version().await,
Some(1),
"a rejected candidate must not consume a service version"
);
assert!(
Arc::ptr_eq(&incumbent, &manager.get_encryption_service().await.expect("service")),
"the published service must still be the incumbent instance"
);
assert!(
manager.get_config().await.expect("config").local_config().is_some(),
"the published configuration must still be the Local one"
);
// And it is not merely present — it still does real work, on both old and
// freshly fetched handles.
let decrypted = manager
.get_manager()
.await
.expect("manager")
.decrypt(DecryptRequest {
ciphertext: dek.ciphertext_blob.clone(),
encryption_context: context.clone(),
grant_tokens: Vec::new(),
})
.await
.expect("the surviving service must still decrypt");
assert_eq!(decrypted.plaintext, dek.plaintext_key);
assert!(manager.health_check().await.expect("health check"), "the survivor is healthy");
}
#[tokio::test]
async fn a_vault_backend_reconfigure_onto_a_dead_address_is_rejected() {
// Start from a Vault-shaped (never-started) configuration so the transition
// guard does not short-circuit the connectivity check, and confirm that the
// candidate's health check is what refuses it.
let manager = KmsServiceManager::new();
manager
.configure(unreachable_vault_config())
.await
.expect("configure is allowed");
// Reconfigure while nothing is running: the candidate must still be
// health-checked, so an unreachable backend cannot be published.
let error = manager
.reconfigure(unreachable_vault_config())
.await
.expect_err("an unreachable candidate must not be published even from a stopped state");
assert!(
format!("{error}").contains("reconfigure") || format!("{error}").contains("backend"),
"the failure must point at the backend, got {error}"
);
assert!(
manager.get_encryption_service().await.is_none(),
"no service may be published by a failed reconfigure"
);
assert!(
manager.get_service_version().await.is_none(),
"no version may be consumed by a failed reconfigure"
);
}
#[tokio::test]
async fn credentials_that_cannot_be_read_fail_closed_at_start() {
// A Vault Agent token sink that is not there: the service must refuse to
// start rather than come up and send unauthenticated requests.
let missing = std::path::PathBuf::from("/nonexistent/rustfs-kms-behavior/vault-token");
let config = KmsConfig {
backend: KmsBackendKind::VaultKv2,
backend_config: BackendConfig::VaultKv2(Box::new(VaultConfig {
address: dead_address(),
auth_method: VaultAuthMethod::token_file(missing),
namespace: None,
mount_path: "transit".to_string(),
kv_mount: "secret".to_string(),
key_path_prefix: "rustfs/kms/resilience".to_string(),
tls: None,
})),
allow_insecure_dev_defaults: true,
timeout: Duration::from_millis(300),
retry_attempts: 1,
..KmsConfig::default()
};
let manager = KmsServiceManager::new();
manager.configure(config).await.expect("configure");
assert!(
manager.start().await.is_err(),
"a missing credential source must keep the service from starting"
);
assert!(
manager.get_encryption_service().await.is_none(),
"no service may be published without usable credentials"
);
// An empty token-file path is a configuration error, caught before start.
let empty_path = KmsConfig {
backend: KmsBackendKind::VaultKv2,
backend_config: BackendConfig::VaultKv2(Box::new(VaultConfig {
address: "https://vault.example.com:8200".to_string(),
auth_method: VaultAuthMethod::token_file(std::path::PathBuf::new()),
namespace: None,
mount_path: "transit".to_string(),
kv_mount: "secret".to_string(),
key_path_prefix: "rustfs/kms/resilience".to_string(),
tls: None,
})),
..KmsConfig::default()
};
assert_configuration_error(empty_path.validate(), "token file path cannot be empty");
}
#[tokio::test]
async fn a_stopped_service_refuses_work_without_losing_its_state() {
// Stopping is not a failure mode, but it is an unavailability the callers
// must handle: handles disappear, the config stays, and a restart recovers.
let kms = TestKms::local().await;
let manager = kms.manager().clone();
let key_id = kms.create_key("stop-and-recover").await;
let context = ctx(&[("bucket", "resilience-behavior")]);
let dek = kms
.kms()
.await
.generate_data_key(GenerateDataKeyRequest {
key_id: key_id.clone(),
key_spec: KeySpec::Aes256,
encryption_context: context.clone(),
})
.await
.expect("generate before stopping");
manager.stop().await.expect("stop");
assert!(manager.get_encryption_service().await.is_none(), "a stopped service hands out no handles");
assert!(!manager.health_check().await.expect("health check"), "a stopped service is unhealthy");
// Stopping twice is idempotent, not an error.
manager.stop().await.expect("a second stop must be a no-op");
manager.start().await.expect("restart after stop");
let recovered = kms
.kms()
.await
.decrypt(DecryptRequest {
ciphertext: dek.ciphertext_blob,
encryption_context: context,
grant_tokens: Vec::new(),
})
.await
.expect("work done before the stop must still be readable after the restart");
assert_eq!(recovered.plaintext, dek.plaintext_key);
}