diff --git a/ATTRIBUTION.toml b/ATTRIBUTION.toml index c9353d2ee..d4ff7802c 100644 --- a/ATTRIBUTION.toml +++ b/ATTRIBUTION.toml @@ -52,7 +52,7 @@ licence = "MIT OR Apache-2.0" licence_file = "LICENSE-MIT" kind = "translation" usage = "Czech UI strings, compiled in by crates/ui-egui/src/i18n/mod.rs" -sha256 = "e3e2e7b926767e32d1d4c105749d8f2fa570245b4ed6cdddf0ec6ff0f19eee24" +sha256 = "bd0605ae01a6f0cef6675fb73006ca798338892650102a1df5d3d815bc964c63" [[asset]] path = "crates/ui-egui/src/i18n/pt-br.tsv" @@ -228,7 +228,7 @@ licence = "MIT OR Apache-2.0" licence_file = "LICENSE-MIT" kind = "translation" usage = "Lithuanian UI strings, compiled in by crates/ui-egui/src/i18n/mod.rs" -sha256 = "30b8a65a5fae8cf2c3fd5d43d53991095d2f00042899a66c3949573b28d4e3c4" +sha256 = "22ffe5e2c5b43c7b29df378ad4f9773584467b411f3cf647e7b21f5bce862446" [[asset]] diff --git a/crates/automation/src/signing.rs b/crates/automation/src/signing.rs index 07bf6758c..94fc4bdec 100644 --- a/crates/automation/src/signing.rs +++ b/crates/automation/src/signing.rs @@ -145,8 +145,8 @@ impl Automation { return Err(bad("name must not be empty")); } let password = a.str("password")?; - if password.chars().count() < 6 { - return Err(bad("the password must have at least 6 characters")); + if password.chars().count() < sign::pkcs12::MIN_PASSWORD_CHARS { + return Err(bad(format!("the password must have at least {} characters", sign::pkcs12::MIN_PASSWORD_CHARS))); } let path = self.resolve(a.str("path")?, true)?; let key = match a.opt_str("key")?.unwrap_or("rsa2048") { diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index a7b30c312..ba79a7ade 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -677,7 +677,7 @@ pub fn tools() -> Vec { "country": { "type": "string", "description": "Two-letter country code." }, "key": { "type": "string", "enum": ["rsa2048", "rsa3072", "rsa4096", "p256"] }, "years": { "type": "integer", "minimum": 1, "maximum": 50 }, - "password": { "type": "string", "minLength": 6 }, + "password": { "type": "string", "minLength": pdfcraft_engine::sign::pkcs12::MIN_PASSWORD_CHARS }, "path": { "type": "string", "description": "The .p12 file to write." }, }), &["name", "password", "path"], diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index a7db10e58..59bb5f505 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -1758,7 +1758,7 @@ fn protecting_through_tools() { fn signing_an_encrypted_form_through_tools() { let dir = workdir("sign-encrypted"); let mut a = auto(&dir); - ok(&mut a, "sign_id_create", json!({ "name": "Ada Lovelace", "key": "p256", "password": "secret1", "path": "ada.p12" })); + ok(&mut a, "sign_id_create", json!({ "name": "Ada Lovelace", "key": "p256", "password": "secret12", "path": "ada.p12" })); let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap(); let field = ok(&mut a, "form_add_field", json!({ "doc": doc, "page": 1, "type": "signature", "rect": [20, 200, 180, 250] }))["field"] .as_str() @@ -1771,12 +1771,12 @@ fn signing_an_encrypted_form_through_tools() { assert_eq!(ok(&mut b, "doc_info", json!({ "doc": form }))["security"]["protected"], true); match b.call( "sign_document", - &json!({ "doc": form, "id": "ada.p12", "password": "secret1", "page": 1, "rect": [200, 200, 380, 250], "out": "new.pdf" }), + &json!({ "doc": form, "id": "ada.p12", "password": "secret12", "page": 1, "rect": [200, 200, 380, 250], "out": "new.pdf" }), ) { Err(ToolError::Failed(m)) => assert!(m.contains("adding new signature fields"), "{m}"), other => panic!("a new field needs the permissions password: {other:?}"), } - let r = ok(&mut b, "sign_document", json!({ "doc": form, "id": "ada.p12", "password": "secret1", "field": field, "out": "signed.pdf" })); + let r = ok(&mut b, "sign_document", json!({ "doc": form, "id": "ada.p12", "password": "secret12", "field": field, "out": "signed.pdf" })); assert_eq!((r["signature"]["signer"].as_str(), r["signature"]["status"].as_str()), (Some("Ada Lovelace"), Some("unknown"))); let original = std::fs::read(dir.join("form.pdf")).unwrap(); let signed = std::fs::read(dir.join("signed.pdf")).unwrap(); @@ -1784,7 +1784,7 @@ fn signing_an_encrypted_form_through_tools() { let mut c = auto(&dir); let re = ok(&mut c, "doc_open", json!({ "path": "signed.pdf" }))["doc"].as_u64().unwrap(); assert_eq!(ok(&mut c, "doc_info", json!({ "doc": re }))["security"]["protected"], true, "still encrypted"); - ok(&mut c, "sign_trust", json!({ "paths": ["ada.p12"], "password": "secret1" })); + ok(&mut c, "sign_trust", json!({ "paths": ["ada.p12"], "password": "secret12" })); let list = ok(&mut c, "sign_list", json!({ "doc": re })); assert_eq!((list["all_valid"].as_bool(), list["signatures"][0]["status"].as_str()), (Some(true), Some("valid")), "{list}"); } @@ -4209,12 +4209,12 @@ fn signing_with_an_image_typed_or_drawn_graphic_through_tools() { std::fs::write(dir.join("logo.png"), png).unwrap(); std::fs::write(dir.join("broken.png"), b"broken").unwrap(); let mut a = auto(&dir); - ok(&mut a, "sign_id_create", json!({ "name": "Ada Lovelace", "key": "p256", "password": "secret1", "path": "ada.p12" })); + ok(&mut a, "sign_id_create", json!({ "name": "Ada Lovelace", "key": "p256", "password": "secret12", "path": "ada.p12" })); let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap(); let sign = |a: &mut Automation, rect: [f64; 4], graphic: Value, out: &str| { a.call( "sign_document", - &json!({ "doc": doc, "id": "ada.p12", "password": "secret1", "page": 1, "rect": rect, "graphic": graphic, "out": out }), + &json!({ "doc": doc, "id": "ada.p12", "password": "secret12", "page": 1, "rect": rect, "graphic": graphic, "out": out }), ) }; for (graphic, why) in [ @@ -4234,7 +4234,7 @@ fn signing_with_an_image_typed_or_drawn_graphic_through_tools() { // A missing file fails as it does for every tool that reads one. assert!(matches!(sign(&mut a, [20.0, 200.0, 180.0, 250.0], json!({ "image": "missing.png" }), "bad.pdf"), Err(ToolError::Failed(_)))); // An invisible signature shows nothing, so a graphic for one is a mistake. - let r = a.call("sign_document", &json!({ "doc": doc, "id": "ada.p12", "password": "secret1", "graphic": { "text": "Ada" }, "out": "bad.pdf" })); + let r = a.call("sign_document", &json!({ "doc": doc, "id": "ada.p12", "password": "secret12", "graphic": { "text": "Ada" }, "out": "bad.pdf" })); assert!(matches!(r, Err(ToolError::InvalidArgs(_))), "{r:?}"); assert!(!dir.join("bad.pdf").exists()); @@ -4270,11 +4270,13 @@ fn digital_ids_signing_and_validation_through_tools() { let id = ok( &mut a, "sign_id_create", - json!({ "name": "Ada Lovelace", "organization": "Analytical Engines", "email": "ada@example.com", "country": "GB", "key": "p256", "password": "secret1", "path": "ada.p12" }), + json!({ "name": "Ada Lovelace", "organization": "Analytical Engines", "email": "ada@example.com", "country": "GB", "key": "p256", "password": "secret12", "path": "ada.p12" }), ); assert_eq!(id["certificate"]["subject"], "C=GB, O=Analytical Engines, CN=Ada Lovelace, E=ada@example.com"); assert_eq!(id["certificate"]["self_signed"], true); assert!(matches!(a.call("sign_id_create", &json!({ "name": "X", "password": "short", "path": "x.p12" })), Err(ToolError::InvalidArgs(_)))); + // Eight characters at least (#1847): seven is refused. + assert!(matches!(a.call("sign_id_create", &json!({ "name": "X", "password": "seven77", "path": "x.p12" })), Err(ToolError::InvalidArgs(_)))); let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap(); assert_eq!(ok(&mut a, "sign_list", json!({ "doc": doc }))["count"], 0); @@ -4297,7 +4299,7 @@ fn digital_ids_signing_and_validation_through_tools() { let r = ok( &mut a, "sign_document", - json!({ "doc": doc, "id": "ada.p12", "password": "secret1", "page": 2, "rect": [20, 200, 180, 250], "reason": "Approved", "location": "London", "out": "signed.pdf" }), + json!({ "doc": doc, "id": "ada.p12", "password": "secret12", "page": 2, "rect": [20, 200, 180, 250], "reason": "Approved", "location": "London", "out": "signed.pdf" }), ); assert_eq!(r["signature"]["status"], "unknown"); assert_eq!(r["signature"]["signer"], "Ada Lovelace"); @@ -4307,7 +4309,7 @@ fn digital_ids_signing_and_validation_through_tools() { assert!(dir.join("signed.pdf").exists()); // Trust the ID: valid; comment afterwards: still valid, change allowed. - let t = ok(&mut a, "sign_trust", json!({ "paths": ["ada.p12"], "password": "secret1" })); + let t = ok(&mut a, "sign_trust", json!({ "paths": ["ada.p12"], "password": "secret12" })); assert_eq!(t["trusted"].as_array().unwrap().len(), 1); let list = ok(&mut a, "sign_list", json!({ "doc": doc })); assert_eq!((list["all_valid"].as_bool(), list["signatures"][0]["status"].as_str()), (Some(true), Some("valid"))); diff --git a/crates/sign/src/pkcs12.rs b/crates/sign/src/pkcs12.rs index ab83e747c..a71d5c858 100644 --- a/crates/sign/src/pkcs12.rs +++ b/crates/sign/src/pkcs12.rs @@ -6,7 +6,8 @@ //! Files that aren't clean DER still open: trailing whitespace, PEM armour or a bare //! base64 body (#159). //! Writing uses PBES2 AES-256-CBC with PBKDF2-HMAC-SHA-256 and an HMAC-SHA-256 MAC, OpenSSL 3's -//! defaults. +//! algorithms, at [`WRITE_ROUNDS`] rather than OpenSSL's 2048. Opening stops at a fixed amount +//! of key-derivation work, whatever the file asks for. //! //! The envelope is read as BER, not DER: Windows writes `.p12` files with indefinite lengths and //! the AuthenticatedSafe split into OCTET STRING segments, and nothing in the envelope is signed @@ -48,6 +49,42 @@ const PBE_SHA_2DES: &str = "1.2.840.113549.1.12.1.4"; const PBE_SHA_RC2_128: &str = "1.2.840.113549.1.12.1.5"; const PBE_SHA_RC2_40: &str = "1.2.840.113549.1.12.1.6"; +/// Key-derivation rounds for new files: OWASP's figure for PBKDF2-HMAC-SHA-256. The MAC uses the +/// same count, because checking it alone tells whether a guessed password is right (#1847). +pub const WRITE_ROUNDS: u32 = 600_000; + +/// The shortest password a new digital ID may have (NIST SP 800-63B). Opening an existing file +/// takes any password, so nobody is locked out of an ID made before this minimum. +pub const MIN_PASSWORD_CHARS: usize = 8; + +/// The most key-derivation work one [`open`] does, in hash blocks summed over every derivation +/// in the file. The file names its own iteration counts, so without a budget a crafted file +/// hangs the caller on the password check (#1847). A file written by [`write`] needs about +/// 2 million; real files from other tools need far less. +const OPEN_BUDGET: u64 = 10_000_000; + +/// What is left of [`OPEN_BUDGET`] while one file is opened. +struct Budget(u64); + +impl Budget { + /// Spend `rounds` iterations to derive `len` bytes with `alg`, and return them as the `u32` + /// the derivation takes, or refuse the file if the budget runs out. + fn spend(&mut self, alg: DigestAlg, rounds: u64, len: usize) -> Result { + // Every derivation hashes at least once, and repeats the rounds for each output block. + let blocks = len.div_ceil(alg.digest(&[]).len().max(1)).max(1) as u64; + let cost = rounds.max(1).saturating_mul(blocks); + match (self.0.checked_sub(cost), u32::try_from(rounds)) { + (Some(left), Ok(rounds)) => { + self.0 = left; + Ok(rounds) + } + _ => { + Err(SignError::Unsupported(format!("PKCS #12 files that need more than {OPEN_BUDGET} rounds of key derivation ({rounds} asked for)"))) + } + } + } +} + /// What a digital ID file holds: the signing key, its certificate and any others (issuers). #[derive(Debug)] pub struct DigitalId { @@ -182,7 +219,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]) -> 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"))?; @@ -196,7 +233,7 @@ fn pbe_decrypt(alg: &Tlv<'_>, password: &str, ct: &[u8]) -> Result, Sign } let kp = kdf.get(1).ok_or_else(|| bad("PBKDF2 parameters"))?.children()?; let salt = kp.first().ok_or_else(|| bad("salt"))?.octets("salt")?; - let rounds = kp.get(1).ok_or_else(|| bad("iterations"))?.u64()? as u32; + let rounds = kp.get(1).ok_or_else(|| bad("iterations"))?.u64()?; let prf = match kp .iter() .skip(2) @@ -222,13 +259,14 @@ fn pbe_decrypt(alg: &Tlv<'_>, password: &str, ct: &[u8]) -> Result, Sign other => return Err(SignError::Unsupported(format!("PBES2 cipher {}", other.unwrap_or("?")))), }; let iv = sc.get(1).ok_or_else(|| bad("IV"))?.octets("IV")?; + let rounds = budget.spend(prf, rounds, cipher.key_len())?; let key = pbkdf2(prf, password.as_bytes(), &salt, rounds, cipher.key_len())?; cipher.decrypt(&key, &iv, ct) } PBE_SHA_3DES | PBE_SHA_2DES | PBE_SHA_RC2_128 | PBE_SHA_RC2_40 => { let p = params.children()?; let salt = p.first().ok_or_else(|| bad("salt"))?.octets("salt")?; - let rounds = p.get(1).ok_or_else(|| bad("iterations"))?.u64()? as u32; + let rounds = p.get(1).ok_or_else(|| bad("iterations"))?.u64()?; let (cipher, key_len, iv_len) = match o.as_str() { PBE_SHA_3DES => (Cipher::TripleDes, 24, 8), PBE_SHA_2DES => (Cipher::TripleDes, 16, 8), @@ -236,6 +274,9 @@ fn pbe_decrypt(alg: &Tlv<'_>, password: &str, ct: &[u8]) -> Result, Sign _ => (Cipher::Rc2(40), 5, 8), }; let pw = bmp(password); + // Two derivations from the same count: the key, then the IV. + let rounds = budget.spend(DigestAlg::Sha1, rounds, key_len)?; + budget.spend(DigestAlg::Sha1, u64::from(rounds), iv_len)?; 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. @@ -260,7 +301,7 @@ enum BagKind { Cert(Vec), } -fn bags(safe_contents: &[u8], password: &str, out: &mut Vec) -> Result<(), SignError> { +fn bags(safe_contents: &[u8], password: &str, out: &mut Vec, budget: &mut Budget) -> Result<(), SignError> { for bag in Tlv::parse_all_ber(safe_contents)?.children()? { let f = bag.children()?; let Some(id) = f.first().and_then(|o| o.oid().ok()) else { continue }; @@ -282,7 +323,7 @@ fn bags(safe_contents: &[u8], password: &str, out: &mut Vec) -> Result<(), SHROUDED_KEY_BAG => { let p = value.children()?; let [alg, data] = p.as_slice() else { return Err(bad("EncryptedPrivateKeyInfo")) }; - BagKind::Key(pbe_decrypt(alg, password, &data.octets("encrypted private key")?)?) + BagKind::Key(pbe_decrypt(alg, password, &data.octets("encrypted private key")?, budget)?) } CERT_BAG => { let p = value.children()?; @@ -355,6 +396,7 @@ pub fn open(bytes: &[u8], password: &str) -> Result { return Err(SignError::Unsupported("public-key protected PKCS #12 files".into())); } let content = auth_safe.get(1).ok_or_else(|| bad("authSafe content"))?.inner()?.octets("authSafe")?; + let mut budget = Budget(OPEN_BUDGET); // The MAC proves the password before anything is decrypted. It covers the contents of that // OCTET STRING — the segments joined, if the file is segmented, which is what wrote them. if let Some(mac) = pfx.get(2) { @@ -367,8 +409,9 @@ pub fn open(bytes: &[u8], password: &str) -> Result { .ok_or_else(|| SignError::Unsupported(format!("MAC digest {alg_oid}")))?; let expected = digest_info.get(1).ok_or_else(|| bad("MAC"))?.octets("MAC")?; let salt = m.get(1).ok_or_else(|| bad("MAC salt"))?.octets("MAC salt")?; - let iterations = m.get(2).map(|i| i.u64()).transpose()?.unwrap_or(1) as u32; + let iterations = m.get(2).map(|i| i.u64()).transpose()?.unwrap_or(1); let key_len = alg.digest(&[]).len(); + let iterations = budget.spend(alg, iterations, key_len)?; let key = pkcs12_kdf(alg, &bmp(password), &salt, 3, iterations, key_len); if hmac(alg, &key, &content)? != *expected { return Err(SignError::WrongPassword); @@ -380,7 +423,7 @@ pub fn open(bytes: &[u8], password: &str) -> Result { let kind = p.first().ok_or_else(|| bad("ContentInfo"))?.oid()?; let body = p.get(1).ok_or_else(|| bad("ContentInfo content"))?.inner()?; match kind.as_str() { - DATA => bags(&body.octets("SafeContents")?, password, &mut all)?, + DATA => bags(&body.octets("SafeContents")?, password, &mut all, &mut budget)?, ENCRYPTED_DATA => { let ed = body.children()?; let eci = ed.get(1).ok_or_else(|| bad("EncryptedContentInfo"))?.children()?; @@ -397,8 +440,8 @@ pub fn open(bytes: &[u8], password: &str) -> Result { } _ => return Err(bad("encryptedContent")), }; - let plain = pbe_decrypt(alg, password, &ct)?; - bags(&plain, password, &mut all)?; + let plain = pbe_decrypt(alg, password, &ct, &mut budget)?; + bags(&plain, password, &mut all, &mut budget)?; } _ => {} } @@ -441,16 +484,15 @@ fn random(n: usize) -> Result, SignError> { /// Write a digital ID as a `.p12` protected by `password`. pub fn write(id: &DigitalId, password: &str) -> Result, SignError> { - const ITER: u32 = 2048; let encrypt = |plain: &[u8]| -> Result, SignError> { let (salt, iv) = (random(16)?, random(16)?); - let key = pbkdf2(DigestAlg::Sha256, password.as_bytes(), &salt, ITER, 32)?; + let key = pbkdf2(DigestAlg::Sha256, password.as_bytes(), &salt, WRITE_ROUNDS, 32)?; let ct = cbc::Encryptor::::new_from_slices(&key, &iv) .map_err(|_| SignError::Crypto("AES".into()))? .encrypt_padded_vec::(plain); let kdf = der::seq(&[ &der::oid(PBKDF2), - &der::seq(&[&der::octets(&salt), &der::int(ITER as u64), &der::seq(&[&der::oid(HMAC_SHA256), &der::null()])]), + &der::seq(&[&der::octets(&salt), &der::int(WRITE_ROUNDS as u64), &der::seq(&[&der::oid(HMAC_SHA256), &der::null()])]), ]); let alg = der::seq(&[&der::oid(PBES2), &der::seq(&[&kdf, &der::seq(&[&der::oid(AES256_CBC), &der::octets(&iv)])])]); Ok(der::seq(&[&alg, &der::octets(&ct)])) @@ -480,8 +522,31 @@ pub fn write(id: &DigitalId, password: &str) -> Result, SignError> { let ci_key = der::seq(&[&der::oid(DATA), &der::explicit(0, &der::octets(&der::seq(&[&key_bag])))]); let auth_safe = der::seq(&[&ci_certs, &ci_key]); let salt = random(16)?; - let mac_key = pkcs12_kdf(DigestAlg::Sha256, &bmp(password), &salt, 3, ITER, 32); + let mac_key = pkcs12_kdf(DigestAlg::Sha256, &bmp(password), &salt, 3, WRITE_ROUNDS, 32); let mac = hmac(DigestAlg::Sha256, &mac_key, &auth_safe)?; - let mac_data = der::seq(&[&der::seq(&[&DigestAlg::Sha256.algorithm(), &der::octets(&mac)]), &der::octets(&salt), &der::int(ITER as u64)]); + let mac_data = der::seq(&[&der::seq(&[&DigestAlg::Sha256.algorithm(), &der::octets(&mac)]), &der::octets(&salt), &der::int(WRITE_ROUNDS as u64)]); Ok(der::seq(&[&der::int(3), &der::seq(&[&der::oid(DATA), &der::explicit(0, &der::octets(&auth_safe))]), &mac_data])) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn one_budget_covers_every_derivation_in_a_file() { + let mut budget = Budget(OPEN_BUDGET); + // What a file from `write` needs: the MAC and two PBKDF2 derivations. + for _ in 0..3 { + assert_eq!(budget.spend(DigestAlg::Sha256, WRITE_ROUNDS.into(), 32).ok(), Some(WRITE_ROUNDS)); + } + // Six million rounds fit once, but not twice. + assert!(budget.spend(DigestAlg::Sha256, 6_000_000, 32).is_ok()); + assert!(budget.spend(DigestAlg::Sha256, 6_000_000, 32).is_err()); + // Each output block repeats the rounds: 24 bytes of SHA-1 are two blocks. + let mut budget = Budget(OPEN_BUDGET); + assert!(budget.spend(DigestAlg::Sha1, 6_000_000, 24).is_err()); + // Zero rounds still hashes once. + assert_eq!(budget.spend(DigestAlg::Sha1, 0, 20).ok(), Some(0)); + assert_eq!(budget.0, OPEN_BUDGET - 1); + } +} diff --git a/crates/sign/tests/crypto.rs b/crates/sign/tests/crypto.rs index a7e6931c5..ef3d5976d 100644 --- a/crates/sign/tests/crypto.rs +++ b/crates/sign/tests/crypto.rs @@ -185,6 +185,48 @@ fn new_digital_ids_are_self_signed_and_survive_a_p12_round_trip() { } } +#[test] +fn new_digital_ids_use_the_stronger_round_count_for_the_key_and_the_mac() { + let name = Name::build("Grace Hopper", "", "", "", ""); + let now = Time { year: 2026, month: 10, day: 14, hour: 12, minute: 0, second: 0 }; + let key = pdfcraft_sign::PrivateKey::generate_p256().unwrap(); + let certificate = Certificate::self_signed(&name, &key, now, 5, &[0x42]).unwrap(); + let id = pkcs12::DigitalId { key, certificate, chain: Vec::new(), friendly_name: None }; + let p12 = pkcs12::write(&id, "s3cret12").unwrap(); + // OWASP's figure for PBKDF2-HMAC-SHA-256, spelled out so lowering the constant fails here. + const ROUNDS: u64 = 600_000; + // MacData's iteration count, the last field of the file. + let pfx = Tlv::parse_all(&p12).unwrap().children().unwrap(); + let mac = pfx[2].children().unwrap(); + assert_eq!(mac[2].u64().unwrap(), ROUNDS); + // The same INTEGER three times: the MAC and the PBKDF2 parameters of both encrypted parts. + let rounds = pdfcraft_sign::der::int(ROUNDS); + assert_eq!(p12.windows(rounds.len()).filter(|w| *w == rounds.as_slice()).count(), 3); + assert_eq!(pkcs12::open(&p12, "s3cret12").unwrap().certificate, id.certificate); +} + +/// A `.p12` whose MAC asks for `rounds` iterations. Its content is empty: opening it must stop +/// at the MAC, before the content is read. +fn p12_with_mac_rounds(rounds: u64) -> Vec { + use pdfcraft_sign::der; + let auth_safe = der::seq(&[]); + let digest = der::seq(&[&DigestAlg::Sha256.algorithm(), &der::octets(&[0; 32])]); + let mac_data = der::seq(&[&digest, &der::octets(&[1; 16]), &der::int(rounds)]); + der::seq(&[&der::int(3), &der::seq(&[&der::oid("1.2.840.113549.1.7.1"), &der::explicit(0, &der::octets(&auth_safe))]), &mac_data]) +} + +#[test] +fn a_p12_asking_for_too_many_rounds_is_refused_before_hashing() { + // 2^32 + 1 used to wrap to one round; 20 million took seconds and 4 billion about an hour, + // all spent on the password check (#1847). + for rounds in [(1u64 << 32) + 1, 20_000_000, 4_000_000_000] { + let err = pkcs12::open(&p12_with_mac_rounds(rounds), "password").expect_err("refused"); + assert!(matches!(err, SignError::Unsupported(_)), "{rounds}: {err}"); + } + // An ordinary count still gets as far as the password. + assert!(matches!(pkcs12::open(&p12_with_mac_rounds(2048), "password"), Err(SignError::WrongPassword))); +} + #[test] fn certificates_load_from_pem_and_der_and_export_as_pem() { let pem_text = data("rsa.crt.pem"); diff --git a/crates/ui-egui/src/i18n/cs.tsv b/crates/ui-egui/src/i18n/cs.tsv index 1de834d18..f7374a1bc 100644 --- a/crates/ui-egui/src/i18n/cs.tsv +++ b/crates/ui-egui/src/i18n/cs.tsv @@ -2285,7 +2285,7 @@ certificate Subject Subjekt # Digital ID errors, the Signatures panel and the certificate viewer. Enter a name. Zadejte jméno. - The password must have at least 6 characters. Heslo musí mít alespoň 6 znaků. + The password must have at least 8 characters. Heslo musí mít alespoň 8 znaků. The passwords do not match. Hesla se neshodují. There is no folder to save the digital ID in. Není k dispozici složka, do které by se digitální ID uložilo. The password is incorrect. Heslo není správné. diff --git a/crates/ui-egui/src/i18n/lt.tsv b/crates/ui-egui/src/i18n/lt.tsv index c266d89b0..dd8150414 100644 --- a/crates/ui-egui/src/i18n/lt.tsv +++ b/crates/ui-egui/src/i18n/lt.tsv @@ -1703,7 +1703,7 @@ The font {} isn't embedded Šriftas {} neįdėtas The metadata doesn't identify the file as {} Metaduomenys nenurodo, kad failas atitinka {} The password is incorrect. Slaptažodis netinkamas. - The password must have at least 6 characters. Slaptažodį turi sudaryti bent 6 simboliai. + The password must have at least 8 characters. Slaptažodį turi sudaryti bent 8 simboliai. The passwords do not match. Slaptažodžiai nesutampa. The security settings don't allow screen readers to read the text Saugos nuostatos neleidžia ekrano skaitymo programoms skaityti teksto The signature algorithm and the SignerInfo's digest algorithm name different hashes; the signature was made with the SignerInfo's. Parašo algoritmas ir „SignerInfo“ santraukos algoritmas nurodo skirtingas maišos funkcijas; pasirašyta naudojant „SignerInfo“ nurodytą funkciją. diff --git a/crates/ui-egui/src/sign_ui.rs b/crates/ui-egui/src/sign_ui.rs index 98a511de1..29147a992 100644 --- a/crates/ui-egui/src/sign_ui.rs +++ b/crates/ui-egui/src/sign_ui.rs @@ -357,8 +357,10 @@ impl PdfCraftApp { if d.name.trim().is_empty() { return Err("Enter a name.".into()); } - if d.password.chars().count() < 6 { - return Err("The password must have at least 6 characters.".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()); @@ -1416,7 +1418,7 @@ mod tests { // 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: "secret1".into(), confirm: "secret1".into(), ..NewIdDraft::default() }; + NewIdDraft { name: "Grace Hopper".into(), key, password: "secret12".into(), confirm: "secret12".into(), ..NewIdDraft::default() }; app.sign_draft = Some(draft); let i = app.create_digital_id()?; Ok(PathBuf::from(&app.digital_ids[i].path)) @@ -1433,7 +1435,7 @@ mod tests { let saved = create_in(&dir).unwrap(); assert_eq!(saved, dir.join("Grace Hopper 3.p12")); assert_eq!(std::fs::read_to_string(dir.join("victim.txt")).unwrap(), "keep me"); - assert!(sign::pkcs12::open(&std::fs::read(&saved).unwrap(), "secret1").is_ok()); + assert!(sign::pkcs12::open(&std::fs::read(&saved).unwrap(), "secret12").is_ok()); let _ = std::fs::remove_dir_all(&dir); } diff --git a/crates/ui-egui/tests/signing.rs b/crates/ui-egui/tests/signing.rs index 3e1b64c51..7b321d4da 100644 --- a/crates/ui-egui/tests/signing.rs +++ b/crates/ui-egui/tests/signing.rs @@ -109,7 +109,7 @@ fn drawing_a_signature_creating_an_id_signing_and_trusting() { h.run_steps(1); field(&h, "Name").type_text("Grace Hopper"); h.state_mut().sign_draft.as_mut().unwrap().new_id.key = 3; - for (label, text) in [("Password", "secret1"), ("Confirm Password", "secret1")] { + for (label, text) in [("Password", "secret12"), ("Confirm Password", "secret12")] { field(&h, label).click(); h.run_steps(1); field(&h, label).type_text(text); @@ -131,7 +131,7 @@ fn drawing_a_signature_creating_an_id_signing_and_trusting() { h.get_by_label("Sign").click(); h.run_steps(3); h.get_by_label("The password is incorrect."); - h.state_mut().sign_draft.as_mut().unwrap().password = "secret1".into(); + h.state_mut().sign_draft.as_mut().unwrap().password = "secret12".into(); field(&h, "Reason").click(); h.run_steps(1); field(&h, "Reason").type_text("I approve");