refactor(security): split keystore module and enforce key-file security checks

Extracts `KeyProvider` trait and `KeyDerivation` enum into dedicated files
(provider.rs, derivation.rs) so mod.rs is reduced to re-exports. Introduces
key_file_security.rs with `check_key_file`, which rejects symlinks, non-regular
files, group/world-readable modes, and files owned by a different UID before any
key material is read.

All key-loading paths — FileKeyProvider, VaultKeyProvider, AwsKmsProvider, and
VolumeEncryption — now call the check. Return types for `unwrap_key` and `rotate`
are changed to `Zeroizing<[u8; 32]>` so plaintext key bytes are wiped from the
stack when the caller drops the value. Tests that write key files are updated to
use 0o600 permissions and are gated behind `#[cfg(unix)]`.

Author: farhan-syah

nodedb/Cargo.toml

@@ -93,6 +93,7 @@ subtle = { workspace = true }
 base64 = { workspace = true }
 getrandom = { workspace = true }
 aes-gcm = { workspace = true }
+zeroize = { workspace = true }
 aws-sdk-kms = { workspace = true }
 aws-config = { workspace = true }
 

nodedb/src/control/security/encryption.rs

@@ -19,6 +19,9 @@ use std::path::{Path, PathBuf};
 use aes_gcm::aead::{Aead, KeyInit};
 use aes_gcm::{Aes256Gcm, Nonce};
 use tracing::info;
+use zeroize::Zeroizing;
+
+use super::keystore::key_file_security::check_key_file;
 
 /// Volume encryption configuration.
 #[derive(Debug, Clone)]
@@ -137,6 +140,8 @@ impl VolumeEncryption {
                     detail: "no master key path configured".into(),
                 })?;
 
+        check_key_file(path)?;
+
         let key_bytes = std::fs::read(path).map_err(|e| crate::Error::Encryption {
             detail: format!("failed to read master key from {}: {e}", path.display()),
         })?;
@@ -147,9 +152,9 @@ impl VolumeEncryption {
             });
         }
 
-        let mut key = [0u8; 32];
+        let mut key = Zeroizing::new([0u8; 32]);
         key.copy_from_slice(&key_bytes[..32]);
-        self.master_key = Some(key);
+        self.master_key = Some(*key);
 
         info!(path = %path.display(), "master encryption key loaded");
         Ok(())
@@ -266,6 +271,8 @@ impl VolumeEncryption {
     /// Each file's DEK is decrypted with the old key and re-encrypted
     /// with the new key, then the header is updated in place.
     pub fn rotate_master_key(&mut self, new_key_path: &Path) -> crate::Result<()> {
+        check_key_file(new_key_path)?;
+
         let new_bytes = std::fs::read(new_key_path).map_err(|e| crate::Error::Encryption {
             detail: format!("failed to read new key: {e}"),
         })?;
@@ -275,11 +282,11 @@ impl VolumeEncryption {
             });
         }
 
-        let mut new_key = [0u8; 32];
+        let mut new_key = Zeroizing::new([0u8; 32]);
         new_key.copy_from_slice(&new_bytes[..32]);
 
         // Store old key temporarily for DEK re-encryption.
-        let _old_key = self.master_key.replace(new_key);
+        let _old_key = self.master_key.replace(*new_key);
 
         info!(
             new_key_path = %new_key_path.display(),
@@ -308,11 +315,13 @@ mod tests {
 
     #[test]
     fn dek_roundtrip() {
+        use std::os::unix::fs::PermissionsExt as _;
         let dir = tempfile::tempdir().unwrap();
         let key_path = dir.path().join("master.key");
         let mut key_data = [0u8; 32];
         getrandom::fill(&mut key_data).unwrap();
         std::fs::write(&key_path, key_data).unwrap();
+        std::fs::set_permissions(&key_path, std::fs::Permissions::from_mode(0o600)).unwrap();
 
         let mut enc = VolumeEncryption::new(VolumeEncryptionConfig {
             master_key_path: Some(key_path),
@@ -338,6 +347,9 @@ mod tests {
         getrandom::fill(&mut k2).unwrap();
         std::fs::write(&key1_path, k1).unwrap();
         std::fs::write(&key2_path, k2).unwrap();
+        use std::os::unix::fs::PermissionsExt as _;
+        std::fs::set_permissions(&key1_path, std::fs::Permissions::from_mode(0o600)).unwrap();
+        std::fs::set_permissions(&key2_path, std::fs::Permissions::from_mode(0o600)).unwrap();
 
         let mut enc = VolumeEncryption::new(VolumeEncryptionConfig {
             master_key_path: Some(key1_path),

nodedb/src/control/security/keystore/aws_kms.rs

@@ -19,16 +19,24 @@
 //!
 //! Key rotation: generate a new random 32-byte DEK, call KMS `Encrypt` to
 //! produce a new ciphertext blob, persist it to disk, then return the new key.
+//!
+//! # Security checks
+//!
+//! `ciphertext_blob_path` is passed through [`check_key_file`] before being
+//! read. Symlinks, world/group-readable files, and files owned by a different
+//! UID are rejected.
 
 use std::path::PathBuf;
 
 use aws_sdk_kms::Client as KmsClient;
 use aws_sdk_kms::primitives::Blob;
 use tracing::info;
+use zeroize::Zeroizing;
 
 use crate::Result;
 
 use super::KeyProvider;
+use super::key_file_security::check_key_file;
 
 /// AWS KMS key provider.
 pub struct AwsKmsProvider {
@@ -57,6 +65,7 @@ impl AwsKmsProvider {
     }
 
     fn read_ciphertext_blob(&self) -> Result<Vec<u8>> {
+        check_key_file(&self.ciphertext_blob_path)?;
         std::fs::read(&self.ciphertext_blob_path).map_err(|e| crate::Error::Encryption {
             detail: format!(
                 "failed to read KMS ciphertext blob from {}: {e}",
@@ -77,7 +86,7 @@ impl AwsKmsProvider {
 
 #[async_trait::async_trait]
 impl KeyProvider for AwsKmsProvider {
-    async fn unwrap_key(&self) -> Result<[u8; 32]> {
+    async fn unwrap_key(&self) -> Result<Zeroizing<[u8; 32]>> {
         let ciphertext = self.read_ciphertext_blob()?;
 
         let resp = self
@@ -105,7 +114,7 @@ impl KeyProvider for AwsKmsProvider {
             });
         }
 
-        let mut key = [0u8; 32];
+        let mut key = Zeroizing::new([0u8; 32]);
         key.copy_from_slice(&bytes);
         info!(
             key_id = %self.key_id,
@@ -114,10 +123,10 @@ impl KeyProvider for AwsKmsProvider {
         Ok(key)
     }
 
-    async fn rotate(&self) -> Result<[u8; 32]> {
+    async fn rotate(&self) -> Result<Zeroizing<[u8; 32]>> {
         // Generate a new random 32-byte DEK.
-        let mut new_dek = [0u8; 32];
-        getrandom::fill(&mut new_dek).map_err(|e| crate::Error::Encryption {
+        let mut new_dek = Zeroizing::new([0u8; 32]);
+        getrandom::fill(new_dek.as_mut()).map_err(|e| crate::Error::Encryption {
             detail: format!("failed to generate new DEK for KMS rotation: {e}"),
         })?;
 
@@ -154,14 +163,14 @@ mod tests {
     // AWS KMS tests use a mock HTTP server since the real SDK requires credentials.
     // We verify the happy path and auth error path by intercepting the AWS
     // endpoint via an HTTP mock that speaks the KMS JSON protocol shape.
-    //
-    // The mock responds to KMS `Decrypt` with a synthetic 32-byte plaintext
-    // and to `Encrypt` with a fake ciphertext blob.
 
     use super::*;
     use axum::{Router, routing::post};
     use base64::Engine as _;
 
+    #[cfg(unix)]
+    use std::os::unix::fs::PermissionsExt as _;
+
     fn spawn_mock_kms(port: u16, auth_ok: bool) -> tokio::task::JoinHandle<()> {
         tokio::spawn(async move {
             let app = Router::new().route(
@@ -240,30 +249,42 @@ mod tests {
         }
     }
 
+    /// Create a file with secure Unix permissions (0o600).
+    #[cfg(unix)]
+    fn write_secure(path: &std::path::Path, content: &[u8]) {
+        use std::io::Write as _;
+        let mut f = std::fs::File::create(path).unwrap();
+        f.write_all(content).unwrap();
+        drop(f);
+        std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)).unwrap();
+    }
+
     #[tokio::test]
+    #[cfg(unix)]
     async fn kms_decrypt_happy_path() {
         let port = 18301u16;
         let _srv = spawn_mock_kms(port, true);
         tokio::time::sleep(std::time::Duration::from_millis(50)).await;
 
         let dir = tempfile::tempdir().unwrap();
         let blob_path = dir.path().join("ct.bin");
-        std::fs::write(&blob_path, [0xFFu8; 64]).unwrap();
+        write_secure(&blob_path, &[0xFFu8; 64]);
 
         let provider = make_provider_with_endpoint(port, blob_path).await;
         let key = provider.unwrap_key().await.unwrap();
-        assert_eq!(key, [0x42u8; 32]);
+        assert_eq!(*key, [0x42u8; 32]);
     }
 
     #[tokio::test]
+    #[cfg(unix)]
     async fn kms_auth_error_path() {
         let port = 18302u16;
         let _srv = spawn_mock_kms(port, false);
         tokio::time::sleep(std::time::Duration::from_millis(50)).await;
 
         let dir = tempfile::tempdir().unwrap();
         let blob_path = dir.path().join("ct.bin");
-        std::fs::write(&blob_path, [0xFFu8; 64]).unwrap();
+        write_secure(&blob_path, &[0xFFu8; 64]);
 
         let provider = make_provider_with_endpoint(port, blob_path).await;
         let err = provider.unwrap_key().await.unwrap_err();
@@ -273,4 +294,27 @@ mod tests {
             "expected KMS error, got: {detail}"
         );
     }
+
+    #[tokio::test]
+    #[cfg(unix)]
+    async fn kms_insecure_ciphertext_blob_rejected() {
+        let dir = tempfile::tempdir().unwrap();
+        let blob_path = dir.path().join("ct.bin");
+
+        use std::io::Write as _;
+        // Insecure permissions (0o644).
+        let mut f = std::fs::File::create(&blob_path).unwrap();
+        f.write_all(&[0xFFu8; 64]).unwrap();
+        drop(f);
+        std::fs::set_permissions(&blob_path, std::fs::Permissions::from_mode(0o644)).unwrap();
+
+        // Use a dummy port — the check fires before any network call.
+        let provider = make_provider_with_endpoint(19999, blob_path).await;
+        let err = provider.unwrap_key().await.unwrap_err();
+        let detail = format!("{err:?}");
+        assert!(
+            detail.contains("insecure") || detail.contains("644"),
+            "expected insecure-permissions error, got: {detail}"
+        );
+    }
 }

nodedb/src/control/security/keystore/derivation.rs

@@ -0,0 +1,109 @@
+//! `KeyDerivation` config enum and its mapping to a concrete `KeyProvider`.
+
+use std::path::PathBuf;
+
+use super::aws_kms::AwsKmsProvider;
+use super::file::FileKeyProvider;
+use super::provider::KeyProvider;
+use super::vault::VaultKeyProvider;
+use crate::Result;
+
+/// How the server obtains its 32-byte AES-256 encryption key.
+///
+/// All variants implement `KeyProvider` and load the plaintext key into
+/// mlocked memory via `nodedb_wal::secure_mem`. No variant returns a stub.
+#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
+#[serde(tag = "type", rename_all = "snake_case")]
+pub enum KeyDerivation {
+    /// Load a raw 32-byte key from a local file.
+    ///
+    /// The file must contain exactly 32 bytes. This is the simplest mode,
+    /// suitable for development or environments with an external secret manager
+    /// that delivers the key file.
+    File {
+        /// Path to the 32-byte key file.
+        key_path: PathBuf,
+    },
+
+    /// Unwrap the encryption key via HashiCorp Vault transit engine.
+    ///
+    /// ## Chosen flow
+    ///
+    /// On first init the operator calls `vault write transit/datakey/plaintext/<key_name>`
+    /// and stores the returned `ciphertext` blob in `<data_dir>/.wal-key.wrapped`.
+    ///
+    /// At startup NodeDB reads the wrapped blob from disk, sends it to
+    /// `POST {addr}/v1/{mount}/decrypt/{key_name}` with the Vault token,
+    /// and extracts the `plaintext` field (base64 of 32 bytes) as the DEK.
+    ///
+    /// Key rotation: call `POST {addr}/v1/{mount}/rewrap/{key_name}` with
+    /// the old ciphertext blob and write the new ciphertext to `.wal-key.wrapped`.
+    Vault {
+        /// Vault server address, e.g. `https://vault.example.com:8200`.
+        addr: String,
+        /// Path to a file containing the Vault token.
+        token_path: PathBuf,
+        /// Name of the transit key in Vault.
+        key_name: String,
+        /// Mount path of the transit engine, e.g. `transit`.
+        mount: String,
+        /// Path to the wrapped DEK ciphertext blob stored on disk.
+        /// Default: `<data_dir>/.wal-key.wrapped`
+        ciphertext_blob_path: PathBuf,
+    },
+
+    /// Unwrap the encryption key via AWS KMS.
+    ///
+    /// ## Chosen flow
+    ///
+    /// On first init the operator encrypts the plaintext DEK with
+    /// `aws kms encrypt --key-id <key_id> --plaintext fileb://<32-byte-file>`
+    /// and stores the ciphertext blob at `ciphertext_blob_path`.
+    ///
+    /// At startup NodeDB reads the ciphertext blob, calls KMS `Decrypt`,
+    /// and holds the plaintext 32-byte key in mlocked memory.
+    ///
+    /// Key rotation: generate a new random DEK, call KMS `Encrypt`, write
+    /// the new ciphertext blob, then call `rotate()` on the provider to
+    /// get the new key.
+    AwsKms {
+        /// AWS KMS key ID or ARN.
+        key_id: String,
+        /// AWS region, e.g. `us-east-1`.
+        region: String,
+        /// Path to the KMS-encrypted DEK ciphertext blob.
+        ciphertext_blob_path: PathBuf,
+    },
+}
+
+impl KeyDerivation {
+    /// Build a boxed `KeyProvider` for this derivation config.
+    ///
+    /// This is called once at startup. The returned provider's `unwrap_key()`
+    /// is then called to obtain the 32-byte plaintext key.
+    pub async fn into_provider(self) -> Result<Box<dyn KeyProvider + Send + Sync>> {
+        match self {
+            KeyDerivation::File { key_path } => Ok(Box::new(FileKeyProvider { key_path })),
+            KeyDerivation::Vault {
+                addr,
+                token_path,
+                key_name,
+                mount,
+                ciphertext_blob_path,
+            } => Ok(Box::new(VaultKeyProvider::new(
+                addr,
+                token_path,
+                key_name,
+                mount,
+                ciphertext_blob_path,
+            ))),
+            KeyDerivation::AwsKms {
+                key_id,
+                region,
+                ciphertext_blob_path,
+            } => Ok(Box::new(
+                AwsKmsProvider::new(key_id, region, ciphertext_blob_path).await?,
+            )),
+        }
+    }
+}