From fa52469dbed7954638f602aadf9afb955221f373 Mon Sep 17 00:00:00 2001 From: feraremag2680 Date: Fri, 9 Oct 2026 15:15:32 +0200 Subject: [PATCH] sign: read the .p12 files Windows writes (BER, not DER) (#453) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Importing a digital ID exported by Windows failed with "PKCS #12: not a PKCS #12 file" — at the second byte of the file, before the password was looked at. Windows writes a .p12 in BER: the PFX, its ContentInfo and the [0] around the AuthenticatedSafe all use the indefinite length form (0x80 … 00 00), and the AuthenticatedSafe itself is an OCTET STRING split into segments. der.rs took definite lengths only, so every one of those files was turned away. OpenSSL writes definite DER, which is why the fixtures never showed it. A Tlv now carries the rules it was read with, and its children are read the same way: Tlv::parse is the strict DER reader it always was, Tlv::parse_ber also takes the indefinite form, and Tlv::octets joins the segments of a constructed OCTET STRING, borrowing when there is nothing to join. Both are depth-bounded. pkcs12 reads the envelope with the BER reader; x509 and cms keep the strict one, so nothing that is verified over its own bytes changes. The MAC decides what is allowed here: it covers the contents of the authSafe OCTET STRING, the segments joined, which is what the writer MACed. So the file is never normalised to DER — that would change the octets the MAC was computed over and turn a right password into a wrong one. The test builds the Windows shape out of the OpenSSL fixtures, twice: the outer wrappers in BER with the MacData intact, which is what proves the MAC is taken over the right octets, and then the AuthenticatedSafe re-encoded as well, with the MacData dropped (RFC 7292 §4 makes it optional), for a file that is BER the whole way down. Claude-Session: https://claude.ai/code/session_013Q5dhMC58jk1ePNW37xe1b Co-authored-by: Fernando Areán Co-authored-by: Claude Opus 5 --- crates/sign/src/der.rs | 169 +++++++++++++++++++++++++++++++---- crates/sign/src/pkcs12.rs | 65 ++++++++------ crates/sign/tests/crypto.rs | 70 ++++++++++++++- parity/acrobat-features.toml | 4 +- 4 files changed, 263 insertions(+), 45 deletions(-) diff --git a/crates/sign/src/der.rs b/crates/sign/src/der.rs index bedbf07257..949e7a92b1 100644 --- a/crates/sign/src/der.rs +++ b/crates/sign/src/der.rs @@ -1,7 +1,14 @@ -//! A small DER reader and writer (ITU-T X.690): just what X.509, CMS and PKCS#12 need. +//! A small BER/DER reader and writer (ITU-T X.690): just what X.509, CMS and PKCS#12 need. //! //! [`Tlv`] borrows from the input and keeps its full encoding (`raw`), so signed structures can //! be verified over the exact bytes that were signed. +//! +//! Reading is strict DER by default, which is what everything signed must use. [`Tlv::parse_ber`] +//! also accepts the two BER constructions a reader meets in practice — the indefinite length form +//! and strings split into segments — which PKCS #12 needs: Windows writes `.p12` files that way. +//! Writing is always DER. + +use std::borrow::Cow; use crate::SignError; @@ -11,6 +18,8 @@ pub mod tag { pub const INTEGER: u8 = 0x02; pub const BIT_STRING: u8 = 0x03; pub const OCTET_STRING: u8 = 0x04; + /// An OCTET STRING split into segments (BER only, X.690 §8.21); see [`super::Tlv::octets`]. + pub const OCTET_STRING_SEGMENTS: u8 = 0x24; pub const NULL: u8 = 0x05; pub const OID: u8 = 0x06; pub const UTF8_STRING: u8 = 0x0C; @@ -36,56 +45,107 @@ fn bad(what: &str) -> SignError { SignError::Malformed(what.to_string()) } -/// One DER element. +/// Which encoding rules a reader accepts. #[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Form { + /// DER: every length definite, every string primitive. Anything whose bytes are signed is read + /// this way, so that what is verified is exactly what was written. + Der, + /// BER as well: the indefinite length form (X.690 §8.1.3.6) and strings split into segments + /// (§8.21). + Ber, +} + +/// One BER/DER element. +#[derive(Clone, Copy, Debug, Eq)] pub struct Tlv<'a> { pub tag: u8, - /// The contents octets. + /// The contents octets (without the end-of-contents octets of an indefinite length). pub value: &'a [u8], - /// The whole encoding (identifier, length and contents). + /// The whole encoding (identifier, length, contents and any end-of-contents octets). pub raw: &'a [u8], + /// The rules this element was read with. Its children are read the same way. + form: Form, +} + +/// Two elements are equal when they encode the same thing; the rules they were read with are not +/// part of that. +impl PartialEq for Tlv<'_> { + fn eq(&self, other: &Self) -> bool { + self.tag == other.tag && self.value == other.value && self.raw == other.raw + } } impl<'a> Tlv<'a> { - /// Parse one element at the start of `input`; returns it and the rest. + /// Parse one DER element at the start of `input`; returns it and the rest. pub fn parse(input: &'a [u8]) -> Result<(Tlv<'a>, &'a [u8]), SignError> { + Tlv::read(input, Form::Der, 0) + } + + /// Parse one BER element at the start of `input`; returns it and the rest. + pub fn parse_ber(input: &'a [u8]) -> Result<(Tlv<'a>, &'a [u8]), SignError> { + Tlv::read(input, Form::Ber, 0) + } + + fn read(input: &'a [u8], form: Form, depth: usize) -> Result<(Tlv<'a>, &'a [u8]), SignError> { let (&tag, rest) = input.split_first().ok_or_else(|| bad("truncated DER"))?; if tag & 0x1F == 0x1F { return Err(bad("multi-byte DER tags are not supported")); } let (&first, rest) = rest.split_first().ok_or_else(|| bad("truncated DER length"))?; - let (len, rest) = if first < 0x80 { - (first as usize, rest) + // `trailer` is the two end-of-contents octets, which belong to the element but not to its + // contents. + let (len, rest, trailer) = if first < 0x80 { + (first as usize, rest, 0) + } else if first == 0x80 { + if form == Form::Der { + return Err(bad("indefinite lengths are not allowed in DER")); + } + if tag & 0x20 == 0 { + return Err(bad("an indefinite length needs a constructed element")); + } + (contents_len(rest, form, depth)?, rest, 2) } else { let n = (first & 0x7F) as usize; - if n == 0 || n > 4 || rest.len() < n { + if n > 4 || rest.len() < n { return Err(bad("unsupported DER length")); } let len = rest[..n].iter().fold(0usize, |acc, b| (acc << 8) | *b as usize); - (len, &rest[n..]) + (len, &rest[n..], 0) }; - if rest.len() < len { + let end = len.checked_add(trailer).ok_or_else(|| bad("DER length overflows"))?; + if rest.len() < end { return Err(bad("DER element runs past the end")); } + // `end <= rest.len()`, so neither sum below can overflow. let header = input.len() - rest.len(); - Ok((Tlv { tag, value: &rest[..len], raw: &input[..header + len] }, &rest[len..])) + Ok((Tlv { tag, value: &rest[..len], raw: &input[..header + end], form }, &rest[end..])) } - /// Parse `input` as exactly one element. + /// Parse `input` as exactly one DER element. pub fn parse_all(input: &'a [u8]) -> Result, SignError> { - let (t, rest) = Tlv::parse(input)?; + Tlv::only(input, Form::Der) + } + + /// Parse `input` as exactly one BER element. + pub fn parse_all_ber(input: &'a [u8]) -> Result, SignError> { + Tlv::only(input, Form::Ber) + } + + fn only(input: &'a [u8], form: Form) -> Result, SignError> { + let (t, rest) = Tlv::read(input, form, 0)?; if !rest.is_empty() { return Err(bad("trailing bytes after DER element")); } Ok(t) } - /// The children of a constructed element. + /// The children of a constructed element, read with the same rules as their parent. pub fn children(&self) -> Result>, SignError> { let mut out = Vec::new(); let mut rest = self.value; while !rest.is_empty() { - let (t, r) = Tlv::parse(rest)?; + let (t, r) = Tlv::read(rest, self.form, 0)?; out.push(t); rest = r; } @@ -99,7 +159,34 @@ impl<'a> Tlv<'a> { /// The element inside an EXPLICIT tag. pub fn inner(&self) -> Result, SignError> { - Tlv::parse_all(self.value) + Tlv::only(self.value, self.form) + } + + /// An OCTET STRING's contents, joining the segments of BER's constructed form (X.690 §8.21). + /// Borrowed in the ordinary case, so the common path copies nothing. + pub fn octets(&self, what: &str) -> Result, SignError> { + self.join(what, 0) + } + + fn join(&self, what: &str, depth: usize) -> Result, SignError> { + /// Deep enough for anything real: a writer that segments at all segments once. + const MAX_DEPTH: usize = 16; + match self.tag { + tag::OCTET_STRING => Ok(Cow::Borrowed(self.value)), + tag::OCTET_STRING_SEGMENTS if self.form == Form::Ber => { + if depth >= MAX_DEPTH { + return Err(bad(&format!("{what}: OCTET STRING segments nested too deeply"))); + } + // The join is never larger than the segments it is made of, so this is bounded by + // the input. + let mut out: Vec = Vec::with_capacity(self.value.len()); + for s in self.children()? { + out.extend_from_slice(&s.join(what, depth + 1)?); + } + Ok(Cow::Owned(out)) + } + found => Err(bad(&format!("{what}: expected an OCTET STRING, found tag {found:#04x}"))), + } } /// An OID in dotted form. @@ -172,6 +259,26 @@ impl<'a> Tlv<'a> { } } +/// How many contents octets an indefinite-length element has: everything up to the matching +/// end-of-contents octets, which have to be there (X.690 §8.1.5). The contents are elements, so +/// finding the end means reading them, and one of those may be indefinite too — hence the depth +/// limit. Every step consumes at least two octets, so the walk always ends. +fn contents_len(input: &[u8], form: Form, depth: usize) -> Result { + /// Far deeper than any real file nests, and shallow enough never to exhaust the stack. + const MAX_DEPTH: usize = 48; + if depth >= MAX_DEPTH { + return Err(bad("BER elements nested too deeply")); + } + let mut rest = input; + loop { + match rest { + [] => return Err(bad("an indefinite length with no end-of-contents")), + [0, 0, ..] => return Ok(input.len() - rest.len()), + _ => rest = Tlv::read(rest, form, depth + 1)?.1, + } + } +} + /// A UTC time. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct Time { @@ -439,6 +546,36 @@ mod tests { assert!(Tlv::parse(&[0x30, 5, 1]).is_err()); } + #[test] + fn reads_ber_indefinite_lengths_and_segmented_strings() { + // SEQUENCE { INTEGER 3, OCTET STRING { "hi", "!" } }, every wrapper in the indefinite form: + // the shape Windows gives a .p12 file. + let ber: &[u8] = &[0x30, 0x80, 0x02, 0x01, 0x03, 0x24, 0x80, 0x04, 0x02, b'h', b'i', 0x04, 0x01, b'!', 0, 0, 0, 0]; + let e = Tlv::parse_all(ber).unwrap_err().to_string(); + assert!(e.contains("indefinite lengths are not allowed in DER"), "{e}"); + + let t = Tlv::parse_all_ber(ber).unwrap(); + assert_eq!(t.tag, tag::SEQUENCE); + assert_eq!(t.raw, ber, "the end-of-contents octets belong to the element"); + assert_eq!(t.value, &ber[2..ber.len() - 2], "but not to its contents"); + let c = t.children().unwrap(); + assert_eq!(c[0].u64().unwrap(), 3); + assert_eq!(c[1].octets("segments").unwrap().as_ref(), b"hi!", "the segments are joined"); + assert!(c[0].octets("not a string").is_err()); + + // Definite lengths read the same either way, and a segmented string stays a DER error. + assert_eq!(Tlv::parse_all_ber(&int(65537)).unwrap().u64().unwrap(), 65537); + assert_eq!(Tlv::parse_all_ber(&octets(b"hi!")).unwrap().octets("plain").unwrap().as_ref(), b"hi!"); + assert!(Tlv::parse_all(&[0x24, 2, 0x04, 0]).unwrap().octets("segments").is_err()); + + // Malformed: no end-of-contents, and an indefinite length on a primitive element. + assert!(Tlv::parse_all_ber(&ber[..ber.len() - 2]).is_err()); + assert!(Tlv::parse_all_ber(&[0x04, 0x80, 0, 0]).is_err()); + // Nesting is bounded, so a file made of nothing but wrappers fails instead of crashing. + let deep = [[0x30u8, 0x80].repeat(400), [0u8, 0].repeat(400)].concat(); + assert!(Tlv::parse_all_ber(&deep).is_err()); + } + #[test] fn times() { let t = Time { year: 2026, month: 10, day: 2, hour: 9, minute: 30, second: 5 }; diff --git a/crates/sign/src/pkcs12.rs b/crates/sign/src/pkcs12.rs index 11d1e717dd..69d4e14e09 100644 --- a/crates/sign/src/pkcs12.rs +++ b/crates/sign/src/pkcs12.rs @@ -7,12 +7,18 @@ //! base64 body (#159). //! Writing uses PBES2 AES-256-CBC with PBKDF2-HMAC-SHA-256 and an HMAC-SHA-256 MAC, OpenSSL 3's //! defaults. +//! +//! 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 +//! over its own bytes. The certificates inside it are still read as strict DER, which is how they +//! were signed. + +use std::borrow::Cow; use base64::Engine as _; use cbc::cipher::block_padding::Pkcs7; use cbc::cipher::{BlockModeDecrypt, BlockModeEncrypt, InnerIvInit, KeyIvInit}; use hmac::Mac; -use std::borrow::Cow; use crate::SignError; use crate::der::{self, Tlv, tag}; @@ -182,7 +188,7 @@ fn pbe_decrypt(alg: &Tlv<'_>, password: &str, ct: &[u8]) -> Result, Sign return Err(SignError::Unsupported("PBES2 key derivation other than PBKDF2".into())); } let kp = kdf.get(1).ok_or_else(|| bad("PBKDF2 parameters"))?.children()?; - let salt = kp.first().ok_or_else(|| bad("salt"))?.expect(tag::OCTET_STRING, "salt")?.value; + 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 prf = match kp .iter() @@ -208,13 +214,13 @@ fn pbe_decrypt(alg: &Tlv<'_>, password: &str, ct: &[u8]) -> Result, Sign Some(DES_EDE3_CBC) => Cipher::TripleDes, other => return Err(SignError::Unsupported(format!("PBES2 cipher {}", other.unwrap_or("?")))), }; - let iv = sc.get(1).ok_or_else(|| bad("IV"))?.expect(tag::OCTET_STRING, "IV")?.value; - let key = pbkdf2(prf, password.as_bytes(), salt, rounds, cipher.key_len()); - cipher.decrypt(&key, iv, ct) + let iv = sc.get(1).ok_or_else(|| bad("IV"))?.octets("IV")?; + 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"))?.expect(tag::OCTET_STRING, "salt")?.value; + 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 (cipher, key_len, iv_len) = match o.as_str() { PBE_SHA_3DES => (Cipher::TripleDes, 24, 8), @@ -223,13 +229,13 @@ fn pbe_decrypt(alg: &Tlv<'_>, password: &str, ct: &[u8]) -> Result, Sign _ => (Cipher::Rc2(40), 5, 8), }; let pw = bmp(password); - let mut key = pkcs12_kdf(DigestAlg::Sha1, &pw, salt, 1, rounds, key_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. let k1 = key[..8].to_vec(); key.extend(k1); } - let iv = pkcs12_kdf(DigestAlg::Sha1, &pw, salt, 2, rounds, iv_len); + let iv = pkcs12_kdf(DigestAlg::Sha1, &pw, &salt, 2, rounds, iv_len); cipher.decrypt(&key, &iv, ct) } other => Err(SignError::Unsupported(format!("PKCS #12 encryption {other}"))), @@ -248,7 +254,7 @@ enum BagKind { } fn bags(safe_contents: &[u8], password: &str, out: &mut Vec) -> Result<(), SignError> { - for bag in Tlv::parse_all(safe_contents)?.children()? { + 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 }; let Some(value) = f.get(1).filter(|v| v.tag == tag::ctx(0)).and_then(|v| v.inner().ok()) else { continue }; @@ -258,7 +264,7 @@ fn bags(safe_contents: &[u8], password: &str, out: &mut Vec) -> Result<(), let p = a.children()?; let v = p.get(1).and_then(|s| s.children().ok()).and_then(|s| s.into_iter().next()); match (p.first().and_then(|o| o.oid().ok()).as_deref(), v) { - (Some(LOCAL_KEY_ID), Some(v)) => local_key_id = Some(v.value.to_vec()), + (Some(LOCAL_KEY_ID), Some(v)) => local_key_id = v.octets("local key id").ok().map(Cow::into_owned), (Some(FRIENDLY_NAME), Some(v)) => friendly_name = v.text(), _ => {} } @@ -269,15 +275,15 @@ 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.value)?) + BagKind::Key(pbe_decrypt(alg, password, &data.octets("encrypted private key")?)?) } CERT_BAG => { let p = value.children()?; if p.first().and_then(|o| o.oid().ok()).as_deref() != Some(X509_CERT) { continue; } - let Some(c) = p.get(1).and_then(|c| c.inner().ok()) else { continue }; - BagKind::Cert(c.value.to_vec()) + let Some(c) = p.get(1).and_then(|c| c.inner().ok()).and_then(|c| c.octets("certificate").ok()) else { continue }; + BagKind::Cert(c.into_owned()) } _ => continue, }; @@ -330,7 +336,7 @@ fn pem_body(pem: &[u8]) -> Result<&[u8], SignError> { /// Open a `.p12` / `.pfx` file. A wrong password is [`SignError::WrongPassword`]. pub fn open(bytes: &[u8], password: &str) -> Result { let der = normalize(bytes)?; - let pfx = Tlv::parse_all(&der) + let pfx = Tlv::parse_all_ber(&der) .map_err(|e| match e { // Keep "not a PKCS #12 file" but carry the exact reason through. SignError::Malformed(why) => bad(&format!("not a PKCS #12 file: {why}")), @@ -341,38 +347,45 @@ pub fn open(bytes: &[u8], password: &str) -> Result { if auth_safe.first().map(|o| o.oid()).transpose()?.as_deref() != Some(DATA) { return Err(SignError::Unsupported("public-key protected PKCS #12 files".into())); } - let content = auth_safe.get(1).ok_or_else(|| bad("authSafe content"))?.inner()?.expect(tag::OCTET_STRING, "authSafe")?.value; - // The MAC proves the password before anything is decrypted. + let content = auth_safe.get(1).ok_or_else(|| bad("authSafe content"))?.inner()?.octets("authSafe")?; + // 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) { let m = mac.children()?; let digest_info = m.first().ok_or_else(|| bad("MacData"))?.children()?; let alg = digest_info.first().ok_or_else(|| bad("MAC algorithm"))?.children()?; let alg_oid = alg.first().ok_or_else(|| bad("MAC algorithm"))?.oid()?; let alg = DigestAlg::from_oid(&alg_oid).ok_or_else(|| SignError::Unsupported(format!("MAC digest {alg_oid}")))?; - let expected = digest_info.get(1).ok_or_else(|| bad("MAC"))?.value; - let salt = m.get(1).ok_or_else(|| bad("MAC salt"))?.value; + 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 key_len = alg.digest(&[]).len(); - let key = pkcs12_kdf(alg, &bmp(password), salt, 3, iterations, key_len); - if hmac(alg, &key, content)? != expected { + let key = pkcs12_kdf(alg, &bmp(password), &salt, 3, iterations, key_len); + if hmac(alg, &key, &content)? != *expected { return Err(SignError::WrongPassword); } } let mut all = Vec::new(); - for ci in Tlv::parse_all(content)?.children()? { + for ci in Tlv::parse_all_ber(&content)?.children()? { let p = ci.children()?; 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.expect(tag::OCTET_STRING, "SafeContents")?.value, password, &mut all)?, + DATA => bags(&body.octets("SafeContents")?, password, &mut all)?, ENCRYPTED_DATA => { let ed = body.children()?; let eci = ed.get(1).ok_or_else(|| bad("EncryptedContentInfo"))?.children()?; let alg = eci.get(1).ok_or_else(|| bad("content encryption algorithm"))?; - let ct = match eci.get(2) { - Some(t) if t.tag == tag::ctx_prim(0) => t.value.to_vec(), - // Constructed form: a sequence of OCTET STRING segments. - Some(t) if t.tag == tag::ctx(0) => t.children()?.iter().flat_map(|s| s.value.iter().copied()).collect(), + let ct: Cow<'_, [u8]> = match eci.get(2) { + Some(t) if t.tag == tag::ctx_prim(0) => Cow::Borrowed(t.value), + // Constructed form: the OCTET STRING segments, joined. + Some(t) if t.tag == tag::ctx(0) => { + let mut v = Vec::with_capacity(t.value.len()); + for s in t.children()? { + v.extend_from_slice(&s.octets("encryptedContent")?); + } + Cow::Owned(v) + } _ => return Err(bad("encryptedContent")), }; let plain = pbe_decrypt(alg, password, &ct)?; diff --git a/crates/sign/tests/crypto.rs b/crates/sign/tests/crypto.rs index f66179bfdf..ef9c570a13 100644 --- a/crates/sign/tests/crypto.rs +++ b/crates/sign/tests/crypto.rs @@ -1,6 +1,6 @@ //! The cryptographic core against files made by OpenSSL 3 (tests/data/README.md). -use pdfcraft_sign::der::Time; +use pdfcraft_sign::der::{Time, Tlv}; use pdfcraft_sign::keys::DigestAlg; use pdfcraft_sign::{Certificate, Name, PublicKey, SignError, cms, pkcs12}; @@ -58,6 +58,74 @@ fn opens_every_openssl_flavour_of_pkcs12() { assert_eq!(chain.certificate.key_usage.map(|u| u & 0b11), Some(0b11), "digitalSignature + nonRepudiation"); } +/// One element with BER's indefinite length: contents, then the end-of-contents octets. +fn indefinite(tag: u8, body: &[u8]) -> Vec { + let mut v = vec![tag, 0x80]; + v.extend_from_slice(body); + v.extend([0, 0]); + v +} + +/// An OCTET STRING split into segments, BER's constructed form. +fn segmented(bytes: &[u8]) -> Vec { + let parts: Vec = bytes.chunks(64).flat_map(der::octets).collect(); + indefinite(0x24, &parts) +} + +/// A `.p12` in the shape Windows writes one: the PFX, its ContentInfo and the `[0]` holding the +/// AuthenticatedSafe all in the indefinite form, and the AuthenticatedSafe split into segments. +/// Nothing changes meaning, and the MAC still covers the same octets, so whatever a reader makes +/// of it, it is making it of the form and not of the contents. +/// +/// With `deep`, every ContentInfo inside is rewritten the same way, as a real Windows file has it. +/// That re-encodes the AuthenticatedSafe, which is what the MAC covers, so the MacData is dropped +/// — RFC 7292 §4 makes it optional — and a wrong password is then caught by the decryption. +fn as_windows_writes_it(der: &[u8], deep: bool) -> Vec { + let pfx = Tlv::parse_all(der).unwrap().children().unwrap(); + let auth = pfx[1].children().unwrap(); + let mut content = auth[1].inner().unwrap().value.to_vec(); + if deep { + let mut inner = Vec::new(); + for ci in Tlv::parse_all(&content).unwrap().children().unwrap() { + let p = ci.children().unwrap(); + let body = p[1].inner().unwrap(); + // A Data content is an OCTET STRING, so it gets segmented; an EncryptedData is a + // SEQUENCE and stays as it is inside its indefinite wrappers. + let wrapped = if body.tag == 0x04 { segmented(body.value) } else { body.raw.to_vec() }; + let zero = indefinite(0xA0, &wrapped); + inner.extend(indefinite(0x30, &[p[0].raw, zero.as_slice()].concat())); + } + content = indefinite(0x30, &inner); + } + let zero = indefinite(0xA0, &segmented(&content)); + let mut body = pfx[0].raw.to_vec(); + body.extend(indefinite(0x30, &[auth[0].raw, zero.as_slice()].concat())); + if !deep { + for extra in &pfx[2..] { + body.extend_from_slice(extra.raw); + } + } + indefinite(0x30, &body) +} + +#[test] +fn opens_pkcs12_as_windows_writes_it() { + for file in ["rsa-aes.p12", "rsa-legacy.p12", "chain.p12"] { + let der = data(file); + let want = pkcs12::open(&der, "test").unwrap(); + for deep in [false, true] { + let ber = as_windows_writes_it(&der, deep); + assert_ne!(ber, der, "{file} ({deep})"); + let got = pkcs12::open(&ber, "test").unwrap_or_else(|e| panic!("{file} in BER (deep: {deep}): {e}")); + assert_eq!(got.certificate.raw, want.certificate.raw, "{file} ({deep})"); + assert_eq!(got.key.public_key(), want.key.public_key(), "{file} ({deep})"); + assert_eq!(got.chain.len(), want.chain.len(), "{file} ({deep})"); + assert_eq!(got.friendly_name, want.friendly_name, "{file} ({deep})"); + assert!(matches!(pkcs12::open(&ber, "nope"), Err(SignError::WrongPassword)), "{file} ({deep})"); + } + } +} + #[test] fn certificates_parse_as_openssl_made_them() { let c = Certificate::parse(&pem("rsa.crt.pem")).unwrap(); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 627fe94bf3..7da0de8f7d 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5786,8 +5786,8 @@ milestone = "M9" status = "shipped" commands = ["sign.digital"] tools = ["sign_document"] -evidence = ["crates/sign/tests/crypto.rs::opens_every_openssl_flavour_of_pkcs12", "crates/sign/tests/pdf.rs::signing_then_validating_with_and_without_trust", "crates/ui-egui/tests/signing.rs::certifying_without_a_visible_signature", "crates/automation/tests/automation.rs::digital_ids_signing_and_validation_through_tools"] -notes = "PBES2 (AES, 3DES) and legacy SHA-1 3DES/RC2 files; RSA, P-256, P-384" +evidence = ["crates/sign/tests/crypto.rs::opens_every_openssl_flavour_of_pkcs12", "crates/sign/tests/crypto.rs::opens_pkcs12_as_windows_writes_it", "crates/sign/tests/pdf.rs::signing_then_validating_with_and_without_trust", "crates/ui-egui/tests/signing.rs::certifying_without_a_visible_signature", "crates/automation/tests/automation.rs::digital_ids_signing_and_validation_through_tools"] +notes = "PBES2 (AES, 3DES) and legacy SHA-1 3DES/RC2 files; RSA, P-256, P-384; both the DER OpenSSL writes and the BER Windows writes (indefinite lengths, segmented AuthenticatedSafe)" [[feature]] id = "sign.macos-keychain"