feat(kms): enable safe local KMS evaluation workflow

This commit is contained in:
唐小鸭
2026-07-29 13:01:27 +08:00
parent 7d698abc1f
commit 353984d48b
6 changed files with 551 additions and 140 deletions
Generated
+1
View File
@@ -9460,6 +9460,7 @@ dependencies = [
"serde",
"serde_json",
"sha2 0.11.0",
"subtle",
"temp-env",
"tempfile",
"thiserror 2.0.19",
+1
View File
@@ -45,6 +45,7 @@ chacha20poly1305 = { workspace = true }
rand = { workspace = true, features = ["serde"] }
base64 = { workspace = true }
sha2 = { workspace = true }
subtle = { workspace = true }
zeroize = { workspace = true, features = ["derive"] }
# Configuration and storage
+209 -126
View File
@@ -35,7 +35,6 @@ use std::collections::HashMap;
use std::path::{Component, Path, PathBuf};
use std::time::Duration;
use tokio::fs;
use tokio::sync::RwLock;
use tracing::{debug, warn};
/// Reject key identifiers that would not name a single file directly inside the key
@@ -77,8 +76,6 @@ const LOCAL_KMS_ARGON2_P_COST: u32 = 1;
/// Local KMS client that stores keys in local files
pub struct LocalKmsClient {
config: LocalConfig,
/// In-memory cache of loaded keys for performance
key_cache: RwLock<HashMap<String, MasterKeyInfo>>,
/// Master encryption key for encrypting stored keys
master_cipher: Option<Aes256Gcm>,
/// Legacy pre-beta.9 master cipher for reading pre-Argon2 key files
@@ -139,13 +136,14 @@ impl LocalKmsClient {
(None, None)
};
Ok(Self {
let client = Self {
config,
key_cache: RwLock::new(HashMap::new()),
master_cipher,
legacy_master_cipher,
dek_crypto: AesDekCrypto::new(),
})
};
client.validate_existing_keys().await?;
Ok(client)
}
/// Derive a 256-bit key from the master key string using a persistent Argon2id salt.
@@ -193,10 +191,35 @@ impl LocalKmsClient {
let mut salt = [0u8; LOCAL_KMS_MASTER_KEY_SALT_LEN];
rand::rng().fill(&mut salt[..]);
fs::write(&salt_path, salt).await?;
Self::set_file_permissions(&salt_path, config.file_permissions).await?;
debug!(path = ?salt_path, "Local KMS master key salt created");
Ok(salt)
let temp_path = config
.key_dir
.join(format!("{LOCAL_KMS_MASTER_KEY_SALT_FILE}.tmp-{}", uuid::Uuid::new_v4()));
fs::write(&temp_path, salt).await?;
Self::set_file_permissions(&temp_path, config.file_permissions).await?;
match fs::hard_link(&temp_path, &salt_path).await {
Ok(()) => {
if let Err(error) = fs::remove_file(&temp_path).await {
warn!(path = ?temp_path, %error, "Failed to remove Local KMS salt temporary file");
}
debug!(path = ?salt_path, "Local KMS master key salt created");
Ok(salt)
}
Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => {
let _ = fs::remove_file(&temp_path).await;
let bytes = fs::read(&salt_path).await?;
bytes.try_into().map_err(|_| {
KmsError::configuration_error(format!(
"Local KMS master key salt at {} must be exactly {} bytes",
salt_path.display(),
LOCAL_KMS_MASTER_KEY_SALT_LEN
))
})
}
Err(error) => {
let _ = fs::remove_file(&temp_path).await;
Err(error.into())
}
}
}
#[cfg(unix)]
@@ -242,6 +265,12 @@ impl LocalKmsClient {
let content = fs::read(&key_path).await?;
let stored_key: StoredMasterKey = serde_json::from_slice(&content)?;
if stored_key.key_id != key_id {
return Err(KmsError::invalid_key(format!(
"Local KMS key file identity mismatch: expected {key_id:?}, found {:?}",
stored_key.key_id
)));
}
let encrypted_bytes = BASE64
.decode(&stored_key.encrypted_key_material)
@@ -326,8 +355,43 @@ impl LocalKmsClient {
/// Save a master key to disk
async fn save_master_key(&self, master_key: &MasterKeyInfo, key_material: &[u8]) -> Result<()> {
let key_path = self.master_key_path(&master_key.key_id)?;
let content = self.encode_master_key(master_key, key_material)?;
let temp_path = key_path.with_extension(format!("tmp-{}", uuid::Uuid::new_v4()));
fs::write(&temp_path, &content).await?;
Self::set_file_permissions(&temp_path, self.config.file_permissions).await?;
fs::rename(&temp_path, &key_path).await?;
// Encrypt key material if master cipher is available
debug!(key_id = %master_key.key_id, path = ?key_path, "Local KMS master key saved");
Ok(())
}
async fn save_new_master_key(&self, master_key: &MasterKeyInfo, key_material: &[u8]) -> Result<()> {
let key_path = self.master_key_path(&master_key.key_id)?;
let content = self.encode_master_key(master_key, key_material)?;
let temp_path = key_path.with_extension(format!("tmp-{}", uuid::Uuid::new_v4()));
fs::write(&temp_path, &content).await?;
Self::set_file_permissions(&temp_path, self.config.file_permissions).await?;
match fs::hard_link(&temp_path, &key_path).await {
Ok(()) => {
if let Err(error) = fs::remove_file(&temp_path).await {
warn!(path = ?temp_path, %error, "Failed to remove Local KMS key temporary file");
}
debug!(key_id = %master_key.key_id, path = ?key_path, "Local KMS master key created");
Ok(())
}
Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => {
let _ = fs::remove_file(&temp_path).await;
Err(KmsError::key_already_exists(&master_key.key_id))
}
Err(error) => {
let _ = fs::remove_file(&temp_path).await;
Err(error.into())
}
}
}
fn encode_master_key(&self, master_key: &MasterKeyInfo, key_material: &[u8]) -> Result<Vec<u8>> {
let (encrypted_key_material, nonce, at_rest_protection) = if let Some(ref cipher) = self.master_cipher {
let mut nonce_bytes = [0u8; 12];
rand::rng().fill(&mut nonce_bytes[..]);
@@ -362,17 +426,7 @@ impl LocalKmsClient {
at_rest_protection,
};
let content = serde_json::to_vec_pretty(&stored_key)?;
// Write to temporary file first, then rename for atomicity
let temp_path = key_path.with_extension("tmp");
fs::write(&temp_path, &content).await?;
Self::set_file_permissions(&temp_path, self.config.file_permissions).await?;
fs::rename(&temp_path, &key_path).await?;
debug!(key_id = %master_key.key_id, path = ?key_path, "Local KMS master key saved");
Ok(())
serde_json::to_vec_pretty(&stored_key).map_err(Into::into)
}
/// Get the actual key material for a master key
@@ -381,6 +435,23 @@ impl LocalKmsClient {
Ok(key_material)
}
async fn validate_existing_keys(&self) -> Result<()> {
let mut entries = fs::read_dir(&self.config.key_dir).await?;
while let Some(entry) = entries.next_entry().await? {
let path = entry.path();
if path.extension().is_none_or(|extension| extension != "key") {
continue;
}
let key_id = path
.file_stem()
.and_then(|stem| stem.to_str())
.ok_or_else(|| KmsError::configuration_error("Local KMS key file name must be valid UTF-8"))?;
self.decode_stored_key(key_id).await?;
}
Ok(())
}
/// Encrypt data using a master key
async fn encrypt_with_master_key(&self, key_id: &str, plaintext: &[u8]) -> Result<(Vec<u8>, Vec<u8>)> {
// Load the actual master key material
@@ -513,11 +584,7 @@ impl KmsClient for LocalKmsClient {
let master_key = MasterKeyInfo::new_with_description(key_id.to_string(), algorithm.to_string(), Some(created_by), None);
// Save to disk
self.save_master_key(&master_key, &key_material).await?;
// Cache the key
let mut cache = self.key_cache.write().await;
cache.insert(key_id.to_string(), master_key.clone());
self.save_new_master_key(&master_key, &key_material).await?;
debug!(key_id, "Local KMS master key created");
Ok(master_key)
@@ -526,23 +593,7 @@ impl KmsClient for LocalKmsClient {
async fn describe_key(&self, key_id: &str, _context: Option<&OperationContext>) -> Result<KeyInfo> {
debug!("Describing key: {}", key_id);
// Check cache first
{
let cache = self.key_cache.read().await;
if let Some(master_key) = cache.get(key_id) {
return Ok(master_key.clone().into());
}
}
// Load from disk
let master_key = self.load_master_key(key_id).await?;
// Update cache
{
let mut cache = self.key_cache.write().await;
cache.insert(key_id.to_string(), master_key.clone());
}
Ok(master_key.into())
}
@@ -602,10 +653,6 @@ impl KmsClient for LocalKmsClient {
let key_material = self.get_key_material(key_id).await?;
self.save_master_key(&master_key, &key_material).await?;
// Update cache
let mut cache = self.key_cache.write().await;
cache.insert(key_id.to_string(), master_key);
debug!(key_id, "Local KMS key enabled");
Ok(())
}
@@ -621,10 +668,6 @@ impl KmsClient for LocalKmsClient {
let key_material = self.get_key_material(key_id).await?;
self.save_master_key(&master_key, &key_material).await?;
// Update cache
let mut cache = self.key_cache.write().await;
cache.insert(key_id.to_string(), master_key);
debug!(key_id, "Local KMS key disabled");
Ok(())
}
@@ -646,10 +689,6 @@ impl KmsClient for LocalKmsClient {
let key_material = self.get_key_material(key_id).await?;
self.save_master_key(&master_key, &key_material).await?;
// Update cache
let mut cache = self.key_cache.write().await;
cache.insert(key_id.to_string(), master_key);
debug!(key_id, "Local KMS key deletion scheduled");
Ok(())
}
@@ -665,31 +704,17 @@ impl KmsClient for LocalKmsClient {
let key_material = self.get_key_material(key_id).await?;
self.save_master_key(&master_key, &key_material).await?;
// Update cache
let mut cache = self.key_cache.write().await;
cache.insert(key_id.to_string(), master_key);
debug!(key_id, "Local KMS key deletion canceled");
Ok(())
}
async fn rotate_key(&self, key_id: &str, _context: Option<&OperationContext>) -> Result<MasterKeyInfo> {
debug!("Rotating key: {}", key_id);
let mut master_key = self.load_master_key(key_id).await?;
master_key.version += 1;
master_key.rotated_at = Some(Zoned::now());
// Generate new key material
let key_material = generate_key_material(&master_key.algorithm)?;
self.save_master_key(&master_key, &key_material).await?;
// Update cache
let mut cache = self.key_cache.write().await;
cache.insert(key_id.to_string(), master_key.clone());
debug!(key_id, "Local KMS key rotated");
Ok(master_key)
if !fs::try_exists(self.master_key_path(key_id)?).await? {
return Err(KmsError::key_not_found(key_id));
}
Err(KmsError::invalid_operation(
"Local KMS key rotation is unavailable until historical key versions can be retained",
))
}
async fn health_check(&self) -> Result<()> {
@@ -745,11 +770,6 @@ impl KmsBackend for LocalKmsBackend {
async fn create_key(&self, request: CreateKeyRequest) -> Result<CreateKeyResponse> {
let key_id = request.key_name.unwrap_or_else(|| uuid::Uuid::new_v4().to_string());
// `save_master_key` writes through a temp file and renames over the destination, so
// creating a key under an existing name would replace its material and silently
// destroy the ability to decrypt everything wrapped under it. The sibling
// `KmsClient::create_key` has always refused this; the backend path did not, and
// this is the path the admin API uses.
if self.client.master_key_path(&key_id)?.exists() {
return Err(KmsError::key_already_exists(&key_id));
}
@@ -767,11 +787,8 @@ impl KmsBackend for LocalKmsBackend {
request.description.clone(),
);
// Save to disk and cache
self.client.save_master_key(&master_key, &key_material).await?;
let mut cache = self.client.key_cache.write().await;
cache.insert(key_id.clone(), master_key.clone());
// Save to disk
self.client.save_new_master_key(&master_key, &key_material).await?;
master_key
};
@@ -888,10 +905,6 @@ impl KmsBackend for LocalKmsBackend {
.await
.map_err(|e| KmsError::internal_error(format!("Failed to delete key file: {e}")))?;
// Remove from cache
let mut cache = self.client.key_cache.write().await;
cache.remove(key_id);
debug!(key_id, "Local KMS key deleted immediately");
// Return success response for immediate deletion
@@ -935,10 +948,6 @@ impl KmsBackend for LocalKmsBackend {
self.client.save_master_key(&master_key, &existing_key_material).await?;
// Update cache
let mut cache = self.client.key_cache.write().await;
cache.insert(key_id.to_string(), master_key.clone());
// Convert master_key to KeyMetadata for response
let key_metadata = KeyMetadata {
key_id: master_key.key_id.clone(),
@@ -986,10 +995,6 @@ impl KmsBackend for LocalKmsBackend {
self.client.save_master_key(&master_key, &existing_key_material).await?;
// Update cache
let mut cache = self.client.key_cache.write().await;
cache.insert(key_id.to_string(), master_key.clone());
// Convert master_key to KeyMetadata for response
let key_metadata = KeyMetadata {
key_id: master_key.key_id.clone(),
@@ -1189,6 +1194,18 @@ mod tests {
.expect("stored encrypted key should deserialize");
assert_eq!(stored.at_rest_protection, StoredKeyProtection::EncryptedMasterKey);
assert_eq!(stored.nonce.len(), 12);
let wrong_master_error = match LocalKmsClient::new(LocalConfig {
key_dir: client.config.key_dir.clone(),
master_key: Some("wrong-master-key".to_string()),
file_permissions: Some(0o600),
})
.await
{
Ok(_) => panic!("wrong master key must fail initialization"),
Err(error) => error,
};
assert!(matches!(wrong_master_error, KmsError::CryptographicError { .. }));
}
#[tokio::test]
@@ -1228,17 +1245,57 @@ mod tests {
master_key: None,
file_permissions: Some(0o600),
};
let client_without_master = LocalKmsClient::new(config)
.await
.expect("client without master key should still initialize in dev-mode tests");
let err = client_without_master
.describe_key("encrypted-key", None)
.await
.expect_err("encrypted key should require a master key to read");
let err = match LocalKmsClient::new(config).await {
Ok(_) => panic!("initialization must reject an unreadable encrypted key"),
Err(error) => error,
};
assert!(err.to_string().contains("requires a configured master key"));
}
#[tokio::test]
async fn local_key_rotation_is_rejected_without_overwriting_key_material() {
let (client, _temp_dir) = create_test_client().await;
let key_id = "rotation-key";
client.create_key(key_id, "AES_256", None).await.expect("create key");
let original_material = client.get_key_material(key_id).await.expect("load original material");
let error = client
.rotate_key(key_id, None)
.await
.expect_err("rotation must remain unavailable without historical key versions");
assert!(matches!(error, KmsError::InvalidOperation { .. }));
assert_eq!(
client.get_key_material(key_id).await.expect("reload original material"),
original_material
);
}
#[tokio::test]
async fn startup_rejects_key_file_with_mismatched_embedded_id() {
let (client, temp_dir) = create_dev_mode_client().await;
client.create_key("file-name", "AES_256", None).await.expect("create key");
let key_path = client.master_key_path("file-name").expect("valid key id");
let mut stored: serde_json::Value =
serde_json::from_slice(&fs::read(&key_path).await.expect("read key file")).expect("decode key file");
stored["key_id"] = serde_json::json!("embedded-name");
fs::write(&key_path, serde_json::to_vec_pretty(&stored).expect("encode mismatched key"))
.await
.expect("write mismatched key");
let error = match LocalKmsClient::new(LocalConfig {
key_dir: temp_dir.path().to_path_buf(),
master_key: None,
file_permissions: Some(0o600),
})
.await
{
Ok(_) => panic!("mismatched key identity must fail initialization"),
Err(error) => error,
};
assert!(matches!(error, KmsError::InvalidKey { .. }));
}
#[tokio::test]
async fn test_load_master_key_accepts_legacy_rfc3339_timestamp() {
let (client, _temp_dir) = create_dev_mode_client().await;
@@ -1329,17 +1386,6 @@ mod tests {
)
.await
.expect("write beta.5 fixture");
let mut explicit_protection = stored_key.clone();
let explicit_object = explicit_protection.as_object_mut().expect("beta.5 fixture is a JSON object");
explicit_object.insert("key_id".to_string(), serde_json::json!("beta5-explicit-key"));
explicit_object.insert("at_rest_protection".to_string(), serde_json::json!("encrypted-master-key"));
fs::write(
temp_dir.path().join("beta5-explicit-key.key"),
serde_json::to_vec_pretty(&explicit_protection).expect("serialize explicit-protection fixture"),
)
.await
.expect("write explicit-protection fixture");
let client = LocalKmsClient::new(LocalConfig {
key_dir: temp_dir.path().to_path_buf(),
master_key: Some("beta5-test-master-key".to_string()),
@@ -1353,24 +1399,34 @@ mod tests {
.await
.expect("decrypt beta.5 SHA-256 protected key");
assert_eq!(material, vec![0x42; 32]);
let mut explicit_protection = stored_key.clone();
let explicit_object = explicit_protection.as_object_mut().expect("beta.5 fixture is a JSON object");
explicit_object.insert("key_id".to_string(), serde_json::json!("beta5-explicit-key"));
explicit_object.insert("at_rest_protection".to_string(), serde_json::json!("encrypted-master-key"));
fs::write(
temp_dir.path().join("beta5-explicit-key.key"),
serde_json::to_vec_pretty(&explicit_protection).expect("serialize explicit-protection fixture"),
)
.await
.expect("write explicit-protection fixture");
let explicit_error = client
.get_key_material("beta5-explicit-key")
.await
.expect_err("explicit current protection must not fall back to the beta.5 KDF");
assert!(matches!(explicit_error, KmsError::CryptographicError { .. }));
let wrong_key_client = LocalKmsClient::new(LocalConfig {
let wrong_key_error = match LocalKmsClient::new(LocalConfig {
key_dir: temp_dir.path().to_path_buf(),
master_key: Some("wrong-beta5-master-key".to_string()),
file_permissions: Some(0o600),
})
.await
.expect("initialize local KMS with wrong beta.5 master key");
let error = wrong_key_client
.get_key_material("beta5-key")
.await
.expect_err("wrong beta.5 master key must not decrypt the fixture");
assert!(matches!(error, KmsError::CryptographicError { .. }));
{
Ok(_) => panic!("wrong beta.5 master key must fail initialization"),
Err(error) => error,
};
assert!(matches!(wrong_key_error, KmsError::CryptographicError { .. }));
}
/// R03-CAN-072 / R03-CAN-073: key identifiers arrive from request input, so every path
@@ -1429,9 +1485,7 @@ mod tests {
}
}
/// R07-CAN-103: `save_master_key` writes a temp file and renames over the destination,
/// so creating a key under an existing name would replace its material and silently
/// destroy the ability to decrypt anything wrapped under it.
/// R07-CAN-103: creating a duplicate key must preserve its original material.
#[tokio::test]
async fn backend_create_key_refuses_to_replace_existing_key_material() {
let temp_dir = TempDir::new().expect("Failed to create temp dir");
@@ -1469,4 +1523,33 @@ mod tests {
.expect("original key material must survive the refused create");
assert_eq!(original, after, "existing key material must not be replaced");
}
#[tokio::test]
async fn concurrent_backend_create_allows_only_one_writer() {
let temp_dir = TempDir::new().expect("create key directory");
let config = || LocalConfig {
key_dir: temp_dir.path().to_path_buf(),
master_key: Some("test-master-key".to_string()),
file_permissions: Some(0o600),
};
let first = LocalKmsBackend {
client: LocalKmsClient::new(config()).await.expect("create first client"),
};
let second = LocalKmsBackend {
client: LocalKmsClient::new(config()).await.expect("create second client"),
};
let request = || CreateKeyRequest {
key_name: Some("concurrent-key".to_string()),
..Default::default()
};
let (first_result, second_result) = tokio::join!(first.create_key(request()), second.create_key(request()));
assert_ne!(first_result.is_ok(), second_result.is_ok(), "exactly one create must succeed");
let error = first_result
.err()
.or_else(|| second_result.err())
.expect("one create must fail");
assert!(matches!(error, KmsError::KeyAlreadyExists { .. }));
assert_eq!(first.client.get_key_material("concurrent-key").await.expect("load key").len(), 32);
}
}
+2 -2
View File
@@ -177,8 +177,8 @@ mod tests {
let service1 = manager.get_encryption_service().await.expect("Service should be available");
// Reconfigure to new service (zero-downtime)
let temp_dir2 = TempDir::new().expect("Failed to create temp dir");
let config2 = KmsConfig::local(temp_dir2.path().to_path_buf()).with_insecure_development_defaults();
let mut config2 = config1;
config2.timeout = std::time::Duration::from_secs(45);
manager.reconfigure(config2).await.expect("Reconfiguration should succeed");
// Verify version 2
+162 -6
View File
@@ -20,10 +20,12 @@ use crate::error::{KmsError, Result};
use crate::manager::KmsManager;
use crate::service::ObjectEncryptionService;
use arc_swap::ArcSwap;
use sha2::{Digest, Sha256};
use std::sync::{
Arc, OnceLock,
atomic::{AtomicU64, Ordering},
};
use subtle::ConstantTimeEq;
use tokio::sync::{Mutex, RwLock};
use tracing::{debug, error, info, warn};
@@ -31,6 +33,53 @@ const LOG_COMPONENT_KMS: &str = "kms";
const LOG_SUBSYSTEM_SERVICE: &str = "service";
const EVENT_KMS_SERVICE_STATE: &str = "kms_service_state";
fn local_master_key_fingerprint(master_key: Option<&str>) -> [u8; 32] {
let mut digest = Sha256::new();
digest.update([u8::from(master_key.is_some())]);
if let Some(master_key) = master_key {
digest.update(master_key.as_bytes());
}
digest.finalize().into()
}
fn validate_local_transition(current: Option<&KmsConfig>, new: &KmsConfig) -> Result<()> {
let Some(current) = current else {
return Ok(());
};
let BackendConfig::Local(current_local) = &current.backend_config else {
return Ok(());
};
let BackendConfig::Local(new_local) = &new.backend_config else {
return Err(KmsError::configuration_error("Local KMS backend cannot be changed after configuration"));
};
if current_local.key_dir != new_local.key_dir {
return Err(KmsError::configuration_error(
"Local KMS key directory cannot be changed after configuration",
));
}
if current_local.file_permissions != new_local.file_permissions {
return Err(KmsError::configuration_error(
"Local KMS file permissions cannot be changed after configuration",
));
}
if current.allow_insecure_dev_defaults != new.allow_insecure_dev_defaults {
return Err(KmsError::configuration_error(
"Local KMS development mode cannot be changed after configuration",
));
}
let current_master_key = local_master_key_fingerprint(current_local.master_key.as_deref());
let new_master_key = local_master_key_fingerprint(new_local.master_key.as_deref());
if !bool::from(current_master_key.ct_eq(&new_master_key)) {
return Err(KmsError::configuration_error(
"Local KMS master key cannot be changed after configuration",
));
}
Ok(())
}
/// KMS service status
#[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)]
pub enum KmsServiceStatus {
@@ -106,7 +155,12 @@ impl KmsServiceManager {
/// Configure KMS with new configuration
pub async fn configure(&self, new_config: KmsConfig) -> Result<()> {
let _guard = self.lifecycle_mutex.lock().await;
new_config.validate()?;
{
let config = self.config.read().await;
validate_local_transition(config.as_ref(), &new_config)?;
}
// Update configuration
{
@@ -254,11 +308,9 @@ impl KmsServiceManager {
"KMS service reconfiguring"
);
new_config.validate()?;
// Configure with new config
{
let mut config = self.config.write().await;
*config = Some(new_config.clone());
let config = self.config.read().await;
validate_local_transition(config.as_ref(), &new_config)?;
}
// Create new service version without stopping old one
@@ -268,6 +320,11 @@ impl KmsServiceManager {
// Get old version for logging (lock-free read)
let old_version = self.current_service.load().as_ref().as_ref().map(|sv| sv.version);
{
let mut config = self.config.write().await;
*config = Some(new_config);
}
// Atomically switch to new service version (lock-free, instant CAS operation)
// This is a true atomic operation - no waiting for locks, instant switch
// Old service will be dropped when no more Arc references exist
@@ -304,8 +361,6 @@ impl KmsServiceManager {
Err(e) => {
let err_msg = format!("Failed to reconfigure KMS: {e}");
error!("{}", err_msg);
let mut status = self.status.write().await;
*status = KmsServiceStatus::Error(err_msg.clone());
Err(KmsError::backend_error(&err_msg))
}
}
@@ -477,4 +532,105 @@ mod tests {
};
assert!(static_config.secret_key.is_empty());
}
#[tokio::test]
async fn forbidden_local_master_key_change_preserves_running_config_and_service() {
use crate::types::{CreateKeyRequest, KeyUsage};
use std::collections::HashMap;
use tempfile::TempDir;
let key_dir = TempDir::new().expect("create local KMS directory");
let config = |master_key: &str| {
let mut config = KmsConfig::local(key_dir.path().to_path_buf());
let BackendConfig::Local(local) = &mut config.backend_config else {
panic!("local constructor must create local backend config");
};
local.master_key = Some(master_key.to_string());
config.allow_insecure_dev_defaults = true;
config
};
let manager = KmsServiceManager::new();
manager
.configure(config("working-master-key"))
.await
.expect("configure local KMS");
manager.start().await.expect("start local KMS");
manager
.get_manager()
.await
.expect("running KMS manager")
.create_key(CreateKeyRequest {
key_name: Some("existing-key".to_string()),
key_usage: KeyUsage::EncryptDecrypt,
description: None,
policy: None,
tags: HashMap::new(),
origin: None,
})
.await
.expect("create encrypted key");
let service_version = manager.get_service_version().await;
let error = manager
.reconfigure(config("wrong-master-key"))
.await
.expect_err("local master key change must be rejected");
assert!(error.to_string().contains("master key cannot be changed"));
assert_eq!(manager.get_status().await, KmsServiceStatus::Running);
assert_eq!(manager.get_service_version().await, service_version);
let current = manager.get_config().await.expect("working config must remain");
let BackendConfig::Local(local) = current.backend_config else {
panic!("working config must remain local");
};
assert_eq!(local.master_key.as_deref(), Some("working-master-key"));
}
#[tokio::test]
async fn configure_cannot_replace_existing_local_backend() {
use base64::Engine as _;
use tempfile::TempDir;
let key_dir = TempDir::new().expect("create local KMS directory");
let mut local = KmsConfig::local(key_dir.path().to_path_buf());
local.allow_insecure_dev_defaults = true;
let manager = KmsServiceManager::new();
manager.configure(local.clone()).await.expect("configure local KMS");
let encoded_key = base64::engine::general_purpose::STANDARD.encode([0x5au8; 32]);
let error = manager
.configure(KmsConfig::static_kms("static-key".to_string(), encoded_key))
.await
.expect_err("existing local backend must be immutable");
assert!(error.to_string().contains("backend cannot be changed"));
let current = manager.get_config().await.expect("local config must remain");
assert!(matches!(current.backend_config, BackendConfig::Local(_)));
}
#[tokio::test]
async fn reconfigure_allows_safe_local_runtime_settings_only() {
use tempfile::TempDir;
let key_dir = TempDir::new().expect("create local KMS directory");
let mut initial = KmsConfig::local(key_dir.path().to_path_buf());
initial.allow_insecure_dev_defaults = true;
let manager = KmsServiceManager::new();
manager.configure(initial.clone()).await.expect("configure local KMS");
manager.start().await.expect("start local KMS");
let mut updated = initial;
updated.default_key_id = Some("evaluation-key".to_string());
updated.timeout = std::time::Duration::from_secs(45);
updated.enable_cache = false;
manager
.reconfigure(updated.clone())
.await
.expect("update safe local settings");
let current = manager.get_config().await.expect("updated config");
assert_eq!(current.default_key_id, updated.default_key_id);
assert_eq!(current.timeout, updated.timeout);
assert!(!current.enable_cache);
}
}
+176 -6
View File
@@ -79,10 +79,40 @@ fn kms_service_control_actions() -> Vec<Action> {
vec![Action::KmsAction(KmsAction::ServiceControlAction)]
}
fn normalize_configure_request_auth(
fn normalize_configure_request_secrets(
request: &mut ConfigureKmsRequest,
existing_config: Option<&KmsConfig>,
) -> Result<(), String> {
if existing_config.is_some_and(|config| matches!(&config.backend_config, rustfs_kms::BackendConfig::Local(_)))
&& !matches!(request, ConfigureKmsRequest::Local(_))
{
return Err("Changing from the Local KMS backend is not supported".to_string());
}
if let ConfigureKmsRequest::Local(request) = request
&& let Some(KmsConfig {
backend_config: rustfs_kms::BackendConfig::Local(existing),
allow_insecure_dev_defaults,
..
}) = existing_config
{
if request.key_dir != existing.key_dir {
return Err("Changing the Local KMS key directory is not supported".to_string());
}
match request.file_permissions {
Some(permissions) if Some(permissions) != existing.file_permissions => {
return Err("Changing Local KMS file permissions is not supported".to_string());
}
None => request.file_permissions = existing.file_permissions,
Some(_) => {}
}
if request.master_key.as_deref().is_some_and(|master_key| !master_key.is_empty()) {
return Err("Changing the Local KMS master key is not supported".to_string());
}
request.master_key.clone_from(&existing.master_key);
request.allow_insecure_dev_defaults = Some(*allow_insecure_dev_defaults);
}
let needs_existing_auth = match request {
ConfigureKmsRequest::VaultKv2(req) => token_is_blank(&req.auth_method),
ConfigureKmsRequest::VaultTransit(req) => token_is_blank(&req.auth_method),
@@ -350,9 +380,9 @@ impl Operation for ConfigureKmsHandler {
);
let service_manager = kms_service_manager_from_context();
let existing_config = service_manager.get_redacted_config().await;
let existing_config = service_manager.get_config().await;
if let Err(e) = normalize_configure_request_auth(&mut configure_request, existing_config.as_ref()) {
if let Err(e) = normalize_configure_request_secrets(&mut configure_request, existing_config.as_ref()) {
return Ok(S3Response::new((StatusCode::BAD_REQUEST, Body::from(e))));
}
@@ -895,9 +925,9 @@ impl Operation for ReconfigureKmsHandler {
);
let service_manager = kms_service_manager_from_context();
let existing_config = service_manager.get_redacted_config().await;
let existing_config = service_manager.get_config().await;
if let Err(e) = normalize_configure_request_auth(&mut configure_request, existing_config.as_ref()) {
if let Err(e) = normalize_configure_request_secrets(&mut configure_request, existing_config.as_ref()) {
return Ok(S3Response::new((StatusCode::BAD_REQUEST, Body::from(e))));
}
@@ -986,8 +1016,12 @@ impl Operation for ReconfigureKmsHandler {
#[cfg(test)]
mod tests {
use super::{decode_persisted_kms_config, ensure_kms_config_persistable, kms_configure_actions, kms_service_control_actions};
use super::{
decode_persisted_kms_config, ensure_kms_config_persistable, kms_configure_actions, kms_service_control_actions,
normalize_configure_request_secrets,
};
use rustfs_policy::policy::action::{Action, AdminAction, KmsAction};
use std::path::PathBuf;
use tempfile::TempDir;
fn assert_has_action(actions: &[Action], action: Action) {
@@ -1084,4 +1118,140 @@ mod tests {
assert!(ensure_kms_config_persistable(&config).is_err());
}
#[test]
fn local_reconfigure_preserves_hidden_master_key_for_same_directory() {
let key_dir = PathBuf::from("/var/lib/rustfs/kms");
let mut existing = rustfs_kms::KmsConfig::local(key_dir.clone());
let rustfs_kms::BackendConfig::Local(existing_local) = &mut existing.backend_config else {
panic!("local constructor must create local backend config");
};
existing_local.master_key = Some("stored-master-key".to_string());
let mut request = rustfs_kms::ConfigureKmsRequest::Local(rustfs_kms::ConfigureLocalKmsRequest {
key_dir,
master_key: None,
file_permissions: Some(0o600),
default_key_id: Some("experience-key".to_string()),
timeout_seconds: Some(30),
retry_attempts: Some(3),
enable_cache: Some(true),
max_cached_keys: Some(1000),
cache_ttl_seconds: Some(3600),
allow_insecure_dev_defaults: Some(false),
});
normalize_configure_request_secrets(&mut request, Some(&existing)).expect("normalize local request");
let rustfs_kms::ConfigureKmsRequest::Local(request) = request else {
panic!("request must remain local");
};
assert_eq!(request.master_key.as_deref(), Some("stored-master-key"));
}
#[test]
fn local_reconfigure_preserves_unspecified_legacy_file_permissions() {
let key_dir = PathBuf::from("/var/lib/rustfs/kms");
let mut existing = rustfs_kms::KmsConfig::local(key_dir.clone());
let rustfs_kms::BackendConfig::Local(existing_local) = &mut existing.backend_config else {
panic!("local constructor must create local backend config");
};
existing_local.master_key = Some("stored-master-key".to_string());
existing_local.file_permissions = None;
let mut request = rustfs_kms::ConfigureKmsRequest::Local(rustfs_kms::ConfigureLocalKmsRequest {
key_dir,
master_key: None,
file_permissions: None,
default_key_id: Some("experience-key".to_string()),
timeout_seconds: None,
retry_attempts: None,
enable_cache: None,
max_cached_keys: None,
cache_ttl_seconds: None,
allow_insecure_dev_defaults: Some(false),
});
normalize_configure_request_secrets(&mut request, Some(&existing)).expect("normalize legacy local request");
let rustfs_kms::ConfigureKmsRequest::Local(request) = request else {
panic!("request must remain local");
};
assert!(request.file_permissions.is_none());
}
#[test]
fn local_reconfigure_does_not_reuse_master_key_for_different_directory() {
let mut existing = rustfs_kms::KmsConfig::local(PathBuf::from("/var/lib/rustfs/kms"));
let rustfs_kms::BackendConfig::Local(existing_local) = &mut existing.backend_config else {
panic!("local constructor must create local backend config");
};
existing_local.master_key = Some("stored-master-key".to_string());
let mut request = rustfs_kms::ConfigureKmsRequest::Local(rustfs_kms::ConfigureLocalKmsRequest {
key_dir: PathBuf::from("/var/lib/rustfs/other-kms"),
master_key: None,
file_permissions: Some(0o600),
default_key_id: None,
timeout_seconds: None,
retry_attempts: None,
enable_cache: None,
max_cached_keys: None,
cache_ttl_seconds: None,
allow_insecure_dev_defaults: Some(false),
});
let error = normalize_configure_request_secrets(&mut request, Some(&existing))
.expect_err("changing the local key directory must be rejected");
assert_eq!(error, "Changing the Local KMS key directory is not supported");
}
#[test]
fn local_reconfigure_rejects_file_permission_and_master_key_changes() {
let key_dir = PathBuf::from("/var/lib/rustfs/kms");
let mut existing = rustfs_kms::KmsConfig::local(key_dir.clone());
let rustfs_kms::BackendConfig::Local(existing_local) = &mut existing.backend_config else {
panic!("local constructor must create local backend config");
};
existing_local.master_key = Some("stored-master-key".to_string());
existing_local.file_permissions = Some(0o600);
let request = |master_key, file_permissions| {
rustfs_kms::ConfigureKmsRequest::Local(rustfs_kms::ConfigureLocalKmsRequest {
key_dir: key_dir.clone(),
master_key,
file_permissions,
default_key_id: None,
timeout_seconds: None,
retry_attempts: None,
enable_cache: None,
max_cached_keys: None,
cache_ttl_seconds: None,
allow_insecure_dev_defaults: Some(false),
})
};
let mut permissions_change = request(None, Some(0o666));
let permissions_error = normalize_configure_request_secrets(&mut permissions_change, Some(&existing))
.expect_err("changing file permissions must be rejected");
assert_eq!(permissions_error, "Changing Local KMS file permissions is not supported");
let mut master_key_change = request(Some("replacement-master-key".to_string()), Some(0o600));
let master_key_error = normalize_configure_request_secrets(&mut master_key_change, Some(&existing))
.expect_err("changing the master key must be rejected");
assert_eq!(master_key_error, "Changing the Local KMS master key is not supported");
let mut backend_change = rustfs_kms::ConfigureKmsRequest::Static(rustfs_kms::ConfigureStaticKmsRequest {
key_id: "static-key".to_string(),
secret_key: "not-used-by-normalization".to_string(),
default_key_id: None,
timeout_seconds: None,
retry_attempts: None,
enable_cache: None,
max_cached_keys: None,
cache_ttl_seconds: None,
allow_insecure_dev_defaults: None,
});
let backend_error = normalize_configure_request_secrets(&mut backend_change, Some(&existing))
.expect_err("changing from the local backend must be rejected");
assert_eq!(backend_error, "Changing from the Local KMS backend is not supported");
}
}