fix(kms): report accurate Vault KV2 security contract and disable unsafe rotation (#5474)

crates/kms/src/api_types.rs

@@ -71,7 +71,10 @@ impl fmt::Debug for ConfigureLocalKmsRequest {
     }
 }
 
-/// Request to configure KMS with Vault KV v2 + Transit backend
+/// Request to configure KMS with the Vault KV v2 storage backend.
+///
+/// This backend stores master key material directly in KV v2; confidentiality relies on
+/// Vault ACLs and KV v2 at-rest encryption, with no Transit wrapping involved.
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(deny_unknown_fields)]
 pub struct ConfigureVaultKmsRequest {
@@ -82,7 +85,8 @@ pub struct ConfigureVaultKmsRequest {
     pub auth_method: VaultAuthMethod,
     /// Vault namespace (Vault Enterprise, optional)
     pub namespace: Option<String>,
-    /// Transit engine mount path
+    /// Deprecated: legacy Transit engine mount path. Still accepted so older clients keep
+    /// working, but the Vault KV2 backend never uses it.
     pub mount_path: Option<String>,
     /// KV engine mount path for storing keys  
     pub kv_mount: Option<String>,
@@ -192,7 +196,7 @@ pub enum ConfigureKmsRequest {
     /// Configure with Local backend
     #[serde(alias = "local", alias = "Local")]
     Local(ConfigureLocalKmsRequest),
-    /// Configure with Vault KV v2 + Transit backend
+    /// Configure with the Vault KV v2 storage backend
     #[serde(
         rename = "VaultKV2",
         alias = "Vault",
@@ -333,7 +337,7 @@ pub enum BackendSummary {
         /// File permissions (octal)
         file_permissions: Option<u32>,
     },
-    /// Vault KV v2 + Transit backend summary
+    /// Vault KV v2 storage backend summary
     #[serde(alias = "vault")]
     VaultKv2 {
         /// Vault server address
@@ -344,7 +348,8 @@ pub enum BackendSummary {
         has_stored_credentials: bool,
         /// Namespace (if configured)
         namespace: Option<String>,
-        /// Transit engine mount path
+        /// Deprecated: legacy Transit mount path. Unused by the backend; kept only so the
+        /// serialized response shape stays stable for existing consumers.
         mount_path: String,
         /// KV engine mount path
         kv_mount: String,
@@ -621,6 +626,52 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_deserialize_vault_kv2_configure_request_mount_path_optional_but_accepted() {
+        // deny_unknown_fields regression guard: mount_path is deprecated but must remain
+        // accepted so older clients that still send it do not get a 400.
+        let with_mount_path = serde_json::json!({
+            "backend_type": "VaultKV2",
+            "address": "http://127.0.0.1:8200",
+            "auth_method": { "Token": { "token": "dev-root-token" } },
+            "mount_path": "transit"
+        });
+        let request: ConfigureKmsRequest =
+            serde_json::from_value(with_mount_path).expect("request with deprecated mount_path should deserialize");
+        let config = request.to_kms_config();
+        assert_eq!(config.vault_config().expect("vault-kv2 config").mount_path, "transit");
+
+        let without_mount_path = serde_json::json!({
+            "backend_type": "VaultKV2",
+            "address": "http://127.0.0.1:8200",
+            "auth_method": { "Token": { "token": "dev-root-token" } }
+        });
+        let request: ConfigureKmsRequest =
+            serde_json::from_value(without_mount_path).expect("request without mount_path should deserialize");
+        let config = request.to_kms_config();
+        assert_eq!(config.vault_config().expect("vault-kv2 config").mount_path, "transit");
+    }
+
+    #[test]
+    fn test_vault_kv2_status_summary_does_not_mention_transit() {
+        let config = KmsConfig::vault(
+            url::Url::parse("https://vault.example.com:8200").expect("vault URL"),
+            "summary-token".to_string(),
+        );
+        let response = KmsStatusResponse {
+            status: KmsServiceStatus::Running,
+            backend_type: Some(config.backend.clone()),
+            healthy: Some(true),
+            config_summary: Some(KmsConfigSummary::from(&config)),
+        };
+
+        let json = serde_json::to_string(&response).expect("kms status response should serialize");
+        assert!(
+            !json.contains("Transit"),
+            "vault-kv2 status output must not describe the backend as Transit: {json}"
+        );
+    }
+
     #[test]
     fn test_deserialize_vault_transit_configure_request() {
         let cases = ["VaultTransit", "vault-transit", "vault_transit"];

crates/kms/src/backends/vault.rs

@@ -150,17 +150,18 @@ impl VaultKmsClient {
         format!("{}/{}", self.key_path_prefix, key_id)
     }
 
-    /// Encrypt key material using Vault's transit engine
+    /// Encode key material for KV2 storage.
+    ///
+    /// This is plain Base64 encoding, not encryption: the KV2 backend stores master key
+    /// material as-is and relies on Vault ACLs plus KV2 at-rest encryption for
+    /// confidentiality. Any identity with KV read access to the key path can recover the
+    /// plaintext master key.
     async fn encrypt_key_material(&self, key_material: &[u8]) -> Result<String> {
-        // For simplicity, we'll base64 encode the key material
-        // In a production setup, you would use Vault's transit engine for additional encryption
         Ok(general_purpose::STANDARD.encode(key_material))
     }
 
-    /// Decrypt key material
+    /// Decode key material from KV2 storage (plain Base64, see `encrypt_key_material`).
     async fn decrypt_key_material(&self, encrypted_material: &str) -> Result<Vec<u8>> {
-        // For simplicity, we'll base64 decode the key material
-        // In a production setup, you would use Vault's transit engine for decryption
         general_purpose::STANDARD
             .decode(encrypted_material)
             .map_err(|e| KmsError::cryptographic_error("decrypt", e.to_string()))
@@ -529,33 +530,13 @@ impl KmsClient for VaultKmsClient {
         Ok(())
     }
 
-    async fn rotate_key(&self, key_id: &str, _context: Option<&OperationContext>) -> Result<MasterKeyInfo> {
-        debug!("Rotating key: {}", key_id);
-
-        let mut key_data = self.get_key_data(key_id).await?;
-        key_data.version += 1;
-
-        // Generate new key material
-        let key_material = generate_key_material(&key_data.algorithm)?;
-        key_data.encrypted_key_material = self.encrypt_key_material(&key_material).await?;
-
-        self.store_key_data(key_id, &key_data).await?;
-
-        let master_key = MasterKeyInfo {
-            key_id: key_id.to_string(),
-            version: key_data.version,
-            algorithm: key_data.algorithm,
-            usage: key_data.usage,
-            status: key_data.status,
-            description: None, // Rotate preserves existing description (would need key lookup)
-            metadata: key_data.metadata,
-            created_at: key_data.created_at,
-            rotated_at: Some(Zoned::now()),
-            created_by: None,
-        };
-
-        debug!(key_id, "Vault KMS key rotated");
-        Ok(master_key)
+    async fn rotate_key(&self, _key_id: &str, _context: Option<&OperationContext>) -> Result<MasterKeyInfo> {
+        // Rotation previously overwrote the stored master key with fresh material, which
+        // permanently orphaned every DEK wrapped by the prior version. Reject before any
+        // storage access so existing material can never be touched.
+        Err(KmsError::invalid_operation(
+            "Vault KV2 rotation is unavailable until versioned key material retention lands",
+        ))
     }
 
     async fn health_check(&self) -> Result<()> {
@@ -585,6 +566,9 @@ impl KmsClient for VaultKmsClient {
         BackendInfo::new("vault-kv2".to_string(), "0.1.0".to_string(), self.config.address.clone(), true)
             .with_metadata("kv_mount".to_string(), self.kv_mount.clone())
             .with_metadata("key_prefix".to_string(), self.key_path_prefix.clone())
+            // Master key material is protected only by Vault ACLs and KV2 at-rest
+            // encryption; there is no additional cryptographic wrapping.
+            .with_metadata("at_rest_protection".to_string(), "vault-kv2-acl".to_string())
     }
 }
 
@@ -950,6 +934,54 @@ mod tests {
         }
     }
 
+    #[tokio::test]
+    async fn test_vault_kv2_rotate_key_rejected_without_touching_storage() {
+        // No Vault instance needed: rotation must be rejected before any storage access,
+        // so the call cannot read or overwrite key material.
+        let client = VaultKmsClient::new(integration_vault_config()).await.expect("client");
+
+        let err = client
+            .rotate_key("any-key", None)
+            .await
+            .expect_err("Vault KV2 rotation must be rejected");
+        assert!(matches!(err, KmsError::InvalidOperation { .. }), "expected InvalidOperation, got {err:?}");
+        assert!(err.to_string().contains("rotation is unavailable"));
+    }
+
+    #[tokio::test]
+    async fn test_vault_kv2_backend_info_reports_at_rest_protection() {
+        let client = VaultKmsClient::new(integration_vault_config()).await.expect("client");
+
+        let info = client.backend_info();
+        assert_eq!(info.backend_type, "vault-kv2");
+        assert_eq!(info.metadata.get("at_rest_protection").map(String::as_str), Some("vault-kv2-acl"));
+        // The KV2 backend must not present itself as Transit-backed.
+        assert!(!format!("{info:?}").contains("Transit"));
+    }
+
+    #[tokio::test]
+    #[ignore] // Requires a running Vault instance (dev mode)
+    async fn test_vault_kv2_rotate_rejected_and_material_untouched() {
+        let client = VaultKmsClient::new(integration_vault_config()).await.expect("client");
+
+        let key_id = format!("rotate-{}", uuid::Uuid::new_v4());
+        client.create_key(&key_id, "AES_256", None).await.expect("create");
+        let before = client.get_key_data(&key_id).await.expect("read");
+
+        let err = client
+            .rotate_key(&key_id, None)
+            .await
+            .expect_err("Vault KV2 rotation must be rejected");
+        assert!(matches!(err, KmsError::InvalidOperation { .. }));
+
+        let after = client.get_key_data(&key_id).await.expect("reread");
+        assert_eq!(
+            after.encrypted_key_material, before.encrypted_key_material,
+            "rejected rotation must leave stored key material untouched"
+        );
+        assert_eq!(after.version, before.version, "rejected rotation must not bump the key version");
+    }
+
     #[tokio::test]
     #[ignore] // Requires a running Vault instance (dev mode)
     async fn test_corrupted_key_material_does_not_regenerate() {

crates/kms/src/config.rs

@@ -49,6 +49,10 @@ fn default_vault_transit_metadata_key_prefix() -> String {
     DEFAULT_VAULT_TRANSIT_METADATA_KEY_PREFIX.to_string()
 }
 
+fn default_vault_kv2_mount_path() -> String {
+    "transit".to_string()
+}
+
 pub const KMS_CONFIG_REDACTION_RULES: &[RedactionRule] = &[
     RedactionRule::new("kms.local.master_key", RedactionLevel::Secret, "local backend key encryption material"),
     RedactionRule::new("kms.vault.token", RedactionLevel::Secret, "vault authentication token"),
@@ -100,7 +104,9 @@ pub(crate) fn redacted_secret_option(value: Option<&str>) -> Option<&'static str
 /// KMS backend types
 #[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq, Eq)]
 pub enum KmsBackend {
-    /// Vault KV v2 + Transit backend (key metadata in KV, wrapping via Transit)
+    /// Vault KV v2 storage backend: master key material is stored directly in KV v2.
+    /// Confidentiality relies on Vault ACLs, KV v2 at-rest encryption, and TLS; the
+    /// backend performs no Transit wrapping of key material.
     #[serde(rename = "VaultKV2", alias = "Vault")]
     VaultKv2,
     /// Vault Transit backend using Vault as the cryptographic source of truth
@@ -162,7 +168,7 @@ impl Default for KmsConfig {
 pub enum BackendConfig {
     /// Local backend configuration
     Local(LocalConfig),
-    /// Vault KV v2 + Transit backend configuration
+    /// Vault KV v2 storage backend configuration
     #[serde(rename = "VaultKV2", alias = "Vault")]
     VaultKv2(Box<VaultConfig>),
     /// Vault Transit backend configuration
@@ -270,7 +276,11 @@ impl StaticConfig {
     }
 }
 
-/// Vault KV v2 + Transit backend configuration (metadata in KV, key wrapping via Transit)
+/// Vault KV v2 backend configuration.
+///
+/// Key material and metadata are stored directly in KV v2; any identity with KV read
+/// access to the key path can recover plaintext master key material. Use the Vault
+/// Transit backend when cryptographic isolation of key material is required.
 #[derive(Clone, Serialize, Deserialize)]
 pub struct VaultConfig {
     /// Vault server URL
@@ -279,7 +289,10 @@ pub struct VaultConfig {
     pub auth_method: VaultAuthMethod,
     /// Vault namespace (Vault Enterprise)
     pub namespace: Option<String>,
-    /// Transit engine mount path
+    /// Deprecated: legacy Transit engine mount path. The Vault KV2 backend never calls
+    /// the Transit engine, so this value is unused; the field is retained (and
+    /// defaulted) only so previously persisted configurations keep deserializing.
+    #[serde(default = "default_vault_kv2_mount_path")]
     pub mount_path: String,
     /// KV engine mount path for storing keys
     pub kv_mount: String,
@@ -439,7 +452,12 @@ impl KmsConfig {
         }
     }
 
-    /// Create a new KMS configuration for Vault backend with token authentication (recommended for production)
+    /// Create a new KMS configuration for the Vault KV v2 backend with token authentication.
+    ///
+    /// Master key material is stored directly in Vault KV v2: confidentiality relies on
+    /// Vault ACLs, KV v2 at-rest encryption, and TLS. KV read access to the key path is
+    /// equivalent to holding the plaintext master keys. Use [`KmsConfig::vault_transit`]
+    /// when key material must never be readable through Vault storage APIs.
     pub fn vault(address: Url, token: String) -> Self {
         Self {
             backend: KmsBackend::VaultKv2,
@@ -452,7 +470,10 @@ impl KmsConfig {
         }
     }
 
-    /// Create a new KMS configuration for Vault backend with AppRole authentication (recommended for production)
+    /// Create a new KMS configuration for the Vault KV v2 backend with AppRole authentication.
+    ///
+    /// Shares the security boundary described on [`KmsConfig::vault`]: key material lives
+    /// in KV v2 and is protected only by Vault ACLs and KV v2 at-rest encryption.
     pub fn vault_approle(address: Url, role_id: String, secret_id: String) -> Self {
         Self {
             backend: KmsBackend::VaultKv2,
@@ -620,9 +641,8 @@ impl KmsConfig {
                     validate_vault_development_defaults("Vault KV2", &config.address, &config.auth_method, config.tls.as_ref())?;
                 }
 
-                if config.mount_path.is_empty() {
-                    return Err(KmsError::configuration_error("Vault KV2 mount path cannot be empty"));
-                }
+                // `mount_path` is deprecated and unused by this backend, so an empty value
+                // is deliberately not an error.
 
                 // Validate TLS configuration if using HTTPS
                 if config.address.starts_with("https://")
@@ -747,11 +767,21 @@ impl KmsConfig {
                 let token = get_env_str("RUSTFS_KMS_VAULT_TOKEN", "dev-token");
                 let skip_tls_verify = get_env_bool(ENV_KMS_VAULT_SKIP_TLS_VERIFY, false);
 
+                let mount_path = match get_env_opt_str("RUSTFS_KMS_VAULT_MOUNT_PATH") {
+                    Some(path) => {
+                        tracing::warn!(
+                            "RUSTFS_KMS_VAULT_MOUNT_PATH is deprecated for the Vault KV2 backend: it never calls the Transit engine and the value is stored but unused"
+                        );
+                        path
+                    }
+                    None => default_vault_kv2_mount_path(),
+                };
+
                 config.backend_config = BackendConfig::VaultKv2(Box::new(VaultConfig {
                     address,
                     auth_method: VaultAuthMethod::Token { token },
                     namespace: get_env_opt_str("RUSTFS_KMS_VAULT_NAMESPACE"),
-                    mount_path: get_env_str("RUSTFS_KMS_VAULT_MOUNT_PATH", "transit"),
+                    mount_path,
                     kv_mount: get_env_str("RUSTFS_KMS_VAULT_KV_MOUNT", "secret"),
                     key_path_prefix: get_env_str("RUSTFS_KMS_VAULT_KEY_PREFIX", "rustfs/kms/keys"),
                     tls: vault_tls_config(skip_tls_verify),
@@ -1044,6 +1074,73 @@ mod tests {
         assert!(config.vault_config().is_some());
     }
 
+    #[test]
+    fn test_persisted_vault_kv2_config_without_mount_path_deserializes() {
+        // Configurations persisted after mount_path was deprecated may omit the field;
+        // it must default instead of failing deserialization.
+        let raw = r#"{
+            "backend": "VaultKV2",
+            "backend_config": {
+                "VaultKV2": {
+                    "address": "http://127.0.0.1:8200",
+                    "auth_method": { "Token": { "token": "t" } },
+                    "namespace": null,
+                    "kv_mount": "secret",
+                    "key_path_prefix": "rustfs/kms/keys",
+                    "tls": null
+                }
+            },
+            "default_key_id": null,
+            "timeout": {"secs": 30, "nanos": 0},
+            "retry_attempts": 3,
+            "enable_cache": true,
+            "cache_config": {
+                "max_keys": 1000,
+                "ttl": {"secs": 3600, "nanos": 0},
+                "enable_metrics": true
+            }
+        }"#;
+        let config: KmsConfig = serde_json::from_str(raw).expect("persisted kms config without mount_path");
+        assert_eq!(config.backend, KmsBackend::VaultKv2);
+        let vault = config.vault_config().expect("vault-kv2 config");
+        assert_eq!(vault.mount_path, "transit");
+    }
+
+    #[test]
+    fn test_vault_kv2_empty_mount_path_passes_validation() {
+        let address = Url::parse("https://vault.example.com:8200").expect("Valid URL");
+        let mut config = KmsConfig::vault(address, "test-token".to_string());
+        if let BackendConfig::VaultKv2(vault) = &mut config.backend_config {
+            vault.mount_path = String::new();
+        }
+        assert!(config.validate().is_ok(), "deprecated mount_path must not be required");
+    }
+
+    #[test]
+    fn test_vault_kv2_sources_do_not_claim_transit_wrapping() {
+        let sources = [
+            ("config.rs", include_str!("config.rs")),
+            ("api_types.rs", include_str!("api_types.rs")),
+            ("backends/vault.rs", include_str!("backends/vault.rs")),
+            ("lib.rs", include_str!("lib.rs")),
+        ];
+        // Assemble the needles at runtime so this guard does not match its own source.
+        let needles = [
+            format!("wrapping via {}", "Transit"),
+            format!("KV v2 + {}", "Transit"),
+            format!("KV2+{}", "Transit"),
+            format!("you would use Vault's {} engine", "transit"),
+        ];
+        for (name, source) in sources {
+            for needle in &needles {
+                assert!(
+                    !source.contains(needle.as_str()),
+                    "{name} still describes the Vault KV2 backend with `{needle}`"
+                );
+            }
+        }
+    }
+
     #[test]
     fn test_legacy_persisted_vault_transit_config_uses_metadata_defaults() {
         let raw = r#"{

crates/kms/src/lib.rs

@@ -20,7 +20,7 @@
 //!
 //! ## Features
 //!
-//! - **Multiple Backends**: Local file storage, Vault KV2+Transit, and Vault Transit (optional)
+//! - **Multiple Backends**: Local file storage, Vault KV2 (plain KV storage), and Vault Transit (optional)
 //! - **Object Encryption**: Transparent S3-compatible object encryption
 //! - **Streaming Encryption**: Memory-efficient encryption for large files
 //! - **Key Management**: Full lifecycle management of encryption keys

docs/operations/kms-backend-security.md

@@ -0,0 +1,78 @@
+# KMS backend security properties
+
+RustFS ships several KMS backends. They differ not only in deployment effort
+but in **where master key material lives and who can read it**. Pick a backend
+based on the confidentiality boundary you need, not on the name alone.
+
+## Backend comparison
+
+| Backend | Config tag | Master key material location | At-rest protection of key material | Rotation | Intended use |
+| --- | --- | --- | --- | --- | --- |
+| Local | `Local` | Files under `key_dir`, encrypted with the configured local master key | Local master key (AES-GCM) + file permissions | Rejected (no versioned retention yet) | Development; single-node setups that accept host-level trust |
+| Static | `Static` | Provided out-of-band via environment/file; never persisted by RustFS | Operator-managed secret distribution | Rejected (read-only backend) | Simple deployments with an external secret manager |
+| Vault KV2 | `VaultKV2` (legacy alias `Vault`) | Stored **directly** in Vault KV v2 (Base64-encoded plaintext) | Vault ACLs + KV v2 at-rest encryption + TLS only | Rejected (no versioned retention yet) | Deployments that accept Vault KV ACLs as the sole confidentiality boundary |
+| Vault Transit | `VaultTransit` | Key-encryption keys never leave Vault; only Transit ciphertext is visible outside | Vault Transit engine (cryptographic isolation) | Via Vault Transit key versioning | Deployments that need key material to be unreadable through storage APIs |
+
+## Vault KV2: what the backend does and does not do
+
+The Vault KV2 backend uses Vault purely as a **secure storage** service:
+
+- Master key material is generated by RustFS and written to KV v2 as a
+  Base64-encoded value (`encrypted_key_material` is an encoding, not a
+  ciphertext).
+- The backend never calls the Vault Transit engine. The `mount_path`
+  configuration field and the `RUSTFS_KMS_VAULT_MOUNT_PATH` environment
+  variable are deprecated leftovers: they are accepted for compatibility and
+  ignored.
+- Data-encryption keys (DEKs) handed to the object-encryption path are still
+  wrapped with AES-256-GCM under the master key; the statement above concerns
+  the master key's storage in Vault, not the DEK envelope.
+- The backend reports this boundary in its `backend_info` metadata as
+  `at_rest_protection: vault-kv2-acl`.
+- Key rotation is rejected (`InvalidOperation`) until versioned key material
+  retention lands; rotating by overwriting the stored key would orphan every
+  DEK wrapped by the previous version.
+
+> **Warning: KV read access is equivalent to holding the master keys.**
+> Any Vault identity (token, AppRole, or policy) that can `read` the RustFS
+> key path in KV v2 can recover the plaintext master key material and decrypt
+> every object protected by those keys. Treat KV read grants on that path with
+> the same care as handing out the keys themselves. If this is not acceptable,
+> use the Vault Transit backend instead.
+
+## Minimal Vault policy for the KV2 backend
+
+Scope the RustFS token/AppRole to exactly the KV v2 mount and key prefix it is
+configured with (defaults shown: mount `secret`, prefix `rustfs/kms/keys`),
+and grant no other identity read access to that subtree:
+
+```hcl
+# RustFS KMS (Vault KV2 backend) — key storage only, no Transit access needed.
+path "secret/data/rustfs/kms/keys/*" {
+  capabilities = ["create", "read", "update"]
+}
+
+path "secret/metadata/rustfs/kms/keys/*" {
+  capabilities = ["list", "read", "delete"]
+}
+```
+
+Notes:
+
+- `delete` on the metadata path is required for permanent key deletion
+  (`force_immediate`); drop it if you never hard-delete keys.
+- Do not attach `sudo`, wildcard mounts, or Transit paths to this policy; the
+  KV2 backend does not use them.
+- Auditing KV reads on the key prefix is strongly recommended: every read
+  event is a potential master-key disclosure.
+
+## Choosing between Vault KV2 and Vault Transit
+
+Use **Vault Transit** (`VaultTransit`) when key material must be
+cryptographically isolated from anyone holding storage-level read access:
+Transit keeps key-encryption keys inside Vault and only ever returns
+ciphertext, and supports server-side key versioning/rotation.
+
+Use **Vault KV2** only when you accept that the Vault ACL on the key path *is*
+the confidentiality boundary and you want the operational simplicity of a
+single KV mount.

rustfs/src/config/cli.rs

@@ -314,7 +314,7 @@ pub struct ServerOpts {
     #[arg(long, default_value_t = false, env = "RUSTFS_KMS_ENABLE")]
     pub kms_enable: bool,
 
-    /// KMS backend type: local, vault or vault-kv2 (Vault KV2+Transit), vault-transit
+    /// KMS backend type: local, vault or vault-kv2 (plain Vault KV v2 storage), vault-transit
     #[arg(long, default_value_t = rustfs_config::DEFAULT_KMS_BACKEND.to_string(), env = "RUSTFS_KMS_BACKEND")]
     pub kms_backend: String,