From e8eed9f8d1a3568f388336871cbec33c556334a5 Mon Sep 17 00:00:00 2001 From: Christian Visintin Date: Sat, 28 Feb 2026 15:26:42 +0100 Subject: [PATCH] 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. --- Cargo.lock | 127 +++++++++++++++++++++--------------- Cargo.toml | 6 +- src/main.rs | 2 - src/utils/crypto.rs | 152 +++++++++++++++++++++++++++++++++++++------- 4 files changed, 210 insertions(+), 77 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c957069..e28da9d 100644 --- 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" diff --git a/Cargo.toml b/Cargo.toml index e3836f8..e82da5e 100644 --- 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" diff --git a/src/main.rs b/src/main.rs index 949e420..278f0c7 100644 --- 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}; diff --git a/src/utils/crypto.rs b/src/utils/crypto.rs index 66936e8..62f2091 100644 --- 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 { - 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 { + // 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 { + 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 { + type CbcDec = cbc::Decryptor; + + 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::(&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()); } }