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
This commit is contained in:
2026-06-09 23:46:27 +02:00
parent 81ac1475ad
commit 725d33939e
2 changed files with 59 additions and 1 deletions
+9 -1
View File
@@ -120,7 +120,15 @@ fn read_key_file(path: &Path) -> Result<SecretBytes32, FcryError> {
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}",
+50
View File
@@ -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();