From 72311bc1015dee23c6e2f0d3198476fdeb26a1e9 Mon Sep 17 00:00:00 2001 From: oracola91 <118899664+oracola91@users.noreply.github.com> Date: Mon, 9 Nov 2026 02:50:39 +0200 Subject: [PATCH] Signature validation takes the certification from the catalog (#6453) --- crates/sign/src/pdf/types.rs | 4 +- crates/sign/src/pdf/validate.rs | 53 +++++++++++++++++++----- crates/sign/tests/it/pdf.rs | 73 +++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 12 deletions(-) diff --git a/crates/sign/src/pdf/types.rs b/crates/sign/src/pdf/types.rs index 089cc2e9d7..93883e7584 100644 --- a/crates/sign/src/pdf/types.rs +++ b/crates/sign/src/pdf/types.rs @@ -41,7 +41,7 @@ pub mod kind { pub const PAGES: &str = "pages added or removed"; pub const OTHER: &str = "other changes"; /// Not a change but the reason there is nothing to compare: the signed revision itself could not - /// be read, so what came after it cannot be allowed. + /// be read, or does not hold the signature, so what came after it cannot be allowed. pub const UNREADABLE: &str = "the signed version could not be read"; /// Every word above, for an interface to translate. @@ -177,6 +177,8 @@ pub const WORDINGS: &[&str] = &[ "The signature value does not match the signer's certificate: the signature is corrupt.", "The signature could not be checked ({}).", "The signature could not be verified ({}).", + // Whether the signature certifies the document. + "This signature carries certification permissions, but the document does not name it as its certification, so it is checked as an approval signature.", // What came after the signature. "This document has not been modified since this signature was applied.", "The document has been modified since this signature was applied, but the changes are permitted ({}).", diff --git a/crates/sign/src/pdf/validate.rs b/crates/sign/src/pdf/validate.rs index 09a4e5264c..52bf79351f 100644 --- a/crates/sign/src/pdf/validate.rs +++ b/crates/sign/src/pdf/validate.rs @@ -38,9 +38,9 @@ pub(super) fn is_doc_timestamp(d: &Dict) -> bool { d.name(b"SubFilter") == Some(b"ETSI.RFC3161") && matches!(d.name(b"Type"), Some(b"DocTimeStamp" | b"Sig")) } -/// The DocMDP permission level (`/P`, 1–3) of signature value `v`, or `None` for an approval -/// signature. -fn docmdp(doc: &Document, v: &Dict) -> Option { +/// The DocMDP permission level (`/P`, 1 to 3) that signature value `v` asks for in its `/Reference`, +/// or `None` when it asks for none. +fn docmdp_reference(doc: &Document, v: &Dict) -> Option { v.get(b"Reference") .map(|r| doc.resolve(r)) .and_then(|r| r.as_array().cloned()) @@ -51,6 +51,31 @@ fn docmdp(doc: &Document, v: &Dict) -> Option { .map(|r| r.get(b"TransformParams").map(|p| doc.resolve(p)).and_then(|p| p.as_dict().and_then(|p| p.int(b"P"))).unwrap_or(2).clamp(1, 3) as u8) } +/// Whether the catalog's `/Perms /DocMDP` in `doc` is the signature dictionary `v`. +fn names_certification(doc: &Document, v: &Dict) -> bool { + let Some(root) = doc.root() else { return false }; + let catalog = doc.get(root); + let Some(perms) = catalog.as_dict().and_then(|c| c.get(b"Perms")).map(|p| doc.resolve(p)) else { return false }; + let Some(named) = perms.as_dict().and_then(|p| p.get(b"DocMDP")).map(|d| doc.resolve(d)) else { return false }; + named.as_dict() == Some(v) +} + +/// Set `info.certify` from `doc`, the revision signature value `v` covers. A document has at most +/// one certification, the signature its catalog's `/Perms /DocMDP` names (ISO 32000-2, 12.8.2.2), +/// so a DocMDP reference in any other signature dictionary does not choose the permissions later +/// changes are judged by. Such a signature is checked as an approval signature, with a note. +fn set_certification(doc: &Document, v: &Dict, info: &mut SignatureInfo) { + let asked = docmdp_reference(doc, v); + info.certify = asked.filter(|_| names_certification(doc, v)); + let note = Detail::plain( + "This signature carries certification permissions, but the document does not name it as its certification, so it is checked as an approval signature.", + ); + info.details.retain(|d| *d != note); + if asked.is_some() && info.certify.is_none() { + info.details.push(note); + } +} + pub(super) fn validate_into(doc: &Document, bytes: &[u8], trust: &TrustStore, v: &Dict, info: &mut SignatureInfo, cache: &DigestCache) { // A document timestamp can also be the value of a signature field (Documenso writes // `Timestamp_1` that way). Its `/Contents` is a bare RFC 3161 token, not a signature over the @@ -68,7 +93,7 @@ pub(super) fn validate_into(doc: &Document, bytes: &[u8], trust: &TrustStore, v: info.contact = text(doc, v, b"ContactInfo"); info.sub_filter = v.name(b"SubFilter").map(|s| String::from_utf8_lossy(s).into_owned()); info.signer = text(doc, v, b"Name"); - info.certify = docmdp(doc, v); + set_certification(doc, v, info); let invalid = |info: &mut SignatureInfo, why: Detail| { info.status = Status::Invalid; info.details.push(why); @@ -243,18 +268,24 @@ fn finish_validation(context: SignatureValidation<'_>, verified: VerifiedSignatu info.modification = if covered == bytes.len() || bytes[covered..].iter().all(|b| b.is_ascii_whitespace() || *b == 0) { Modification::None } else { - // The permissions are the signed revision's: a later one can rewrite `/P` (#1788). This - // signature is the one whose byte range ends that revision. + // The permissions are the signed revision's: a later one can rewrite `/P` (#1788) or the + // catalog's `/Perms`. This signature is the one whose byte range ends that revision. A + // revision that holds no such signature does not say which permissions apply, so what + // came after it is judged as when the revision cannot be read: not permitted. let old = cache.revision(doc, bytes, covered); - if let Some(old) = &old - && let Some(v) = sig_fields(old) + let signed = old.as_ref().and_then(|old| { + sig_fields(old) .iter() .filter_map(|f| f.signature(old)) .find(|v| nums(old, v, b"ByteRange").is_some_and(|b| b.len() == 4 && b[2] + b[3] == covered as f64)) - { - info.certify = docmdp(old, &v); + }); + match (old, signed) { + (Some(old), Some(v)) => { + set_certification(&old, &v, info); + classify_changes(doc, Some(old), info.certify) + } + _ => classify_changes(doc, None, info.certify), } - classify_changes(doc, old, info.certify) }; let mut problems = false; // The changes are handed over as the words they are, not stitched into the sentence, so an diff --git a/crates/sign/tests/it/pdf.rs b/crates/sign/tests/it/pdf.rs index de55fdca6c..e765941c9a 100644 --- a/crates/sign/tests/it/pdf.rs +++ b/crates/sign/tests/it/pdf.rs @@ -538,6 +538,79 @@ fn a_document_timestamp_keeps_a_no_changes_certification_valid() { assert!(matches!(&s.modification, Modification::Allowed(k) if k.contains(&kind::SIGNATURE)), "{:?}", s.modification); } +/// Signature field "Approval" whose value, object 4, asks for DocMDP P 1 in its `/Reference`; +/// object 5 is another signature dictionary. `perms` goes into the catalog. The signature itself +/// is a dummy: which signature certifies is decided before its bytes are checked. +fn certification_of(perms: &str) -> pdfcraft_sign::SignatureInfo { + let catalog = format!("<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [3 0 R] >> {perms} >>"); + let bytes = write_pdf(&[ + catalog.as_bytes(), + b"<< /Type /Pages /Kids [] /Count 0 >>", + b"<< /FT /Sig /T (Approval) /V 4 0 R >>", + b"<< /Type /Sig /Filter /Adobe.PPKLite /SubFilter /adbe.pkcs7.detached /Reference [<< /Type /SigRef /TransformMethod /DocMDP /TransformParams << /Type /TransformParams /P 1 /V /1.2 >> >>] >>", + b"<< /Type /Sig /Filter /Adobe.PPKLite /SubFilter /adbe.pkcs7.detached >>", + ]); + signatures(&open(&bytes), &bytes, &TrustStore { certs: Vec::new() }).into_iter().find(|s| s.field == "Approval").unwrap() +} + +/// Only the signature the catalog's `/Perms /DocMDP` names certifies the document: a DocMDP +/// reference in any other signature dictionary leaves it an approval signature, with a note. +#[test] +fn only_the_signature_the_catalog_names_certifies() { + let note = "This signature carries certification permissions, but the document does not name it as its certification, so it is checked as an approval signature."; + let noted = |s: &pdfcraft_sign::SignatureInfo| s.details.iter().filter(|d| d.message == note).count(); + let named = certification_of("/Perms << /DocMDP 4 0 R >>"); + assert_eq!((named.certify, noted(&named)), (Some(1), 0), "{:?}", named.details); + for perms in ["", "/Perms << /DocMDP 5 0 R >>", "/Perms << /UR3 4 0 R >>"] { + let s = certification_of(perms); + assert_eq!((s.certify, noted(&s)), (None, 1), "{perms:?}: {:?}", s.details); + } +} + +/// [`certified_p1`] with a fraction after its last `/ByteRange` number, signed again over the +/// edited bytes. The ranges still cover whole bytes, so the signature verifies, but no signature +/// in the signed revision has a byte range that ends exactly where that revision ends. +fn certified_p1_with_a_fractional_range() -> (TrustStore, Vec) { + let (id, trust, mut signed) = certified_p1(); + let prefix = b"/ByteRange ["; + let at = signed.windows(prefix.len()).position(|w| w == prefix).unwrap() + prefix.len(); + let end = at + signed[at..].iter().position(|b| *b == b']').unwrap(); + let text = std::str::from_utf8(&signed[at..end]).unwrap().to_owned(); + let range: Vec = text.split_ascii_whitespace().map(|n| n.parse().unwrap()).collect(); + let mut fractional = format!("{}.5", text.trim_end()).into_bytes(); + assert!(fractional.len() <= end - at, "no room for the fraction in {text:?}"); + fractional.resize(end - at, b' '); + signed[at..end].copy_from_slice(&fractional); + let (start, stop) = (range[1], range[2]); + signed[start + 1..stop - 1].fill(b'0'); + let alg = id.key.preferred_digest(); + let digest = alg.digest(&[&signed[..start], &signed[stop..]]); + let cms = pdfcraft_sign::cms::sign_detached(&id.key, &id.certificate, &id.chain, alg, &digest).unwrap(); + let hex: Vec = cms.iter().flat_map(|b| format!("{b:02X}").into_bytes()).collect(); + signed[start + 1..start + 1 + hex.len()].copy_from_slice(&hex); + (trust, signed) +} + +/// When the signed revision holds no signature whose byte range ends it, nothing signed says +/// which permissions apply: later changes are not permitted, whatever the latest catalog says. +#[test] +fn changes_after_an_unmatched_signed_revision_are_not_permitted() { + let (trust, signed) = certified_p1_with_a_fractional_range(); + let check = |bytes: &[u8]| signatures(&open(bytes), bytes, &trust).into_iter().find(|s| s.signed).unwrap(); + let s = check(&signed); + assert_eq!((s.status, &s.modification), (Status::Valid, &Modification::None), "{:?}", s.details); + let edited = edit_after(&signed, |doc| { + let root = doc.root().unwrap(); + doc.update_dict(root, |c| { + c.remove(b"Perms"); + }) + .unwrap(); + add_comment(doc); + }); + let s = check(&edited); + assert_eq!((s.status, s.modification), (Status::Invalid, Modification::Disallowed(vec![kind::UNREADABLE]))); +} + #[test] fn invisible_signatures_and_refusals() { let id = pkcs12::open(&data("ec-p256.p12"), "test").unwrap();