From 2216f00cfd0d4ef4fe2ebb5324f3240ff2dae642 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=94=90=E5=B0=8F=E9=B8=AD?= Date: Tue, 28 Jul 2026 17:02:18 +0800 Subject: [PATCH] fix(kms): unify persisted SSE data key envelopes (#5343) * feat(kms): implement secure handling of static KMS secret keys and enhance encryption context validation * feat: enhance local SSE DEK handling with JSON envelope format and versioning --- .vscode/launch.json | 27 ++- crates/ecstore/src/object_api/readers.rs | 64 ++++++- crates/kms/src/api_types.rs | 8 + crates/kms/src/backends/static_kms.rs | 180 ++++++++++++++++--- crates/kms/src/config.rs | 24 +++ crates/kms/src/manager.rs | 26 +-- crates/kms/src/service.rs | 12 +- crates/kms/src/service_manager.rs | 28 +++ docs/architecture/compat-cleanup-register.md | 1 + docs/operations/rolling-restart.md | 10 ++ rustfs/src/admin/handlers/kms_dynamic.rs | 46 ++++- rustfs/src/admin/handlers/kms_management.rs | 4 +- rustfs/src/error.rs | 33 ++++ rustfs/src/storage/sse.rs | 115 +++++++++--- 14 files changed, 499 insertions(+), 79 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index f113b1d5a..394c1e979 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -172,7 +172,7 @@ ], }, { - "name": "Debug executable target/debug/rustfs with sse", + "name": "Debug executable target/debug/rustfs with sse kms", "type": "lldb", "request": "launch", "program": "${workspaceFolder}/target/debug/rustfs", @@ -200,7 +200,7 @@ // 2. kms local backend test key // "RUSTFS_KMS_ENABLE": "true", // "RUSTFS_KMS_BACKEND": "local", - // "RUSTFS_KMS_KEY_DIR": "./target/kms-key-dir", + // "RUSTFS_KMS_KEY_DIR": "/tmp/kms-key-dir", // "RUSTFS_KMS_LOCAL_MASTER_KEY": "my-secret-key", // Some Password // "RUSTFS_KMS_DEFAULT_KEY_ID": "rustfs-master-key", @@ -228,6 +228,29 @@ "rust" ], }, + { + "name": "Debug executable target/debug/rustfs with local sse", + "type": "lldb", + "request": "launch", + "program": "${workspaceFolder}/target/debug/rustfs", + "args": [], + "cwd": "${workspaceFolder}", + "env": { + "RUSTFS_ACCESS_KEY": "rustfsadmin", + "RUSTFS_SECRET_KEY": "rustfsadmin", + "RUSTFS_VOLUMES": "./target/volumes/test{1...4}", + "RUSTFS_ADDRESS": ":9000", + "RUSTFS_CONSOLE_ENABLE": "true", + "RUSTFS_CONSOLE_ADDRESS": "127.0.0.1:9001", + "RUSTFS_OBS_LOG_DIRECTORY": "./target/logs", + "RUSTFS_UNSAFE_BYPASS_DISK_CHECK": "true", + "RUSTFS_SSE_S3_MASTER_KEY": "xGb3aYSp825j2tPpg8JrUzghiXsIkfdOtmrsJ/iafiM=", + "RUST_LOG": "rustfs=debug,ecstore=debug,s3s=debug,iam=debug", + }, + "sourceLanguages": [ + "rust" + ], + }, { "type": "lldb", "request": "launch", diff --git a/crates/ecstore/src/object_api/readers.rs b/crates/ecstore/src/object_api/readers.rs index c249f64b3..1330cc10b 100644 --- a/crates/ecstore/src/object_api/readers.rs +++ b/crates/ecstore/src/object_api/readers.rs @@ -28,7 +28,6 @@ use md5::{Digest, Md5}; use rustfs_kms::{KmsUnavailableError, is_data_key_envelope, types::ObjectEncryptionContext}; use rustfs_utils::http::{SSEC_ALGORITHM_HEADER, SSEC_KEY_HEADER, SSEC_KEY_MD5_HEADER}; use rustfs_utils::path::path_join_buf; -#[cfg(feature = "rio-v2")] use serde::Deserialize; #[cfg(feature = "rio-v2")] use sha2::Sha256; @@ -44,6 +43,7 @@ const INTERNAL_ENCRYPTION_IV_HEADER: &str = "x-rustfs-encryption-iv"; const INTERNAL_ENCRYPTION_ORIGINAL_SIZE_HEADER: &str = "x-rustfs-encryption-original-size"; const SSEC_ORIGINAL_SIZE_HEADER: &str = "x-amz-server-side-encryption-customer-original-size"; const DEFAULT_SSE_ALGORITHM: &str = "AES256"; +const LOCAL_SSE_DEK_FORMAT_VERSION: u8 = 1; #[cfg(feature = "rio-v2")] const DARE_PAYLOAD_SIZE: i64 = 64 * 1024; #[cfg(feature = "rio-v2")] @@ -1716,16 +1716,39 @@ fn decrypt_local_sse_dek(encrypted_dek: &[u8], _kms_key_id: &str, object_context fn decrypt_rustfs_local_sse_dek(encrypted_dek: &[u8]) -> Result<[u8; 32]> { let encrypted_dek = std::str::from_utf8(encrypted_dek).map_err(|_| Error::other("managed DEK is not valid UTF-8"))?; - let parts: Vec<&str> = encrypted_dek.split(':').collect(); - if parts.len() != 2 { - return Err(Error::other("invalid managed DEK format")); + #[derive(Deserialize)] + #[serde(deny_unknown_fields)] + struct LocalSseDekEnvelope<'a> { + version: u8, + nonce: &'a str, + ciphertext: &'a str, } + let (nonce, ciphertext) = match serde_json::from_str::>(encrypted_dek) { + Ok(envelope) => { + if envelope.version != LOCAL_SSE_DEK_FORMAT_VERSION { + return Err(Error::other(format!("unsupported managed DEK format version: {}", envelope.version))); + } + (envelope.nonce, envelope.ciphertext) + } + Err(_) => { + // DEPRECATED: read-only compatibility for persisted colon-delimited DEKs. + // RUSTFS_COMPAT_TODO(sse-local-dek-json-v1): Remove after all supported upgrades have rewritten legacy DEKs. + let Some((nonce, ciphertext)) = encrypted_dek.split_once(':') else { + return Err(Error::other("invalid managed DEK format")); + }; + if ciphertext.contains(':') { + return Err(Error::other("invalid managed DEK format")); + } + (nonce, ciphertext) + } + }; + let nonce_vec = BASE64_STANDARD - .decode(parts[0]) + .decode(nonce) .map_err(|_| Error::other("invalid managed DEK nonce"))?; let ciphertext = BASE64_STANDARD - .decode(parts[1]) + .decode(ciphertext) .map_err(|_| Error::other("invalid managed DEK ciphertext"))?; let nonce_array: [u8; 12] = nonce_vec @@ -2324,9 +2347,36 @@ mod tests { let cipher = Aes256Gcm::new(&key); let nonce = Nonce::from([0u8; 12]); let ciphertext = cipher.encrypt(&nonce, dek.as_slice()).expect("encrypt managed dek"); + serde_json::json!({ + "version": LOCAL_SSE_DEK_FORMAT_VERSION, + "nonce": BASE64_STANDARD.encode(nonce), + "ciphertext": BASE64_STANDARD.encode(ciphertext), + }) + .to_string() + } + + fn encrypt_legacy_managed_dek_for_test(dek: [u8; 32], master_key: [u8; 32]) -> String { + let key = Key::::from(master_key); + let cipher = Aes256Gcm::new(&key); + let nonce = Nonce::from([0u8; 12]); + let ciphertext = cipher.encrypt(&nonce, dek.as_slice()).expect("encrypt legacy managed dek"); format!("{}:{}", BASE64_STANDARD.encode(nonce), BASE64_STANDARD.encode(ciphertext)) } + #[test] + fn decrypt_rustfs_local_sse_dek_rejects_unknown_json_version() { + let envelope = serde_json::json!({ + "version": LOCAL_SSE_DEK_FORMAT_VERSION + 1, + "nonce": BASE64_STANDARD.encode([0u8; 12]), + "ciphertext": BASE64_STANDARD.encode([0u8; 48]), + }) + .to_string(); + + let error = + decrypt_rustfs_local_sse_dek(envelope.as_bytes()).expect_err("unknown local SSE DEK versions must fail closed"); + assert!(error.to_string().contains("unsupported managed DEK format version")); + } + #[cfg(feature = "rio-v2")] fn seal_managed_s3_object_key_for_test( bucket: &str, @@ -2433,7 +2483,7 @@ mod tests { async_with_vars([("__RUSTFS_SSE_SIMPLE_CMK", Some(BASE64_STANDARD.encode([7u8; 32])))], async { let data_key = [0x24; 32]; let base_nonce = [0x14; 12]; - let encrypted_dek = encrypt_managed_dek_for_test(data_key, [7u8; 32]); + let encrypted_dek = encrypt_legacy_managed_dek_for_test(data_key, [7u8; 32]); let metadata = HashMap::from([ ( INTERNAL_ENCRYPTION_KEY_HEADER.to_string(), diff --git a/crates/kms/src/api_types.rs b/crates/kms/src/api_types.rs index dba7e5de7..3af643c48 100644 --- a/crates/kms/src/api_types.rs +++ b/crates/kms/src/api_types.rs @@ -161,6 +161,14 @@ pub struct ConfigureStaticKmsRequest { pub allow_insecure_dev_defaults: Option, } +impl Drop for ConfigureStaticKmsRequest { + fn drop(&mut self) { + use zeroize::Zeroize; + + self.secret_key.zeroize(); + } +} + impl fmt::Debug for ConfigureStaticKmsRequest { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("ConfigureStaticKmsRequest") diff --git a/crates/kms/src/backends/static_kms.rs b/crates/kms/src/backends/static_kms.rs index 0dc36f456..8372429a9 100644 --- a/crates/kms/src/backends/static_kms.rs +++ b/crates/kms/src/backends/static_kms.rs @@ -23,16 +23,17 @@ use crate::backends::{BackendInfo, KmsBackend, KmsClient}; use crate::config::{BackendConfig, KmsConfig}; +use crate::encryption::DataKeyEnvelope; use crate::error::{KmsError, Result}; use crate::types::*; use aes_gcm::{ Aes256Gcm, Key, Nonce, - aead::{Aead, KeyInit}, + aead::{Aead, KeyInit, Payload}, }; use async_trait::async_trait; use jiff::Zoned; use rand::RngExt; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use tracing::debug; use zeroize::Zeroizing; @@ -41,6 +42,11 @@ const NONCE_SIZE: usize = 12; /// AES-256 key size in bytes. const KEY_SIZE: usize = 32; +fn context_aad(context: &HashMap) -> Result> { + let canonical: BTreeMap<&str, &str> = context.iter().map(|(key, value)| (key.as_str(), value.as_str())).collect(); + serde_json::to_vec(&canonical).map_err(Into::into) +} + /// Static single-key KMS backend. /// /// Uses a pre-configured AES-256 key to derive data encryption keys. This is a @@ -111,21 +117,35 @@ impl KmsClient for StaticKmsBackend { let key = Key::::from(*self.key); let cipher = Aes256Gcm::new(&key); let nonce = Nonce::from(nonce_bytes); + let aad = context_aad(&request.encryption_context)?; let encrypted = cipher - .encrypt(&nonce, plaintext.as_ref()) + .encrypt( + &nonce, + Payload { + msg: plaintext.as_ref(), + aad: &aad, + }, + ) .map_err(|e| KmsError::cryptographic_error("AES-256-GCM encrypt", e.to_string()))?; - // Ciphertext format: encrypted_dek || nonce - let mut ciphertext = encrypted; - ciphertext.extend_from_slice(&nonce_bytes); + let envelope = DataKeyEnvelope { + key_id: uuid::Uuid::new_v4().to_string(), + master_key_id: request.master_key_id.clone(), + key_spec: request.key_spec.clone(), + encrypted_key: encrypted, + nonce: nonce_bytes.to_vec(), + encryption_context: request.encryption_context.clone(), + created_at: Zoned::now(), + }; + let ciphertext = serde_json::to_vec(&envelope)?; Ok(DataKeyInfo::new( self.key_id.clone(), - 0, // version is always 0 for static KMS + 0, Some(plaintext.to_vec()), ciphertext, - "AES_256".to_string(), + request.key_spec.clone(), )) } @@ -141,14 +161,28 @@ impl KmsClient for StaticKmsBackend { let key = Key::::from(*self.key); let cipher = Aes256Gcm::new(&key); let nonce = Nonce::from(nonce_bytes); + let aad = context_aad(&request.encryption_context)?; let encrypted = cipher - .encrypt(&nonce, request.plaintext.as_ref()) + .encrypt( + &nonce, + Payload { + msg: request.plaintext.as_ref(), + aad: &aad, + }, + ) .map_err(|e| KmsError::cryptographic_error("AES-256-GCM encrypt", e.to_string()))?; - // Ciphertext format: encrypted_data || nonce - let mut ciphertext = encrypted; - ciphertext.extend_from_slice(&nonce_bytes); + let envelope = DataKeyEnvelope { + key_id: uuid::Uuid::new_v4().to_string(), + master_key_id: request.key_id.clone(), + key_spec: "AES_256".to_string(), + encrypted_key: encrypted, + nonce: nonce_bytes.to_vec(), + encryption_context: request.encryption_context.clone(), + created_at: Zoned::now(), + }; + let ciphertext = serde_json::to_vec(&envelope)?; Ok(EncryptResponse { ciphertext, @@ -159,21 +193,39 @@ impl KmsClient for StaticKmsBackend { } async fn decrypt(&self, request: &DecryptRequest, _context: Option<&OperationContext>) -> Result> { - if request.ciphertext.len() < NONCE_SIZE + 1 { - return Err(KmsError::cryptographic_error("decrypt", "Ciphertext too short for static KMS format")); + let envelope: DataKeyEnvelope = serde_json::from_slice(&request.ciphertext) + .map_err(|error| KmsError::cryptographic_error("parse", format!("Failed to parse data key envelope: {error}")))?; + if envelope.master_key_id != self.key_id { + return Err(KmsError::key_not_found(&envelope.master_key_id)); } - // Split ciphertext: encrypted_data || nonce(12) - let split_at = request.ciphertext.len() - NONCE_SIZE; - let encrypted = &request.ciphertext[..split_at]; - let nonce_slice = &request.ciphertext[split_at..]; + for (key, expected_value) in &envelope.encryption_context { + match request.encryption_context.get(key) { + Some(actual_value) if actual_value == expected_value => {} + Some(actual_value) => { + return Err(KmsError::context_mismatch(format!( + "Context mismatch for key '{key}': expected '{expected_value}', got '{actual_value}'" + ))); + } + None if request.encryption_context.is_empty() => {} + None => return Err(KmsError::context_mismatch(format!("Missing context key '{key}'"))), + } + } let key = Key::::from(*self.key); let cipher = Aes256Gcm::new(&key); - let nonce = Nonce::try_from(nonce_slice).map_err(|_| KmsError::cryptographic_error("nonce", "invalid nonce length"))?; + let nonce = Nonce::try_from(envelope.nonce.as_slice()) + .map_err(|_| KmsError::cryptographic_error("nonce", "invalid nonce length"))?; + let aad = context_aad(&envelope.encryption_context)?; let plaintext = cipher - .decrypt(&nonce, encrypted) + .decrypt( + &nonce, + Payload { + msg: envelope.encrypted_key.as_ref(), + aad: &aad, + }, + ) .map_err(|e| KmsError::cryptographic_error("AES-256-GCM decrypt", e.to_string()))?; Ok(plaintext) @@ -317,7 +369,13 @@ impl KmsBackend for StaticKmsBackend { } async fn generate_data_key(&self, request: GenerateDataKeyRequest) -> Result { - let gen_req = GenerateKeyRequest::new(request.key_id.clone(), request.key_spec.as_str().to_string()); + let gen_req = GenerateKeyRequest { + master_key_id: request.key_id.clone(), + key_spec: request.key_spec.as_str().to_string(), + key_length: None, + encryption_context: request.encryption_context, + grant_tokens: Vec::new(), + }; let data_key = ::generate_data_key(self, &gen_req, None).await?; let plaintext_key = data_key @@ -378,8 +436,9 @@ impl KmsBackend for StaticKmsBackend { #[cfg(test)] mod tests { use super::*; - use crate::backends::KmsClient; + use crate::backends::{KmsBackend as KmsBackendTrait, KmsClient}; use crate::config::{BackendConfig, KmsBackend, StaticConfig}; + use crate::encryption::is_data_key_envelope; use base64::Engine as _; use base64::engine::general_purpose::STANDARD as BASE64; @@ -430,8 +489,12 @@ mod tests { assert_eq!(data_key.version, 0); assert!(data_key.plaintext.is_some()); assert_eq!(data_key.plaintext.as_ref().expect("plaintext should be set").len(), 32); - // Ciphertext should be: encrypted(32) + tag(16) + nonce(12) - assert_eq!(data_key.ciphertext.len(), 32 + 16 + NONCE_SIZE); + let envelope: DataKeyEnvelope = + serde_json::from_slice(&data_key.ciphertext).expect("static data key should use a KMS envelope"); + assert_eq!(envelope.master_key_id, key_id); + assert_eq!(envelope.encrypted_key.len(), 32 + 16); + assert_eq!(envelope.nonce.len(), NONCE_SIZE); + assert_eq!(envelope.encryption_context.get("bucket").map(String::as_str), Some("test-bucket")); // Decrypt the data key let decrypt_request = @@ -443,6 +506,75 @@ mod tests { assert_eq!(decrypted.as_slice(), data_key.plaintext.as_deref().expect("plaintext should exist")); } + #[tokio::test] + async fn generated_data_key_uses_kms_envelope_for_sse_read_routing() { + let (backend, key_id, _key) = create_test_backend().await; + let request = GenerateKeyRequest::new(key_id, "AES_256".to_string()) + .with_context("bucket".to_string(), "source-bucket".to_string()) + .with_context("object".to_string(), "source-object".to_string()); + + let data_key = KmsClient::generate_data_key(&backend, &request, None) + .await + .expect("generate static KMS data key"); + + assert!( + is_data_key_envelope(&data_key.ciphertext), + "static KMS ciphertext must use the KMS envelope recognized by the SSE read path" + ); + } + + #[tokio::test] + async fn generated_data_key_rejects_a_different_encryption_context() { + let (backend, key_id, _key) = create_test_backend().await; + let generate_request = GenerateDataKeyRequest { + key_id, + key_spec: KeySpec::Aes256, + encryption_context: HashMap::from([ + ("bucket".to_string(), "source-bucket".to_string()), + ("object".to_string(), "source-object".to_string()), + ]), + }; + let generated = KmsBackendTrait::generate_data_key(&backend, generate_request) + .await + .expect("generate context-bound static KMS data key"); + let decrypt_request = DecryptRequest { + ciphertext: generated.ciphertext_blob, + encryption_context: HashMap::from([ + ("bucket".to_string(), "different-bucket".to_string()), + ("object".to_string(), "different-object".to_string()), + ]), + grant_tokens: Vec::new(), + }; + + let error = KmsBackendTrait::decrypt(&backend, decrypt_request) + .await + .expect_err("a static KMS data key must not decrypt under a different object context"); + + assert!(matches!(error, KmsError::ContextMismatch { .. })); + } + + #[tokio::test] + async fn generated_data_key_rejects_tampered_envelope_context() { + let (backend, key_id, _key) = create_test_backend().await; + let request = GenerateKeyRequest::new(key_id, "AES_256".to_string()) + .with_context("bucket".to_string(), "source-bucket".to_string()); + let generated = KmsClient::generate_data_key(&backend, &request, None) + .await + .expect("generate context-bound data key"); + let mut envelope: DataKeyEnvelope = serde_json::from_slice(&generated.ciphertext).expect("parse static KMS envelope"); + envelope + .encryption_context + .insert("bucket".to_string(), "different-bucket".to_string()); + let decrypt_request = DecryptRequest::new(serde_json::to_vec(&envelope).expect("serialize tampered envelope")) + .with_context("bucket".to_string(), "different-bucket".to_string()); + + let error = KmsClient::decrypt(&backend, &decrypt_request, None) + .await + .expect_err("tampering with authenticated envelope context must fail"); + + assert!(matches!(error, KmsError::CryptographicError { .. })); + } + #[tokio::test] async fn test_generate_data_key_wrong_key_id() { let (backend, _key_id, _key) = create_test_backend().await; diff --git a/crates/kms/src/config.rs b/crates/kms/src/config.rs index cf6cc2177..3d2a469a5 100644 --- a/crates/kms/src/config.rs +++ b/crates/kms/src/config.rs @@ -214,9 +214,18 @@ pub struct StaticConfig { /// Key identifier (name) for the single configured key pub key_id: String, /// Base64-encoded 32-byte AES-256 key material (zeroed on drop) + #[serde(skip_serializing, default)] pub secret_key: String, } +impl Drop for StaticConfig { + fn drop(&mut self) { + use zeroize::Zeroize; + + self.secret_key.zeroize(); + } +} + impl fmt::Debug for StaticConfig { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("StaticConfig") @@ -1054,6 +1063,21 @@ mod tests { assert!(serialized.contains("persisted-token-secret")); } + #[test] + fn static_kms_config_serialization_does_not_expose_key_material() { + use base64::Engine as _; + + let encoded_key = base64::engine::general_purpose::STANDARD.encode([0x5au8; 32]); + let config = KmsConfig::static_kms("static-key".to_string(), encoded_key.clone()); + + let serialized = serde_json::to_string(&config).expect("static KMS config should serialize"); + + assert!( + !serialized.contains(&encoded_key), + "persisted static KMS configuration must not contain plaintext key material" + ); + } + #[test] fn test_config_validation() { let mut config = KmsConfig { diff --git a/crates/kms/src/manager.rs b/crates/kms/src/manager.rs index 1fb489e1a..3cfc8612c 100644 --- a/crates/kms/src/manager.rs +++ b/crates/kms/src/manager.rs @@ -31,19 +31,25 @@ use tokio::sync::RwLock; pub struct KmsManager { backend: Arc, cache: Arc>, - config: KmsConfig, + default_key_id: Option, + enable_cache: bool, } impl KmsManager { /// Create a new KMS manager with the given backend and config pub fn new(backend: Arc, config: KmsConfig) -> Self { let cache = Arc::new(RwLock::new(KmsCache::new(config.cache_config.max_keys as u64))); - Self { backend, cache, config } + Self { + backend, + cache, + default_key_id: config.default_key_id, + enable_cache: config.enable_cache, + } } /// Get the default key ID if configured pub fn get_default_key_id(&self) -> Option<&String> { - self.config.default_key_id.as_ref() + self.default_key_id.as_ref() } /// Create a new master key @@ -51,7 +57,7 @@ impl KmsManager { let response = self.backend.create_key(request).await?; // Cache the key metadata if enabled - if self.config.enable_cache { + if self.enable_cache { let mut cache = self.cache.write().await; cache.put_key_metadata(&response.key_id, &response.key_metadata).await; } @@ -77,7 +83,7 @@ impl KmsManager { /// Describe a key pub async fn describe_key(&self, request: DescribeKeyRequest) -> Result { // Check cache first if enabled - if self.config.enable_cache { + if self.enable_cache { let cache = self.cache.read().await; if let Some(cached_metadata) = cache.get_key_metadata(&request.key_id).await { return Ok(DescribeKeyResponse { @@ -89,7 +95,7 @@ impl KmsManager { // Get from backend and cache let response = self.backend.describe_key(request).await?; - if self.config.enable_cache { + if self.enable_cache { let mut cache = self.cache.write().await; cache .put_key_metadata(&response.key_metadata.key_id, &response.key_metadata) @@ -106,7 +112,7 @@ impl KmsManager { /// Get cache statistics pub async fn cache_stats(&self) -> Option<(u64, u64)> { - if self.config.enable_cache { + if self.enable_cache { let cache = self.cache.read().await; Some(cache.stats()) } else { @@ -116,7 +122,7 @@ impl KmsManager { /// Clear the cache pub async fn clear_cache(&self) -> Result<()> { - if self.config.enable_cache { + if self.enable_cache { let mut cache = self.cache.write().await; cache.clear().await; } @@ -128,7 +134,7 @@ impl KmsManager { let response = self.backend.delete_key(request).await?; // Remove from cache if enabled and key is being deleted - if self.config.enable_cache { + if self.enable_cache { let mut cache = self.cache.write().await; cache.remove_key_metadata(&response.key_id).await; } @@ -141,7 +147,7 @@ impl KmsManager { let response = self.backend.cancel_key_deletion(request).await?; // Update cache if enabled - if self.config.enable_cache { + if self.enable_cache { let mut cache = self.cache.write().await; cache.put_key_metadata(&response.key_id, &response.key_metadata).await; } diff --git a/crates/kms/src/service.rs b/crates/kms/src/service.rs index a7f91c645..2cac74953 100644 --- a/crates/kms/src/service.rs +++ b/crates/kms/src/service.rs @@ -350,11 +350,7 @@ impl ObjectEncryptionService { encryption_context: context.clone(), }; - let data_key = self - .kms_manager - .generate_data_key(request) - .await - .map_err(|e| KmsError::backend_error(format!("Failed to generate data key: {e}")))?; + let data_key = self.kms_manager.generate_data_key(request).await?; let plaintext_key = data_key.plaintext_key; @@ -431,11 +427,7 @@ impl ObjectEncryptionService { grant_tokens: Vec::new(), }; - let decrypt_response = self - .kms_manager - .decrypt(decrypt_request) - .await - .map_err(|e| KmsError::backend_error(format!("Failed to decrypt data key: {e}")))?; + let decrypt_response = self.kms_manager.decrypt(decrypt_request).await?; // Create cipher let cipher = create_cipher(&algorithm, &decrypt_response.plaintext)?; diff --git a/crates/kms/src/service_manager.rs b/crates/kms/src/service_manager.rs index f81d799fb..b3a37ca22 100644 --- a/crates/kms/src/service_manager.rs +++ b/crates/kms/src/service_manager.rs @@ -94,6 +94,16 @@ impl KmsServiceManager { self.config.read().await.clone() } + /// Get configuration for status and management responses without static key material. + pub async fn get_redacted_config(&self) -> Option { + let mut config = self.config.read().await.clone()?; + if let BackendConfig::Static(static_config) = &mut config.backend_config { + use zeroize::Zeroize; + static_config.secret_key.zeroize(); + } + Some(config) + } + /// Configure KMS with new configuration pub async fn configure(&self, new_config: KmsConfig) -> Result<()> { new_config.validate()?; @@ -449,4 +459,22 @@ mod tests { assert_eq!(manager.get_status().await, KmsServiceStatus::NotConfigured); assert!(manager.get_config().await.is_none()); } + + #[tokio::test] + async fn redacted_config_omits_static_key_material() { + use base64::Engine as _; + + let manager = KmsServiceManager::new(); + let encoded_key = base64::engine::general_purpose::STANDARD.encode([0x5au8; 32]); + manager + .configure(KmsConfig::static_kms("static-key".to_string(), encoded_key)) + .await + .expect("configure static KMS"); + + let config = manager.get_redacted_config().await.expect("redacted config"); + let BackendConfig::Static(static_config) = config.backend_config else { + panic!("expected static config"); + }; + assert!(static_config.secret_key.is_empty()); + } } diff --git a/docs/architecture/compat-cleanup-register.md b/docs/architecture/compat-cleanup-register.md index e89e4b8cd..cdd62a4b1 100644 --- a/docs/architecture/compat-cleanup-register.md +++ b/docs/architecture/compat-cleanup-register.md @@ -20,6 +20,7 @@ for later deletion. - `heal-status-rpc-v1` node heal status capability: new peers treat an unimplemented BackgroundHealStatus RPC as an explicitly incomplete rolling-upgrade response. Remove the fallback after the minimum supported RustFS peer version implements BackgroundHealStatus. - `backlog-1316` legacy encrypted multipart range seek: the feature remains opt-in until every server that can initiate, write, or complete multipart uploads supports the candidate-to-final marker protocol and uploadId commit lock, and pre-upgrade multipart uploads have drained. Remove the RUSTFS_ENCRYPTED_RANGE_SEEK switch after the minimum supported release does so; keep the quorum marker and malformed-layout full-read guards permanently. - `rustfs-5063` pre-beta.9 Local KMS recovery: persisted Local KMS configs from beta.8 and earlier predate the explicit insecure-development flag, and encrypted key files use the legacy SHA-256 KDF. Remove the config fallback after supported upgrades have rewritten or explicitly resaved all pre-beta.9 configs with the development-default field, and remove the legacy KDF after supported upgrades have rewritten all pre-beta.9 Local KMS key files with explicit at-rest protection. +- `sse-local-dek-json-v1` legacy local SSE DEK decoding: releases before the JSON envelope wrote wrapped DEKs as `base64(nonce):base64(ciphertext)`, so readers retain that decoder while all new writes use the versioned JSON envelope. Remove the colon decoder after the minimum supported direct-upgrade release writes JSON envelopes and migration tooling has rewritten every retained legacy object. ## Review Checklist diff --git a/docs/operations/rolling-restart.md b/docs/operations/rolling-restart.md index 076a0bc9f..83a52bfef 100644 --- a/docs/operations/rolling-restart.md +++ b/docs/operations/rolling-restart.md @@ -9,6 +9,16 @@ failure pattern reported in rustfs/rustfs#4304. > format. Replacing the executable and restarting is safe; no migration step > runs on startup. +> [!WARNING] +> The release that switches local SSE wrapped DEKs from the legacy +> `base64(nonce):base64(ciphertext)` representation to the versioned JSON +> envelope is a deliberate exception. Do not run that release together with +> an older RustFS version: older nodes cannot read objects written with the +> JSON envelope. Freeze every source of object mutation, including client +> writes and background lifecycle or replication work, upgrade every node, +> and then resume traffic. Downgrading or rolling back after new encrypted +> objects are written is not supported. + ## TL;DR - **Rolling restart (no downtime):** restart **one node at a time**, and wait diff --git a/rustfs/src/admin/handlers/kms_dynamic.rs b/rustfs/src/admin/handlers/kms_dynamic.rs index edcff88b1..e4753542d 100644 --- a/rustfs/src/admin/handlers/kms_dynamic.rs +++ b/rustfs/src/admin/handlers/kms_dynamic.rs @@ -36,6 +36,8 @@ use tracing::{error, info, instrument, warn}; /// Path to store KMS configuration in the cluster metadata const KMS_CONFIG_PATH: &str = "config/kms_config.json"; +const STATIC_KMS_LOCAL_CONFIG_REQUIRED: &str = + "Static KMS must be configured through RUSTFS_KMS_STATIC_SECRET_KEY or RUSTFS_KMS_STATIC_SECRET_KEY_FILE"; const LOG_COMPONENT_ADMIN: &str = "admin"; const LOG_SUBSYSTEM_KMS: &str = "kms"; const EVENT_ADMIN_KMS_DYNAMIC_STATE: &str = "admin_kms_dynamic_state"; @@ -106,9 +108,25 @@ fn normalize_configure_request_auth( Ok(()) } +fn ensure_kms_config_persistable(config: &KmsConfig) -> Result<(), String> { + if matches!(&config.backend_config, rustfs_kms::BackendConfig::Static(_)) { + return Err(STATIC_KMS_LOCAL_CONFIG_REQUIRED.to_string()); + } + Ok(()) +} + +fn ensure_kms_request_persistable(request: &ConfigureKmsRequest) -> Result<(), String> { + if matches!(request, ConfigureKmsRequest::Static(_)) { + return Err(STATIC_KMS_LOCAL_CONFIG_REQUIRED.to_string()); + } + Ok(()) +} + /// Save KMS configuration to cluster storage #[instrument(skip(config))] async fn save_kms_config(config: &KmsConfig) -> Result<(), String> { + ensure_kms_config_persistable(config)?; + let context = current_app_context(); let Some(store) = current_object_store_handle_for_context(context.as_deref()) else { return Err("Storage layer not initialized".to_string()); @@ -332,12 +350,16 @@ impl Operation for ConfigureKmsHandler { ); let service_manager = kms_service_manager_from_context(); - let existing_config = service_manager.get_config().await; + let existing_config = service_manager.get_redacted_config().await; if let Err(e) = normalize_configure_request_auth(&mut configure_request, existing_config.as_ref()) { return Ok(S3Response::new((StatusCode::BAD_REQUEST, Body::from(e)))); } + if let Err(e) = ensure_kms_request_persistable(&configure_request) { + return Ok(S3Response::new((StatusCode::BAD_REQUEST, Body::from(e)))); + } + // Convert request to KmsConfig let kms_config = configure_request.to_kms_config(); @@ -753,7 +775,7 @@ impl Operation for GetKmsStatusHandler { let service_manager = kms_service_manager_from_context(); let status = service_manager.get_status().await; - let config = service_manager.get_config().await; + let config = service_manager.get_redacted_config().await; // Get backend type and health status let backend_type = config.as_ref().map(|c| c.backend.clone()); @@ -873,12 +895,16 @@ impl Operation for ReconfigureKmsHandler { ); let service_manager = kms_service_manager_from_context(); - let existing_config = service_manager.get_config().await; + let existing_config = service_manager.get_redacted_config().await; if let Err(e) = normalize_configure_request_auth(&mut configure_request, existing_config.as_ref()) { return Ok(S3Response::new((StatusCode::BAD_REQUEST, Body::from(e)))); } + if let Err(e) = ensure_kms_request_persistable(&configure_request) { + return Ok(S3Response::new((StatusCode::BAD_REQUEST, Body::from(e)))); + } + // Convert request to KmsConfig let kms_config = configure_request.to_kms_config(); @@ -960,7 +986,7 @@ impl Operation for ReconfigureKmsHandler { #[cfg(test)] mod tests { - use super::{decode_persisted_kms_config, kms_configure_actions, kms_service_control_actions}; + use super::{decode_persisted_kms_config, ensure_kms_config_persistable, kms_configure_actions, kms_service_control_actions}; use rustfs_policy::policy::action::{Action, AdminAction, KmsAction}; use tempfile::TempDir; @@ -1046,4 +1072,16 @@ mod tests { assert!(!config.allow_insecure_dev_defaults); assert!(config.validate().is_ok()); } + + #[test] + fn static_kms_config_is_not_persisted_with_cluster_configuration() { + use base64::Engine as _; + + let config = rustfs_kms::KmsConfig::static_kms( + "static-key".to_string(), + base64::engine::general_purpose::STANDARD.encode([0x5au8; 32]), + ); + + assert!(ensure_kms_config_persistable(&config).is_err()); + } } diff --git a/rustfs/src/admin/handlers/kms_management.rs b/rustfs/src/admin/handlers/kms_management.rs index b958654ab..bbeca1d90 100644 --- a/rustfs/src/admin/handlers/kms_management.rs +++ b/rustfs/src/admin/handlers/kms_management.rs @@ -193,7 +193,7 @@ impl Operation for KmsStatusHandler { hit_count: hits, miss_count: misses, }); - let config = kms_service_manager_from_context().get_config().await; + let config = kms_service_manager_from_context().get_redacted_config().await; let response = KmsStatusResponse { backend_type: config @@ -243,7 +243,7 @@ impl Operation for KmsConfigHandler { }; let config = kms_service_manager_from_context() - .get_config() + .get_redacted_config() .await .ok_or_else(|| s3_error!(InternalError, "KMS config not available"))?; diff --git a/rustfs/src/error.rs b/rustfs/src/error.rs index 831f7314a..f4cf54dd1 100644 --- a/rustfs/src/error.rs +++ b/rustfs/src/error.rs @@ -245,6 +245,21 @@ impl From for ApiError { }; } + if let StorageError::Io(ref io_err) = err + && matches!( + io_err + .get_ref() + .and_then(|inner| inner.downcast_ref::()), + Some(rustfs_kms::KmsError::BackendError { .. }) + ) + { + return ApiError { + code: S3ErrorCode::ServiceUnavailable, + message: ApiError::error_code_to_message(&S3ErrorCode::ServiceUnavailable), + source: Some(Box::new(err)), + }; + } + let code = match &err { StorageError::NotImplemented => S3ErrorCode::NotImplemented, StorageError::InvalidArgument(_, _, _) => S3ErrorCode::InvalidArgument, @@ -487,6 +502,14 @@ mod tests { assert_eq!(api_error.message, "The service is unavailable. Please retry."); } + #[test] + fn test_kms_backend_unavailable_maps_to_retryable_error() { + let api_error = ApiError::from(StorageError::other(rustfs_kms::KmsError::backend_error("Vault connection refused"))); + + assert_eq!(api_error.code, S3ErrorCode::ServiceUnavailable); + assert_eq!(api_error.message, "The service is unavailable. Please retry."); + } + #[test] fn test_unknown_authoritative_quota_usage_maps_to_retryable_error() { let api_error = ApiError::from(QuotaError::UsageUnavailable { @@ -497,6 +520,16 @@ mod tests { assert_eq!(api_error.message, "The service is unavailable. Please retry."); } + #[test] + fn test_kms_cryptographic_error_is_not_retryable() { + let api_error = ApiError::from(StorageError::other(rustfs_kms::KmsError::cryptographic_error( + "decrypt", + "authentication failed", + ))); + + assert_eq!(api_error.code, S3ErrorCode::InternalError); + } + #[test] fn test_api_error_from_storage_error_mappings() { let test_cases = vec![ diff --git a/rustfs/src/storage/sse.rs b/rustfs/src/storage/sse.rs index e24ee5c90..36aa2116b 100644 --- a/rustfs/src/storage/sse.rs +++ b/rustfs/src/storage/sse.rs @@ -91,6 +91,7 @@ use rustfs_kms::{DataKey, KmsUnavailableError, is_data_key_envelope, types::Obje use rustfs_utils::get_env_opt_str; use s3s::S3ErrorCode; use s3s::dto::ServerSideEncryption; +use serde::{Deserialize, Serialize}; #[cfg(feature = "rio-v2")] use sha2::Sha256; use std::collections::HashMap; @@ -1885,6 +1886,10 @@ struct KmsSseDekProvider { service_manager: Option>, } +fn kms_operation_error(error: rustfs_kms::KmsError) -> ApiError { + ApiError::from(StorageError::other(error)) +} + impl KmsSseDekProvider { /// Create a new KMS-backed provider pub async fn new() -> Result { @@ -1936,7 +1941,7 @@ impl SseDekProvider for KmsSseDekProvider { let (data_key, encrypted_data_key) = service .create_data_key(&kms_key_option, context) .await - .map_err(|e| ApiError::from(StorageError::other(format!("Failed to create data key: {}", e))))?; + .map_err(kms_operation_error)?; Ok((data_key, encrypted_data_key)) } @@ -1954,7 +1959,7 @@ impl SseDekProvider for KmsSseDekProvider { let data_key = service .decrypt_data_key(encrypted_dek, context) .await - .map_err(|e| ApiError::from(StorageError::other(format!("Failed to decrypt data key: {}", e))))?; + .map_err(kms_operation_error)?; Ok(data_key.plaintext_key) } @@ -1973,7 +1978,7 @@ impl SseDekProvider for KmsSseDekProvider { let data_key = service .decrypt_legacy_data_key(encrypted_dek) .await - .map_err(|e| ApiError::from(StorageError::other(format!("Failed to decrypt legacy data key: {e}"))))?; + .map_err(kms_operation_error)?; Ok(data_key.plaintext_key) } @@ -1998,6 +2003,16 @@ pub(crate) struct LocalSseDekProvider { master_key: [u8; 32], } +const LOCAL_SSE_DEK_FORMAT_VERSION: u8 = 1; + +#[derive(Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +struct LocalSseDekEnvelope<'a> { + version: u8, + nonce: &'a str, + ciphertext: &'a str, +} + /// Test-only alias so existing test code that references `TestSseDekProvider` /// continues to compile without changes. #[cfg(test)] @@ -2099,22 +2114,51 @@ impl LocalSseDekProvider { .encrypt(&nonce, dek.as_slice()) .map_err(|_| ApiError::from(StorageError::other("Failed to encrypt DEK")))?; - // nonce:ciphertext - Ok(format!("{}:{}", BASE64_STANDARD.encode(nonce), BASE64_STANDARD.encode(ciphertext))) + let nonce = BASE64_STANDARD.encode(nonce); + let ciphertext = BASE64_STANDARD.encode(ciphertext); + serde_json::to_string(&LocalSseDekEnvelope { + version: LOCAL_SSE_DEK_FORMAT_VERSION, + nonce: &nonce, + ciphertext: &ciphertext, + }) + .map_err(|e| ApiError::from(StorageError::other(format!("Failed to serialize encrypted DEK: {e}")))) } // Simple decryption of DEK pub(crate) fn decrypt_dek(encrypted_dek: &str, cmk_value: [u8; 32]) -> Result<[u8; 32], ApiError> { - let parts: Vec<&str> = encrypted_dek.split(':').collect(); - if parts.len() != 2 { - return Err(ApiError::from(StorageError::other("Invalid encrypted DEK format"))); - } - + let envelope = serde_json::from_str::>(encrypted_dek); + let (nonce, ciphertext) = match envelope { + Ok(envelope) => { + if envelope.version != LOCAL_SSE_DEK_FORMAT_VERSION { + return Err(ApiError::from(StorageError::other(format!( + "Unsupported encrypted DEK format version: {}", + envelope.version + )))); + } + (envelope.nonce, envelope.ciphertext) + } + Err(json_error) if encrypted_dek.trim_start().starts_with('{') => { + return Err(ApiError::from(StorageError::other(format!( + "Invalid encrypted DEK JSON format: {json_error}" + )))); + } + Err(_) => { + // DEPRECATED: read-only compatibility for persisted colon-delimited DEKs. + // RUSTFS_COMPAT_TODO(sse-local-dek-json-v1): Remove after all supported upgrades have rewritten legacy DEKs. + let Some((nonce, ciphertext)) = encrypted_dek.split_once(':') else { + return Err(ApiError::from(StorageError::other("Invalid encrypted DEK format"))); + }; + if ciphertext.contains(':') { + return Err(ApiError::from(StorageError::other("Invalid encrypted DEK format"))); + } + (nonce, ciphertext) + } + }; let nonce_vec = BASE64_STANDARD - .decode(parts[0]) + .decode(nonce) .map_err(|_| ApiError::from(StorageError::other("Invalid nonce format")))?; let ciphertext = BASE64_STANDARD - .decode(parts[1]) + .decode(ciphertext) .map_err(|_| ApiError::from(StorageError::other("Invalid ciphertext format")))?; let key = Key::::from(cmk_value); @@ -2596,10 +2640,10 @@ mod tests { SseDekProvider, SsecParams, StorageError, TestSseDekProvider, apply_managed_decryption_material, apply_managed_encryption_material, encryption_material_to_metadata, extract_server_side_encryption_from_headers, extract_ssec_params_from_headers, extract_ssekms_context_from_headers, generate_ssec_nonce, is_managed_sse, - map_get_object_reader_error, mark_encrypted_multipart_metadata, normalize_managed_metadata, reset_sse_dek_provider, - resolve_effective_kms_key_id, sse_decryption, sse_encryption, sse_prepare_encryption, strip_managed_encryption_metadata, - validate_sse_headers_for_read, validate_sse_headers_for_write, validate_ssec_for_read, validate_ssec_params, - verify_ssec_key_match, + kms_operation_error, map_get_object_reader_error, mark_encrypted_multipart_metadata, normalize_managed_metadata, + reset_sse_dek_provider, resolve_effective_kms_key_id, sse_decryption, sse_encryption, sse_prepare_encryption, + strip_managed_encryption_metadata, validate_sse_headers_for_read, validate_sse_headers_for_write, validate_ssec_for_read, + validate_ssec_params, verify_ssec_key_match, }; #[cfg(feature = "rio-v2")] use super::{ @@ -2622,6 +2666,15 @@ mod tests { assert!(super::parse_simple_sse_cmk(&zero).is_err()); } + #[test] + fn kms_operation_errors_preserve_retryability_classification() { + let unavailable = kms_operation_error(rustfs_kms::KmsError::backend_error("connection refused")); + let corrupt = kms_operation_error(rustfs_kms::KmsError::cryptographic_error("decrypt", "authentication failed")); + + assert_eq!(unavailable.code, S3ErrorCode::ServiceUnavailable); + assert_eq!(corrupt.code, S3ErrorCode::InternalError); + } + #[test] fn parse_simple_sse_cmk_accepts_valid_32_byte_key() { let mut key = [0u8; 32]; @@ -4023,17 +4076,25 @@ mod tests { } #[test] - fn test_encrypt_dek_uses_random_nonce_prefixes() { + fn test_encrypt_dek_writes_json_with_random_nonces() { let dek = [0x11u8; 32]; let cmk = [0x22u8; 32]; let encrypted_a = TestSseDekProvider::encrypt_dek(dek, cmk).expect("first DEK wrap should succeed"); let encrypted_b = TestSseDekProvider::encrypt_dek(dek, cmk).expect("second DEK wrap should succeed"); - let nonce_a = encrypted_a.split(':').next().expect("wrapped DEK should contain nonce"); - let nonce_b = encrypted_b.split(':').next().expect("wrapped DEK should contain nonce"); + let envelope_a: super::LocalSseDekEnvelope = + serde_json::from_str(&encrypted_a).expect("first wrapped DEK should be a JSON envelope"); + let envelope_b: super::LocalSseDekEnvelope = + serde_json::from_str(&encrypted_b).expect("second wrapped DEK should be a JSON envelope"); - assert_ne!(nonce_a, nonce_b, "each DEK wrap should use a distinct nonce prefix"); + assert_eq!(envelope_a.version, super::LOCAL_SSE_DEK_FORMAT_VERSION); + assert_eq!(envelope_b.version, super::LOCAL_SSE_DEK_FORMAT_VERSION); + assert_ne!(envelope_a.nonce, envelope_b.nonce, "each DEK wrap should use a distinct nonce"); + assert_eq!( + TestSseDekProvider::decrypt_dek(&encrypted_a, cmk).expect("first JSON envelope should decrypt"), + dek + ); } #[test] @@ -4051,6 +4112,20 @@ mod tests { assert_eq!(decrypted, dek); } + #[test] + fn test_decrypt_dek_rejects_unknown_json_version() { + let envelope = serde_json::json!({ + "version": super::LOCAL_SSE_DEK_FORMAT_VERSION + 1, + "nonce": BASE64_STANDARD.encode([0u8; 12]), + "ciphertext": BASE64_STANDARD.encode([0u8; 48]), + }) + .to_string(); + + let error = TestSseDekProvider::decrypt_dek(&envelope, [0x55u8; 32]) + .expect_err("unknown JSON envelope versions must fail closed"); + assert!(error.message.contains("Unsupported encrypted DEK format version")); + } + #[tokio::test] async fn test_sse_encryption_fails_closed_without_local_sse_master_key() { let _guard = lock_sse_test_state().await;