diff --git a/Cargo.lock b/Cargo.lock index 9384aeee8..8e207b71a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3751,6 +3751,7 @@ version = "0.4.0" dependencies = [ "aes", "aws-lc-rs", + "base64 0.22.1", "cbc", "des", "getrandom 0.4.3", diff --git a/crates/sign/Cargo.toml b/crates/sign/Cargo.toml index ddb23cdd7..a09a78bde 100644 --- a/crates/sign/Cargo.toml +++ b/crates/sign/Cargo.toml @@ -15,6 +15,7 @@ pdfcraft-cos = { workspace = true } pdfcraft-annot = { workspace = true } pdfcraft-fonts = { workspace = true } thiserror.workspace = true +base64 = "0.22" sha1 = "0.11" sha2 = "0.11" p256 = { version = "0.14", default-features = false, features = ["ecdsa", "std"] } diff --git a/crates/sign/src/pkcs12.rs b/crates/sign/src/pkcs12.rs index 419d25c11..11d1e717d 100644 --- a/crates/sign/src/pkcs12.rs +++ b/crates/sign/src/pkcs12.rs @@ -3,12 +3,16 @@ //! //! Reading supports PBES2 (PBKDF2 with HMAC-SHA-1/256/384/512; AES-128/192/256-CBC or //! 3DES-CBC) and the legacy PKCS #12 PBE schemes (SHA-1 with 3DES or RC2), and checks the MAC. +//! 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. +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}; @@ -282,9 +286,57 @@ fn bags(safe_contents: &[u8], password: &str, out: &mut Vec) -> Result<(), Ok(()) } +fn normalize(bytes: &[u8]) -> Result, SignError> { + let start = bytes.iter().position(|b| !b.is_ascii_whitespace()).unwrap_or(0); + let end = bytes.iter().rposition(|b| !b.is_ascii_whitespace()).map_or(0, |i| i + 1); + let trimmed = bytes.get(start..end).unwrap_or(b""); // hostile input: no slicing by hand + + // 1. A DER file starts with a SEQUENCE tag: borrow, no work. + if trimmed.first() == Some(&0x30) { + return Ok(Cow::Borrowed(trimmed)); + } + // 2. PEM armor: base64 body between the BEGIN/END lines. + if trimmed.starts_with(b"-----BEGIN") { + let body = pem_body(trimmed)?; + return Ok(Cow::Owned(decode_base64(body)?)); + } + // 3. Raw base64 text, armour or not (a real DER PFX always contains bytes + // outside the base64 alphabet, so this can't swallow a valid file). + if trimmed.iter().all(|b| b.is_ascii_alphanumeric() || *b == b'+' || *b == b'/' || *b == b'=' || b.is_ascii_whitespace()) { + return Ok(Cow::Owned(decode_base64(trimmed)?)); + } + // 4. Not recognisable: hand the bytes to Tlv::parse_all for a clear error. + Ok(Cow::Borrowed(trimmed)) +} + +fn decode_base64(input: &[u8]) -> Result, SignError> { + let compact: Vec = input.iter().copied().filter(|b| !b.is_ascii_whitespace()).collect(); + base64::engine::general_purpose::STANDARD.decode(&compact).map_err(|e| SignError::Malformed(format!("PKCS #12: base64: {e}"))) +} + +/// The base64 lines between "-----BEGIN …-----" and "-----END …-----". +fn pem_body(pem: &[u8]) -> Result<&[u8], SignError> { + let text = std::str::from_utf8(pem).map_err(|_| bad("PEM file is not valid text"))?; + let begin = text.find("-----BEGIN ").ok_or_else(|| bad("PEM file has no BEGIN marker"))?; + let body_start = text[begin..].find('\n').map_or(text.len(), |i| begin + i + 1); + let body_end = text.find("-----END ").filter(|end| *end >= body_start).ok_or_else(|| bad("PEM file has no END marker"))?; + let body = text.as_bytes().get(body_start..body_end).ok_or_else(|| bad("PEM body"))?; + if !body.iter().any(|b| !b.is_ascii_whitespace()) { + return Err(bad("PEM file has an empty body")); + } + Ok(body) +} + /// Open a `.p12` / `.pfx` file. A wrong password is [`SignError::WrongPassword`]. pub fn open(bytes: &[u8], password: &str) -> Result { - let pfx = Tlv::parse_all(bytes).map_err(|_| bad("not a PKCS #12 file"))?.children()?; + let der = normalize(bytes)?; + let pfx = Tlv::parse_all(&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}")), + e => e, + })? + .children()?; let auth_safe = pfx.get(1).ok_or_else(|| bad("authSafe"))?.children()?; if auth_safe.first().map(|o| o.oid()).transpose()?.as_deref() != Some(DATA) { return Err(SignError::Unsupported("public-key protected PKCS #12 files".into())); diff --git a/crates/sign/tests/crypto.rs b/crates/sign/tests/crypto.rs index f771911f2..f66179bfd 100644 --- a/crates/sign/tests/crypto.rs +++ b/crates/sign/tests/crypto.rs @@ -209,6 +209,40 @@ fn crls_verify_and_report_revocation() { ); } +/// Issue #159: `.p12` files that aren't clean DER still open — trailing +/// whitespace, PEM armour or a bare base64 body — and real damage still fails, +/// with the reason in the message. +#[test] +fn opens_wrapped_pkcs12_files_and_still_rejects_broken_ones() { + use base64::Engine as _; + let der = data("rsa-aes.p12"); + let signer = pkcs12::open(&der, "test").unwrap(); + + // An editor or a download appends whitespace. + let mut trailing = der.clone(); + trailing.extend_from_slice(b"\r\n \n"); + assert_eq!(pkcs12::open(&trailing, "test").unwrap().certificate, signer.certificate); + + // PEM armour, the way OpenSSL and government portals present them. + let b64 = base64::engine::general_purpose::STANDARD.encode(&der); + let mut pem = String::from("-----BEGIN PKCS12-----\n"); + for line in b64.as_bytes().chunks(64) { + pem.push_str(std::str::from_utf8(line).unwrap()); + pem.push('\n'); + } + pem.push_str("-----END PKCS12-----\n"); + assert_eq!(pkcs12::open(pem.as_bytes(), "test").unwrap().certificate, signer.certificate); + + // The base64 body alone, armour stripped. + assert_eq!(pkcs12::open(b64.as_bytes(), "test").unwrap().certificate, signer.certificate); + + // A truncated file still fails, and says why. + let err = pkcs12::open(&der[..64], "test").expect_err("truncated file"); + assert!(err.to_string().contains("runs past the end"), "{err}"); + let err = pkcs12::open(b"", "test").expect_err("empty file"); + assert!(err.to_string().contains("truncated DER"), "{err}"); +} + #[test] fn ocsp_responses_verify_and_match_the_certificate() { let id = pkcs12::open(&data("ec-p256.p12"), "test").unwrap();