diff --git a/Cargo.lock b/Cargo.lock index 1fc55e9583..781255619d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4416,6 +4416,7 @@ dependencies = [ "sha2 0.11.1", "stringprep", "thiserror 2.0.21", + "zeroize", ] [[package]] @@ -4749,6 +4750,7 @@ dependencies = [ "sha2 0.11.1", "sha3", "thiserror 2.0.21", + "zeroize", ] [[package]] @@ -4810,6 +4812,7 @@ dependencies = [ "web-time", "winsafe", "xattr", + "zeroize", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index fb6b7f4544..ab8b964ce8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -84,6 +84,7 @@ hayro-jbig2 = "0.3" # foundation thiserror = "2" +zeroize = "1" # Linear-time matching (no backtracking) for caller-supplied redaction patterns. `perf-dfa` gives # the lazy-DFA cache that `RegexBuilder::dfa_size_limit` bounds (the limit is inert without it). regex = { version = "1", default-features = false, features = ["std", "unicode", "perf-dfa"] } diff --git a/crates/crypt/Cargo.toml b/crates/crypt/Cargo.toml index 9edb008f2c..dcc7d2b935 100644 --- a/crates/crypt/Cargo.toml +++ b/crates/crypt/Cargo.toml @@ -16,6 +16,7 @@ md-5 = "0.11" sha2 = "0.11" stringprep = "0.1" thiserror.workspace = true +zeroize.workspace = true [dev-dependencies] proptest.workspace = true diff --git a/crates/crypt/src/lib.rs b/crates/crypt/src/lib.rs index 9478e74130..8b520d281d 100644 --- a/crates/crypt/src/lib.rs +++ b/crates/crypt/src/lib.rs @@ -19,6 +19,7 @@ mod rc4; use md5::{Digest as _, Md5}; use sha2::{Sha256, Sha384, Sha512}; +use zeroize::Zeroizing; pub use aes_cbc::{aes_cbc_decrypt, aes_cbc_encrypt}; pub use rc4::rc4; @@ -147,9 +148,9 @@ pub enum StreamKind<'a> { #[derive(Clone, Debug)] pub struct SecurityHandler { dict: EncryptDict, - key: Vec, + key: Zeroizing>, auth: Auth, - recovered_user: Option>, + recovered_user: Option>>, methods: Methods, } @@ -190,7 +191,7 @@ impl Methods { } impl SecurityHandler { - fn new(dict: EncryptDict, key: Vec, auth: Auth, recovered_user: Option>) -> Self { + fn new(dict: EncryptDict, key: Zeroizing>, auth: Auth, recovered_user: Option>>) -> Self { let methods = Methods::new(&dict); Self { dict, key, auth, recovered_user, methods } } @@ -248,7 +249,7 @@ impl SecurityHandler { if methods.string != Method::Identity || methods.stream != Method::Identity { return None; } - Some(Self { dict, key: Vec::new(), auth: Auth::User, recovered_user: None, methods }) + Some(Self { dict, key: Zeroizing::new(Vec::new()), auth: Auth::User, recovered_user: None, methods }) } /// Whether the handler has the file key: false for [`SecurityHandler::without_key`]. @@ -289,7 +290,7 @@ impl SecurityHandler { /// For R2–R4 documents opened with the owner password: the user password, which owner /// authentication recovers (Algorithm 7). Lets components that only accept user passwords /// open the document too. - pub fn recovered_user_password(&self) -> Option> { + pub fn recovered_user_password(&self) -> Option>> { self.recovered_user.clone() } @@ -306,7 +307,7 @@ impl SecurityHandler { if !matches!(self.dict.r, 5 | 6) || !self.has_key() { return None; } - let plain = aes_cbc_decrypt(&self.key, &[0; 16], self.dict.perms.first_chunk::<16>()?, false); + let plain = Zeroizing::new(aes_cbc_decrypt(&self.key, &[0; 16], self.dict.perms.first_chunk::<16>()?, false)); let p = i32::from_le_bytes(*plain.first_chunk::<4>()?); (plain.get(9..12) == Some(&b"adb"[..])).then_some(p) } @@ -348,7 +349,7 @@ impl SecurityHandler { /// the same way, so a page it shows is one this crate can read and save. AES-128 keys are /// left as they are: `AESV2` takes a 128-bit key (Table 25), and Acrobat's extension is only /// seen with RC4. Every key PdfCraft writes under `/V 4` is 16 bytes already. - fn object_key(&self, method: Method, num: u32, generation: u16) -> Vec { + fn object_key(&self, method: Method, num: u32, generation: u16) -> Zeroizing> { if method == Method::Aes256 { return self.key.clone(); } @@ -363,8 +364,9 @@ impl SecurityHandler { if method == Method::Aes128 { h.update(b"sAlT"); } - let digest = h.finalize(); - digest[..(key.len() + 5).min(16)].to_vec() + let mut digest = Zeroizing::new(h.finalize().to_vec()); + digest.truncate((key.len() + 5).min(16)); + digest } fn apply(&self, method: Method, num: u32, generation: u16, data: &[u8], encrypt: bool) -> Vec { @@ -456,31 +458,31 @@ pub fn pdfdoc_byte(c: char) -> Option { /// Revisions 2–4: the password in PDFDocEncoding (§7.6.4.3.2, Algorithm 2), at most 32 bytes. /// Characters PDFDocEncoding lacks are dropped. -fn legacy_password(pw: &str) -> Vec { - pw.chars().filter_map(pdfdoc_byte).take(32).collect() +fn legacy_password(pw: &str) -> Zeroizing> { + Zeroizing::new(pw.chars().filter_map(pdfdoc_byte).take(32).collect()) } /// The bytes an R2–R4 password may have been written as: PDFDocEncoding, as the specification /// asks; and for a password with characters PDFDocEncoding lacks (ğ, ş, İ, …), which it gives /// no bytes, also its UTF-8 bytes, which qpdf and other tools write for such passwords. -fn legacy_candidates(pw: &str) -> Vec> { +fn legacy_candidates(pw: &str) -> Vec>> { let mut out = vec![legacy_password(pw)]; if pw.chars().any(|c| pdfdoc_byte(c).is_none()) { - out.push(pw.bytes().take(32).collect()); + out.push(Zeroizing::new(pw.bytes().take(32).collect())); } out } /// Revisions 5–6: SASLprep (RFC 4013), UTF-8, at most 127 bytes. -fn sasl_password(pw: &str) -> Vec { - let prepped = stringprep::saslprep(pw).map(|c| c.into_owned()).unwrap_or_else(|_| pw.to_string()); - let mut bytes = prepped.into_bytes(); +fn sasl_password(pw: &str) -> Zeroizing> { + let prepped = Zeroizing::new(stringprep::saslprep(pw).map(|c| c.into_owned()).unwrap_or_else(|_| pw.to_string())); + let mut bytes = Zeroizing::new(prepped.as_bytes().to_vec()); bytes.truncate(127); bytes } -fn pad(pw: &[u8]) -> [u8; 32] { - let mut out = PADDING; +fn pad(pw: &[u8]) -> Zeroizing<[u8; 32]> { + let mut out = Zeroizing::new(PADDING); let n = pw.len().min(32); out[..n].copy_from_slice(&pw[..n]); out[n..].copy_from_slice(&PADDING[..32 - n]); @@ -494,20 +496,20 @@ fn key_len(d: &EncryptDict) -> usize { // ── Revisions 2–4 ────────────────────────────────────────────────────────────────────────────── /// Algorithm 2: file key from a (user) password. -fn file_key_r4(d: &EncryptDict, id0: &[u8], pw: &[u8]) -> Vec { +fn file_key_r4(d: &EncryptDict, id0: &[u8], pw: &[u8]) -> Zeroizing> { let n = key_len(d); let mut h = Md5::new(); - h.update(pad(pw)); + h.update(*pad(pw)); h.update(&d.o[..d.o.len().min(32)]); h.update(d.p.to_le_bytes()); h.update(id0); if d.r >= 4 && !d.encrypt_metadata { h.update([0xFF; 4]); } - let mut digest = h.finalize().to_vec(); + let mut digest = Zeroizing::new(h.finalize().to_vec()); if d.r >= 3 { for _ in 0..50 { - digest = Md5::digest(&digest[..n]).to_vec(); + digest = Zeroizing::new(Md5::digest(&digest[..n]).to_vec()); } } digest.truncate(n); @@ -524,7 +526,7 @@ fn u_value_r4(d: &EncryptDict, id0: &[u8], key: &[u8]) -> Vec { h.update(id0); let mut x = rc4(key, &h.finalize()); for i in 1..=19u8 { - let k: Vec = key.iter().map(|b| b ^ i).collect(); + let k = Zeroizing::new(key.iter().map(|b| b ^ i).collect::>()); x = rc4(&k, &x); } x.extend_from_slice(&[0u8; 16]); @@ -536,7 +538,7 @@ fn u_value_r4(d: &EncryptDict, id0: &[u8], key: &[u8]) -> Vec { /// Some producers ignore `/EncryptMetadata false` when deriving the key (they omit the /// `FF FF FF FF` of Algorithm 2, step f). Viewers open those files, so if the strict derivation /// fails we retry the other way. -fn user_key_r4(d: &EncryptDict, id0: &[u8], pw: &[u8]) -> Option> { +fn user_key_r4(d: &EncryptDict, id0: &[u8], pw: &[u8]) -> Option>> { let check = |d: &EncryptDict| { let key = file_key_r4(d, id0, pw); let u = u_value_r4(d, id0, &key); @@ -547,54 +549,57 @@ fn user_key_r4(d: &EncryptDict, id0: &[u8], pw: &[u8]) -> Option> { } /// The RC4 key derived from an owner password (Algorithm 3, steps a–d). -fn owner_rc4_key(d: &EncryptDict, owner_pw: &[u8]) -> Vec { +fn owner_rc4_key(d: &EncryptDict, owner_pw: &[u8]) -> Zeroizing> { let n = key_len(d); - let mut digest = Md5::digest(pad(owner_pw)).to_vec(); + let mut digest = Zeroizing::new(Md5::digest(*pad(owner_pw)).to_vec()); if d.r >= 3 { for _ in 0..50 { - digest = Md5::digest(&digest).to_vec(); + digest = Zeroizing::new(Md5::digest(&*digest).to_vec()); } } digest.truncate(n); digest } +type SecretBytes = Zeroizing>; + /// Algorithm 7: authenticate an owner password (recover the user password from `/O`). /// Returns the file key and the recovered user password (padding removed). -fn owner_key_r4(d: &EncryptDict, id0: &[u8], pw: &[u8]) -> Option<(Vec, Vec)> { +fn owner_key_r4(d: &EncryptDict, id0: &[u8], pw: &[u8]) -> Option<(SecretBytes, SecretBytes)> { let rk = owner_rc4_key(d, pw); - let mut user = d.o[..d.o.len().min(32)].to_vec(); + let mut user = Zeroizing::new(d.o[..d.o.len().min(32)].to_vec()); if d.r == 2 { - user = rc4(&rk, &user); + user = Zeroizing::new(rc4(&rk, &user)); } else { for i in (0..=19u8).rev() { - let k: Vec = rk.iter().map(|b| b ^ i).collect(); - user = rc4(&k, &user); + let k = Zeroizing::new(rk.iter().map(|b| b ^ i).collect::>()); + user = Zeroizing::new(rc4(&k, &user)); } } let key = user_key_r4(d, id0, &user)?; // The recovered value is the padded password: strip the padding suffix. let len = (0..=32).find(|&i| user.len() >= 32 && user[i..32] == PADDING[..32 - i]).unwrap_or(user.len().min(32)); - Some((key, user[..len].to_vec())) + user.truncate(len); + Some((key, user)) } // ── Revisions 5–6 ────────────────────────────────────────────────────────────────────────────── /// The hash of a password for R5 (SHA-256) or R6 (Algorithm 2.B). -fn hash_r6(r: i64, pw: &[u8], salt: &[u8], udata: &[u8]) -> Vec { - let mut k = { +fn hash_r6(r: i64, pw: &[u8], salt: &[u8], udata: &[u8]) -> Zeroizing> { + let mut k = Zeroizing::new({ let mut h = Sha256::new(); h.update(pw); h.update(salt); h.update(udata); h.finalize().to_vec() - }; + }); if r == 5 { return k; } let mut round = 0u32; loop { - let mut k1 = Vec::with_capacity(64 * (pw.len() + k.len() + udata.len())); + let mut k1 = Zeroizing::new(Vec::with_capacity(64 * (pw.len() + k.len() + udata.len()))); for _ in 0..64 { k1.extend_from_slice(pw); k1.extend_from_slice(&k); @@ -603,13 +608,13 @@ fn hash_r6(r: i64, pw: &[u8], salt: &[u8], udata: &[u8]) -> Vec { // `k` is always a SHA-2 digest (32, 48 or 64 bytes): key = its first 16 bytes, IV = the next 16. let Some((key, rest)) = k.split_first_chunk::<16>() else { break }; let Some(iv) = rest.first_chunk::<16>() else { break }; - let e = aes_cbc_encrypt(key, iv, &k1, false); + let e = Zeroizing::new(aes_cbc_encrypt(key, iv, &k1, false)); let m = e.iter().take(16).map(|b| u32::from(*b)).sum::() % 3; - k = match m { - 0 => Sha256::digest(&e).to_vec(), - 1 => Sha384::digest(&e).to_vec(), - _ => Sha512::digest(&e).to_vec(), - }; + k = Zeroizing::new(match m { + 0 => Sha256::digest(&*e).to_vec(), + 1 => Sha384::digest(&*e).to_vec(), + _ => Sha512::digest(&*e).to_vec(), + }); round += 1; if round >= 64 && e.last().is_none_or(|b| u32::from(*b) <= round - 32) { break; @@ -620,19 +625,19 @@ fn hash_r6(r: i64, pw: &[u8], salt: &[u8], udata: &[u8]) -> Vec { } /// Algorithm 2.A: authenticate as owner (`owner = true`) or user; returns the file key. -fn key_r6(d: &EncryptDict, pw: &[u8], owner: bool) -> Result>, CryptError> { +fn key_r6(d: &EncryptDict, pw: &[u8], owner: bool) -> Result>>, CryptError> { if d.u.len() < 48 || d.o.len() < 48 { return Err(CryptError::Malformed("/O and /U must be 48 bytes for R5/R6".into())); } let (hashed, encrypted, udata) = if owner { (&d.o[..48], &d.oe, &d.u[..48]) } else { (&d.u[..48], &d.ue, &[][..]) }; - if hash_r6(d.r, pw, &hashed[32..40], udata) != hashed[..32] { + if *hash_r6(d.r, pw, &hashed[32..40], udata) != hashed[..32] { return Ok(None); } if encrypted.len() < 32 { return Err(CryptError::Malformed("/OE or /UE must be 32 bytes".into())); } let k = hash_r6(d.r, pw, &hashed[40..48], udata); - Ok(Some(aes_cbc_decrypt(&k, &[0; 16], &encrypted[..32], false))) + Ok(Some(Zeroizing::new(aes_cbc_decrypt(&k, &[0; 16], &encrypted[..32], false)))) } // ── Creating encryption ──────────────────────────────────────────────────────────────────────── @@ -654,7 +659,7 @@ pub enum Algorithm { /// dropped), at most 32 bytes; for R6 its SASLprep (RFC 4013: soft hyphens and the like removed, /// other spaces made plain, then NFKC; the password as given if SASLprep refuses it), in UTF-8, /// at most 127 bytes. -fn prepared_password(algorithm: Algorithm, password: &str) -> Vec { +fn prepared_password(algorithm: Algorithm, password: &str) -> Zeroizing> { match algorithm { Algorithm::Rc4_40 | Algorithm::Rc4_128 | Algorithm::Aes128 => legacy_password(password), Algorithm::Aes256 => sasl_password(password), @@ -672,7 +677,7 @@ pub fn effective_password(algorithm: Algorithm, password: &str) -> Vec { let prepared = prepared_password(algorithm, password); match algorithm { Algorithm::Rc4_40 | Algorithm::Rc4_128 | Algorithm::Aes128 => pad(&prepared).to_vec(), - Algorithm::Aes256 => prepared, + Algorithm::Aes256 => prepared.to_vec(), } } @@ -737,10 +742,10 @@ pub fn create(params: &NewEncryption, id0: &[u8]) -> Result= 3 { for i in 1..=19u8 { - let k: Vec = rk.iter().map(|b| b ^ i).collect(); + let k = Zeroizing::new(rk.iter().map(|b| b ^ i).collect::>()); o = rc4(&k, &o); } } @@ -754,15 +759,15 @@ pub fn create(params: &NewEncryption, id0: &[u8]) -> Result> = &h.key; + let _: &Option>> = &h.recovered_user; + assert_eq!(h.recovered_user_password().as_deref().map(Vec::as_slice), Some(&b"user pw"[..]), "{alg:?}"); assert_eq!(SecurityHandler::open(d, ID, Some("user pw")).unwrap().recovered_user_password(), None); } let d = new(Algorithm::Rc4_128, "", "owner", -1).dict().clone(); - assert_eq!(SecurityHandler::open(d, ID, Some("owner")).unwrap().recovered_user_password().as_deref(), Some(&b""[..])); + assert_eq!(SecurityHandler::open(d, ID, Some("owner")).unwrap().recovered_user_password().as_deref().map(Vec::as_slice), Some(&b""[..])); } /// /O, /U and the first /ID of files qpdf 12.4.2 wrote (`--static-id`), with the file keys @@ -393,7 +395,7 @@ fn legacy_passwords_are_pdfdoc_encoded_with_a_utf8_fallback() { assert_eq!((user.auth(), user.file_key().to_vec()), (Auth::User, hex("3564ffcea186122595ad55ee157da71f"))); let owner = SecurityHandler::open(r3.clone(), &id, Some("Öğretmen")).unwrap(); assert_eq!((owner.auth(), owner.file_key().to_vec()), (Auth::Owner, hex("3564ffcea186122595ad55ee157da71f"))); - assert_eq!(owner.recovered_user_password(), Some(vec![0x9A, b'l', 0x9A, b'k', 0xA0])); + assert_eq!(owner.recovered_user_password(), Some(Zeroizing::new(vec![0x9A, b'l', 0x9A, b'k', 0xA0]))); // Dropping ı and € (the old Latin-1 conversion) or a near miss doesn't open it. for wrong in ["lk", "ilik€", "ılık", "Ogretmen"] { assert_eq!(SecurityHandler::open(r3.clone(), &id, Some(wrong)).unwrap_err(), CryptError::WrongPassword, "{wrong}"); @@ -423,6 +425,6 @@ fn pdfdoc_encoding_round_trips() { for c in ['ğ', 'ş', 'İ', '\u{A0}', '\u{85}', 'Ж', '日'] { assert_eq!(pdfdoc_byte(c), None, "{c}"); } - assert_eq!(legacy_candidates("Çok"), [vec![0xC7, b'o', b'k']]); - assert_eq!(legacy_candidates("şifre"), [b"ifre".to_vec(), "şifre".as_bytes().to_vec()]); + assert_eq!(legacy_candidates("Çok"), [Zeroizing::new(vec![0xC7, b'o', b'k'])]); + assert_eq!(legacy_candidates("şifre"), [Zeroizing::new(b"ifre".to_vec()), Zeroizing::new("şifre".as_bytes().to_vec())]); } diff --git a/crates/sign/Cargo.toml b/crates/sign/Cargo.toml index bfb04a26cb..a72d91ad2a 100644 --- a/crates/sign/Cargo.toml +++ b/crates/sign/Cargo.toml @@ -17,6 +17,7 @@ pdfcraft-content = { workspace = true } pdfcraft-fonts = { workspace = true } pdfcraft-model = { workspace = true } thiserror.workspace = true +zeroize.workspace = true # ASN.1 UTCTime/GeneralizedTime conversion goes through chrono (no hand-rolled calendar code). chrono = { workspace = true } base64 = "0.23" diff --git a/crates/sign/src/keys.rs b/crates/sign/src/keys.rs index 0521c5d914..b5d466ae81 100644 --- a/crates/sign/src/keys.rs +++ b/crates/sign/src/keys.rs @@ -4,6 +4,8 @@ //! (constant-time; ADR-0009: the `rsa` crate is never used for private keys there). ECDSA //! signing uses RustCrypto with deterministic nonces (RFC 6979). +use zeroize::Zeroizing; + use crate::SignError; use crate::der::{self, Tlv, tag}; @@ -588,7 +590,7 @@ pub struct PrivateKey { inner: Inner, public: PublicKey, /// The PKCS#8 `PrivateKeyInfo` it came from (to write it back into a PKCS#12 file). - pkcs8: Vec, + pub(crate) pkcs8: Zeroizing>, } impl std::fmt::Debug for PrivateKey { @@ -601,7 +603,7 @@ impl PrivateKey { /// A key held outside PdfCraft's memory (its public half is `public`). It can't be /// exported to a PKCS #12 file. pub fn external(public: PublicKey, key: std::sync::Arc) -> PrivateKey { - PrivateKey { inner: Inner::External(key), public, pkcs8: Vec::new() } + PrivateKey { inner: Inner::External(key), public, pkcs8: Zeroizing::new(Vec::new()) } } /// Whether the key lives outside the app (and so can't be exported). @@ -631,7 +633,7 @@ impl PrivateKey { ); #[cfg(target_arch = "wasm32")] let inner = Inner::Rsa; - Ok(PrivateKey { inner, public: PublicKey::Rsa { n, e }, pkcs8: info.to_vec() }) + Ok(PrivateKey { inner, public: PublicKey::Rsa { n, e }, pkcs8: Zeroizing::new(info.to_vec()) }) } oid::EC => { // ECPrivateKey: version, privateKey OCTET STRING, [0] parameters, [1] publicKey. @@ -650,12 +652,12 @@ impl PrivateKey { oid::P256 => { let k = p256::ecdsa::SigningKey::from_slice(d).map_err(|_| SignError::Malformed("P-256 key".into()))?; let public = PublicKey::P256(k.verifying_key().to_sec1_point(false).as_bytes().to_vec()); - Ok(PrivateKey { inner: Inner::P256(k), public, pkcs8: info.to_vec() }) + Ok(PrivateKey { inner: Inner::P256(k), public, pkcs8: Zeroizing::new(info.to_vec()) }) } oid::P384 => { let k = p384::ecdsa::SigningKey::from_slice(d).map_err(|_| SignError::Malformed("P-384 key".into()))?; let public = PublicKey::P384(k.verifying_key().to_sec1_point(false).as_bytes().to_vec()); - Ok(PrivateKey { inner: Inner::P384(k), public, pkcs8: info.to_vec() }) + Ok(PrivateKey { inner: Inner::P384(k), public, pkcs8: Zeroizing::new(info.to_vec()) }) } other => Err(SignError::Unsupported(format!("elliptic curve {other}"))), } @@ -689,13 +691,15 @@ impl PrivateKey { /// A new P-256 key. pub fn generate_p256() -> Result { loop { - let mut d = [0u8; 32]; - getrandom::fill(&mut d).map_err(|e| SignError::Crypto(format!("no randomness: {e}")))?; - if let Ok(k) = p256::ecdsa::SigningKey::from_slice(&d) { + let mut d = Zeroizing::new([0u8; 32]); + getrandom::fill(&mut *d).map_err(|e| SignError::Crypto(format!("no randomness: {e}")))?; + if let Ok(k) = p256::ecdsa::SigningKey::from_slice(&*d) { let point = k.verifying_key().to_sec1_point(false).as_bytes().to_vec(); // PKCS#8 with an ECPrivateKey (RFC 5915) carrying the public key. - let ec = der::seq(&[&der::int(1), &der::octets(&d), &der::explicit(1, &der::bit_string(&point))]); - let info = der::seq(&[&der::int(0), &der::seq(&[&der::oid(oid::EC), &der::oid(oid::P256)]), &der::octets(&ec)]); + let d_octets = Zeroizing::new(der::octets(&*d)); + let ec = Zeroizing::new(der::seq(&[&der::int(1), &d_octets, &der::explicit(1, &der::bit_string(&point))])); + let ec_octets = Zeroizing::new(der::octets(&ec)); + let info = Zeroizing::new(der::seq(&[&der::int(0), &der::seq(&[&der::oid(oid::EC), &der::oid(oid::P256)]), &ec_octets])); return PrivateKey::from_pkcs8(&info); } } diff --git a/crates/sign/src/pkcs12.rs b/crates/sign/src/pkcs12.rs index a71d5c8584..77a1019ff6 100644 --- a/crates/sign/src/pkcs12.rs +++ b/crates/sign/src/pkcs12.rs @@ -20,6 +20,7 @@ use base64::Engine as _; use cbc::cipher::block_padding::Pkcs7; use cbc::cipher::{BlockModeDecrypt, BlockModeEncrypt, InnerIvInit, KeyIvInit}; use hmac::Mac; +use zeroize::Zeroizing; use crate::SignError; use crate::der::{self, Tlv, tag}; @@ -100,14 +101,14 @@ fn bad(what: &str) -> SignError { } /// The password as a BMPString with its terminating NUL (RFC 7292 Appendix B.1). -fn bmp(password: &str) -> Vec { - let mut v: Vec = password.encode_utf16().flat_map(u16::to_be_bytes).collect(); +fn bmp(password: &str) -> Zeroizing> { + let mut v = Zeroizing::new(password.encode_utf16().flat_map(u16::to_be_bytes).collect::>()); v.extend([0, 0]); v } /// The PKCS #12 key derivation function (RFC 7292 Appendix B.2). -fn pkcs12_kdf(alg: DigestAlg, password: &[u8], salt: &[u8], id: u8, iterations: u32, n: usize) -> Vec { +fn pkcs12_kdf(alg: DigestAlg, password: &[u8], salt: &[u8], id: u8, iterations: u32, n: usize) -> Zeroizing> { // Callers only pass the SHA-1/SHA-2 digests PKCS #12 files use (`open` restricts the MAC digest). let (u, v) = match alg { DigestAlg::Sha1 => (20, 64), @@ -115,25 +116,25 @@ fn pkcs12_kdf(alg: DigestAlg, password: &[u8], salt: &[u8], id: u8, iterations: DigestAlg::Sha256 => (32, 64), DigestAlg::Sha384 => (48, 128), DigestAlg::Sha512 => (64, 128), - _ => return Vec::new(), + _ => return Zeroizing::new(Vec::new()), }; let _ = u; - let fill = |s: &[u8]| -> Vec { + let fill = |s: &[u8]| -> Zeroizing> { if s.is_empty() { - return Vec::new(); + return Zeroizing::new(Vec::new()); } let len = v * s.len().div_ceil(v); - s.iter().cycle().take(len).copied().collect() + Zeroizing::new(s.iter().cycle().take(len).copied().collect()) }; let d = vec![id; v]; - let mut i: Vec = [fill(salt), fill(password)].concat(); - let mut out = Vec::with_capacity(n); + let mut i = Zeroizing::new([fill(salt).as_slice(), fill(password).as_slice()].concat()); + let mut out = Zeroizing::new(Vec::with_capacity(n)); while out.len() < n { - let mut a = alg.digest(&[&d, &i]); + let mut a = Zeroizing::new(alg.digest(&[&d, &i])); for _ in 1..iterations.max(1) { - a = alg.digest(&[&a]); + a = Zeroizing::new(alg.digest(&[&a])); } - let b: Vec = a.iter().cycle().take(v).copied().collect(); + let b = Zeroizing::new(a.iter().cycle().take(v).copied().collect::>()); for chunk in i.chunks_mut(v) { // chunk = (chunk + b + 1) mod 2^(8v) let mut carry = 1u16; @@ -169,7 +170,7 @@ impl Cipher { } } - fn decrypt(self, key: &[u8], iv: &[u8], ct: &[u8]) -> Result, SignError> { + fn decrypt(self, key: &[u8], iv: &[u8], ct: &[u8]) -> Result>, SignError> { let wrong = || SignError::WrongPassword; let r = match self { Cipher::Aes128 => cbc::Decryptor::::new_from_slices(key, iv).map_err(|_| wrong())?.decrypt_padded_vec::(ct), @@ -181,7 +182,7 @@ impl Cipher { cbc::Decryptor::::inner_iv_slice_init(c, iv).map_err(|_| wrong())?.decrypt_padded_vec::(ct) } }; - r.map_err(|_| wrong()) + r.map(Zeroizing::new).map_err(|_| wrong()) } } @@ -205,8 +206,8 @@ fn hmac(alg: DigestAlg, key: &[u8], data: &[u8]) -> Result, SignError> { } } -fn pbkdf2(prf: DigestAlg, password: &[u8], salt: &[u8], rounds: u32, len: usize) -> Result, SignError> { - let mut out = vec![0u8; len]; +fn pbkdf2(prf: DigestAlg, password: &[u8], salt: &[u8], rounds: u32, len: usize) -> Result>, SignError> { + let mut out = Zeroizing::new(vec![0u8; len]); match prf { DigestAlg::Sha1 => pbkdf2::pbkdf2_hmac::(password, salt, rounds, &mut out), DigestAlg::Sha224 => pbkdf2::pbkdf2_hmac::(password, salt, rounds, &mut out), @@ -219,7 +220,7 @@ fn pbkdf2(prf: DigestAlg, password: &[u8], salt: &[u8], rounds: u32, len: usize) } /// Decrypt `ct` with the password-based scheme `alg` (an AlgorithmIdentifier). -fn pbe_decrypt(alg: &Tlv<'_>, password: &str, ct: &[u8], budget: &mut Budget) -> Result, SignError> { +fn pbe_decrypt(alg: &Tlv<'_>, password: &str, ct: &[u8], budget: &mut Budget) -> Result>, SignError> { let parts = alg.children()?; let o = parts.first().ok_or_else(|| bad("algorithm"))?.oid()?; let params = parts.get(1).ok_or_else(|| bad("algorithm parameters"))?; @@ -280,8 +281,7 @@ fn pbe_decrypt(alg: &Tlv<'_>, password: &str, ct: &[u8], budget: &mut Budget) -> let mut key = pkcs12_kdf(DigestAlg::Sha1, &pw, &salt, 1, rounds, key_len); if key.len() == 16 && matches!(cipher, Cipher::TripleDes) { // Two-key 3DES: K1 K2 K1. - let k1 = key[..8].to_vec(); - key.extend(k1); + key.extend_from_within(..8); } let iv = pkcs12_kdf(DigestAlg::Sha1, &pw, &salt, 2, rounds, iv_len); cipher.decrypt(&key, &iv, ct) @@ -297,7 +297,7 @@ struct Bag { } enum BagKind { - Key(Vec), + Key(Zeroizing>), Cert(Vec), } @@ -319,7 +319,7 @@ fn bags(safe_contents: &[u8], password: &str, out: &mut Vec, budget: &mut B } } let kind = match id.as_str() { - KEY_BAG => BagKind::Key(value.raw.to_vec()), + KEY_BAG => BagKind::Key(Zeroizing::new(value.raw.to_vec())), SHROUDED_KEY_BAG => { let p = value.children()?; let [alg, data] = p.as_slice() else { return Err(bad("EncryptedPrivateKeyInfo")) }; @@ -549,4 +549,16 @@ mod tests { assert_eq!(budget.spend(DigestAlg::Sha1, 0, 20).ok(), Some(0)); assert_eq!(budget.0, OPEN_BUDGET - 1); } + + #[test] + fn key_and_kdf_buffers_are_zeroizing() { + let key = PrivateKey::generate_p256().unwrap(); + let _: &Zeroizing> = &key.pkcs8; + let pw: Zeroizing> = bmp("secret12"); + assert_eq!(&*pw, &[0, b's', 0, b'e', 0, b'c', 0, b'r', 0, b'e', 0, b't', 0, b'1', 0, b'2', 0, 0]); + let kdf: Zeroizing> = pkcs12_kdf(DigestAlg::Sha256, &pw, &[1, 2, 3, 4], 1, 1, 32); + assert_eq!(kdf.len(), 32); + let derived: Zeroizing> = pbkdf2(DigestAlg::Sha256, b"secret12", &[1, 2, 3, 4], 1, 32).unwrap(); + assert_eq!(derived.len(), 32); + } } diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index 7f9f39894e..b4e7022220 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -34,6 +34,7 @@ eframe = { workspace = true, features = ["default_fonts", "wgpu", "glow", "acces serde = { workspace = true } serde_json = { workspace = true } log = { workspace = true } +zeroize = { workspace = true } # UTC/calendar display (combine modified-time stamp, the form date picker) goes through chrono # (no hand-rolled calendar code); std-only, so it stays wasm-safe. chrono = { workspace = true } diff --git a/crates/ui-egui/src/app/control.rs b/crates/ui-egui/src/app/control.rs index 9381664527..a545b4165a 100644 --- a/crates/ui-egui/src/app/control.rs +++ b/crates/ui-egui/src/app/control.rs @@ -161,6 +161,7 @@ impl PdfCraftApp { if let Some(Dialog::Marks(kind)) = self.dialog { self.marks_draft.opened(kind); } + dialogs::discard_inactive_password_drafts(self); } ("tools", _) if value == "full" => self.expand_all_tools(), ("tools", _) => { diff --git a/crates/ui-egui/src/app/files.rs b/crates/ui-egui/src/app/files.rs index 1a873a7c11..3f53813222 100644 --- a/crates/ui-egui/src/app/files.rs +++ b/crates/ui-egui/src/app/files.rs @@ -627,11 +627,12 @@ impl PdfCraftApp { /// Answer the password prompt (`None` cancels). pub fn submit_password(&mut self, password: Option) { - let Some(p) = self.password_prompt.take() else { return }; - let Some(pw) = password else { + let Some(mut p) = self.password_prompt.take() else { return }; + zeroize::Zeroize::zeroize(&mut p.input); + let Some(pw) = password.map(zeroize::Zeroizing::new) else { // A cancelled recovery goes back on offer; its snapshot stays in the store and must // not attach itself to whatever document is unlocked next (#813). - if let Some(meta) = p.recovered { + if let Some(meta) = p.recovered.take() { self.recoverable.push(meta); } #[cfg(not(target_arch = "wasm32"))] @@ -669,7 +670,9 @@ impl PdfCraftApp { #[cfg(not(target_arch = "wasm32"))] let (path_for_watch, stamp_for_watch) = (p.path.clone(), p.stamp); let after_open = p.after_open.clone(); - match self.try_open(&p.name, p.path, p.bytes, Some(&pw), p.activate) { + let opened = self.try_open(&p.name, p.path.take(), p.bytes.clone(), Some(&pw), p.activate); + drop(pw); + match opened { Err(e) => self.notify_fmt("Couldn't open {name}: {e}", &[("name", &p.name), ("e", &e.to_string())]), // A recovered encrypted document is open once its prompt is gone. Ok(()) if self.password_prompt.is_none() => { @@ -706,7 +709,7 @@ impl PdfCraftApp { // and the tool waiting for it. Ok(()) => { if let Some(prompt) = self.password_prompt.as_mut() { - prompt.recovered = p.recovered; + prompt.recovered = p.recovered.take(); prompt.then = p.then; prompt.after_open = after_open; prompt.display_override = p.display_override; @@ -726,8 +729,11 @@ impl PdfCraftApp { /// Answer the permissions password prompt (`None` cancels). pub fn submit_unlock(&mut self, password: Option) { let Some(mut p) = self.unlock_prompt.take() else { return }; - let Some(pw) = password else { return }; - match self.session.unlock(p.doc, &pw) { + zeroize::Zeroize::zeroize(&mut p.input); + let Some(mut pw) = password else { return }; + let unlocked = self.session.unlock(p.doc, &pw); + zeroize::Zeroize::zeroize(&mut pw); + match unlocked { // Carry on with what asked for the password, if its document is still the one shown. Ok(()) => match p.then { Some(command) if self.active_ids().is_some_and(|(_, id)| id == p.doc) => { @@ -736,7 +742,6 @@ impl PdfCraftApp { _ => self.notify_tr("Restrictions lifted"), }, Err(pdfcraft_engine::EditError::WrongPassword) => { - p.input.clear(); p.error = Some("Incorrect password. Try again.".to_string()); self.unlock_prompt = Some(p); } diff --git a/crates/ui-egui/src/commands.rs b/crates/ui-egui/src/commands.rs index ac52586062..1ad3e0f5ed 100644 --- a/crates/ui-egui/src/commands.rs +++ b/crates/ui-egui/src/commands.rs @@ -113,7 +113,7 @@ impl PdfCraftApp { /// Last-resort guard (AGENTS.md §4): a command that panics is reported and the app, with its /// open documents, keeps running. Edits are applied to a copy, so the document is unchanged. pub fn execute(&mut self, id: &str) -> bool { - match pdfcraft_engine::guard(|| self.execute_unguarded(id)) { + let done = match pdfcraft_engine::guard(|| self.execute_unguarded(id)) { Ok(done) => done, Err(m) => { // A command stopped part way through closing or replacing a tab must not leave @@ -122,7 +122,9 @@ impl PdfCraftApp { self.notify_fmt("That didn't work: an internal error stopped it ({m}). Your documents are unchanged.", &[("m", m.as_str())]); true } - } + }; + crate::dialogs::discard_inactive_password_drafts(self); + done } /// Why the command can't run right now, for the user. diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 0f93ae8e2d..f156285f89 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -31,6 +31,17 @@ use prompts::{full_screen_prompt, link_prompt, password, remote_link_prompt, sav const INFO_KEYS: [&str; 4] = ["Title", "Author", "Subject", "Keywords"]; +/// Drop (and zeroize) password drafts as soon as their owning dialog is inactive. +/// Commands and control requests also call this: close/reopen can happen between frames. +pub(crate) fn discard_inactive_password_drafts(app: &mut PdfCraftApp) { + if app.dialog != Some(Dialog::Protect) { + app.protect_draft = Default::default(); + } + if app.dialog != Some(Dialog::Sign) { + app.sign_draft = None; + } +} + /// What the dialog's widgets asked for this frame; acted on after the modal is drawn. #[derive(Default)] struct Pending { @@ -106,6 +117,7 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) { if app.dialog != Some(Dialog::FieldTable) { app.field_table_draft = None; } + discard_inactive_password_drafts(app); // A movable window, not this modal: the page under it shows the table as it changes. if app.dialog == Some(Dialog::FieldTable) { crate::field_table_ui::show(app, ctx); @@ -2214,6 +2226,7 @@ fn finish_close(app: &mut PdfCraftApp, dialog: Dialog, modal_closed: bool, req: } else { app.dialog = Some(req.next.unwrap_or(dialog)); } + discard_inactive_password_drafts(app); if let Some(cmd) = req.link_command { app.execute(cmd); } @@ -2282,3 +2295,134 @@ fn draft_changes(app: &PdfCraftApp) -> Option> { fn yes(b: bool) -> String { tl!(if b { "Yes" } else { "No" }).to_string() } + +#[cfg(test)] +mod password_draft_tests { + use super::*; + use egui_kittest::Harness; + + fn seed_password_drafts(app: &mut PdfCraftApp) { + app.start_signing(0, None, None, None); + let sign = app.sign_draft.as_mut().unwrap(); + sign.step = crate::sign_ui::SignStep::Choose; + sign.password = "sign password".into(); + sign.new_id.password = "new ID password".into(); + sign.new_id.confirm = "new ID password".into(); + sign.new_id.file_password = "import password".into(); + app.protect_draft.password = "view password".into(); + app.protect_draft.confirm = "view password".into(); + app.protect_draft.protection.open_password = Some("open password".into()); + app.protect_draft.protection.permissions_password = Some("owner password".into()); + } + + fn assert_protect_cleared(app: &PdfCraftApp) { + assert_eq!(app.protect_draft, crate::protect::ProtectDraft::default()); + } + + #[test] + fn inactive_password_drafts_are_cleared_before_early_returns() { + // None and FieldTable return before the ordinary modal-close path; About replaces it. + for dialog in [None, Some(Dialog::FieldTable), Some(Dialog::About)] { + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(|_| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + seed_password_drafts(&mut app); + app.dialog = dialog; + app + }); + h.run_steps(2); + assert_protect_cleared(h.state()); + assert!(h.state().sign_draft.is_none()); + } + } + + #[test] + fn rendering_preserves_only_the_active_password_draft() { + for dialog in [Dialog::Protect, Dialog::Sign] { + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(|_| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + seed_password_drafts(&mut app); + app.dialog = Some(dialog); + app + }); + h.run_steps(2); + if dialog == Dialog::Protect { + assert_eq!(h.state().protect_draft.password, "view password"); + assert_eq!(h.state().protect_draft.confirm, "view password"); + assert!(h.state().sign_draft.is_none()); + } else { + assert_protect_cleared(h.state()); + let sign = h.state().sign_draft.as_ref().unwrap(); + assert_eq!(sign.password, "sign password"); + assert_eq!(sign.new_id.password, "new ID password"); + assert_eq!(sign.new_id.confirm, "new ID password"); + assert_eq!(sign.new_id.file_password, "import password"); + } + } + } + + #[test] + fn control_close_and_reopen_clears_passwords_without_a_render_between_requests() { + let mut app = PdfCraftApp::new(); + // The control queue can handle several ui.set requests before the next rendered frame. + // Repeat both close and replacement flows so stale drafts cannot return on later opens. + for _ in 0..2 { + for replacement in ["none", "about", "sign", "certify"] { + app.set_option("dialog", "protect").unwrap(); + app.protect_draft.password = "old password".into(); + app.protect_draft.confirm = "old password".into(); + app.set_option("dialog", replacement).unwrap(); + assert_protect_cleared(&app); + app.set_option("dialog", "protect").unwrap(); + assert_protect_cleared(&app); + assert!(app.sign_draft.is_none()); + } + for replacement in ["none", "about", "protect"] { + seed_password_drafts(&mut app); + app.set_option("dialog", replacement).unwrap(); + assert!(app.sign_draft.is_none(), "inactive Sign draft retained after {replacement}"); + app.set_option("dialog", "sign").unwrap(); + let sign = app.sign_draft.as_ref().unwrap(); + assert!(sign.password.is_empty()); + assert_eq!(sign.new_id, crate::sign_ui::NewIdDraft::default()); + } + } + } + + #[test] + fn selecting_the_active_protect_dialog_keeps_its_password() { + let mut app = PdfCraftApp::new(); + app.set_option("dialog", "protect").unwrap(); + app.protect_draft.password = "current password".into(); + app.protect_draft.confirm = "current password".into(); + app.set_option("dialog", "protect").unwrap(); + assert_eq!(app.protect_draft.password, "current password"); + assert_eq!(app.protect_draft.confirm, "current password"); + } + + #[test] + fn commands_discard_replaced_password_drafts_before_the_next_control_request() { + let mut app = PdfCraftApp::new(); + for _ in 0..2 { + seed_password_drafts(&mut app); + app.dialog = Some(Dialog::Protect); + assert!(app.execute("help.about")); + assert_eq!(app.dialog, Some(Dialog::About)); + assert_protect_cleared(&app); + assert!(app.sign_draft.is_none()); + app.set_option("dialog", "protect").unwrap(); + assert_protect_cleared(&app); + + seed_password_drafts(&mut app); + assert!(app.execute("help.shortcuts")); + assert_eq!(app.dialog, Some(Dialog::Shortcuts)); + assert_protect_cleared(&app); + assert!(app.sign_draft.is_none()); + app.set_option("dialog", "sign").unwrap(); + let sign = app.sign_draft.as_ref().unwrap(); + assert!(sign.password.is_empty()); + assert_eq!(sign.new_id, crate::sign_ui::NewIdDraft::default()); + } + } +} diff --git a/crates/ui-egui/src/dialogs/prompts.rs b/crates/ui-egui/src/dialogs/prompts.rs index 91e069bca3..4b69afbfb6 100644 --- a/crates/ui-egui/src/dialogs/prompts.rs +++ b/crates/ui-egui/src/dialogs/prompts.rs @@ -351,7 +351,7 @@ pub(super) fn password(app: &mut PdfCraftApp, ctx: &egui::Context) { let message = crate::i18n::fmt(tl!("“{name}” is protected. Enter a password to open it."), &[("name", &prompt.name)]); match password_modal(ctx, "password", tl!("Password required"), message, &mut prompt.input, prompt.error.as_deref(), tl!("Open")) { Some(PasswordAnswer::Submit) => { - let pw = prompt.input.clone(); + let pw = std::mem::take(&mut prompt.input); app.submit_password(Some(pw)); } Some(PasswordAnswer::Cancel) => app.submit_password(None), @@ -366,7 +366,7 @@ pub(super) fn unlock(app: &mut PdfCraftApp, ctx: &egui::Context) { crate::i18n::fmt(tl!("“{name}” has security restrictions. Enter its permissions password to lift them."), &[("name", &prompt.name)]); match password_modal(ctx, "unlock", tl!("Permissions password"), message, &mut prompt.input, prompt.error.as_deref(), tl!("Unlock")) { Some(PasswordAnswer::Submit) => { - let pw = prompt.input.clone(); + let pw = std::mem::take(&mut prompt.input); app.submit_unlock(Some(pw)); } Some(PasswordAnswer::Cancel) => app.submit_unlock(None), diff --git a/crates/ui-egui/src/dialogs/protect.rs b/crates/ui-egui/src/dialogs/protect.rs index ffc8b53f8e..cc79f3c82c 100644 --- a/crates/ui-egui/src/dialogs/protect.rs +++ b/crates/ui-egui/src/dialogs/protect.rs @@ -25,6 +25,20 @@ impl Default for ProtectDraft { } } +impl Drop for ProtectDraft { + fn drop(&mut self) { + use zeroize::Zeroize as _; + self.password.zeroize(); + self.confirm.zeroize(); + if let Some(pw) = &mut self.protection.open_password { + pw.zeroize(); + } + if let Some(pw) = &mut self.protection.permissions_password { + pw.zeroize(); + } + } +} + impl ProtectDraft { /// Why Apply is disabled, if it is. pub fn problem(&self) -> Option<&'static str> { @@ -251,4 +265,40 @@ mod tests { assert_eq!(strength("Abcdefg1").0, "Medium"); assert_eq!(strength("Correct-Horse-9").0, "Strong"); } + + #[test] + fn closing_the_protect_dialog_clears_the_password_draft() { + use egui_kittest::{Harness, kittest::Queryable}; + + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(|_| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.dialog = Some(crate::Dialog::Protect); + app.protect_draft.password = "TopSecret123!".into(); + app.protect_draft.confirm = "TopSecret123!".into(); + app + }); + h.run_steps(2); + + // Clicking Cancel closes Dialog::Protect and clears the draft passwords. + h.get_by_label("Cancel").click(); + h.run_steps(2); + assert_eq!(h.state().dialog, None); + assert!(h.state().protect_draft.password.is_empty(), "password must not linger after cancelling Dialog::Protect"); + assert!(h.state().protect_draft.confirm.is_empty(), "confirm must not linger after cancelling Dialog::Protect"); + + // Pressing Escape on Dialog::Protect also clears the draft passwords. + { + let app = h.state_mut(); + app.dialog = Some(crate::Dialog::Protect); + app.protect_draft.password = "AnotherSecret!".into(); + app.protect_draft.confirm = "AnotherSecret!".into(); + } + h.run_steps(2); + h.key_press(egui::Key::Escape); + h.run_steps(2); + assert_eq!(h.state().dialog, None); + assert!(h.state().protect_draft.password.is_empty(), "password must not linger after closing Dialog::Protect"); + assert!(h.state().protect_draft.confirm.is_empty(), "confirm must not linger after closing Dialog::Protect"); + } } diff --git a/crates/ui-egui/src/platform/open_worker.rs b/crates/ui-egui/src/platform/open_worker.rs index 274071ea78..94052f6368 100644 --- a/crates/ui-egui/src/platform/open_worker.rs +++ b/crates/ui-egui/src/platform/open_worker.rs @@ -37,7 +37,7 @@ struct Job { context: OpenContext, name: String, path: Option, - password: Option, + password: Option>, input: Input, reply: Sender, #[cfg(test)] @@ -80,7 +80,7 @@ impl OpenWorker { context: OpenContext, name: String, path: Option, - password: Option, + password: Option>, input: Input, ) -> Result, String> { let (reply, receiver) = mpsc::channel(); @@ -178,7 +178,7 @@ fn prepare(job: Job) -> ResultKind { if matches!(pdfcraft_engine::source_kind(&job.name, &bytes), Some(SourceKind::Image | SourceKind::Text)) { return ResultKind::Conversion(bytes); } - let result = job.context.prepare_open(job.name, job.path, bytes.clone(), job.password.as_deref()); + let result = job.context.prepare_open(job.name, job.path, bytes.clone(), job.password.as_deref().map(String::as_str)); ResultKind::Prepared { bytes, result: Box::new(result) } } diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index d4f5e09be4..9acbc370f5 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -356,13 +356,19 @@ pub struct PasswordPrompt { pub(crate) display_override: Option<(canvas::PageLayout, bool)>, } +impl Drop for PasswordPrompt { + fn drop(&mut self) { + zeroize::Zeroize::zeroize(&mut self.input); + } +} + #[cfg(not(target_arch = "wasm32"))] pub(crate) struct OpenRequest { pub(crate) name: String, pub(crate) path: Option, pub(crate) stamp: Option, pub(crate) input: open_worker::Input, - pub(crate) password: Option, + pub(crate) password: Option>, pub(crate) activate: bool, pub(crate) after_open: Option, pub(crate) recovered: Option, @@ -380,6 +386,12 @@ pub struct UnlockPrompt { pub then: Option<&'static str>, } +impl Drop for UnlockPrompt { + fn drop(&mut self) { + zeroize::Zeroize::zeroize(&mut self.input); + } +} + impl UnlockPrompt { /// Ask for document `doc`'s permissions password, then run command `then`, if any. pub(crate) fn new(doc: &pdfcraft_engine::Document, then: Option<&'static str>) -> Self { diff --git a/crates/ui-egui/src/tools/sign_ui.rs b/crates/ui-egui/src/tools/sign_ui.rs index 299f66329c..56410eeb33 100644 --- a/crates/ui-egui/src/tools/sign_ui.rs +++ b/crates/ui-egui/src/tools/sign_ui.rs @@ -90,6 +90,15 @@ impl Default for NewIdDraft { } } +impl Drop for NewIdDraft { + fn drop(&mut self) { + use zeroize::Zeroize as _; + self.password.zeroize(); + self.confirm.zeroize(); + self.file_password.zeroize(); + } +} + /// What a visible signature shows on its left (Acrobat: Configure Signature Appearance ▸ Text, /// Draw or Image). The one last signed with is remembered. #[derive(Clone, Debug, Default, PartialEq, serde::Serialize, serde::Deserialize)] @@ -152,6 +161,13 @@ impl SignDraft { } } +impl Drop for SignDraft { + fn drop(&mut self) { + use zeroize::Zeroize as _; + self.password.zeroize(); + } +} + /// What a page asks of the signing flow. #[derive(Clone, Debug, Default, PartialEq)] pub struct SignView { @@ -426,51 +442,57 @@ impl PdfCraftApp { /// Configure New Digital ID ▸ Create: a self-signed ID saved as a .p12 next to the app data /// (or in `export_dir_override`). Returns its list index. fn create_digital_id(&mut self) -> Result { - let Some(d) = self.sign_draft.as_ref().map(|d| d.new_id.clone()) else { return Err("nothing to create".into()) }; - if d.name.trim().is_empty() { - return Err("Enter a name.".into()); - } - // The message is a catalogue key, so it spells the number out; keep the two in step. - const _: () = assert!(sign::pkcs12::MIN_PASSWORD_CHARS == 8); - if d.password.chars().count() < sign::pkcs12::MIN_PASSWORD_CHARS { - return Err("The password must have at least 8 characters.".into()); - } - if d.password != d.confirm { - return Err("The passwords do not match.".into()); - } - let key = match KEY_ALGORITHMS[d.key.min(KEY_ALGORITHMS.len() - 1)].1 { - "rsa3072" => PrivateKey::generate_rsa(3072), - "rsa4096" => PrivateKey::generate_rsa(4096), - "p256" => PrivateKey::generate_p256(), - _ => PrivateKey::generate_rsa(2048), - } - .map_err(|e| e.to_string())?; - let now = self.session.now_secs(); - let dn = Name::build(&d.name, &d.unit, &d.organization, &d.email, &d.country); - let serial = sign::keys::DigestAlg::Sha256.digest(&[d.name.as_bytes(), &now.to_be_bytes()])[..8].to_vec(); - let cert = Certificate::self_signed(&dn, &key, sign::Time::from_unix(now), 5, &serial).map_err(|e| e.to_string())?; - let id = DigitalId { key, certificate: cert.clone(), chain: Vec::new(), friendly_name: Some(d.name.trim().to_string()) }; - let p12 = sign::pkcs12::write(&id, &d.password).map_err(|e| e.to_string())?; - let dir = match &self.export_dir_override { - Some(p) => Some(PathBuf::from(p)), - None => id_dir(), - } - .ok_or("There is no folder to save the digital ID in.")?; - std::fs::create_dir_all(&dir).map_err(|e| e.to_string())?; - let stem: String = d.name.trim().chars().map(|c| if c.is_alphanumeric() || c == ' ' || c == '-' { c } else { '_' }).collect(); - let path = save_new_id_file(&dir, &stem, &p12).map_err(|e| e.to_string())?; + let (path, cert) = { + let Some(d) = self.sign_draft.as_ref().map(|d| &d.new_id) else { return Err("nothing to create".into()) }; + if d.name.trim().is_empty() { + return Err("Enter a name.".into()); + } + // The message is a catalogue key, so it spells the number out; keep the two in step. + const _: () = assert!(sign::pkcs12::MIN_PASSWORD_CHARS == 8); + if d.password.chars().count() < sign::pkcs12::MIN_PASSWORD_CHARS { + return Err("The password must have at least 8 characters.".into()); + } + if d.password != d.confirm { + return Err("The passwords do not match.".into()); + } + let key = match KEY_ALGORITHMS[d.key.min(KEY_ALGORITHMS.len() - 1)].1 { + "rsa3072" => PrivateKey::generate_rsa(3072), + "rsa4096" => PrivateKey::generate_rsa(4096), + "p256" => PrivateKey::generate_p256(), + _ => PrivateKey::generate_rsa(2048), + } + .map_err(|e| e.to_string())?; + let now = self.session.now_secs(); + let dn = Name::build(&d.name, &d.unit, &d.organization, &d.email, &d.country); + let serial = sign::keys::DigestAlg::Sha256.digest(&[d.name.as_bytes(), &now.to_be_bytes()])[..8].to_vec(); + let cert = Certificate::self_signed(&dn, &key, sign::Time::from_unix(now), 5, &serial).map_err(|e| e.to_string())?; + let id = DigitalId { key, certificate: cert.clone(), chain: Vec::new(), friendly_name: Some(d.name.trim().to_string()) }; + let p12 = sign::pkcs12::write(&id, &d.password).map_err(|e| e.to_string())?; + let dir = match &self.export_dir_override { + Some(p) => Some(PathBuf::from(p)), + None => id_dir(), + } + .ok_or("There is no folder to save the digital ID in.")?; + std::fs::create_dir_all(&dir).map_err(|e| e.to_string())?; + let stem: String = d.name.trim().chars().map(|c| if c.is_alphanumeric() || c == ' ' || c == '-' { c } else { '_' }).collect(); + let path = save_new_id_file(&dir, &stem, &p12).map_err(|e| e.to_string())?; + (path, cert) + }; Ok(self.add_digital_id(&path.to_string_lossy(), &cert)) } /// Configure New Digital ID ▸ Use a Digital ID from a file. fn import_digital_id(&mut self) -> Result { - let Some(d) = self.sign_draft.as_ref().map(|d| d.new_id.clone()) else { return Err("nothing to import".into()) }; - let bytes = std::fs::read(&d.file).map_err(|e| format!("{}: {e}", d.file))?; - let id = sign::pkcs12::open(&bytes, &d.file_password).map_err(|e| match e { - sign::SignError::WrongPassword => "The password is incorrect.".to_string(), - e => e.to_string(), - })?; - Ok(self.add_digital_id(&d.file, &id.certificate)) + let (file, cert) = { + let Some(d) = self.sign_draft.as_ref().map(|d| &d.new_id) else { return Err("nothing to import".into()) }; + let bytes = std::fs::read(&d.file).map_err(|e| format!("{}: {e}", d.file))?; + let id = sign::pkcs12::open(&bytes, &d.file_password).map_err(|e| match e { + sign::SignError::WrongPassword => "The password is incorrect.".to_string(), + e => e.to_string(), + })?; + (d.file.clone(), id.certificate) + }; + Ok(self.add_digital_id(&file, &cert)) } /// Sign as …: open the ID, sign, save the signed file (Save As), and show it. @@ -481,7 +503,7 @@ impl PdfCraftApp { return Err(String::new()); } let Some((_, doc_id)) = self.active_ids() else { return Err("no document".into()) }; - let d = self.sign_draft.clone().ok_or("nothing to sign")?; + let d = self.sign_draft.as_ref().ok_or("nothing to sign")?; let entry = d.selected.and_then(|i| self.digital_ids.get(i)).filter(|e| e.unusable.is_none()).cloned().ok_or("Choose a digital ID.")?; let id = if entry.path.starts_with("keychain:") { keychain_id(&entry.path)? @@ -781,8 +803,10 @@ mod tests { let mut draft = SignDraft::new(0, None, None, None, &[]); // P-256 keeps the test fast. let key = KEY_ALGORITHMS.iter().position(|k| k.1 == "p256").unwrap(); - draft.new_id = - NewIdDraft { name: "Grace Hopper".into(), key, password: "secret12".into(), confirm: "secret12".into(), ..NewIdDraft::default() }; + draft.new_id.name = "Grace Hopper".into(); + draft.new_id.key = key; + draft.new_id.password = "secret12".into(); + draft.new_id.confirm = "secret12".into(); app.sign_draft = Some(draft); let i = app.create_digital_id()?; Ok(PathBuf::from(&app.digital_ids[i].path)) @@ -1036,4 +1060,74 @@ mod tests { assert_eq!(bar(&[signature(None, Valid)]), Some(("circle-check", "Signed and all signatures are valid."))); assert_eq!(bar(&[SignatureInfo { signed: false, ..signature(None, Invalid) }]), None); } + + #[test] + fn dismissing_or_stepping_back_in_the_sign_dialog_clears_passwords() { + use egui_kittest::{Harness, kittest::Queryable}; + + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(|_| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.digital_ids = vec![listed("/ids/grace.p12", "Grace Hopper")]; + let mut draft = SignDraft::new(0, None, None, None, &app.digital_ids); + draft.step = SignStep::SignAs; + draft.password = "secret12".into(); + app.sign_draft = Some(draft); + app.dialog = Some(crate::Dialog::Sign); + app + }); + h.run_steps(2); + + // Stepping back from SignAs to Choose clears the typed digital ID password. + h.get_by_label("Back").click(); + h.run_steps(2); + assert_eq!(h.state().sign_draft.as_ref().unwrap().step, SignStep::Choose); + assert!(h.state().sign_draft.as_ref().unwrap().password.is_empty(), "Back from SignAs must clear password"); + + // Stepping back from Configure to Choose resets the new-ID password fields. + { + let d = h.state_mut().sign_draft.as_mut().unwrap(); + d.step = SignStep::Configure; + d.new_id.password = "newsecret12".into(); + d.new_id.confirm = "newsecret12".into(); + d.new_id.file_password = "filesecret12".into(); + } + h.run_steps(2); + h.get_by_label("Back").click(); + h.run_steps(2); + let d = h.state().sign_draft.as_ref().unwrap(); + assert_eq!(d.step, SignStep::Choose); + assert!(d.new_id.password.is_empty() && d.new_id.confirm.is_empty() && d.new_id.file_password.is_empty()); + + // Dismissing the modal with Escape clears sign_draft completely. + h.state_mut().sign_draft.as_mut().unwrap().password = "secret12".into(); + h.key_press(egui::Key::Escape); + h.run_steps(2); + assert_eq!(h.state().dialog, None); + assert!(h.state().sign_draft.is_none(), "sign_draft must be cleared when Dialog::Sign is dismissed"); + + // Importing a digital ID file and signing work by borrowing the draft in place. + let dir = scratch(); + let saved = create_in(&dir).unwrap(); + let mut import_app = PdfCraftApp::new(); + let mut import_draft = SignDraft::new(0, None, None, None, &[]); + import_draft.new_id.create = false; + import_draft.new_id.file = saved.to_string_lossy().into_owned(); + import_draft.new_id.file_password = "secret12".into(); + import_app.sign_draft = Some(import_draft); + assert_eq!(import_app.import_digital_id(), Ok(0)); + assert_eq!(import_app.digital_ids[0].name, "Grace Hopper"); + + let bytes = pdfcraft_engine::Session::new().create_blank(200.0, 200.0, 1).unwrap(); + import_app.open_bytes("unsigned.pdf", None, bytes.as_ref().clone()).unwrap(); + let out_pdf = dir.join("signed.pdf"); + import_app.save_override = Some(out_pdf.to_string_lossy().into_owned()); + let mut sign_draft = SignDraft::new(0, None, None, None, &import_app.digital_ids); + sign_draft.step = SignStep::SignAs; + sign_draft.password = "secret12".into(); + import_app.sign_draft = Some(sign_draft); + assert_eq!(import_app.finish_signing(), Ok(())); + assert!(out_pdf.exists()); + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/crates/ui-egui/src/tools/sign_ui/dialog.rs b/crates/ui-egui/src/tools/sign_ui/dialog.rs index 61e9aac042..2e5a319573 100644 --- a/crates/ui-egui/src/tools/sign_ui/dialog.rs +++ b/crates/ui-egui/src/tools/sign_ui/dialog.rs @@ -206,6 +206,7 @@ fn configure(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> bool { error(ui, &d.error); ui.horizontal(|ui| { if !app.digital_ids.is_empty() && widgets::pill_button(ui, tl!("Back"), false).clicked() { + d.new_id = NewIdDraft::default(); d.step = SignStep::Choose; d.error = None; } @@ -471,6 +472,7 @@ fn sign_as(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> bool { error(ui, &d.error); ui.horizontal(|ui| { if widgets::pill_button(ui, tl!("Back"), false).clicked() { + zeroize::Zeroize::zeroize(&mut d.password); d.step = SignStep::Choose; d.error = None; } diff --git a/crates/ui-egui/src/workflows/combine_ui/files.rs b/crates/ui-egui/src/workflows/combine_ui/files.rs index 85634c2aff..9c8b3d25ed 100644 --- a/crates/ui-egui/src/workflows/combine_ui/files.rs +++ b/crates/ui-egui/src/workflows/combine_ui/files.rs @@ -128,6 +128,12 @@ pub(crate) struct Preview { #[derive(Clone, Default)] pub struct Secret(pub(super) String); +impl Drop for Secret { + fn drop(&mut self) { + zeroize::Zeroize::zeroize(&mut self.0); + } +} + impl std::fmt::Debug for Secret { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str(if self.0.is_empty() { "\"\"" } else { "…" })