From d74e60b2d59ac6af7263c4b6a3c3444e0851319d Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 14 Nov 2026 12:43:40 -0500 Subject: [PATCH] craft-pdf: refuse to create RC4 encryption without an explicit legacy call The create() API exposed RC4-40 and RC4-128 behind a doc comment, so an app could write files whose encryption protects nothing. create() now returns Unsupported for both schemes; create_legacy() and Document::set_encryption_legacy() are the explicit ways to write a file that must stay readable by an old reader. Tests and the encrypt example, which read and write every algorithm, use the legacy entry points. --- crates/pdf/README.md | 9 ++++-- crates/pdf/examples/cos_protect.rs | 3 +- crates/pdf/src/cos/document/open.rs | 16 ++++++++++- crates/pdf/src/crypt.rs | 28 +++++++++++++++++-- crates/pdf/src/crypt/tests.rs | 24 +++++++++++++++- crates/pdf/tests/cos/bounded_writer.rs | 3 +- crates/pdf/tests/cos/encryption.rs | 10 ++++--- crates/pdf/tests/cos/file_identifier.rs | 5 ++-- .../pdf/tests/crypt/permissions_revision.rs | 6 ++-- crates/pdf/tests/crypt/proptests.rs | 12 +++++--- 10 files changed, 94 insertions(+), 22 deletions(-) diff --git a/crates/pdf/README.md b/crates/pdf/README.md index 8cd2c3f25..210b79849 100644 --- a/crates/pdf/README.md +++ b/crates/pdf/README.md @@ -202,9 +202,12 @@ PDF files. The reader decrypts on its own and does not use it. It covers revisio block is ignored and bad padding is left in. - `permissions()` gives the `/P` flags; for R5/R6 a permission counts only when `/Perms` grants it too. Owners have every permission. -- `create(&NewEncryption, id0)` builds the `/Encrypt` values for RC4-40, RC4-128, AES-128 or - AES-256. The caller supplies 32 bytes of entropy (`seed`), so the module needs no OS access - and builds for the web. `file_id` derives a file identifier from the same seed. +- `create(&NewEncryption, id0)` builds the `/Encrypt` values for AES-128 or AES-256. It refuses + the legacy RC4-40 and RC4-128 schemes, because RC4 is broken (RFC 7465); `create_legacy` + writes those, for a file that must stay readable by an old reader. `Document::set_encryption` + refuses them too, with `set_encryption_legacy` as the explicit way in. The caller supplies 32 + bytes of entropy (`seed`), so the module needs no OS access and builds for the web. `file_id` + derives a file identifier from the same seed. - `EncryptDict` holds the dictionary's values. The caller reads them from, and writes them to, its own object model; the module does not parse PDF syntax. Keys and prepared passwords are zeroized when dropped. diff --git a/crates/pdf/examples/cos_protect.rs b/crates/pdf/examples/cos_protect.rs index c40bccdce..b12c700cf 100644 --- a/crates/pdf/examples/cos_protect.rs +++ b/crates/pdf/examples/cos_protect.rs @@ -22,7 +22,8 @@ fn main() { _ => craft_pdf::cos::Algorithm::Aes256, }; let mut doc = craft_pdf::cos::Document::open(std::sync::Arc::new(std::fs::read(input).unwrap())).unwrap(); - doc.set_encryption(&craft_pdf::cos::NewEncryption { + // `rc4-40` and `rc4-128` are the legacy schemes, which `set_encryption` refuses. + doc.set_encryption_legacy(&craft_pdf::cos::NewEncryption { algorithm, user_password: user, owner_password: owner, diff --git a/crates/pdf/src/cos/document/open.rs b/crates/pdf/src/cos/document/open.rs index 46a6423a9..4991412ce 100644 --- a/crates/pdf/src/cos/document/open.rs +++ b/crates/pdf/src/cos/document/open.rs @@ -414,7 +414,20 @@ impl Document { /// Protect the document with a password (§7.6.4). Takes effect on the next save, which is /// always a full rewrite. Returns the handler (authenticated as owner). + /// + /// The legacy RC4 schemes are refused (RC4 is broken, `RFC 7465`): [`Self::set_encryption_legacy`] + /// writes them for a file an old reader must open. pub fn set_encryption(&mut self, params: &crate::crypt::NewEncryption) -> Result<(), CosError> { + self.set_encryption_with(params, false) + } + + /// [`Self::set_encryption`] with the legacy RC4-40 and RC4-128 schemes allowed. Pass it only + /// for a file that must stay readable by a reader without AES. + pub fn set_encryption_legacy(&mut self, params: &crate::crypt::NewEncryption) -> Result<(), CosError> { + self.set_encryption_with(params, true) + } + + fn set_encryption_with(&mut self, params: &crate::crypt::NewEncryption, legacy: bool) -> Result<(), CosError> { // The file identifier is part of the key; make sure it exists and keep it. let id = self.trailer.get(b"ID").map(|id| self.resolve(id)); let id0 = match id.as_deref() { @@ -426,7 +439,8 @@ impl Document { id } }; - let h = crate::crypt::create(params, &id0).map_err(|e| CosError::Security(e.to_string()))?; + let built = if legacy { crate::crypt::create_legacy(params, &id0) } else { crate::crypt::create(params, &id0) }; + let h = built.map_err(|e| CosError::Security(e.to_string()))?; let d = h.dict(); let s = |b: &[u8]| Object::String(crate::cos::PdfString { bytes: b.to_vec(), hex: true }); let mut e = Dict::new(); diff --git a/crates/pdf/src/crypt.rs b/crates/pdf/src/crypt.rs index 069627db9..fff09b7a7 100644 --- a/crates/pdf/src/crypt.rs +++ b/crates/pdf/src/crypt.rs @@ -651,9 +651,9 @@ fn key_r6(d: &EncryptDict, pw: &[u8], owner: bool) -> Result bool { + matches!(self, Algorithm::Rc4_40 | Algorithm::Rc4_128) + } +} + /// A password as [`create`] prepares it: for R2–R4 its PDFDocEncoding (characters it lacks /// dropped), at most 32 bytes; for R6 its SASLprep (RFC 4013: soft hyphens and the like removed, /// other spaces made plain, then NFKC; the password as given if SASLprep refuses it), in UTF-8, @@ -725,7 +733,23 @@ pub fn file_id(seed: &[u8; 32]) -> Vec { } /// Build the `/Encrypt` values and an authenticated (owner) handler for a new encryption. +/// +/// The legacy RC4 schemes are refused, because RC4 is broken (`RFC 7465`) and a file written with +/// it protects nothing: [`create_legacy`] is the explicit way to write them. pub fn create(params: &NewEncryption, id0: &[u8]) -> Result { + if params.algorithm.is_legacy() { + return Err(CryptError::Unsupported("RC4 encryption is legacy (RFC 7465); use create_legacy for a file an old reader must open".into())); + } + create_any(params, id0) +} + +/// [`create`] without the RC4 gate: writes RC4-40 and RC4-128 too. For a file that must stay +/// readable by a reader without AES, and for tests that read and write every algorithm. +pub fn create_legacy(params: &NewEncryption, id0: &[u8]) -> Result { + create_any(params, id0) +} + +fn create_any(params: &NewEncryption, id0: &[u8]) -> Result { // R2 reserves bits 7-32; later revisions reserve bits 7-8 and 13-32. // Reserved bits must be 1, and bits 1-2 must be 0 (§7.6.4.2). let reserved = if params.algorithm == Algorithm::Rc4_40 { !0x3F } else { !0xFFF | 0xC0 }; diff --git a/crates/pdf/src/crypt/tests.rs b/crates/pdf/src/crypt/tests.rs index 04bb7c61e..59fc20e44 100644 --- a/crates/pdf/src/crypt/tests.rs +++ b/crates/pdf/src/crypt/tests.rs @@ -52,13 +52,35 @@ fn aes_padding_is_added_and_tolerantly_removed() { const ID: &[u8] = b"0123456789abcdef"; fn new(alg: Algorithm, user: &str, owner: &str, perms: i32) -> SecurityHandler { - create( + // These tests read and write every algorithm, RC4 included. + create_legacy( &NewEncryption { algorithm: alg, user_password: user, owner_password: owner, permissions: perms, encrypt_metadata: true, seed: [42; 32] }, ID, ) .unwrap() } +/// New files are not written with RC4: `create` refuses it, and the explicitly named +/// `create_legacy` is the only way to write it. +#[test] +fn create_refuses_rc4_unless_asked() { + let params = |algorithm| NewEncryption { + algorithm, + user_password: "user", + owner_password: "owner", + permissions: -1, + encrypt_metadata: true, + seed: [42; 32], + }; + for (alg, revision) in [(Algorithm::Rc4_40, 2), (Algorithm::Rc4_128, 3)] { + assert!(matches!(create(¶ms(alg), ID), Err(CryptError::Unsupported(_))), "{alg:?}"); + assert_eq!(create_legacy(¶ms(alg), ID).unwrap().dict().r, revision, "{alg:?}"); + } + for alg in [Algorithm::Aes128, Algorithm::Aes256] { + assert!(create(¶ms(alg), ID).is_ok(), "AES is not legacy: {alg:?}"); + } +} + const ALL: [Algorithm; 4] = [Algorithm::Rc4_40, Algorithm::Rc4_128, Algorithm::Aes128, Algorithm::Aes256]; #[test] diff --git a/crates/pdf/tests/cos/bounded_writer.rs b/crates/pdf/tests/cos/bounded_writer.rs index b8fe79370..5677ca542 100644 --- a/crates/pdf/tests/cos/bounded_writer.rs +++ b/crates/pdf/tests/cos/bounded_writer.rs @@ -139,7 +139,8 @@ fn full_rewrites_preserve_graph_across_multiple_object_streams_and_evictions() { } fn protect(doc: &mut Document, algorithm: Algorithm) { - doc.set_encryption(&NewEncryption { + // Every algorithm, RC4 included, has to survive a rewrite. + doc.set_encryption_legacy(&NewEncryption { algorithm, user_password: "user", owner_password: "owner", diff --git a/crates/pdf/tests/cos/encryption.rs b/crates/pdf/tests/cos/encryption.rs index 923b04e39..2ac53cb69 100644 --- a/crates/pdf/tests/cos/encryption.rs +++ b/crates/pdf/tests/cos/encryption.rs @@ -39,9 +39,11 @@ fn fixture_with(extra: &str) -> Vec { out } +/// Every algorithm, RC4-40 and RC4-128 included: those two are the legacy schemes, which +/// `set_encryption` refuses. fn protect(alg: Algorithm, user: &str, owner: &str, permissions: i32) -> Vec { let mut doc = Document::open(Arc::new(fixture())).unwrap(); - doc.set_encryption(&NewEncryption { + doc.set_encryption_legacy(&NewEncryption { algorithm: alg, user_password: user, owner_password: owner, @@ -287,7 +289,7 @@ fn signature_contents_stay_unencrypted() { c.set(b"Test".to_vec(), Object::Array(vec![Object::Ref(sig), Object::Ref(untyped), Object::Ref(note)])) }) .unwrap(); - doc.set_encryption(&NewEncryption { + doc.set_encryption_legacy(&NewEncryption { algorithm: alg, user_password: "", owner_password: "owner", @@ -423,7 +425,7 @@ fn an_earlier_revision_opens_with_the_documents_own_credentials() { // Another file, encrypted with other keys: the document's key is no use, and it says so. let other = { let mut doc = Document::open(Arc::new(fixture())).unwrap(); - doc.set_encryption(&NewEncryption { + doc.set_encryption_legacy(&NewEncryption { algorithm: alg, user_password: "user", owner_password: "owner", @@ -444,7 +446,7 @@ fn an_earlier_revision_opens_with_the_documents_own_credentials() { /// instead of as an indirect object. fn protect_inline(alg: Algorithm, user: &str, owner: &str) -> Vec { let mut doc = Document::open(Arc::new(fixture())).unwrap(); - doc.set_encryption(&NewEncryption { + doc.set_encryption_legacy(&NewEncryption { algorithm: alg, user_password: user, owner_password: owner, diff --git a/crates/pdf/tests/cos/file_identifier.rs b/crates/pdf/tests/cos/file_identifier.rs index 551db4ebd..5ca0f0f54 100644 --- a/crates/pdf/tests/cos/file_identifier.rs +++ b/crates/pdf/tests/cos/file_identifier.rs @@ -122,7 +122,8 @@ fn caller_entropy_makes_the_new_identifier_reproducible() { fn an_encrypted_rewrite_keeps_the_first_identifier_string_its_key_depends_on() { for algorithm in [Algorithm::Rc4_128, Algorithm::Aes128, Algorithm::Aes256] { let mut doc = reopen(source(true)); - doc.set_encryption(&NewEncryption { + // The loop covers RC4-128, which `set_encryption` refuses. + doc.set_encryption_legacy(&NewEncryption { algorithm, user_password: "user", owner_password: "owner", @@ -160,7 +161,7 @@ fn unusable_ids() -> Vec<(&'static str, Option)> { /// An encrypted file whose key is derived from the empty string, with its trailer `/ID` replaced by `id`. fn protected_with_id(algorithm: Algorithm, id: Option) -> Vec { let mut doc = reopen(source_with(Some("/ID [<> <>]"))); - doc.set_encryption(&NewEncryption { + doc.set_encryption_legacy(&NewEncryption { algorithm, user_password: "user", owner_password: "owner", diff --git a/crates/pdf/tests/crypt/permissions_revision.rs b/crates/pdf/tests/crypt/permissions_revision.rs index 06a5a5b98..a58868637 100644 --- a/crates/pdf/tests/crypt/permissions_revision.rs +++ b/crates/pdf/tests/crypt/permissions_revision.rs @@ -3,7 +3,7 @@ //! //! Bit n has the mask `1 << (n - 1)`. Readers use bits 3 to 6 and 9 to 12 and ignore the rest. -use craft_pdf::crypt::{Algorithm, Auth, NewEncryption, Permissions, SecurityHandler, create}; +use craft_pdf::crypt::{Algorithm, Auth, NewEncryption, Permissions, SecurityHandler, create_legacy}; const ID: &[u8] = b"0123456789abcdef"; const T: bool = true; @@ -18,7 +18,7 @@ fn cells(p: &Permissions) -> [bool; 8] { /// Creates a file with `flags` at revision `revision`, opens it with the user password, and /// returns the user's permissions. fn user_view(alg: Algorithm, revision: i64, flags: i32) -> Permissions { - let made = create( + let made = create_legacy( &NewEncryption { algorithm: alg, user_password: "user", owner_password: "owner", permissions: flags, encrypt_metadata: true, seed: [42; 32] }, ID, ) @@ -56,7 +56,7 @@ fn revision_three_rows_hold_through_a_created_and_reopened_file() { #[test] fn owner_access_gives_every_flag() { - let made = create( + let made = create_legacy( &NewEncryption { algorithm: Algorithm::Rc4_128, user_password: "user", diff --git a/crates/pdf/tests/crypt/proptests.rs b/crates/pdf/tests/crypt/proptests.rs index a81808ab9..7b9ede4d3 100644 --- a/crates/pdf/tests/crypt/proptests.rs +++ b/crates/pdf/tests/crypt/proptests.rs @@ -19,7 +19,7 @@ use craft_pdf::crypt::{ Algorithm, Auth, CryptError, Method, NewEncryption, PADDING, SecurityHandler, StreamKind, aes_cbc_decrypt, aes_cbc_encrypt, create, - effective_password, pdfdoc_byte, rc4, + create_legacy, effective_password, pdfdoc_byte, rc4, }; use proptest::prelude::*; @@ -92,8 +92,12 @@ fn expected_auth(algorithm: Algorithm, user_target: &[u8], owner_target: &[u8], } fn make(algorithm: Algorithm, user: &str, owner: &str) -> SecurityHandler { - create(&NewEncryption { algorithm, user_password: user, owner_password: owner, permissions: -1, encrypt_metadata: true, seed: [0x5e; 32] }, ID) - .unwrap() + // These properties read and write every algorithm, RC4 included. + create_legacy( + &NewEncryption { algorithm, user_password: user, owner_password: owner, permissions: -1, encrypt_metadata: true, seed: [0x5e; 32] }, + ID, + ) + .unwrap() } proptest! { @@ -457,7 +461,7 @@ proptest! { } // Revisions below 4 always encrypt metadata, whatever the caller asked for. - let legacy = create(&NewEncryption { algorithm: Algorithm::Rc4_128, user_password: "user", owner_password: "owner", permissions: -1, encrypt_metadata: false, seed: [0x5e; 32] }, ID).unwrap(); + let legacy = create_legacy(&NewEncryption { algorithm: Algorithm::Rc4_128, user_password: "user", owner_password: "owner", permissions: -1, encrypt_metadata: false, seed: [0x5e; 32] }, ID).unwrap(); prop_assert!(legacy.dict().encrypt_metadata && legacy.encrypts_metadata(), "V<4 always encrypts metadata, as create() documents"); }