From 152a996b6686e2e337f508fd20ffcc86f115bcbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20R=2E=20Escobar?= Date: Sat, 14 Feb 2026 13:43:02 +0100 Subject: [PATCH] fix: replace XOR cipher with ChaCha20-Poly1305 AEAD for secret encryption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous secret store used a repeating-key XOR cipher which is cryptographically broken: - Deterministic (no nonce) — identical plaintexts produce identical ciphertexts - No authentication — tampered ciphertext decrypts silently - Vulnerable to known-plaintext attacks (e.g., "sk-" prefix reveals key bytes) Replace with ChaCha20-Poly1305 authenticated encryption: - Random 12-byte nonce per encryption (non-deterministic) - Poly1305 authentication tag detects tampering - Uses the same 32-byte key file (no migration needed for keys) New ciphertext format is `enc2:`. Legacy `enc:` values (XOR) are still decryptable for backward compatibility during migration. Adds chacha20poly1305 0.10 crate (pure Rust, no C dependencies). New tests: tamper detection, wrong-key rejection, nonce uniqueness, truncation handling, legacy XOR backward compatibility. CWE-327 / CRIT-1 Co-Authored-By: Claude Opus 4.6 --- Cargo.lock | 83 +++++++++++++++++++ Cargo.toml | 3 + src/security/secrets.rs | 174 +++++++++++++++++++++++++++++++++------- 3 files changed, 232 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 747e2d9..00da71f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,16 @@ # It is not intended for manual editing. version = 4 +[[package]] +name = "aead" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d122413f284cf2d62fb1b7db97e02edb8cda96d769b16e443a4f6195e35662b0" +dependencies = [ + "crypto-common", + "generic-array", +] + [[package]] name = "ahash" version = "0.8.12" @@ -163,6 +173,30 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" +[[package]] +name = "chacha20" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3613f74bd2eac03dad61bd53dbe620703d4371614fe0bc3b9f04dd36fe4e818" +dependencies = [ + "cfg-if", + "cipher", + "cpufeatures", +] + +[[package]] +name = "chacha20poly1305" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "10cd79432192d1c0f4e1a0fef9527696cc039165d729fb41b3f4f4f354c2dc35" +dependencies = [ + "aead", + "chacha20", + "cipher", + "poly1305", + "zeroize", +] + [[package]] name = "chrono" version = "0.4.43" @@ -174,6 +208,17 @@ dependencies = [ "windows-link", ] +[[package]] +name = "cipher" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "773f3b9af64447d2ce9850330c473515014aa235e6a783b02db81ff39e4a3dad" +dependencies = [ + "crypto-common", + "inout", + "zeroize", +] + [[package]] name = "clap" version = "4.5.58" @@ -255,6 +300,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "78c8292055d1c1df0cce5d180393dc8cce0abec0a7102adb6c7b1eef6016d60a" dependencies = [ "generic-array", + "rand_core 0.6.4", "typenum", ] @@ -769,6 +815,15 @@ dependencies = [ "hashbrown 0.16.1", ] +[[package]] +name = "inout" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "879f10e63c20629ecabbb64a8010319738c66a5cd0c29b02d63d272b03751d01" +dependencies = [ + "generic-array", +] + [[package]] name = "ipnet" version = "2.11.0" @@ -911,6 +966,12 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" +[[package]] +name = "opaque-debug" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c08d65885ee38876c4f86fa503fb49d7b507c2b62552df7c70b2fce627e06381" + [[package]] name = "option-ext" version = "0.2.0" @@ -941,6 +1002,17 @@ version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7edddbd0b52d732b21ad9a5fab5c704c14cd949e5e9a1ec5929a24fded1b904c" +[[package]] +name = "poly1305" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8159bd90725d2df49889a078b54f4f79e87f1f8a8444194cdca81d38f5393abf" +dependencies = [ + "cpufeatures", + "opaque-debug", + "universal-hash", +] + [[package]] name = "potential_utf" version = "0.1.4" @@ -1762,6 +1834,16 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b4ac048d71ede7ee76d585517add45da530660ef4390e49b098733c6e897f254" +[[package]] +name = "universal-hash" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc1de2c688dc15305988b563c3854064043356019f97a4b46276fe734c4f07ea" +dependencies = [ + "crypto-common", + "subtle", +] + [[package]] name = "untrusted" version = "0.9.0" @@ -2282,6 +2364,7 @@ version = "0.1.0" dependencies = [ "anyhow", "async-trait", + "chacha20poly1305", "chrono", "clap", "console", diff --git a/Cargo.toml b/Cargo.toml index 566a63a..08f75b0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,6 +40,9 @@ thiserror = "2.0" # UUID generation uuid = { version = "1.11", default-features = false, features = ["v4", "std"] } +# Authenticated encryption (AEAD) for secret store +chacha20poly1305 = "0.10" + # Async traits async-trait = "0.1" diff --git a/src/security/secrets.rs b/src/security/secrets.rs index ee1d22d..8b770e5 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -1,23 +1,37 @@ // Encrypted secret store — defense-in-depth for API keys and tokens. // -// Secrets are encrypted using a random key stored in `~/.zeroclaw/.secret_key` -// with restrictive file permissions (0600). The config file stores only -// hex-encoded ciphertext, never plaintext keys. +// Secrets are encrypted using ChaCha20-Poly1305 AEAD with a random key stored +// in `~/.zeroclaw/.secret_key` with restrictive file permissions (0600). The +// config file stores only hex-encoded ciphertext, never plaintext keys. +// +// Each encryption generates a fresh random 12-byte nonce, prepended to the +// ciphertext. The Poly1305 authentication tag prevents tampering. // // This prevents: // - Plaintext exposure in config files // - Casual `grep` or `git log` leaks // - Accidental commit of raw API keys +// - Known-plaintext attacks (unlike the previous XOR cipher) +// - Ciphertext tampering (authenticated encryption) // // For sovereign users who prefer plaintext, `secrets.encrypt = false` disables this. +// +// Migration: values with the legacy `enc:` prefix (XOR cipher) are decrypted +// using the old algorithm for backward compatibility. New encryptions always +// produce `enc2:` (ChaCha20-Poly1305). use anyhow::{Context, Result}; +use chacha20poly1305::aead::{Aead, KeyInit, OsRng}; +use chacha20poly1305::{AeadCore, ChaCha20Poly1305, Key, Nonce}; use std::fs; use std::path::{Path, PathBuf}; -/// Length of the random encryption key in bytes. +/// Length of the random encryption key in bytes (256-bit, matches `ChaCha20`). const KEY_LEN: usize = 32; +/// ChaCha20-Poly1305 nonce length in bytes. +const NONCE_LEN: usize = 12; + /// Manages encrypted storage of secrets (API keys, tokens, etc.) #[derive(Debug, Clone)] pub struct SecretStore { @@ -36,37 +50,72 @@ impl SecretStore { } } - /// Encrypt a plaintext secret. Returns hex-encoded ciphertext prefixed with `enc:`. + /// Encrypt a plaintext secret. Returns hex-encoded ciphertext prefixed with `enc2:`. + /// Format: `enc2:` (12 + N + 16 bytes). /// If encryption is disabled, returns the plaintext as-is. pub fn encrypt(&self, plaintext: &str) -> Result { if !self.enabled || plaintext.is_empty() { return Ok(plaintext.to_string()); } - let key = self.load_or_create_key()?; - let ciphertext = xor_cipher(plaintext.as_bytes(), &key); - Ok(format!("enc:{}", hex_encode(&ciphertext))) + let key_bytes = self.load_or_create_key()?; + let key = Key::from_slice(&key_bytes); + let cipher = ChaCha20Poly1305::new(key); + + let nonce = ChaCha20Poly1305::generate_nonce(&mut OsRng); + let ciphertext = cipher + .encrypt(&nonce, plaintext.as_bytes()) + .map_err(|e| anyhow::anyhow!("Encryption failed: {e}"))?; + + // Prepend nonce to ciphertext for storage + let mut blob = Vec::with_capacity(NONCE_LEN + ciphertext.len()); + blob.extend_from_slice(&nonce); + blob.extend_from_slice(&ciphertext); + + Ok(format!("enc2:{}", hex_encode(&blob))) } - /// Decrypt a secret. If the value starts with `enc:`, it's decrypted. - /// Otherwise, it's returned as-is (backward-compatible with plaintext configs). + /// Decrypt a secret. + /// - `enc2:` prefix → ChaCha20-Poly1305 (current format) + /// - `enc:` prefix → legacy XOR cipher (backward compatibility for migration) + /// - No prefix → returned as-is (plaintext config) pub fn decrypt(&self, value: &str) -> Result { - if !value.starts_with("enc:") { - return Ok(value.to_string()); - } + if let Some(hex_str) = value.strip_prefix("enc2:") { + let blob = + hex_decode(hex_str).context("Failed to decode encrypted secret (corrupt hex)")?; + anyhow::ensure!( + blob.len() > NONCE_LEN, + "Encrypted value too short (missing nonce)" + ); - let hex_str = &value[4..]; // strip "enc:" prefix - let ciphertext = - hex_decode(hex_str).context("Failed to decode encrypted secret (corrupt hex)")?; - let key = self.load_or_create_key()?; - let plaintext_bytes = xor_cipher(&ciphertext, &key); - String::from_utf8(plaintext_bytes) - .context("Decrypted secret is not valid UTF-8 — wrong key or corrupt data") + let (nonce_bytes, ciphertext) = blob.split_at(NONCE_LEN); + let nonce = Nonce::from_slice(nonce_bytes); + let key_bytes = self.load_or_create_key()?; + let key = Key::from_slice(&key_bytes); + let cipher = ChaCha20Poly1305::new(key); + + let plaintext_bytes = cipher + .decrypt(nonce, ciphertext) + .map_err(|_| anyhow::anyhow!("Decryption failed — wrong key or tampered data"))?; + + String::from_utf8(plaintext_bytes) + .context("Decrypted secret is not valid UTF-8 — corrupt data") + } else if let Some(hex_str) = value.strip_prefix("enc:") { + // Legacy XOR cipher — decrypt for backward compatibility + let ciphertext = hex_decode(hex_str) + .context("Failed to decode legacy encrypted secret (corrupt hex)")?; + let key = self.load_or_create_key()?; + let plaintext_bytes = xor_cipher(&ciphertext, &key); + String::from_utf8(plaintext_bytes) + .context("Decrypted legacy secret is not valid UTF-8 — wrong key or corrupt data") + } else { + Ok(value.to_string()) + } } - /// Check if a value is already encrypted. + /// Check if a value is already encrypted (current or legacy format). pub fn is_encrypted(value: &str) -> bool { - value.starts_with("enc:") + value.starts_with("enc2:") || value.starts_with("enc:") } /// Load the encryption key from disk, or create one if it doesn't exist. @@ -157,7 +206,7 @@ mod tests { let secret = "sk-my-secret-api-key-12345"; let encrypted = store.encrypt(secret).unwrap(); - assert!(encrypted.starts_with("enc:"), "Should have enc: prefix"); + assert!(encrypted.starts_with("enc2:"), "Should have enc2: prefix"); assert_ne!(encrypted, secret, "Should not be plaintext"); let decrypted = store.decrypt(&encrypted).unwrap(); @@ -176,7 +225,7 @@ mod tests { fn decrypt_plaintext_passthrough() { let tmp = TempDir::new().unwrap(); let store = SecretStore::new(tmp.path(), true); - // Values without "enc:" prefix are returned as-is (backward compat) + // Values without "enc:"/"enc2:" prefix are returned as-is (backward compat) let result = store.decrypt("sk-plaintext-key").unwrap(); assert_eq!(result, "sk-plaintext-key"); } @@ -191,7 +240,8 @@ mod tests { #[test] fn is_encrypted_detects_prefix() { - assert!(SecretStore::is_encrypted("enc:aabbcc")); + assert!(SecretStore::is_encrypted("enc2:aabbcc")); + assert!(SecretStore::is_encrypted("enc:aabbcc")); // legacy assert!(!SecretStore::is_encrypted("sk-plaintext")); assert!(!SecretStore::is_encrypted("")); } @@ -214,13 +264,20 @@ mod tests { } #[test] - fn same_key_used_across_calls() { + fn encrypting_same_value_produces_different_ciphertext() { let tmp = TempDir::new().unwrap(); let store = SecretStore::new(tmp.path(), true); let e1 = store.encrypt("secret").unwrap(); let e2 = store.encrypt("secret").unwrap(); - assert_eq!(e1, e2, "Same key should produce same ciphertext"); + assert_ne!( + e1, e2, + "AEAD with random nonce should produce different ciphertext each time" + ); + + // Both should still decrypt to the same value + assert_eq!(store.decrypt(&e1).unwrap(), "secret"); + assert_eq!(store.decrypt(&e2).unwrap(), "secret"); } #[test] @@ -260,10 +317,71 @@ mod tests { fn corrupt_hex_returns_error() { let tmp = TempDir::new().unwrap(); let store = SecretStore::new(tmp.path(), true); - let result = store.decrypt("enc:not-valid-hex!!"); + let result = store.decrypt("enc2:not-valid-hex!!"); assert!(result.is_err()); } + #[test] + fn tampered_ciphertext_detected() { + let tmp = TempDir::new().unwrap(); + let store = SecretStore::new(tmp.path(), true); + let encrypted = store.encrypt("sensitive-data").unwrap(); + + // Flip a bit in the ciphertext (after the "enc2:" prefix) + let hex_str = &encrypted[5..]; + let mut blob = hex_decode(hex_str).unwrap(); + // Modify a byte in the ciphertext portion (after the 12-byte nonce) + if blob.len() > NONCE_LEN { + blob[NONCE_LEN] ^= 0xff; + } + let tampered = format!("enc2:{}", hex_encode(&blob)); + + let result = store.decrypt(&tampered); + assert!(result.is_err(), "Tampered ciphertext must be rejected"); + } + + #[test] + fn wrong_key_detected() { + let tmp1 = TempDir::new().unwrap(); + let tmp2 = TempDir::new().unwrap(); + let store1 = SecretStore::new(tmp1.path(), true); + let store2 = SecretStore::new(tmp2.path(), true); + + let encrypted = store1.encrypt("secret-for-store1").unwrap(); + let result = store2.decrypt(&encrypted); + assert!(result.is_err(), "Decrypting with a different key must fail"); + } + + #[test] + fn truncated_ciphertext_returns_error() { + let tmp = TempDir::new().unwrap(); + let store = SecretStore::new(tmp.path(), true); + // Only a few bytes — shorter than nonce + let result = store.decrypt("enc2:aabbccdd"); + assert!(result.is_err(), "Too-short ciphertext must be rejected"); + } + + // ── Legacy XOR backward compatibility ─────────────────────── + + #[test] + fn legacy_xor_decrypt_still_works() { + let tmp = TempDir::new().unwrap(); + let store = SecretStore::new(tmp.path(), true); + + // Trigger key creation via an encrypt call + let _ = store.encrypt("setup").unwrap(); + let key = store.load_or_create_key().unwrap(); + + // Manually produce a legacy XOR-encrypted value + let plaintext = "sk-legacy-api-key"; + let ciphertext = xor_cipher(plaintext.as_bytes(), &key); + let legacy_value = format!("enc:{}", hex_encode(&ciphertext)); + + // Store should still be able to decrypt legacy values + let decrypted = store.decrypt(&legacy_value).unwrap(); + assert_eq!(decrypted, plaintext, "Legacy XOR values must still decrypt"); + } + // ── Low-level helpers ─────────────────────────────────────── #[test]