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

- [x] `acroform()` returns signature fields; a direct-`/AcroForm` fixture
      regression-tests the reference-destroying bug.
- [x] `/ByteRange` and `/Contents` are parsed, with the hex string decoded.
- [x] A signature covering the whole file is distinguished from one that
      does not.
- [x] 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.
- [x] The gap between the two ranges is checked to be exactly the
      `/Contents` container.
- [x] `/SubFilter` is classified, and an unknown one is reported by name.
- [x] Signer metadata (`/M`, `/Name`, `/Reason`, `/Location`) is exposed.
- [x] `/DocMDP` permissions are reported when present.
- [x] The API cannot report a signature as cryptographically valid, and a
      test asserts the absence of that capability.
- [x] 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.
- [x] Incremental save over a signed document is shown to preserve the
      signed byte range.
- [x] `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.