fix: replace magic-crypt with aes-gcm for bookmark encryption

magic-crypt has known vulnerabilities. Replace it with aes-gcm for new encryption (authenticated, with random nonces) while keeping a legacy AES-128-CBC decryption path to transparently handle existing bookmarks.
2026-04-02 00:02:17 -07:00 · 2026-02-28 15:26:42 +01:00
parent 1461a68bb3
commit e8eed9f8d1
4 changed files with 210 additions and 77 deletions
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -8,6 +8,16 @@ version = "2.0.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa"

+[[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 = "aes"
 version = "0.8.4"
@@ -19,6 +29,20 @@ dependencies = [
 "cpufeatures 0.2.17",
 ]

+[[package]]
+name = "aes-gcm"
+version = "0.10.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "831010a0f742e1209b3bcea8fab6a8e149051ba6099432c8cb2cc117dec3ead1"
+dependencies = [
+ "aead",
+ "aes",
+ "cipher",
+ "ctr",
+ "ghash",
+ "subtle",
+]
+
 [[package]]
 name = "aho-corasick"
 version = "1.1.4"
@@ -893,15 +917,6 @@ dependencies = [
 "crc-catalog",
 ]

-[[package]]
-name = "crc-any"
-version = "2.5.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a62ec9ff5f7965e4d7280bd5482acd20aadb50d632cf6c1d74493856b011fa73"
-dependencies = [
- "debug-helper",
-]
-
 [[package]]
 name = "crc-catalog"
 version = "2.4.0"
@@ -1026,9 +1041,19 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "78c8292055d1c1df0cce5d180393dc8cce0abec0a7102adb6c7b1eef6016d60a"
 dependencies = [
 "generic-array",
+ "rand_core 0.6.4",
 "typenum",
 ]

+[[package]]
+name = "ctr"
+version = "0.9.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0369ee1ad671834580515889b80f2ea915f23b8be8d0daa4bbaf2ac5c7590835"
+dependencies = [
+ "cipher",
+]
+
 [[package]]
 name = "curve25519-dalek"
 version = "4.1.3"
@@ -1153,12 +1178,6 @@ dependencies = [
 "zeroize",
 ]

-[[package]]
-name = "debug-helper"
-version = "0.3.13"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f578e8e2c440e7297e008bb5486a3a8a194775224bbc23729b0dbdfaeebf162e"
-
 [[package]]
 name = "der"
 version = "0.6.1"
@@ -1252,15 +1271,6 @@ dependencies = [
 "syn",
 ]

-[[package]]
-name = "des"
-version = "0.8.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ffdd80ce8ce993de27e9f063a444a4d53ce8e8db4c1f00cc03af5ad5a9867a1e"
-dependencies = [
- "cipher",
-]
-
 [[package]]
 name = "diff"
 version = "0.1.13"
@@ -1702,6 +1712,16 @@ dependencies = [
 "wasip3",
 ]

+[[package]]
+name = "ghash"
+version = "0.5.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f0d8a4362ccb29cb0b265253fb0a2728f592895ee6854fd9bc13f2ffda266ff1"
+dependencies = [
+ "opaque-debug",
+ "polyval",
+]
+
 [[package]]
 name = "git2"
 version = "0.20.4"
@@ -2692,22 +2712,6 @@ dependencies = [
 "time",
 ]

-[[package]]
-name = "magic-crypt"
-version = "4.0.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "844b6169eeaae32ae8a61855964331a67f12d2afba9170303fbd3e3c2a861a52"
-dependencies = [
- "aes",
- "base64 0.22.1",
- "cbc",
- "crc-any",
- "des",
- "md-5",
- "sha2",
- "tiger",
-]
-
 [[package]]
 name = "md-5"
 version = "0.10.6"
@@ -2948,6 +2952,12 @@ version = "1.21.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "42f5e15c9953c5e4ccceeb2e7382a716482c34515315f7b03532b8b4e8393d2d"

+[[package]]
+name = "opaque-debug"
+version = "0.3.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c08d65885ee38876c4f86fa503fb49d7b507c2b62552df7c70b2fce627e06381"
+
 [[package]]
 name = "open"
 version = "5.3.3"
@@ -3252,6 +3262,18 @@ version = "0.3.32"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "7edddbd0b52d732b21ad9a5fab5c704c14cd949e5e9a1ec5929a24fded1b904c"

+[[package]]
+name = "polyval"
+version = "0.6.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9d1fe60d06143b2430aa532c94cfe9e29783047f06c0d7fd359a9a51b729fa25"
+dependencies = [
+ "cfg-if",
+ "cpufeatures 0.2.17",
+ "opaque-debug",
+ "universal-hash",
+]
+
 [[package]]
 name = "portable-atomic"
 version = "1.13.1"
@@ -4628,9 +4650,13 @@ dependencies = [
 name = "termscp"
 version = "0.19.1"
 dependencies = [
+ "aes",
+ "aes-gcm",
 "argh",
+ "base64 0.22.1",
 "bitflags 2.11.0",
 "bytesize",
+ "cbc",
 "cfg_aliases",
 "chrono",
 "content_inspector",
@@ -4641,7 +4667,7 @@ dependencies = [
 "keyring",
 "lazy-regex",
 "log",
- "magic-crypt",
+ "md-5",
 "notify",
 "notify-rust",
 "nucleo",
@@ -4729,15 +4755,6 @@ dependencies = [
 "syn",
 ]

-[[package]]
-name = "tiger"
-version = "0.2.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "579abbce4ad73b04386dbeb34369c9873a8f9b749c7b99cbf479a2949ff715ed"
-dependencies = [
- "digest",
-]
-
 [[package]]
 name = "time"
 version = "0.3.47"
@@ -5160,6 +5177,16 @@ version = "0.2.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853"

+[[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 = "unsafe-libyaml"
 version = "0.2.11"
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -32,8 +32,12 @@ name = "termscp"
 path = "src/main.rs"

 [dependencies]
+aes = "0.8"
+aes-gcm = "0.10"
 argh = "^0.1"
+base64 = "0.22"
 bitflags = "^2"
+cbc = { version = "0.1", features = ["alloc"] }
 bytesize = "^2"
 chrono = "^0.4"
 content_inspector = "^0.2"
@@ -49,7 +53,7 @@ keyring = { version = "^3", features = [
 ] }
 lazy-regex = "^3"
 log = "^0.4"
-magic-crypt = "4"
+md-5 = "0.10"
 notify = "8"
 notify-rust = { version = "^4", default-features = false, features = ["d"] }
 nucleo = "0.5"
--- a/src/main.rs
+++ b/src/main.rs
@@ -16,8 +16,6 @@ extern crate bitflags;
 extern crate lazy_regex;
 #[macro_use]
 extern crate log;
-#[macro_use]
-extern crate magic_crypt;

 use std::env;
 use std::path::{Path, PathBuf};
--- a/src/utils/crypto.rs
+++ b/src/utils/crypto.rs
@@ -1,41 +1,145 @@
-//! ## Crypto
+//! `crypto` provides utilities for encrypting and decrypting strings.
 //!
-//! `crypto` is the module which provides utilities for crypting
+//! New data is encrypted with AES-128-GCM (authenticated encryption).
+//! Legacy data encrypted by `magic-crypt` (AES-128-CBC with MD5 key derivation
+//! and a zero IV) is transparently decrypted via a fallback path.

-// Ext
-use magic_crypt::MagicCryptTrait;
+use aes::cipher::block_padding::Pkcs7;
+use aes::cipher::{BlockDecryptMut, KeyIvInit};
+use aes_gcm::aead::{Aead, AeadCore};
+use aes_gcm::{Aes128Gcm, KeyInit, Nonce};
+use base64::Engine;
+use base64::engine::general_purpose::STANDARD as B64;
+use md5::{Digest, Md5};

-/// Crypt a string using AES128; output is returned as a BASE64 string
+/// Nonce length for AES-128-GCM (96 bits).
+const GCM_NONCE_LEN: usize = 12;
+
+/// Encrypt a string with AES-128-GCM. The output is `nonce || ciphertext`,
+/// encoded as standard Base64.
 pub fn aes128_b64_crypt(key: &str, input: &str) -> String {
-    let crypter = new_magic_crypt!(key, 128);
-    crypter.encrypt_str_to_base64(input)
+    let derived = derive_gcm_key(key);
+    let cipher = Aes128Gcm::new(&derived.into());
+    let nonce_bytes = Aes128Gcm::generate_nonce(&mut aes_gcm::aead::OsRng);
+    let ciphertext = cipher
+        .encrypt(&nonce_bytes, input.as_bytes())
+        .expect("AES-GCM encryption must not fail for valid inputs");
+
+    let mut combined = Vec::with_capacity(GCM_NONCE_LEN + ciphertext.len());
+    combined.extend_from_slice(&nonce_bytes);
+    combined.extend_from_slice(&ciphertext);
+
+    B64.encode(combined)
 }

-/// Decrypt a string using AES128
-pub fn aes128_b64_decrypt(key: &str, secret: &str) -> Result<String, magic_crypt::MagicCryptError> {
-    let crypter = new_magic_crypt!(key, 128);
-    crypter.decrypt_base64_to_string(secret)
+/// Decrypt a Base64-encoded string. Tries AES-128-GCM first; on failure falls
+/// back to the legacy AES-128-CBC format produced by `magic-crypt`.
+pub fn aes128_b64_decrypt(key: &str, secret: &str) -> Result<String, CryptoError> {
+    // Try modern AES-128-GCM first
+    if let Ok(plaintext) = decrypt_gcm(key, secret) {
+        return Ok(plaintext);
+    }
+    // Fall back to legacy AES-128-CBC (magic-crypt compat)
+    decrypt_legacy_cbc(key, secret)
+}
+
+/// AES-128-GCM decryption. Expects `nonce (12 bytes) || ciphertext` in the
+/// Base64-decoded payload.
+fn decrypt_gcm(key: &str, secret: &str) -> Result<String, CryptoError> {
+    let raw = B64.decode(secret)?;
+    if raw.len() < GCM_NONCE_LEN {
+        return Err(CryptoError::InvalidData);
+    }
+    let (nonce_bytes, ciphertext) = raw.split_at(GCM_NONCE_LEN);
+    let nonce = Nonce::from_slice(nonce_bytes);
+    let derived = derive_gcm_key(key);
+    let cipher = Aes128Gcm::new(&derived.into());
+    let plaintext = cipher
+        .decrypt(nonce, ciphertext)
+        .map_err(|_| CryptoError::AesGcm)?;
+    String::from_utf8(plaintext).map_err(|_| CryptoError::InvalidData)
+}
+
+/// Legacy AES-128-CBC decryption compatible with `magic-crypt` v4.
+/// Key is derived via MD5; IV is 16 zero bytes; padding is PKCS7.
+fn decrypt_legacy_cbc(key: &str, secret: &str) -> Result<String, CryptoError> {
+    type CbcDec = cbc::Decryptor<aes::Aes128>;
+
+    let key_bytes: [u8; 16] = Md5::digest(key.as_bytes()).into();
+    let iv = [0u8; 16];
+    let raw = B64.decode(secret)?;
+    let decryptor = CbcDec::new(&key_bytes.into(), &iv.into());
+    let plaintext = decryptor
+        .decrypt_padded_vec_mut::<Pkcs7>(&raw)
+        .map_err(|_| CryptoError::InvalidData)?;
+    String::from_utf8(plaintext).map_err(|_| CryptoError::InvalidData)
+}
+
+/// Derive a 128-bit key for AES-GCM by hashing the input with MD5.
+fn derive_gcm_key(key: &str) -> [u8; 16] {
+    let digest = Md5::digest(key.as_bytes());
+    let mut out = [0u8; 16];
+    out.copy_from_slice(&digest);
+    out
+}
+
+/// Errors that can occur during crypto operations.
+#[derive(Debug, thiserror::Error)]
+pub enum CryptoError {
+    #[error("base64 decode error: {0}")]
+    Base64(#[from] base64::DecodeError),
+    #[error("AES-GCM decryption failed")]
+    AesGcm,
+    #[error("invalid encrypted data")]
+    InvalidData,
 }

 #[cfg(test)]
 mod tests {
-
    use pretty_assertions::assert_eq;

    use super::*;

    #[test]
-    fn test_utils_crypto_aes128() {
-        let key: &str = "MYSUPERSECRETKEY";
-        let input: &str = "Hello world!";
-        let secret: String = aes128_b64_crypt(key, input);
-        assert_eq!(secret.as_str(), "z4Z6LpcpYqBW4+bkIok+5A==");
-        assert_eq!(
-            aes128_b64_decrypt(key, secret.as_str())
-                .ok()
-                .unwrap()
-                .as_str(),
-            input
-        );
+    fn test_encrypt_decrypt_roundtrip() {
+        let key = "MYSUPERSECRETKEY";
+        let input = "Hello world!";
+        let secret = aes128_b64_crypt(key, input);
+        let decrypted = aes128_b64_decrypt(key, &secret).unwrap();
+        assert_eq!(decrypted, input);
+    }
+
+    #[test]
+    fn test_decrypt_legacy_magic_crypt_ciphertext() {
+        // This ciphertext was produced by magic-crypt v4:
+        //   new_magic_crypt!("MYSUPERSECRETKEY", 128).encrypt_str_to_base64("Hello world!")
+        let key = "MYSUPERSECRETKEY";
+        let legacy_secret = "z4Z6LpcpYqBW4+bkIok+5A==";
+        let decrypted = aes128_b64_decrypt(key, legacy_secret).unwrap();
+        assert_eq!(decrypted, "Hello world!");
+    }
+
+    #[test]
+    fn test_different_encryptions_produce_different_ciphertexts() {
+        let key = "MYSUPERSECRETKEY";
+        let input = "Hello world!";
+        let secret1 = aes128_b64_crypt(key, input);
+        let secret2 = aes128_b64_crypt(key, input);
+        // Due to random nonces, ciphertexts must differ
+        assert_ne!(secret1, secret2);
+        // But both decrypt to the same plaintext
+        assert_eq!(aes128_b64_decrypt(key, &secret1).unwrap(), input);
+        assert_eq!(aes128_b64_decrypt(key, &secret2).unwrap(), input);
+    }
+
+    #[test]
+    fn test_wrong_key_fails() {
+        let secret = aes128_b64_crypt("correct-key", "sensitive data");
+        assert!(aes128_b64_decrypt("wrong-key", &secret).is_err());
+    }
+
+    #[test]
+    fn test_invalid_base64_fails() {
+        assert!(aes128_b64_decrypt("key", "not-valid-base64!!!").is_err());
    }
 }