andodeki
147ca7de23
Closes the gaps in ADR 0005's own merge criteria. The encryption commit shipped with two of its stated criteria unmet, which an audit of the ADR checklist against the corpus caught: - "An AES-256 (/R 6) document opens likewise" had no fixture. The revision 6 key derivation and the AES-256 stream path were implemented and their helpers unit-tested, but neither had ever decrypted a real file. That is exactly the "asserting Ok proves nothing" trap the same ADR warns about, since a key-derivation error can produce plausible output for one algorithm and garbage for another. - "Malformed encrypted fixtures never panic" had no malformed fixtures at all; only well-formed documents were covered. New fixtures, generated by the checked-in script from the specification so they test agreement with the spec rather than with the reader: - encrypted/aes256.pdf, a /V 5 /R 6 document with an empty user password, full /U, /UE, /O and /OE entries and an AESV3 crypt filter. It decrypts, so derive_key_r6, the iterated SHA-256/384/512 hash and the zero-IV unwrap of /UE are now proven end to end rather than in isolation. - encrypted/truncated_u.pdf, a /U shorter than the 48 bytes revision 6 requires, which must be reported rather than indexed past the end. - encrypted/missing_o.pdf, an /Encrypt dictionary with no /O. - encrypted/absurd_length.pdf, a /Length of 999999 bits, which must clamp rather than panic or over-index. All four behave correctly: the AES-256 document decrypts to its marker, and the three malformed ones are refused with typed errors naming the offending entry. Encryption tests go from 13 to 17. The ADR's merge criteria are now ticked, with a note recording that the original commit shipped with the revision 6 path unproven and that this follow-up is what closed it. Not done here: the decrypt fuzz target could not be re-run. The nightly toolchain now available (2026-07-27) fails to build the cc crate that libfuzzer depends on, with errors inside cc itself rather than in this code. The target still compiles under stable and the earlier run of 1,953,940 executions stands; this is recorded rather than quietly skipped. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh (387 tests) Both rustfmt and clippy -D warnings clean.
133 lines
5.8 KiB
Markdown
133 lines
5.8 KiB
Markdown
# ADR 0005: PDF encryption — audited crates, decrypt-on-read, never write
|
||
|
||
- **Status:** Accepted
|
||
- **Date:** 2026-07-28
|
||
- **Review item:** Phase 8, `DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md`
|
||
- **Supersedes:** nothing
|
||
- **Related:** ADR 0003 (incremental save refuses encrypted documents)
|
||
|
||
## Context
|
||
|
||
An encrypted PDF today does something worse than fail: it **succeeds**.
|
||
|
||
Probing a structurally valid RC4-encrypted file through the current parser:
|
||
|
||
```
|
||
parsed OK: pages=1
|
||
page 0 content bytes=44
|
||
content parsed into 0 ops
|
||
```
|
||
|
||
No error, no warning. The document reports one page, the page reports
|
||
content, and the content interprets to nothing because it is ciphertext. The
|
||
user sees a blank page and is told the file is fine.
|
||
|
||
That is precisely the defect class Phase 0 existed to remove — an API
|
||
claiming a capability it does not deliver — and it is the worst remaining
|
||
example in the PDF stack, because it is silent.
|
||
|
||
The gap analysis lists **Encryption (RC4/AES-128/256)** with prerequisites
|
||
"real crypto crates (`ring`, `aes`), byte-range tests". Phase 0 also
|
||
explicitly deleted the previous hand-rolled MD5/SHA/AES/RC4 implementation,
|
||
calling it a "CVE factory", and required that it be replaced with audited
|
||
crates when signatures or encryption were actually needed. That time is now.
|
||
|
||
## Decision
|
||
|
||
Implement the **standard security handler** (PDF 32000-1 §7.6) for reading,
|
||
using audited RustCrypto crates, and never write encryption.
|
||
|
||
### Dependencies
|
||
|
||
`aes`, `cbc`, `rc4`, `md-5`, `sha2` — all RustCrypto, all MIT/Apache-2.0,
|
||
which `deny.toml` already permits. No hand-rolled primitives: the repository
|
||
has a standing rule against them and a documented history of getting it
|
||
wrong.
|
||
|
||
These are added to `pdf-cos` only. The crate already parses untrusted bytes;
|
||
adding decryption there keeps the trust boundary in one place rather than
|
||
spreading key material through the document and graphics layers.
|
||
|
||
### Supported
|
||
|
||
- **V1/R2** — RC4 40-bit.
|
||
- **V2/R3** — RC4 40–128-bit.
|
||
- **V4/R4** — crypt filters selecting RC4 or AES-128 (`/AESV2`).
|
||
- **V5/R6** — AES-256 (`/AESV3`), the SHA-256-based revision 6 algorithm.
|
||
|
||
Both the **empty user password** (the overwhelmingly common case: a document
|
||
encrypted only to set permissions) and an explicitly supplied user or owner
|
||
password.
|
||
|
||
### Not supported, and refused rather than guessed
|
||
|
||
- **Public-key security handlers** (`/Filter /Adobe.PubSec`). Certificate
|
||
handling is a separate problem.
|
||
- Any non-standard `/Filter`. A handler this code does not implement means
|
||
the bytes cannot be trusted, so the document is refused.
|
||
- **Writing encryption.** ADR 0003 already refuses to save an encrypted
|
||
document; this ADR keeps that refusal rather than relaxing it. Saving a
|
||
decrypted document as plaintext would silently strip the protection its
|
||
author applied, which is a security decision no library should make on a
|
||
user's behalf.
|
||
|
||
### Permissions are reported, not enforced
|
||
|
||
`/P` permission bits are parsed and exposed. They are **not** enforced by
|
||
this crate, and the code says so. Permission bits are advisory in any reader
|
||
that has already decrypted the content — enforcing them here would give a
|
||
false impression of security while a caller can read the plaintext anyway.
|
||
The honest position is to surface them and let the host decide.
|
||
|
||
## Non-negotiable rules
|
||
|
||
1. **No hand-rolled cryptography.** Standing repository rule; Phase 0 deleted
|
||
the previous attempt for cause.
|
||
2. **A document whose handler is unsupported is refused**, never partially
|
||
decrypted. A blank page is what the current bug produces and is exactly
|
||
what must not happen.
|
||
3. **A wrong password is a typed error**, distinguishable from a corrupt
|
||
file, so a caller can prompt again rather than reporting damage.
|
||
4. **Decryption is per object.** The key is derived once; each string and
|
||
stream is decrypted with its own object and generation number, as the
|
||
spec requires. Reusing one keystream across objects is a real break.
|
||
5. **The `/Encrypt` dictionary itself is never decrypted**, and neither are
|
||
the strings inside it. Doing so corrupts the key derivation.
|
||
6. **Saving stays refused.** ADR 0003's `SaveError::Encrypted` remains.
|
||
|
||
## Merge criteria
|
||
|
||
- [x] An RC4-encrypted document with an empty user password opens and its
|
||
content stream decrypts to real operators.
|
||
- [x] An AES-128 (`/AESV2`) document opens likewise.
|
||
- [x] An AES-256 (`/R 6`) document opens likewise.
|
||
- [x] A wrong password returns `WrongPassword`, not a corrupt-file error.
|
||
- [x] An unsupported handler returns `UnsupportedHandler` naming it.
|
||
- [x] Strings as well as streams are decrypted.
|
||
- [x] Permission bits are parsed and exposed.
|
||
- [x] Corpus fixtures for each of the above, generated by the checked-in
|
||
script.
|
||
- [x] Malformed encrypted fixtures never panic.
|
||
- [x] Saving an encrypted document is still refused.
|
||
- [x] `TEST_TARGET=pdf ./tools/test-rust-clean.sh` passes, rustfmt and
|
||
clippy `-D warnings` clean.
|
||
|
||
All criteria met as of the follow-up commit that added the AES-256 and
|
||
malformed-encryption fixtures. The original encryption commit shipped with
|
||
the revision 6 code path unit-tested but never run against a real file,
|
||
which is the gap that follow-up closed.
|
||
|
||
## Consequences
|
||
|
||
**Positive.** The silent-blank-page bug is gone. A large class of real-world
|
||
documents — anything encrypted merely to set print permissions — becomes
|
||
readable. The crypto dependency is confined to one crate behind one module.
|
||
|
||
**Negative.** Five new dependencies in a crate that previously had one. They
|
||
are audited and permissively licensed, but they are still supply chain.
|
||
|
||
**Risk.** The likeliest defect is a key-derivation error that happens to
|
||
produce plausible output for one algorithm and garbage for another. The
|
||
mitigation is a fixture per algorithm whose decrypted content is asserted to
|
||
contain known operators, rather than merely asserting that parsing returned
|
||
`Ok`.
|