From 725d33939ec2696dc654e8706fbc32b06980f00a Mon Sep 17 00:00:00 2001 From: ddidderr Date: Tue, 9 Jun 2026 23:46:27 +0200 Subject: [PATCH] fix: read key files through short reads Read --key-file input in a loop until EOF or until the 33-byte rejection threshold is reached. A single read call is enough for ordinary regular files in practice, but FIFOs and process substitution can legally return fewer than 32 bytes before EOF. Treating that first short read as final made the new key-file path fail spuriously for exactly the shell-friendly usage it was meant to support. Keep the existing exact-length policy: 32 bytes is accepted, shorter files are rejected, and 33 or more bytes are rejected with the trailing-newline hint. The intermediate buffer remains zeroizing. Test Plan: - cargo test split_fifo_key_file_read_roundtrips - cargo fmt --check - cargo clippy --all-targets -- -D warnings - cargo test - git diff --check Refs: review finding for --key-file short reads --- src/main.rs | 10 +++++++++- tests/roundtrip.rs | 50 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 438b2fb..309a60c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -120,7 +120,15 @@ fn read_key_file(path: &Path) -> Result { warn_if_key_file_world_readable(path); let mut file = File::open(path)?; let mut buf = Zeroizing::new([0u8; 33]); - let n = file.read(&mut *buf)?; + let mut n = 0usize; + while n < buf.len() { + match file.read(&mut buf[n..]) { + Ok(0) => break, + Ok(read) => n += read, + Err(e) if e.kind() == std::io::ErrorKind::Interrupted => continue, + Err(e) => return Err(e.into()), + } + } if n < 32 { return Err(FcryError::Format(format!( "key file {} is too short: expected exactly 32 bytes, got {n}", diff --git a/tests/roundtrip.rs b/tests/roundtrip.rs index 1da119a..eccfdbd 100644 --- a/tests/roundtrip.rs +++ b/tests/roundtrip.rs @@ -419,6 +419,56 @@ fn non_utf8_key_file_roundtrips() { assert_eq!(fs::read(&rt).unwrap(), data); } +#[cfg(unix)] +#[test] +fn split_fifo_key_file_read_roundtrips() { + use std::ffi::CString; + use std::fs::OpenOptions; + use std::os::unix::ffi::OsStrExt; + use std::thread; + use std::time::Duration; + + let dir = TempDir::new().unwrap(); + let fifo = dir.path().join("key.fifo"); + let fifo_c = CString::new(fifo.as_os_str().as_bytes()).unwrap(); + let rc = unsafe { libc::mkfifo(fifo_c.as_ptr(), 0o600) }; + assert_eq!(rc, 0, "mkfifo failed: {}", std::io::Error::last_os_error()); + + let plain = dir.path().join("plain.bin"); + let ct = dir.path().join("ct.bin"); + let rt = dir.path().join("rt.bin"); + let data = pseudo_random(33, 8192); + fs::write(&plain, &data).unwrap(); + + let fifo_writer = fifo.clone(); + let writer = thread::spawn(move || { + let mut file = OpenOptions::new().write(true).open(&fifo_writer).unwrap(); + file.write_all(&KEY[..8]).unwrap(); + file.flush().unwrap(); + thread::sleep(Duration::from_millis(50)); + file.write_all(&KEY[8..]).unwrap(); + }); + + let enc = fcry() + .arg("-i") + .arg(&plain) + .arg("-o") + .arg(&ct) + .arg("--key-file") + .arg(&fifo) + .output() + .unwrap(); + writer.join().unwrap(); + assert!( + enc.status.success(), + "split FIFO key encrypt failed: {}", + String::from_utf8_lossy(&enc.stderr) + ); + + decrypt_file(&ct, &rt); + assert_eq!(fs::read(&rt).unwrap(), data); +} + #[test] fn roundtrip_passphrase_argon2id() { let dir = TempDir::new().unwrap();