nigig-org/REVIEWS/adr/0010-pdf-signatures.md

ADR 0010: PDF digital signatures — read, report, verify integrity; never sign

• Status: Accepted
• Date: 2026-07-31
• Review item: Phase 8, DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("Signatures", 4–6 weeks, "CMS/X.509 via x509-cert + rsa/p256, trust store, interop tests against OpenSSL")
• Supersedes: nothing
• Related: ADR 0003 (incremental save — the byte range an append preserves is exactly what a signature covers), ADR 0005 (encryption — same rule that audited crates do the cryptography)

Context

A signed PDF today is not merely unverified. It is misreported, in two distinct ways, and one of them is not limited to signatures at all.

1. acroform() silently dropped fields

PdfDocument::acroform dereferenced the /AcroForm entry with self.resolve(), which recurses. That replaced every /Fields [5 0 R] entry with an inline dictionary, so AcroForm::walk saw node.as_ref() == None, concluded the field had no identity to key an edit on, and dropped it.

Probing a signed document whose /AcroForm is a direct dictionary:

acroform present, 0 fields
annotations on page 0: 1

The form is not reported as broken. It is reported as empty, which is indistinguishable from a document that has no fields — so nothing announces the loss.

This is the third appearance of one defect: page_annotations once called self.resolve() on /Annots and destroyed every annotation's obj_ref, and extract_xobjects resolved a reference and then asked the resolved object for as_ref(), leaving every page's XObject map empty. Same mistake, third location.

It was masked because both existing fixtures (acroform.pdf, corpus/forms/all_types.pdf) declare /AcroForm as an indirect reference, where only one level is dereferenced and the /Fields refs survive. A direct /AcroForm dictionary — equally legal, and what the signature fixture uses — hits the bug. The test suite could not have caught it.

2. Nothing reads a signature at all

FieldType::Signature is classified and then nothing else happens. /ByteRange, /Contents, /SubFilter, /M, /Name, /Reason, /DocMDP are never read anywhere in the codebase. A document carrying a signature is presented exactly like an unsigned one.

For a viewer, that is the dangerous direction of failure. A user shown a signed contract with no indication that it is signed — or worse, no indication that the signature covers only part of the file — has been given less information than the file contains.

Decision

Implement signature reading, reporting and byte-range integrity checking. Do not implement cryptographic verification in this ADR, and do not implement signing at all.

That split is deliberate and is the whole point of the decision, so it is stated plainly rather than buried:

In scope

• Parse every signature field: /ByteRange, /Contents, /Filter, /SubFilter, /M, /Name, /Reason, /Location, /ContactInfo.
• Classify the /SubFilter into a known algorithm family (adbe.pkcs7.detached, adbe.pkcs7.sha1, adbe.x509.rsa_sha1, ETSI.CAdES.detached) and report an unknown one by name.
• Byte-range integrity, which needs no cryptography and catches the most common real-world tampering:
  • the four /ByteRange numbers are well-formed, ascending, non-negative and inside the file;
  • the gap between the two ranges is exactly the hex /Contents string, so the signature covers everything except its own container;
  • whether the range reaches the end of the file — a signature that stops short leaves appended bytes uncovered, which is precisely how an incremental-update attack hides content behind a valid-looking signature.
• Report document-level state: is the document signed, how many signatures, does any of them cover the whole file, and is there a /DocMDP transform declaring the permitted modifications.
• A typed SignatureError for every failure mode.

Explicitly out of scope, and refused rather than faked

• Cryptographic verification. Parsing CMS/PKCS#7, walking an X.509 chain, checking validity dates, revocation (CRL/OCSP), and timestamp tokens. This needs x509-cert, rsa, p256, cms and a trust store — a policy decision about which roots to trust, which belongs to the host, not to a parsing library.
• Signing. Creating a signature requires a private key, and ADR 0003 already refuses to save an encrypted document for the same class of reason: a library should not make a security decision on the user's behalf.

The API therefore reports VerificationStatus::NotVerified with a reason, never Valid. There is no code path in this ADR that can return "valid". That is enforced by the type: the Valid variant does not exist. A future ADR that adds real verification adds the variant with the cryptography, in the same change.

This matters more than it might look. The failure mode for a signature feature is not "it doesn't work" — it is a green tick next to a document whose signature was never checked. Refusing to render that tick until the cryptography is real is the only honest position, and making it a type-level guarantee means it cannot be undone by accident.

Non-negotiable rules

1. Nothing claims a signature is valid. No Valid variant exists until real cryptography backs it.
2. A signature that does not cover the whole file is reported as such, prominently, because that is an attack signature and not a curiosity.
3. Byte-range arithmetic is bounds-checked against the real file length. A /ByteRange is attacker-controlled input.
4. An unknown /SubFilter is named, not ignored.
5. No signing, and no private-key handling, anywhere in this crate.
6. The acroform() reference bug is fixed with a regression test that fails against the old code, and the fixture uses a direct /AcroForm dictionary, because an indirect one cannot reproduce it.

Merge criteria

• acroform() returns signature fields; a direct-/AcroForm fixture regression-tests the reference-destroying bug.
• /ByteRange and /Contents are parsed, with the hex string decoded.
• A signature covering the whole file is distinguished from one that does not.
• A /ByteRange that is malformed, out of order, negative or past the end of the file is a typed error, not a panic and not a pass.
• The gap between the two ranges is checked to be exactly the /Contents container.
• /SubFilter is classified, and an unknown one is reported by name.
• Signer metadata (/M, /Name, /Reason, /Location) is exposed.
• /DocMDP permissions are reported when present.
• The API cannot report a signature as cryptographically valid, and a test asserts the absence of that capability.
• Corpus fixtures: a whole-file signature, a partial-coverage signature, a malformed byte range, an unknown SubFilter, and a certification (/DocMDP) signature — all generated by the checked-in script.
• Incremental save over a signed document is shown to preserve the signed byte range.
• TEST_TARGET=pdf ./tools/test-rust-clean.sh passes, rustfmt and clippy -D warnings clean.

All criteria met. TEST_TARGET=pdf went from 497 to 536.

The acroform() regression test was mutation-checked: reverting the one-line fix fails 9 of the 12 signature acceptance tests. A parse_signature fuzz target was added because /ByteRange is four attacker-controlled integers used to index the file.

One implementation bug worth recording, because it is the kind that ships quietly: /Contents was initially hex-decoded. The COS lexer already decodes <...>, so this ran twice — and since the common placeholder blob is 128 zero bytes, which contain no hex digits, it decoded to an empty vector. The signature blob was silently discarded while every other field looked correct. Caught by asserting the blob is non-empty rather than asserting the parse returned Ok.

Consequences

Positive. A viewer can finally tell the user a document is signed, who signed it, and — critically — whether the signature covers the whole file. The byte-range checks catch incremental-update tampering with no cryptography at all. The acroform() fix repairs every form with a direct /AcroForm dictionary, not only signed ones.

Negative. "Signed" without "verified" is a partial answer, and a host must be careful how it presents that. Mitigated by the API being unable to say "valid", so the host cannot accidentally imply it.

Risk. Someone later adds a Valid variant that is set by anything less than real chain verification. Mitigated by rule 1, by the test asserting the capability's absence, and by this paragraph.

Out of scope, deliberately: CMS/PKCS#7 parsing, X.509 chains, trust stores, revocation, timestamps, signature creation, /Perms, usage-rights signatures, and appearance generation for signature widgets.