From 06c4a970a242418e8147e7499417a586c0fd052c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?enes=20y=C4=B1ld=C4=B1z?= Date: Fri, 16 Oct 2026 08:11:00 +0000 Subject: [PATCH] =?UTF-8?q?fix(cli):=20info,=20text,=20render=20and=20audi?= =?UTF-8?q?t=20take=20the=20owner=20password=20of=20R2=E2=80=93R4=20files?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pdfcraft-cli info`, `text`, `render` and `audit` rejected the owner password of RC4-40, RC4-128 and AES-128 files with "the password is incorrect", while `run doc_open` accepted it (#2670, item 8). The renderer authenticates only the user password of revisions 2–4. `Session::open` works around that by handing it the user password that owner authentication recovers, but the CLI passed the typed password straight to `inspect` and `PageRenderer`. `pdfcraft_engine::render_password_for` now gives any render path outside a session the password the renderer takes, sharing the recovery with `Session::open`. The CLI uses it for the renderer and keeps the typed password for the engine session and the cos document, so the owner password still lifts copy restrictions in `text`. The render test decodes the PNG and checks its dimensions and that it has non-white pixels, for every algorithm with either password. Refs #2670 (item 8 only). Checks: cargo fmt --check; cargo clippy -p pdfcraft-cli -p pdfcraft-engine --all-targets -- -D warnings; cargo test -p pdfcraft-cli; cargo test -p pdfcraft-engine (and --lib); cargo xtask layers, assets, parity. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 1 + apps/pdfcraft-cli/Cargo.toml | 4 + apps/pdfcraft-cli/src/main.rs | 24 +++- apps/pdfcraft-cli/tests/owner_password.rs | 154 ++++++++++++++++++++++ crates/engine/src/lib.rs | 22 +++- crates/engine/src/tests.rs | 26 ++++ parity/acrobat-features.toml | 4 +- 7 files changed, 225 insertions(+), 10 deletions(-) create mode 100644 apps/pdfcraft-cli/tests/owner_password.rs diff --git a/Cargo.lock b/Cargo.lock index 91ed3c490..9f923a32a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4022,6 +4022,7 @@ dependencies = [ "pdfcraft-engine", "pdfcraft-platform", "pdfcraft-render", + "png 0.18.1", "rustix 1.1.5", "serde_json", "winresource", diff --git a/apps/pdfcraft-cli/Cargo.toml b/apps/pdfcraft-cli/Cargo.toml index d7b52fbd6..cf960a9a9 100644 --- a/apps/pdfcraft-cli/Cargo.toml +++ b/apps/pdfcraft-cli/Cargo.toml @@ -22,6 +22,10 @@ pdfcraft-automation = { workspace = true } pdfcraft-platform = { workspace = true } base64 = "0.22" +[dev-dependencies] +# Decodes `render` output in tests/owner_password.rs; already a dependency of the engine. +png = "0.18" + [target.'cfg(unix)'.dependencies] # `geteuid` for `ui`'s control-file owner check: safe, so no `unsafe` libc call here. # Already in Cargo.lock through the GUI crates. diff --git a/apps/pdfcraft-cli/src/main.rs b/apps/pdfcraft-cli/src/main.rs index 0e24800d3..69c4371c0 100644 --- a/apps/pdfcraft-cli/src/main.rs +++ b/apps/pdfcraft-cli/src/main.rs @@ -175,7 +175,12 @@ fn audit(args: &[String]) -> Result<(), CliError> { let findings = pdfcraft_audit::audit_redactions(&doc).map_err(|e| format!("{path}: {e}"))?; // Rectangles in points from the top-left of the displayed page, as `doc_audit_redactions` // reports them; the page geometry is read only when there is something to place. - let info = if findings.is_empty() { None } else { Some(inspect(bytes, password).map_err(|e| format!("{path}: {e}"))?) }; + let info = if findings.is_empty() { + None + } else { + let render_password = pdfcraft_engine::render_password_for(bytes.clone(), password); + Some(inspect(bytes, render_password.as_deref()).map_err(|e| format!("{path}: {e}"))?) + }; let items: Vec = findings .into_iter() .map(|f| { @@ -298,7 +303,9 @@ fn info(args: &[String]) -> Result<(), CliError> { let path = *positional(args).first().ok_or("info: missing file")?; let bytes = read(path)?; let password = flag(args, "--password"); - let info = inspect(bytes.clone(), password).map_err(|e| e.to_string())?; + // The renderer takes only the user password of R2–R4 files; `Session::open` takes either. + let render_password = pdfcraft_engine::render_password_for(bytes.clone(), password); + let info = inspect(bytes.clone(), render_password.as_deref()).map_err(|e| e.to_string())?; // Dynamic XFA forms: say what laying the template out gives (what the app shows). let xfa_layout = (info.xfa == Some(pdfcraft_render::Xfa::Dynamic)) .then(|| { @@ -349,12 +356,14 @@ fn text(args: &[String]) -> Result<(), CliError> { } let password = flag(args, "--password"); let bytes = read(path)?; - let mut r = PageRenderer::new(bytes.clone(), RenderConfig { password: password.map(Arc::from), ..Default::default() }); + // The renderer takes only the user password of R2–R4 files; `Session::open` takes either. + let render_password = pdfcraft_engine::render_password_for(bytes.clone(), password); + let mut r = PageRenderer::new(bytes.clone(), RenderConfig { password: render_password.as_deref().map(Arc::from), ..Default::default() }); // A document the renderer could not open (wrong or missing password, unparseable file) has // no pages, and "extract every page" of nothing would print nothing and exit 0 (#132). Ask // `inspect` why instead, so a protected file fails the same way `info` does. if r.page_count() == 0 { - let info = inspect(bytes.clone(), password).map_err(|e| format!("text: {e}"))?; + let info = inspect(bytes.clone(), render_password.as_deref()).map_err(|e| format!("text: {e}"))?; if !info.pages.is_empty() { return Err("text: the document could not be parsed".into()); } @@ -588,9 +597,12 @@ fn render(args: &[String]) -> Result<(), CliError> { // The app renders a display copy that draws comments missing their /AP streams; the CLI // must agree with it (and with independent viewers), while keeping the file it reads intact. let bytes = read(path)?; + let password = flag(args, "--password"); + // The renderer takes only the user password of R2–R4 files; the display copy takes either. + let render_password = pdfcraft_engine::render_password_for(bytes.clone(), password); let mut r = PageRenderer::new( - pdfcraft_engine::display_bytes_for(bytes.clone(), flag(args, "--password")), - RenderConfig { password: flag(args, "--password").map(Arc::from), reject_oversize: true, ..Default::default() }, + pdfcraft_engine::display_bytes_for(bytes.clone(), password), + RenderConfig { password: render_password.as_deref().map(Arc::from), reject_oversize: true, ..Default::default() }, ); let p = r.render(RenderRequest { page: page_index, kind: RequestKind::Pixels, tile: None, scale: dpi / 72.0, tag: 0, preview: false }); if let Some(e) = p.error { diff --git a/apps/pdfcraft-cli/tests/owner_password.rs b/apps/pdfcraft-cli/tests/owner_password.rs new file mode 100644 index 000000000..3542a7d4c --- /dev/null +++ b/apps/pdfcraft-cli/tests/owner_password.rs @@ -0,0 +1,154 @@ +//! `info`, `text` and `render` take the owner password of revision 2–4 files, as `run doc_open` +//! does (#2670, item 8). The renderer authenticates only user passwords of those revisions, so +//! the CLI must hand it the user password that owner authentication recovers. + +use std::process::Command; +use std::sync::Arc; + +use pdfcraft_cos::{Algorithm, NewEncryption}; + +const BIN: &str = env!("CARGO_BIN_EXE_pdfcraft-cli"); + +/// A one-page 200×300 pt PDF reading "Page 1", drawn by `content`. +fn fixture(content: &str) -> Vec { + let stream = format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()); + let objs: [&[u8]; 5] = [ + b"<< /Type /Catalog /Pages 2 0 R >>", + b"<< /Type /Pages /Kids [4 0 R] /Count 1 /MediaBox [0 0 200 300] >>", + b"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>", + b"<< /Type /Page /Parent 2 0 R /Contents 5 0 R /Resources << /Font << /F1 3 0 R >> >> >>", + stream.as_bytes(), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n", i + 1).as_bytes()); + out.extend_from_slice(o); + out.extend_from_slice(b"\nendobj\n"); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out +} + +const PAGE: &str = "BT /F1 24 Tf 20 150 Td (Page 1) Tj ET"; + +/// `fixture` protected with user password "user" and owner password "owner", allowing nothing, +/// so copying its text needs the owner password. +fn protected(algorithm: Algorithm) -> Vec { + protected_with(algorithm, PAGE) +} + +fn protected_with(algorithm: Algorithm, content: &str) -> Vec { + let mut doc = pdfcraft_cos::Document::open(Arc::new(fixture(content))).unwrap(); + doc.set_encryption(&NewEncryption { + algorithm, + user_password: "user", + owner_password: "owner", + permissions: 0, + encrypt_metadata: true, + seed: [3; 32], + }) + .unwrap(); + pdfcraft_cos::write_full(&doc, &Default::default()).unwrap() +} + +fn tmp(name: &str) -> std::path::PathBuf { + let d = std::env::temp_dir().join(format!("pdfcraft-cli-owner-{}", std::process::id())); + std::fs::create_dir_all(&d).unwrap(); + d.join(name) +} + +const ALGORITHMS: [Algorithm; 4] = [Algorithm::Rc4_40, Algorithm::Rc4_128, Algorithm::Aes128, Algorithm::Aes256]; + +#[test] +fn info_accepts_the_owner_password() { + for alg in ALGORITHMS { + let path = tmp(&format!("info-{alg:?}.pdf")); + std::fs::write(&path, protected(alg)).unwrap(); + for pw in ["owner", "user"] { + let out = Command::new(BIN).args(["info", path.to_str().unwrap(), "--password", pw]).output().unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success(), "{alg:?} with {pw:?}: {stderr}"); + let json: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap(); + assert_eq!(json["pages"], 1, "{alg:?} with {pw:?}"); + assert_eq!(json["encrypted"], true, "{alg:?} with {pw:?}"); + } + let out = Command::new(BIN).args(["info", path.to_str().unwrap(), "--password", "wrong"]).output().unwrap(); + assert!(!out.status.success(), "{alg:?}: a wrong password still fails"); + assert!(String::from_utf8_lossy(&out.stderr).contains("incorrect"), "{alg:?}"); + } +} + +#[test] +fn text_accepts_the_owner_password() { + for alg in ALGORITHMS { + let path = tmp(&format!("text-{alg:?}.pdf")); + std::fs::write(&path, protected(alg)).unwrap(); + let out = Command::new(BIN).args(["text", path.to_str().unwrap(), "--password", "owner"]).output().unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success(), "{alg:?}: {stderr}"); + assert!(String::from_utf8_lossy(&out.stdout).contains("Page 1"), "{alg:?}"); + // The user password opens it, but the permissions still forbid copying. + let out = Command::new(BIN).args(["text", path.to_str().unwrap(), "--password", "user"]).output().unwrap(); + assert!(!out.status.success(), "{alg:?}: copying needs the owner password"); + assert!(String::from_utf8_lossy(&out.stderr).contains("don't allow copying"), "{alg:?}"); + } +} + +/// Asserts that `data` is a PNG of the fixture's 200×300 pt page at `dpi` with the text drawn: +/// a blank page of the right size fails too. +fn assert_rendered_page(data: &[u8], dpi: u32, what: &str) { + let mut reader = png::Decoder::new(std::io::Cursor::new(data)).read_info().unwrap_or_else(|e| panic!("{what}: {e}")); + let mut buf = vec![0; reader.output_buffer_size().unwrap()]; + let frame = reader.next_frame(&mut buf).unwrap(); + assert_eq!((frame.width, frame.height), (200 * dpi / 72, 300 * dpi / 72), "{what}"); + // `render` writes 8-bit RGBA (`pdfcraft_engine::export::encode_png`). + assert_eq!((frame.color_type, frame.bit_depth), (png::ColorType::Rgba, png::BitDepth::Eight), "{what}"); + // Opaque pixels with every colour channel below half intensity: ink, not white paper. + let inked = buf[..frame.buffer_size()].as_chunks::<4>().0.iter().filter(|[r, g, b, a]| [r, g, b].iter().all(|c| **c < 128) && *a > 128).count(); + assert!(inked > 100, "{what}: only {inked} inked pixels, the page is blank"); +} + +#[test] +fn render_accepts_the_owner_password() { + const DPI: u32 = 144; + for alg in ALGORITHMS { + let path = tmp(&format!("render-{alg:?}.pdf")); + std::fs::write(&path, protected(alg)).unwrap(); + for pw in ["owner", "user"] { + let png = tmp(&format!("render-{alg:?}-{pw}.png")); + let _ = std::fs::remove_file(&png); + let out = Command::new(BIN) + .args(["render", path.to_str().unwrap(), "--page", "1", "--dpi", &DPI.to_string()]) + .args(["--password", pw, "--out", png.to_str().unwrap()]) + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success(), "{alg:?} with {pw:?}: {stderr}"); + assert_rendered_page(&std::fs::read(&png).unwrap(), DPI, &format!("{alg:?} with {pw:?}")); + } + } +} + +#[test] +fn audit_places_findings_with_the_owner_password() { + // A black box over the text: a faux redaction, so `audit` reads the page geometry too. + let covered = format!("{PAGE} 0 g 10 140 120 40 re f"); + for alg in ALGORITHMS { + let path = tmp(&format!("audit-{alg:?}.pdf")); + std::fs::write(&path, protected_with(alg, &covered)).unwrap(); + let out = Command::new(BIN).args(["audit", path.to_str().unwrap(), "--password", "owner"]).output().unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(!stderr.contains("incorrect"), "{alg:?}: {stderr}"); + let report: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap_or_else(|e| panic!("{alg:?}: {e}: {stderr}")); + assert_eq!(report["findings"][0]["covered_text"], "Page 1", "{alg:?}"); + // Top-left points on the 300 pt high page: the box spans y 140–180 from the bottom. + assert_eq!(report["findings"][0]["rect"], serde_json::json!([10.0, 120.0, 130.0, 160.0]), "{alg:?}"); + } +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 3432cd868..323238e44 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -912,6 +912,24 @@ pub fn display_bytes_for(bytes: Arc>, password: Option<&str>) -> Arc>, password: Option<&str>) -> Option { + let password = password?; + let recovered = + guard(|| pdfcraft_cos::Document::open_with_password(bytes, Some(password)).ok().and_then(|d| recovered_render_password(&d))).ok().flatten(); + Some(recovered.unwrap_or_else(|| password.to_owned())) +} + +/// The user password owner authentication recovered from an R2–R4 `cos`, as the renderer takes it. +fn recovered_render_password(cos: &pdfcraft_cos::Document) -> Option { + cos.security().and_then(|s| s.recovered_user_password()).map(|user| renderer_password(&user)) +} + /// Fails when the renderer still draws text under an applied redaction mark. /// /// Redaction decides what lies under a mark from the positions its own interpreter computes, and @@ -3343,10 +3361,10 @@ impl Session { Ok(info) => (info, password.map(str::to_owned)), Err(OpenError::WrongPassword) => { let user = match &cos { - Ok(Ok(d)) => d.security().and_then(|s| s.recovered_user_password()), + Ok(Ok(d)) => recovered_render_password(d), _ => None, }; - let user = renderer_password(&user.ok_or(OpenError::WrongPassword)?); + let user = user.ok_or(OpenError::WrongPassword)?; (inspect(bytes.clone(), Some(&user))?, Some(user)) } Err(e) => return Err(e), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ec305b1ca..0e0584bab 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1230,6 +1230,32 @@ fn owner_password_of_older_revisions_opens_the_viewer_too() { } } +/// Render paths outside a session (the CLI's `info`, `text`, `render`) get the password the +/// renderer takes: the recovered user password for an R2–R4 owner password, else as given. +#[test] +fn render_password_for_recovers_the_user_password_of_older_revisions() { + for alg in [pdfcraft_cos::Algorithm::Rc4_40, pdfcraft_cos::Algorithm::Rc4_128, pdfcraft_cos::Algorithm::Aes128, pdfcraft_cos::Algorithm::Aes256] { + let mut doc = pdfcraft_cos::Document::open(Arc::new(fixture(1))).unwrap(); + doc.set_encryption(&pdfcraft_cos::NewEncryption { + algorithm: alg, + user_password: "u", + owner_password: "o", + permissions: 0, + encrypt_metadata: true, + seed: [1; 32], + }) + .unwrap(); + let bytes = Arc::new(pdfcraft_cos::write_full(&doc, &Default::default()).unwrap()); + let owner = crate::render_password_for(bytes.clone(), Some("o")); + let expected = if alg == pdfcraft_cos::Algorithm::Aes256 { "o" } else { "u" }; + assert_eq!(owner.as_deref(), Some(expected), "{alg:?}"); + assert!(pdfcraft_render::inspect(bytes.clone(), owner.as_deref()).is_ok(), "{alg:?}: the renderer opens it"); + assert_eq!(crate::render_password_for(bytes.clone(), Some("u")).as_deref(), Some("u"), "{alg:?}"); + assert_eq!(crate::render_password_for(bytes.clone(), Some("wrong")).as_deref(), Some("wrong"), "{alg:?}"); + assert_eq!(crate::render_password_for(bytes, None), None, "{alg:?}"); + } +} + #[test] fn autosave_snapshots_only_changed_documents() { let (mut s, id) = session_with(2); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index c3d319f94..97d57d4fc 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -202,8 +202,8 @@ milestone = "M1" status = "shipped" commands = ["protect.unlock"] tools = ["doc_open"] -evidence = ["crates/engine/src/tests.rs::restricted_documents_refuse_changes_unless_opened_by_the_owner", "crates/engine/src/tests.rs::owner_password_of_older_revisions_opens_the_viewer_too", "crates/crypt/src/tests.rs::owner_authentication_recovers_the_user_password", "crates/engine/src/tests.rs::the_permissions_password_unlocks_an_open_document", "crates/ui-egui/tests/editing.rs::the_security_notice_unlocks_with_the_permissions_password", "crates/ui-egui/tests/editing.rs::remove_security_asks_for_the_permissions_password_first"] -notes = "at open (the password prompt takes either password), or after opening a restricted file that needed no password (#995, #1077): Unlock… on the security notice and in Properties ▸ Security, and changing security asks for it first. Not yet: an automation tool to unlock an open document (reopen with doc_open and the password instead)" +evidence = ["crates/engine/src/tests.rs::restricted_documents_refuse_changes_unless_opened_by_the_owner", "crates/engine/src/tests.rs::owner_password_of_older_revisions_opens_the_viewer_too", "crates/engine/src/tests.rs::render_password_for_recovers_the_user_password_of_older_revisions", "apps/pdfcraft-cli/tests/owner_password.rs::info_accepts_the_owner_password", "apps/pdfcraft-cli/tests/owner_password.rs::text_accepts_the_owner_password", "crates/crypt/src/tests.rs::owner_authentication_recovers_the_user_password", "crates/engine/src/tests.rs::the_permissions_password_unlocks_an_open_document", "crates/ui-egui/tests/editing.rs::the_security_notice_unlocks_with_the_permissions_password", "crates/ui-egui/tests/editing.rs::remove_security_asks_for_the_permissions_password_first"] +notes = "at open (the password prompt takes either password), or after opening a restricted file that needed no password (#995, #1077): Unlock… on the security notice and in Properties ▸ Security, and changing security asks for it first. The CLI's info, text, render and audit take either password too (#2670). Not yet: an automation tool to unlock an open document (reopen with doc_open and the password instead)" [[feature]] id = "core.permissions-enforced"