From fbe079c953d3f2644ac59d40894a07fb061f21b0 Mon Sep 17 00:00:00 2001 From: LetoCoder26 Date: Thu, 5 Nov 2026 21:43:14 -0500 Subject: [PATCH] fix(sign): compare the catalog's /Perms by content under P=1 Rebased #5173 onto 9afa1266. Its later-signature predicate is superseded by #6364's object-based check, so only the /Perms rule lands: under DocMDP P=1 a /Perms that differs from the signed catalog's, by content, is a structure change however it was rewritten. Co-Authored-By: LetoCoder26 --- crates/sign/src/pdf/changes.rs | 73 +++++++++++++++++++++++++++++++++- crates/sign/tests/it/pdf.rs | 29 ++++++++++++++ parity/acrobat-features.toml | 4 +- 3 files changed, 102 insertions(+), 4 deletions(-) diff --git a/crates/sign/src/pdf/changes.rs b/crates/sign/src/pdf/changes.rs index 8cfd823fb0..712c9fcfff 100644 --- a/crates/sign/src/pdf/changes.rs +++ b/crates/sign/src/pdf/changes.rs @@ -217,13 +217,30 @@ pub(super) fn classify_changes(doc: &Document, old: Option, p: Option< // archiving (§12.8.5). Those timestamps draw nothing. Any other signature added since — an // approval signature, an unsigned signature widget, or a widget that paints over the signed // page — is a change the certification forbids, however it is labelled. - if p == Some(1) && allowed.contains(&kind::SIGNATURE) && !only_invisible_timestamps_added(doc, &old) { + if p == Some(1) { + only_validation_data(&mut allowed, &mut disallowed, doc, &old); + } + if disallowed.is_empty() { Modification::Allowed(allowed) } else { Modification::Disallowed(disallowed) } +} + +/// DocMDP P=1 ("no changes allowed") permits only validation data to follow the signed revision: +/// the security store, and document timestamps that draw nothing. A later approval signature, a +/// visible widget or a changed catalog `/Perms` breaks the certification. +fn only_validation_data(allowed: &mut Vec<&'static str>, disallowed: &mut Vec<&'static str>, doc: &Document, old: &Document) { + if allowed.contains(&kind::SIGNATURE) && !only_invisible_timestamps_added(doc, old) { allowed.retain(|k| *k != kind::SIGNATURE); if !disallowed.contains(&kind::SIGNATURE) { disallowed.push(kind::SIGNATURE); } } - if disallowed.is_empty() { Modification::Allowed(allowed) } else { Modification::Disallowed(disallowed) } + // The catalog's /Perms names the certification signature itself. Elsewhere a change to it is + // judged as signing; under P=1 dropping or redirecting it is a change like any other. + // Compared by content: an indirect /Perms rewritten in place keeps its reference (and, shared + // with the trailer's /Info, would otherwise pass as a metadata change). + let perms = |d: &Document| d.root().and_then(|r| d.get(r).as_dict().and_then(|c| c.get(b"Perms").map(|p| (*d.resolve(p)).clone()))); + if perms(doc) != perms(old) && !disallowed.contains(&kind::STRUCTURE) { + disallowed.push(kind::STRUCTURE); + } } /// Whether every signature object `doc` gained or changed since the signed revision `old` is an @@ -892,3 +909,55 @@ fn change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>) } kind::OTHER } + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Arc; + + /// A PDF of `objs` (numbered from 1) whose trailer also holds `trailer_extra`. + fn pdf(objs: &[&str], trailer_extra: &str) -> Vec { + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R {trailer_extra} >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out + } + + /// A P=1 signed revision whose catalog `/Perms` and trailer `/Info` are one indirect + /// dictionary (object 3). A later revision points its `/DocMDP` at another signature + /// dictionary already in the file, in place: the reference stays, no object is added, and + /// as the signed `/Info` the object reads as metadata, but the certification's own record + /// has changed. + #[test] + fn a_perms_dictionary_shared_with_info_rewritten_in_place_is_a_structure_change() { + let signed = pdf( + &[ + "<< /Type /Catalog /Pages 2 0 R /Perms 3 0 R >>", + "<< /Type /Pages /Kids [] /Count 0 >>", + "<< /DocMDP 4 0 R >>", + "<< /Type /Sig >>", + "<< /Type /Sig >>", + ], + "/Info 3 0 R", + ); + let old = Document::open(Arc::new(signed.clone())).unwrap(); + let mut doc = Document::open(Arc::new(signed)).unwrap(); + let other = ObjRef { num: 5, generation: 0 }; + doc.update_dict(ObjRef { num: 3, generation: 0 }, |p| p.set(b"DocMDP".to_vec(), Object::Ref(other))).unwrap(); + let edited = pdfcraft_cos::write_incremental(&doc, &pdfcraft_cos::SaveOptions { object_streams: false, ..Default::default() }).unwrap(); + let doc = Document::open(Arc::new(edited)).unwrap(); + match classify_changes(&doc, Some(old), Some(1)) { + Modification::Disallowed(kinds) => assert!(kinds.contains(&kind::STRUCTURE), "{kinds:?}"), + other => panic!("the certification's /Perms changed in place under P=1: {other:?}"), + } + } +} diff --git a/crates/sign/tests/it/pdf.rs b/crates/sign/tests/it/pdf.rs index 8f327115d2..8c9a827e6d 100644 --- a/crates/sign/tests/it/pdf.rs +++ b/crates/sign/tests/it/pdf.rs @@ -3164,3 +3164,32 @@ fn signing_a_field_with_a_null_value_after_another_signature_keeps_the_first_val all.iter().map(|s| (&s.status, &s.modification)).collect::>() ); } + +#[test] +fn the_catalogs_perms_do_not_change_under_no_changes() { + // /Perms names the certification signature (DocMDP). Under P=1 nothing may change, so a later + // revision that drops it, or points it elsewhere, breaks the certification. + let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap(); + let trust = TrustStore { certs: vec![id.certificate.clone()] }; + let certified = pdfcraft_sign::sign(&open(&fixture()), &id, &SignOptions { certify: Some(1), ..opts() }).unwrap(); + let elsewhere = dict_of(vec![("DocMDP", oref(5))]); + for (how, perms) in [("dropped", None), ("pointed elsewhere", Some(Object::Dict(elsewhere)))] { + let edited = edit_after(&certified, |doc| { + let root = doc.root().unwrap(); + doc.update_dict(root, |c| match &perms { + Some(p) => c.set(b"Perms".to_vec(), p.clone()), + None => { + c.remove(b"Perms"); + } + }) + .unwrap(); + }); + let s = signatures(&open(&edited), &edited, &trust).into_iter().find(|s| s.field == "Signature1").unwrap(); + assert!( + s.status == Status::Invalid && matches!(&s.modification, Modification::Disallowed(k) if k.contains(&kind::STRUCTURE)), + "/Perms {how} under P=1: {:?}, {:?}", + s.status, + s.modification + ); + } +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index ec144d3a07..10b040e09f 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -6535,8 +6535,8 @@ milestone = "M9" status = "shipped" commands = ["sign.panel"] tools = ["sign_list"] -evidence = ["crates/sign/tests/it/pdf.rs::later_changes_are_classified_under_the_signature_permissions", "crates/sign/tests/it/pdf.rs::signing_an_existing_field_and_counter_signing", "crates/engine/src/tests.rs::signing_saving_trusting_and_commenting_afterwards", "crates/sign/tests/it/pdf.rs::validation_data_and_xmp_added_after_signing_are_not_tampering", "crates/sign/tests/it/pdf.rs::a_dss_entry_does_not_make_the_object_it_names_part_of_the_store", "crates/sign/tests/it/pdf.rs::an_existing_dictionary_is_not_a_store_part_because_the_store_names_it", "crates/sign/tests/it/pdf.rs::an_existing_array_stays_what_it_is_when_the_store_points_at_it", "crates/sign/tests/it/pdf.rs::a_signed_store_may_grow_but_not_lose_or_swap_entries", "crates/sign/tests/it/pdf.rs::a_dictionary_labelled_metadata_is_not_a_metadata_change", "crates/sign/tests/it/pdf.rs::a_stream_already_labelled_metadata_when_signed_is_not_metadata_unless_the_catalog_names_it", "crates/sign/tests/it/pdf.rs::the_catalogs_own_xmp_stream_may_still_be_rewritten_after_signing", "crates/sign/tests/it/pdf.rs::rewritten_page_resources_are_page_content_whatever_keys_they_carry", "crates/sign/tests/it/pdf.rs::rewriting_a_certified_forms_xfa_data_is_a_form_fill"] -notes = "revision diff classified as signing, form fill, comments, metadata, page content or structure. Later revisions that add DSS data (new /Certs, /CRLs, /OCSPs, /VRI objects, or ones of the signed store that only grew) or rewrite the catalog's XMP are permitted changes; a /DSS entry that names other objects, or a /Type /DSS or /Type /Metadata label on an existing dictionary or stream, does not excuse changing them (negative tests at every DocMDP level). A signed page's resources (fonts, XObjects, graphics states and what they reach) are page content by the role they held when signed, whatever keys a later revision writes into them (#1787). Rewriting the XFA `datasets` or `form` packet the signed revision named is form fill, as a Reader Extensions (UR3) save of a certified form does (#2759); the packets' roles come from the signed revision" +evidence = ["crates/sign/tests/it/pdf.rs::later_changes_are_classified_under_the_signature_permissions", "crates/sign/tests/it/pdf.rs::signing_an_existing_field_and_counter_signing", "crates/engine/src/tests.rs::signing_saving_trusting_and_commenting_afterwards", "crates/sign/tests/it/pdf.rs::validation_data_and_xmp_added_after_signing_are_not_tampering", "crates/sign/tests/it/pdf.rs::a_dss_entry_does_not_make_the_object_it_names_part_of_the_store", "crates/sign/tests/it/pdf.rs::an_existing_dictionary_is_not_a_store_part_because_the_store_names_it", "crates/sign/tests/it/pdf.rs::an_existing_array_stays_what_it_is_when_the_store_points_at_it", "crates/sign/tests/it/pdf.rs::a_signed_store_may_grow_but_not_lose_or_swap_entries", "crates/sign/tests/it/pdf.rs::a_dictionary_labelled_metadata_is_not_a_metadata_change", "crates/sign/tests/it/pdf.rs::a_stream_already_labelled_metadata_when_signed_is_not_metadata_unless_the_catalog_names_it", "crates/sign/tests/it/pdf.rs::the_catalogs_own_xmp_stream_may_still_be_rewritten_after_signing", "crates/sign/tests/it/pdf.rs::rewritten_page_resources_are_page_content_whatever_keys_they_carry", "crates/sign/tests/it/pdf.rs::rewriting_a_certified_forms_xfa_data_is_a_form_fill", "crates/sign/tests/it/pdf.rs::the_catalogs_perms_do_not_change_under_no_changes"] +notes = "revision diff classified as signing, form fill, comments, metadata, page content or structure. Later revisions that add DSS data (new /Certs, /CRLs, /OCSPs, /VRI objects, or ones of the signed store that only grew) or rewrite the catalog's XMP are permitted changes; a /DSS entry that names other objects, or a /Type /DSS or /Type /Metadata label on an existing dictionary or stream, does not excuse changing them (negative tests at every DocMDP level). A signed page's resources (fonts, XObjects, graphics states and what they reach) are page content by the role they held when signed, whatever keys a later revision writes into them (#1787). Rewriting the XFA `datasets` or `form` packet the signed revision named is form fill, as a Reader Extensions (UR3) save of a certified form does (#2759); the packets' roles come from the signed revision. Under P=1 a change to the catalog's /Perms breaks the certification." [[feature]] id = "sign.view-signed-version"