nigig-org/REVIEWS/adr/0027-pdf-phase6-completion.md
andodeki 374af5ccad
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
feat(pdf): the five Phase 6 bullets the status line omitted
ADR 0027. Asked whether Phase 6 was 100% complete, I checked the plan's
bullets against the code instead of answering from the status line. Five
were not implemented and the status line named none of them:

  detached/ATTACHED signatures   /SubFilter hardcoded to adbe.pkcs7.detached
  PAdES basics                   ETSI.CAdES.detached was a string in a match
  external_signing_test.dart     absent; SigningIdentity needs an in-memory key
  OCSP/CRL lookup                CRL only; OCSP counted, never parsed
  Fulcio identity                absent (optional in the plan)

This is the second time. ADR 0021 recorded the same failure in Phase 4 and
wrote the rule meant to prevent it — enumerate criteria from the plan text
first, then mark each done or explicitly deferred. I wrote that rule and
then produced another prose summary of what I had built. A summary written
from the work cannot show what the work omitted.

PAdES is a real profile, not a label. CAdES signs a set of signed
attributes, one carrying the document digest, and the signature is over
those attributes re-tagged as a SET (RFC 5652 5.4) rather than over the
[0] IMPLICIT SEQUENCE they are carried in. Verification checks the
messageDigest attribute against the document as well as verifying the
attribute signature; without that, a signature over somebody else's digest
would be accepted. /SubFilter now comes from the profile, so a document
cannot claim CAdES while carrying plain PKCS#7.

ExternalSigner is a trait: bytes in, signature out. A smartcard or KMS
never hands out its key, so SigningIdentity could not represent one.
SigningIdentity implements the trait rather than sitting beside it, so
there is one signing path — a second path for hardware keys would be a
second place the byte range could be computed differently.

OCSP is decoded with the der crate already present rather than adding the
ocsp crate for two fields. Revoked from any response beats Good from any
other.

Attached signatures are REFUSED, not deferred. Both attached profiles
(adbe.pkcs7.sha1, adbe.x509.rsa_sha1) are SHA-1 based, and SHA-1 is broken
for signatures. They are parsed so such documents can be read; they cannot
be written, enforced by the absence of a SignatureProfile variant. Same
decision as RC4 in ADR 0024. Recorded as refused rather than not-done,
because "not done" invites someone to finish it.

Four mutations, all killed first attempt: messageDigest not compared,
CAdES verified against the wrong bytes, /SubFilter hardcoded again, OCSP
revoked read as good.

The status line is now the plan's own bullets in a table, one row per spec
item, not prose. Two wrong status lines in the same direction is a pattern,
and the fix is structural: a missing row is visible, a missing sentence is
not. Four rows are left unticked — Fulcio, independent review, Acrobat
interoperability, and signing a document that already has an AcroForm.

qpdf accepts documents under both profiles. pdf: 1289 passed (was 1276).
Coverage 87.96%.
2026-08-18 12:06:59 +00:00

8.4 KiB

ADR 0027: finishing Phase 6 — PAdES, external signing, OCSP, and a status line that stops lying

  • Status: Accepted
  • Date: 2026-08-18
  • Review item: the Phase 6 bullets that ADR 0025 and ADR 0026 left unimplemented while the plan's status line called the phase complete
  • Related: ADR 0025 (signing), ADR 0026 (the security review), ADR 0021 (the process note this ADR is the second violation of), ADR 0024 (encryption — the RC4 precedent for refusing a weak algorithm)

Context

Asked whether Phase 6 was 100% complete, I checked the plan's bullets against the code instead of answering from the status line. Five were not implemented, and the status line named none of them:

Bullet Actual state
"detached/attached signatures" /SubFilter hardcoded to adbe.pkcs7.detached at sign.rs:1401
"PAdES basics" ETSI.CAdES.detached existed only as a string in a name-mapping match
external_signing_test.dart absent — SigningIdentity requires an in-memory key
"OCSP/CRL lookup" CRL only; an OCSP response was counted, never parsed
Fulcio-style identity absent (the plan marks it optional)

This is the second time. ADR 0021 recorded the same failure in Phase 4 and wrote the rule meant to prevent it — enumerate criteria from the plan text first, then mark each done or explicitly deferred. I wrote that rule and then produced another prose status line summarising what I had built. A summary written from the work cannot show what the work omitted.

Decision

PAdES / CAdES, as a real profile rather than a label

SignatureProfile selects Pkcs7Detached or Cades, and the difference is structural, not cosmetic:

  • PKCS#7 detached signs the document bytes directly.
  • CAdES signs a set of signed attributes (RFC 5652 §5.3), one of which — messageDigest — carries the document's hash.

Two things follow, and both are load-bearing. First, the signature is computed over the attributes re-tagged as a SET (§5.4), not over the [0] IMPLICIT SEQUENCE they are carried in; getting that backwards produces a signature that verifies against nothing. Second, verification must check the messageDigest attribute against the document as well as verifying the attribute signature — otherwise a signature over somebody else's digest would be accepted. a_cades_signature_does_not_verify_against_ other_content is the test for exactly that.

/SubFilter now comes from the profile. A document claiming ETSI.CAdES.detached while carrying a plain PKCS#7 signature is malformed even though both halves are individually valid.

External signing

ExternalSigner is a trait: take bytes, return a signature, declare the algorithm and the certificate chain. A smartcard, HSM or cloud KMS never hands out its private key, so SigningIdentity — which owns one — could not represent any of them.

SigningIdentity implements the trait rather than sitting beside it, so there is one signing path and not two. A second path for hardware keys would be a second place the byte range could be computed differently, and that is the most dangerous divergence this module could have.

OCSP

ocsp_status_for_serial decodes the certID serial and the certStatus CHOICE tag directly with the der crate already present, rather than adding the ocsp crate for two fields. The audit surface of a dependency is a real cost against reading a tag byte.

Revoked from any response wins over Good from any other. The asymmetry between Revoked and Good from ADR 0026 stands and is now written down in the function's own documentation: neither CRLs nor OCSP responses are signature-checked, so Revoked is trustworthy — nobody forges a revocation against themselves — while Good means only "this is what the stapled material says".

Attached signatures: refused, not deferred

The two attached profiles are adbe.pkcs7.sha1 and adbe.x509.rsa_sha1. Both are SHA-1 based, and SHA-1 is broken for signature purposes.

They are parsed, so documents using them can be read and reported. They cannot be written: there is no SignatureProfile variant for either, so the refusal is enforced by the type rather than by a check. This is ADR 0024's RC4 decision applied to signatures — a library that offers a broken algorithm as an option will have it selected by somebody who does not know it is broken.

Recording this as refused rather than not done matters. "Not done" invites someone to finish it.

Verification

13 new tests in signing_profiles.rs, alongside the existing 42 across signing_roundtrip.rs, signing_security.rs and sign_document_roundtrip.rs.

The ones that carry weight:

  • a_cades_signature_does_not_verify_against_other_content — the messageDigest check. Without it, CAdES verification would accept a signature over a different document's hash.
  • a_cades_signature_is_structurally_different_from_pkcs7 — if the two profiles produced identical bytes, SignatureProfile would be a label with no effect, which is precisely what the hardcoded /SubFilter amounted to.
  • signing_a_document_as_cades_declares_the_cades_subfilter — asserts the document does not lie about its own profile.
  • an_external_signer_that_fails_is_reported_not_swallowed — an unplugged token must surface as an error; a signing call that quietly produces an unsigned document is the worst available outcome.
  • revoked_beats_good_across_several_responses — a certificate called good by one responder and revoked by another is revoked.

Mutation testing, four mutations, all killed on the first attempt:

Mutation Tests failed
messageDigest not compared with the document 1
CAdES verified against document bytes, not attributes 3
/SubFilter hardcoded again 1
OCSP revoked read as good 2

External: qpdf --check accepts documents signed under both profiles — no syntax or stream encoding errors.

Suite: TEST_TARGET=pdf 1289 passed, 0 failed (was 1276).

Merge criteria

Taken from the Phase 6 plan text, item by item — not from what was built:

  • Document-byte-range signing
  • CMS/PKCS#7 structure
  • Detached signatures
  • Attached signatures — refused by design, SHA-1 only, enforced by the absence of a SignatureProfile variant
  • PAdES basics — CAdES profile with signed attributes
  • RSA identities
  • ECC / Ed25519 identities
  • External signing (external_signing_test.dart)
  • PKIX certificate-chain validation
  • Signature appearance generation
  • Revocation: CRL
  • Revocation: OCSP
  • Permission flags
  • Encryption on save, AES-128 and AES-256
  • Exit: sign → verify round-trip
  • Exit: encrypted-write round-trip
  • Four mutations, all killed
  • Both profiles validate under qpdf
  • Fulcio-style identity — optional in the plan; needs an OIDC round trip and a CA call, which is network access the engine is gated against
  • Independent security review — ADR 0026 was a self-review
  • Acrobat interoperability — untested; Acrobat is stricter than the specification
  • Signing a document that already has an AcroForm — the form is replaced, not merged

Consequences

Positive. Every Phase 6 bullet is implemented, refused with a reason, or listed unticked above. PAdES makes the output usable in European eIDAS-adjacent workflows; ExternalSigner makes hardware-backed keys usable at all, which is how signing is actually done anywhere it matters legally.

Negative. SignatureProfile is a second thing a caller must choose correctly. The default (Pkcs7Detached) is the safe one, and choosing wrong produces a valid signature under the wrong profile rather than an insecure one.

Risk. The CAdES attribute set is minimal: contentType and messageDigest only. Full PAdES B-B additionally wants signingCertificateV2, which binds the signature to a specific certificate and closes a substitution attack that PKIX chain validation already covers here. Adding it is straightforward and is not claimed.

Process consequence. The status line is now the plan's own bullet list in a table, not prose. Prose invites summarising; a table with a row per spec item makes an omission visible as a missing row. Two wrong status lines in the same direction is a pattern, and the fix is structural rather than a resolution to be more careful.