Merge pull request #4391 from Silvertree2010/fix/3605-audit-pdf-encryption-file-key-sa

Seed PDF encryption keys from the OS random source

Cargo.lock

@@ -6325,6 +6325,7 @@ dependencies = [
  "chrono",
  "craft-pdf",
  "flate2",
+ "getrandom 0.4.3",
  "image",
  "jpeg-encoder",
  "krilla",

crates/pdf/Cargo.toml

@@ -34,6 +34,12 @@ roxmltree = "0.21"
 aes = "0.8"
 md-5 = { version = "0.10", default-features = false }
 sha2 = { version = "0.10", default-features = false }
+# The seed of the file key, salts and IVs (encrypt.rs).
+getrandom = "0.4"
+
+[target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dependencies]
+# The browser's crypto.getRandomValues.
+getrandom = { version = "0.4", features = ["wasm_js"] }
 
 [dev-dependencies]
 vectorcraft-testkit = { workspace = true }

crates/pdf/src/encrypt.rs

@@ -300,20 +300,18 @@ struct Entropy {
 }
 
 impl Entropy {
-    /// Seeded with the process's random hash keys (none on the web), the time and `material`.
-    fn new(material: &[&[u8]]) -> Self {
-        use std::hash::BuildHasher;
-        let state = std::hash::RandomState::new();
+    /// Seeded from the OS random source, with the time and `material` mixed in.
+    fn new(material: &[&[u8]]) -> Result<Self, PdfError> {
+        let mut os = [0u8; 32];
+        getrandom::fill(&mut os).map_err(|e| failed(&format!("no random numbers ({e})")))?;
         let mut h = Sha256::new();
-        for i in 0u8..4 {
-            h.update(state.hash_one(i).to_le_bytes());
-        }
+        h.update(os);
         h.update(vectorcraft_doc::metadata::now_unix().unwrap_or_default().to_le_bytes());
         for m in material {
             h.update((m.len() as u64).to_le_bytes());
             h.update(m);
         }
-        Self { seed: h.finalize().into(), counter: 0 }
+        Ok(Self { seed: h.finalize().into(), counter: 0 })
     }
 
     fn bytes<const N: usize>(&mut self) -> [u8; N] {
@@ -598,7 +596,7 @@ pub(crate) fn protect_keeping(pdf: Vec<u8>, set: &PdfSettings) -> Result<(Vec<u8
         Some(Obj::Ref(n, _)) => Some(*n),
         _ => None,
     };
-    let mut rng = Entropy::new(&[sec.open_password.as_bytes(), sec.permissions_password.as_bytes(), &pdf]);
+    let mut rng = Entropy::new(&[sec.open_password.as_bytes(), sec.permissions_password.as_bytes(), &pdf])?;
     let (id, new_id) = match trailer.first_string(b"ID") {
         Some(id) => (id.to_vec(), None),
         None => {

crates/pdf/src/tests/encrypt.rs

@@ -218,3 +218,27 @@ fn damaged_files_never_panic_when_reading_the_owner_password() {
         assert_eq!(crate::encrypt::user_password(junk, "own"), None);
     }
 }
+
+#[test]
+fn the_same_file_and_passwords_get_fresh_keys_each_time() {
+    // Same document, same passwords, same second: the salts and the file key still differ.
+    let mut opts = PdfOptions::default();
+    opts.settings.compatibility = Compatibility::Pdf20;
+    let plain = export(&doc(), &opts).unwrap();
+    opts.settings.security = security("pw", "own");
+    let a = protect(plain.clone(), &opts.settings).unwrap();
+    let b = protect(plain, &opts.settings).unwrap();
+    assert_ne!(encrypt_dict(&a), encrypt_dict(&b));
+    for bytes in [&a, &b] {
+        assert_eq!(shapes_of(bytes, Some("pw")), 1);
+        assert_eq!(shapes_of(bytes, Some("own")), 1);
+    }
+}
+
+#[test]
+fn the_file_key_is_seeded_from_the_os_random_source() {
+    // `RandomState` is no CSPRNG and its keys are fixed on the web: the seed must come from the OS.
+    let src = include_str!("../encrypt.rs");
+    assert!(!src.contains("RandomState"), "encrypt.rs seeds its keys from RandomState");
+    assert!(src.contains("getrandom::fill"), "encrypt.rs doesn't seed its keys from the OS random source");
+}