Merge pull request #6375 from dylanmtaylor/land-sign-remaining

sign: land the still-valid parts of #6358, #5187 and #5173

crates/sign/src/pdf/changes.rs

@@ -68,14 +68,26 @@ pub(super) fn classify_changes(doc: &Document, old: Option<Document>, p: Option<
         };
         placed_annots.extend(found.into_iter().map(|r| r.num));
     }
+    // The signed page tree's nodes: a change to one changes the pages, whatever it is labelled
+    // now. Every node, and every /Kids array object of its own, is read once; past the bound the
+    // tree is not known in full, and no change is judged against it.
+    let page_tree = page_tree_nodes(&old);
+    let page_tree_unbounded = page_tree.is_none();
+    let (page_nodes, kids_arrays) = page_tree.unwrap_or_default();
     // What each signed page draws from, by the role it held when signed (#1787).
     let signed_resources = signed_page_resources(&old, &signed_page_refs);
     let old_info = old.trailer().get(b"Info").and_then(Object::as_ref).map(|r| r.num);
+    // The signed catalog's /AcroForm, when it is an object of its own.
+    let old_form = old.root().and_then(|c| old.get(c).as_dict().and_then(|c| c.get(b"AcroForm").and_then(Object::as_ref))).map(|r| r.num);
     let signed_role = |num: u32| old_content.contains(&num) || signed_pages.contains(&num) || signed_resources.contains_key(&num);
     let (mut allowed, mut disallowed): (Vec<&'static str>, Vec<&'static str>) = (Vec::new(), Vec::new());
     if annots_unbounded {
         disallowed.push(kind::OTHER);
     }
+    if page_tree_unbounded {
+        // Past the bound the tree is not known in full: no change is judged against it.
+        disallowed.push(kind::PAGES);
+    }
     // Every reference the signed revision makes, read only when a slot's generations narrowed.
     let signed_references = OnceCell::new();
     let dss = dss_parts(doc, &old);
@@ -117,6 +129,9 @@ pub(super) fn classify_changes(doc: &Document, old: Option<Document>, p: Option<
         old_xfa_data: &old_xfa_data,
         signed_pages: &signed_pages,
         signed_annots: &signed_annots,
+        page_nodes: &page_nodes,
+        kids_arrays: &kids_arrays,
+        old_form,
         placed_annots: &placed_annots,
         annot_budget: &annot_budget,
         signed_resources: &signed_resources,
@@ -202,13 +217,30 @@ pub(super) fn classify_changes(doc: &Document, old: Option<Document>, 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
@@ -259,10 +291,13 @@ struct ChangedObjectClassifier<'a> {
     old_xfa_data: &'a HashSet<u32>,
     signed_pages: &'a HashSet<u32>,
     signed_annots: &'a HashSet<u32>,
+    page_nodes: &'a HashSet<u32>,
+    kids_arrays: &'a HashSet<u32>,
     placed_annots: &'a HashSet<u32>,
     annot_budget: &'a Cell<usize>,
     signed_resources: &'a HashMap<u32, bool>,
     old_info: Option<u32>,
+    old_form: Option<u32>,
     dss: &'a HashSet<u32>,
 }
 
@@ -275,6 +310,8 @@ impl ChangedObjectClassifier<'_> {
             return Some(kind::SECURITY_STORE);
         }
         match new {
+            // A signed page-tree node's /Kids array of its own lists the pages.
+            Object::Array(_) if before.is_some() && self.kids_arrays.contains(&r.num) => Some(kind::PAGES),
             Object::Array(_) if self.is_signed_annotation_array(r, before) => self.annotation_array_change(new, before),
             Object::Dict(dict) => self.dictionary_change(r, dict, before),
             Object::Stream(stream) => self.stream_change(r, stream, before),
@@ -342,6 +379,15 @@ impl ChangedObjectClassifier<'_> {
         if self.signed_pages.contains(&r.num) {
             // A signed page is judged by what the page shows, whatever its new labels say.
             self.page_change_kind(dict, previous)
+        } else if previous.is_some() && self.page_nodes.contains(&r.num) {
+            // A signed page-tree node is a page-tree change, whatever its new version is labelled.
+            Some(kind::PAGES)
+        } else if let Some(previous) = previous.filter(|_| self.old.root().is_some_and(|root| root.num == r.num)) {
+            // The signed catalog, whatever /Type its new version carries.
+            Some(catalog_kind(self.doc, self.old, dict, previous))
+        } else if let Some(previous) = previous.filter(|_| self.old_form == Some(r.num)) {
+            // The signed form dictionary, held in an object of its own, whatever keys it carries now.
+            Some(acroform_kind(self.doc, self.old, dict, Some(previous)))
         } else if previous.is_some() && Some(r.num) == self.old_info {
             // Only the signed revision's own /Info dictionary is a metadata change.
             Some(kind::METADATA)
@@ -601,6 +647,10 @@ fn acroform(doc: &Document) -> Option<Dict> {
 fn acroform_kind(doc: &Document, old: &Document, now: &Dict, before: Option<&Dict>) -> &'static str {
     let fallback = acroform(old).unwrap_or_default();
     let before = before.unwrap_or(&fallback);
+    // A field taken out of the form: no DocMDP permission covers dropping a signed field.
+    if lost_entries(doc, old, before.get(b"Fields"), now.get(b"Fields")) {
+        return kind::OTHER;
+    }
     let strip = |d: &Dict| -> Vec<(Vec<u8>, Object)> {
         let mut v: Vec<(Vec<u8>, Object)> =
             d.iter().filter(|(k, _)| !matches!(k.as_slice(), b"Fields" | b"SigFlags")).map(|(k, o)| (k.to_vec(), o.clone())).collect();
@@ -724,6 +774,62 @@ fn extensions_kept(doc: &Document, old: &Document, d: &Dict, b: &Dict) -> bool {
     }
 }
 
+/// A change to the signed catalog `b`, now `d`.
+fn catalog_kind(doc: &Document, old: &Document, d: &Dict, b: &Dict) -> &'static str {
+    let keys: &[&[u8]] = &[b"AcroForm", b"DSS", b"Perms", b"Metadata", b"NeedsRendering", b"Extensions"];
+    if without(d, keys) != without(b, keys) {
+        kind::STRUCTURE
+    } else if !extensions_kept(doc, old, d, b) {
+        // Developer extensions (ISO 32000-2 §7.12) only declare what the file uses. A writer
+        // adding a /DSS also declares the ESIC extension it belongs to, so a declaration may be
+        // added, but none that was signed may change or go.
+        kind::STRUCTURE
+    } else if d.get(b"AcroForm") != b.get(b"AcroForm") {
+        // Compare the forms themselves, inline or not.
+        let now = acroform(doc).unwrap_or_default();
+        acroform_kind(doc, old, &now, None)
+    } else if b.get(b"Perms") != d.get(b"Perms") {
+        kind::SIGNATURE
+    } else {
+        kind::METADATA
+    }
+}
+
+/// The nodes of `doc`'s page tree and the /Kids arrays that are objects of their own, by number:
+/// every dictionary with /Kids from the catalog's /Pages down. Each node, and each /Kids array
+/// object, is read once. `None` past a million nodes.
+fn page_tree_nodes(doc: &Document) -> Option<(HashSet<u32>, HashSet<u32>)> {
+    const MAX_NODES: usize = 1_000_000;
+    let (mut nodes, mut arrays) = (HashSet::new(), HashSet::new());
+    let top = doc.root().and_then(|r| doc.get(r).as_dict().and_then(|c| c.get(b"Pages").and_then(Object::as_ref)));
+    let mut stack: Vec<ObjRef> = top.into_iter().collect();
+    while let Some(r) = stack.pop() {
+        if nodes.len() >= MAX_NODES {
+            return None;
+        }
+        let node = doc.get(r);
+        // A leaf (a page) has no /Kids.
+        let Some(kids) = node.as_dict().and_then(|d| d.get(b"Kids")) else { continue };
+        if !nodes.insert(r.num) || kids.as_ref().is_some_and(|a| !arrays.insert(a.num)) {
+            continue;
+        }
+        if let Some(kids) = doc.resolve(kids).as_array() {
+            stack.extend(kids.iter().filter_map(Object::as_ref));
+        }
+    }
+    Some((nodes, arrays))
+}
+
+/// Whether a list of references (`/Fields`) no longer holds an entry it held when signed. An
+/// entry is still held only by a reference that resolves now and that the signed revision
+/// resolved too, so to the same slot (a repaired alias counts): one rewritten to a generation
+/// the slot does not accept names nothing, and one to a newer generation names another object.
+fn lost_entries(doc: &Document, old: &Document, was: Option<&Object>, now: Option<&Object>) -> bool {
+    let live = |d: &Document, r: &ObjRef| !matches!(*d.get(*r), Object::Null);
+    let now: HashSet<u32> = refs(doc, now).into_iter().filter(|r| live(doc, r) && live(old, r)).map(|r| r.num).collect();
+    refs(old, was).iter().filter(|r| live(old, r)).any(|r| !now.contains(&r.num))
+}
+
 fn change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>) -> &'static str {
     let ty = d.name(b"Type");
     // A later revision cannot make an existing object a signature by relabelling it: a signature
@@ -746,31 +852,15 @@ fn change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>)
     }) {
         return kind::OTHER;
     }
+    // Where a signed widget sits and whether it shows are what the page shows: filling the form
+    // changes its value and appearance, never its rectangle or its flags.
+    if before.is_some_and(|b| b.name(b"Subtype") == Some(b"Widget") && (d.get(b"Rect") != b.get(b"Rect") || d.get(b"F") != b.get(b"F"))) {
+        return kind::PAGE_CONTENT;
+    }
     match ty {
         // Only a new signature value or policy: rewriting a signed one is not a signature (#1788).
         Some(b"Sig" | b"DocTimeStamp" | b"SigRef" | b"TransformParams") if before.is_none() => return kind::SIGNATURE,
-        Some(b"Catalog") => {
-            let keys: &[&[u8]] = &[b"AcroForm", b"DSS", b"Perms", b"Metadata", b"NeedsRendering", b"Extensions"];
-            return match before {
-                Some(b) if without(d, keys) != without(b, keys) => kind::STRUCTURE,
-                // Developer extensions (ISO 32000-2 §7.12) only declare what the file uses. A
-                // writer adding a /DSS also declares the ESIC extension it belongs to, so a
-                // declaration may be added, but none that was signed may change or go.
-                Some(b) if !extensions_kept(doc, old, d, b) => kind::STRUCTURE,
-                Some(b) => {
-                    if d.get(b"AcroForm") != b.get(b"AcroForm") {
-                        // Compare the forms themselves, inline or not.
-                        let now = acroform(doc).unwrap_or_default();
-                        acroform_kind(doc, old, &now, None)
-                    } else if b.get(b"Perms") != d.get(b"Perms") {
-                        kind::SIGNATURE
-                    } else {
-                        kind::METADATA
-                    }
-                }
-                None => kind::FORM_FILL,
-            };
-        }
+        Some(b"Catalog") => return before.map_or(kind::FORM_FILL, |b| catalog_kind(doc, old, d, b)),
         Some(b"Page") => {
             return match before {
                 Some(b) if without(d, &[b"Annots"]) != without(b, &[b"Annots"]) => kind::PAGE_CONTENT,
@@ -785,19 +875,32 @@ fn change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>)
         Some(b"Pages") => return if before.is_some() { kind::PAGES } else { kind::STRUCTURE },
         _ => {}
     }
-    match d.name(b"Subtype") {
+    // The role an existing object plays (annotation, form field, the form) is the one it had when
+    // signed: keys its new version carries are written by whoever changed it. An indirect
+    // /OCProperties rewritten with an added /FT still decides which layers show (#1787 judges a
+    // signed page's resources this way; this applies it to every other object).
+    let role = before.unwrap_or(d);
+    // Nor does an existing annotation or widget turned into another kind of annotation edit the
+    // one it was: an empty signature widget rewritten as a /Square that covers signed text is no
+    // signature change, and no comment either.
+    if before.is_some_and(|b| b.name(b"Subtype") != d.name(b"Subtype")) {
+        return kind::OTHER;
+    }
+    // The new version's /FT still decides whether this is a signature: a signed widget relabelled
+    // as a text field is not a signature change, whatever it was when signed.
+    match role.name(b"Subtype") {
         Some(b"Widget") => return if d.name(b"FT") == Some(b"Sig") && signature_ok { kind::SIGNATURE } else { kind::FORM_FILL },
         Some(b"Link") => return kind::LINKS,
-        Some(_) if ty == Some(b"Annot") || d.contains(b"Rect") => return kind::COMMENTS,
+        Some(_) if role.name(b"Type") == Some(b"Annot") || role.contains(b"Rect") => return kind::COMMENTS,
         _ => {}
     }
-    if d.name(b"FT") == Some(b"Sig") && signature_ok {
+    if role.name(b"FT") == Some(b"Sig") && signature_ok {
         return kind::SIGNATURE;
     }
-    if d.contains(b"Fields") {
+    if role.contains(b"Fields") {
         return acroform_kind(doc, old, d, before);
     }
-    if d.contains(b"FT") || (d.contains(b"T") && d.contains(b"Kids")) || d.contains(b"DR") {
+    if role.contains(b"FT") || (role.contains(b"T") && role.contains(b"Kids")) || role.contains(b"DR") {
         return kind::FORM_FILL;
     }
     if before.is_none() {
@@ -806,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<u8> {
+        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:?}"),
+        }
+    }
+}

crates/sign/tests/it/main.rs

@@ -8,4 +8,5 @@ mod crypto;
 mod encrypted_signing;
 mod pdf;
 mod revision_classification;
+mod signed_structure;
 mod store_keys;

crates/sign/tests/it/pdf.rs

@@ -1620,6 +1620,145 @@ fn a_dictionary_labelled_metadata_is_not_a_metadata_change() {
     });
 }
 
+/// A one-page document whose text sits in an optional-content group (object 8) that the
+/// catalog's indirect `/OCProperties` (object 7) lists as shown.
+fn fixture_with_layer() -> Vec<u8> {
+    let content = b"/OC /oc1 BDC BT /F1 14 Tf 20 250 Td (Clause 4: the buyer pays) Tj ET EMC";
+    let objs: Vec<Vec<u8>> = vec![
+        b"<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [6 0 R] >> /OCProperties 7 0 R >>".to_vec(),
+        b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(),
+        b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> /Properties << /oc1 8 0 R >> >> /Annots [6 0 R] >>".to_vec(),
+        [format!("<< /Length {} >>\nstream\n", content.len()).as_bytes(), content, b"\nendstream"].concat(),
+        b"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".to_vec(),
+        b"<< /Type /Annot /Subtype /Widget /FT /Sig /T (Approval) /Rect [150 20 280 70] /P 3 0 R /F 4 >>".to_vec(),
+        b"<< /OCGs [8 0 R] /D << /ON [8 0 R] >> >>".to_vec(),
+        b"<< /Type /OCG /Name (Clause 4) >>".to_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", i + 1).as_bytes());
+        out.extend_from_slice(o);
+        out.extend_from_slice(b"\nendobj\n");
+    }
+    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 >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes());
+    out
+}
+
+/// Hide the clause: rewrite `/OCProperties` (object 7) so its group starts off, adding `extra`.
+fn hide_layer(doc: &mut Document, extra: &[(&str, Object)]) {
+    let mut d = dict_of(vec![("OCGs", Object::Array(vec![oref(8)])), ("D", Object::Dict(dict_of(vec![("OFF", Object::Array(vec![oref(8)]))])))]);
+    for (k, v) in extra {
+        d.set(k.as_bytes().to_vec(), v.clone());
+    }
+    doc.set(pdfcraft_cos::ObjRef { num: 7, generation: 0 }, Object::Dict(d));
+}
+
+#[test]
+fn an_existing_object_given_form_keys_after_signing_is_still_judged() {
+    // An existing object is judged by the role it had when signed, not only when a signed page
+    // draws it (#1787): the catalog's indirect /OCProperties decides which layers show, and a
+    // rewrite that hides the clause stays a change whatever form-field keys it adds.
+    assert_refused_at_every_level(&fixture_with_layer(), "other changes", |doc| hide_layer(doc, &[]));
+    let zeros = Object::Array(vec![Object::Int(0); 4]);
+    for extra in [
+        vec![("FT", Object::name("Tx"))],
+        vec![("DR", Object::Dict(pdfcraft_cos::Dict::new()))],
+        vec![("T", Object::String(PdfString::text("x"))), ("Kids", Object::Array(Vec::new()))],
+        vec![("Subtype", Object::name("Widget")), ("Rect", zeros.clone())],
+        vec![("Subtype", Object::name("Square")), ("Rect", zeros)],
+    ] {
+        assert_refused_at_every_level(&fixture_with_layer(), "other changes", |doc| hide_layer(doc, &extra));
+    }
+}
+
+#[test]
+fn an_unsigned_signature_widget_turned_into_a_comment_is_no_edit_of_it() {
+    // The empty signature field "Approval" (object 6) rewritten as a white /Square annotation that
+    // covers the signed text. Judged by what it was, it is a signature widget; but an annotation
+    // of another kind is no edit that widget permits, at any level.
+    assert_refused_at_every_level(&fixture(), "other changes", |doc| {
+        let white = Object::Array(vec![Object::Int(1); 3]);
+        doc.set(
+            pdfcraft_cos::ObjRef { num: 6, generation: 0 },
+            Object::Dict(dict_of(vec![
+                ("Type", Object::name("Annot")),
+                ("Subtype", Object::name("Square")),
+                ("Rect", Object::Array(vec![Object::Int(0), Object::Int(0), Object::Int(300), Object::Int(300)])),
+                ("C", white.clone()),
+                ("IC", white),
+                ("P", oref(3)),
+            ])),
+        );
+    });
+}
+
+/// `fixture()` with the form an indirect dictionary (object 7) listing the unsigned field.
+fn fixture_with_indirect_acroform() -> Vec<u8> {
+    let objs: Vec<&[u8]> = vec![
+        b"<< /Type /Catalog /Pages 2 0 R /AcroForm 7 0 R >>",
+        b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>",
+        b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> /Annots [6 0 R] >>",
+        b"<< /Length 44 >>\nstream\nBT /F1 14 Tf 20 250 Td (Contract text) Tj ET\nendstream",
+        b"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>",
+        b"<< /Type /Annot /Subtype /Widget /FT /Sig /T (Approval) /Rect [150 20 280 70] /P 3 0 R /F 4 >>",
+        b"<< /Fields [6 0 R] >>",
+    ];
+    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", i + 1).as_bytes());
+        out.extend_from_slice(o);
+        out.extend_from_slice(b"\nendobj\n");
+    }
+    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 >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes());
+    out
+}
+
+#[test]
+fn the_forms_signed_fields_are_not_dropped_after_signing() {
+    // Signed, then timestamped. A later revision drops the form's /Fields, or empties it: the
+    // fields the timestamp covered, the signature among them, are gone from the form. That is no
+    // signature change, so the timestamp reports it as not permitted.
+    let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap();
+    let tsa = TestTsa {
+        id: pkcs12::open(&data("rsa-aes.p12"), "test").unwrap(),
+        time: Time { year: 2026, month: 10, day: 6, hour: 12, minute: 0, second: 0 },
+    };
+    let trust = TrustStore { certs: vec![id.certificate.clone()] };
+    let signed = pdfcraft_sign::sign(&open(&fixture_with_indirect_acroform()), &id, &opts()).unwrap();
+    let stamped = pdfcraft_sign::timestamp_document(&open(&signed), &tsa, "D:20261006120000Z").unwrap();
+    let mut failures = Vec::new();
+    for (how, fields) in [("dropped", None), ("emptied", Some(Object::Array(Vec::new())))] {
+        let edited = edit_after(&stamped, |doc| {
+            doc.update_dict(pdfcraft_cos::ObjRef { num: 7, generation: 0 }, |af| match &fields {
+                Some(f) => af.set(b"Fields".to_vec(), f.clone()),
+                None => {
+                    af.remove(b"Fields");
+                }
+            })
+            .unwrap();
+        });
+        let stamp = signatures(&open(&edited), &edited, &trust).into_iter().find(|s| s.doc_timestamp).expect("the document timestamp");
+        if !matches!(stamp.modification, Modification::Disallowed(_)) {
+            failures.push(format!("/Fields {how}: {:?}, {:?}", stamp.status, stamp.modification));
+        }
+    }
+    assert!(failures.is_empty(), "{}", failures.join("\n"));
+}
+
 /// A one-page document like `fixture()` (objects 1–6) plus a stream, object 7, with the given
 /// dictionary entries and body. `catalog_extra` goes into the catalog; the page draws object 7
 /// as a Form XObject when `drawn`.
@@ -3025,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::<Vec<_>>()
     );
 }
+
+#[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
+        );
+    }
+}

crates/sign/tests/it/signed_structure.rs

@@ -0,0 +1,199 @@
+//! A certified document's page tree, catalog and form keep the roles they had when signed: a
+//! later revision that relabels them, drops a field from the form, or hides or moves a signed
+//! field's widget is no form fill.
+
+use std::sync::Arc;
+
+use pdfcraft_cos::{Document, ObjRef, Object, PdfString, SaveOptions, write_incremental};
+use pdfcraft_sign::{Modification, SignOptions, Status, TrustStore, kind, pkcs12, signatures};
+
+fn data(name: &str) -> Vec<u8> {
+    std::fs::read(format!("{}/tests/data/{name}", env!("CARGO_MANIFEST_DIR"))).unwrap()
+}
+
+fn open(b: &[u8]) -> Document {
+    Document::open(Arc::new(b.to_vec())).unwrap()
+}
+
+/// A classic-xref PDF of `objs`, numbered from 1, with object 1 as the catalog.
+fn write_pdf(objs: &[&str]) -> Vec<u8> {
+    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 >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes());
+    out
+}
+
+fn r(num: u32) -> ObjRef {
+    ObjRef { num, generation: 0 }
+}
+
+/// Two pages, listed by the /Kids array object 6; page 3 holds text field 5, listed in /Fields.
+fn form() -> Vec<u8> {
+    write_pdf(&[
+        "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [5 0 R] >> >>",
+        "<< /Type /Pages /Kids 6 0 R /Count 2 >>",
+        "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Annots [5 0 R] >>",
+        "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] >>",
+        "<< /Type /Annot /Subtype /Widget /FT /Tx /T (Name) /V (Ann) /Rect [20 200 120 220] /P 3 0 R >>",
+        "[3 0 R 4 0 R]",
+    ])
+}
+
+fn certified(p: u8) -> Vec<u8> {
+    let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap();
+    let opts = SignOptions {
+        page: 1,
+        rect: Some([20.0, 100.0, 220.0, 150.0]),
+        date: "D:20261002120000+01'00'".into(),
+        certify: Some(p),
+        ..SignOptions::default()
+    };
+    pdfcraft_sign::sign(&open(&form()), &id, &opts).unwrap()
+}
+
+fn edit_after(signed: &[u8], f: impl FnOnce(&mut Document)) -> Vec<u8> {
+    let mut doc = open(signed);
+    f(&mut doc);
+    write_incremental(&doc, &SaveOptions { object_streams: false, ..SaveOptions::default() }).unwrap()
+}
+
+/// The certification signature of `bytes`: its status and modification verdict.
+fn verdict(bytes: &[u8]) -> (Status, Modification) {
+    let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap();
+    let trust = TrustStore { certs: vec![id.certificate.clone()] };
+    let s = signatures(&open(bytes), bytes, &trust).into_iter().find(|s| s.signed && s.certify.is_some()).unwrap();
+    (s.status, s.modification)
+}
+
+/// `bytes` is refused, for (among others) `expected`.
+fn assert_refused(what: &str, bytes: &[u8], expected: &str) {
+    let (status, modification) = verdict(bytes);
+    assert!(
+        matches!(&modification, Modification::Disallowed(kinds) if kinds.contains(&expected)) && status != Status::Valid,
+        "{what}: {status:?} {modification:?}, expected {expected}"
+    );
+}
+
+#[test]
+fn a_signed_page_tree_node_is_judged_as_the_page_tree() {
+    let signed = certified(2);
+    // Relabelled as a field and dropping a page.
+    assert_refused(
+        "node relabelled",
+        &edit_after(&signed, |doc| {
+            doc.update_dict(r(2), |n| {
+                n.remove(b"Type");
+                n.set(b"T".to_vec(), PdfString::text("x"));
+                n.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(r(4))]));
+                n.set(b"Count".to_vec(), Object::Int(1));
+            })
+            .unwrap();
+        }),
+        kind::PAGES,
+    );
+    // Turning every page through an inherited attribute.
+    assert_refused(
+        "inherited rotation",
+        &edit_after(&signed, |doc| {
+            doc.update_dict(r(2), |n| {
+                n.set(b"DR".to_vec(), Object::Dict(Default::default()));
+                n.set(b"Rotate".to_vec(), Object::Int(180));
+            })
+            .unwrap();
+        }),
+        kind::PAGES,
+    );
+    // Reordering the pages through the node's /Kids array object.
+    assert_refused(
+        "pages reordered",
+        &edit_after(&signed, |doc| doc.set(r(6), Object::Array(vec![Object::Ref(r(4)), Object::Ref(r(3))]))),
+        kind::PAGES,
+    );
+}
+
+#[test]
+fn a_signed_catalog_is_judged_as_the_catalog_whatever_its_type() {
+    let signed = certified(2);
+    assert_refused(
+        "catalog relabelled",
+        &edit_after(&signed, |doc| {
+            let content = doc.add(Object::Stream(pdfcraft_cos::Stream::from_raw(Default::default(), b"0 0 m 300 300 l S".to_vec())));
+            let pages = doc.add(Object::Dict(Default::default()));
+            let mut page = pdfcraft_cos::Dict::new();
+            page.set(b"Parent".to_vec(), Object::Ref(pages));
+            page.set(b"MediaBox".to_vec(), Object::Array([0, 0, 300, 300].map(Object::Int).to_vec()));
+            page.set(b"Contents".to_vec(), Object::Ref(content));
+            let page = doc.add(Object::Dict(page));
+            doc.update_dict(pages, |n| {
+                n.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(page)]));
+                n.set(b"Count".to_vec(), Object::Int(1));
+            })
+            .unwrap();
+            let root = doc.root().unwrap();
+            doc.update_dict(root, |c| {
+                c.remove(b"Type");
+                c.set(b"FT".to_vec(), Object::name("Tx"));
+                c.set(b"Pages".to_vec(), Object::Ref(pages));
+            })
+            .unwrap();
+        }),
+        kind::STRUCTURE,
+    );
+}
+
+#[test]
+fn a_field_dropped_from_the_form_is_no_change_any_level_permits() {
+    for p in [1, 2] {
+        let signed = certified(p);
+        let set_fields = |fields: Vec<Object>| {
+            edit_after(&signed, move |doc| {
+                let root = doc.root().unwrap();
+                let mut form =
+                    doc.get(root).as_dict().and_then(|c| c.get(b"AcroForm")).map(|f| doc.resolve(f)).and_then(|f| f.as_dict().cloned()).unwrap();
+                let kept: Vec<Object> = fields
+                    .into_iter()
+                    .chain(
+                        form.get(b"Fields")
+                            .and_then(Object::as_array)
+                            .into_iter()
+                            .flatten()
+                            .filter(|f| f.as_ref().is_some_and(|f| f.num != 5))
+                            .cloned(),
+                    )
+                    .collect();
+                form.set(b"Fields".to_vec(), Object::Array(kept));
+                doc.update_dict(root, |c| c.set(b"AcroForm".to_vec(), Object::Dict(form))).unwrap();
+            })
+        };
+        assert_refused(&format!("P={p} field dropped"), &set_fields(Vec::new()), kind::OTHER);
+        let dead = ObjRef { num: 5, generation: 3 };
+        assert_refused(&format!("P={p} field listed by a dead generation"), &set_fields(vec![Object::Ref(dead)]), kind::OTHER);
+        // The field's slot taken by a newer generation: another object, listed in its place.
+        let newer = ObjRef { num: 5, generation: 1 };
+        let replaced = edit_after(&set_fields(vec![Object::Ref(newer)]), |doc| {
+            let mut field = doc.get(r(5)).as_dict().cloned().unwrap();
+            field.set(b"V".to_vec(), PdfString::text("Eve"));
+            doc.set(newer, Object::Dict(field));
+        });
+        assert_refused(&format!("P={p} field replaced by a newer generation"), &replaced, kind::OTHER);
+    }
+}
+
+#[test]
+fn a_signed_fields_widget_may_be_filled_but_not_hidden_or_moved() {
+    let signed = certified(2);
+    let edit = |f: &dyn Fn(&mut pdfcraft_cos::Dict)| edit_after(&signed, |doc| doc.update_dict(r(5), |w| f(w)).unwrap());
+    let filled = edit(&|w| w.set(b"V".to_vec(), PdfString::text("Bob")));
+    assert_eq!(verdict(&filled), (Status::Valid, Modification::Allowed(vec![kind::FORM_FILL])));
+    assert_refused("hidden", &edit(&|w| w.set(b"F".to_vec(), Object::Int(2))), kind::PAGE_CONTENT);
+    assert_refused("moved", &edit(&|w| w.set(b"Rect".to_vec(), Object::Array(vec![Object::Int(0); 4]))), kind::PAGE_CONTENT);
+}

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"