Skip to content

der: require a SEQUENCE in Document::decode - #2442

Merged
tarcieri merged 2 commits into
RustCrypto:masterfrom
yuxi-liu-wired:fix/document-decode-requires-sequence
Oct 5, 2026
Merged

tarcieri merged 2 commits into
RustCrypto:masterfrom
yuxi-liu-wired:fix/document-decode-requires-sequence

Conversation

@yuxi-liu-wired

Copy link
Copy Markdown
Contributor

Document is documented as holding "a valid DER-encoded SEQUENCE", and TryFrom<Vec<u8>> checks that through decode_sequence. But Decode for Document, which Document::from_der and TryFrom<&[u8]> use, only peeks the header and reads that many bytes, whatever the tag. So the result depends on which constructor is called:

let int = [0x02, 0x01, 0x00]; // INTEGER 0

Document::from_der(&int);           // Ok
Document::try_from(&int[..]);       // Ok
Document::try_from(int.to_vec());   // Err: expected SEQUENCE, got INTEGER
SecretDocument::try_from(&int[..]); // Ok

A document accepted this way can't be read back from its own output. doc.to_pem(..) succeeds, and Document::from_pem on that PEM fails because it goes through TryFrom<Vec<u8>>. The same applies to read_der_file and encode_msg, which also use the Vec path.

Fix: Decode for Document asserts the SEQUENCE tag, like decode_sequence does. All constructors now agree. Every in-tree caller (spki, pkcs1, pkcs8, sec1) creates documents from SEQUENCE types, so none of them change.

tests/document.rs checks that from_der, TryFrom<&[u8]> and TryFrom<Vec<u8>> all reject INTEGER, OCTET STRING, SET and [0] inputs and all accept a SEQUENCE. It fails on master. cargo hack test --feature-powerset passes for der on stable and on 1.85 (MSRV), as do the test suites of spki, pkcs1, pkcs8, sec1, x509-cert, pkcs12 and cms. The thumbv7em-none-eabi powerset build succeeds, and cargo fmt --check and cargo +1.90.0 clippy --all-features --tests are clean.

Found by fuzzing der's own round-trip promises (DER -> Document -> PEM -> Document). This PR was produced by AI agents (Claude), from the fuzzing and the fix through to this description.

`Document` is documented as holding "a valid DER-encoded `SEQUENCE`",
and `TryFrom<Vec<u8>>` enforces that via `decode_sequence`. But
`Decode for Document`, used by `Document::from_der` and
`TryFrom<&[u8]>`, only peeks the header and reads that many bytes,
whatever the tag. So `Document::from_der(&[0x02, 0x01, 0x00])` (an
INTEGER) succeeds while `Document::try_from(vec![0x02, 0x01, 0x00])`
fails, and a document built from such bytes cannot be read back from
its own `to_pem` output, because `from_pem` goes through the `Vec`
path.

Assert the SEQUENCE tag in `Decode for Document`, as `decode_sequence`
does, so all constructors agree. Adds `tests/document.rs`.

Found by fuzzing der's DER -> Document -> PEM -> Document round trip.
Comment thread der/tests/document.rs Outdated
@tarcieri
tarcieri merged commit b5ce858 into RustCrypto:master Oct 5, 2026
190 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants