Skip to content

Commit b546935

Browse files
committed
fix(wal): enforce key-file security checks and zeroize key bytes on load
`WalEncryptionKey::from_file` now calls `check_key_file_wal` before reading any bytes. The check rejects symlinks (TOCTOU / path-traversal risk), non-regular files, group/world-readable mode bits, and files not owned by the server process UID. The 32-byte key array is wrapped in `Zeroizing` so it is wiped from the stack when dropped. Adds unit tests for 0o600 and 0o400 acceptance, 0o644 rejection, and symlink rejection. The check is duplicated in nodedb-wal (rather than imported from nodedb) to keep the WAL crate free of application-level dependencies.
1 parent beffc9d commit b546935

2 files changed

Lines changed: 158 additions & 1 deletion

File tree

nodedb-wal/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ memmap2 = { workspace = true }
2222
io-uring = { workspace = true, optional = true }
2323
aes-gcm = { workspace = true }
2424
getrandom = { workspace = true }
25+
zeroize = { workspace = true }
2526

2627
[dev-dependencies]
2728
tokio = { workspace = true }

nodedb-wal/src/crypto.rs

Lines changed: 157 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,71 @@ use crate::error::{Result, WalError};
2121
use crate::record::HEADER_SIZE;
2222
use crate::secure_mem;
2323

24+
/// Security gate for WAL key files: enforces no-symlink, regular-file,
25+
/// Unix mode bits (no group/world), and owner-UID checks.
26+
///
27+
/// Mirrors the logic in `nodedb::control::security::keystore::key_file_security`
28+
/// but is duplicated here so that `nodedb-wal` has zero dependency on the
29+
/// `nodedb` application crate.
30+
fn check_key_file_wal(path: &std::path::Path) -> Result<()> {
31+
let symlink_meta = std::fs::symlink_metadata(path).map_err(|e| WalError::EncryptionError {
32+
detail: format!("cannot stat WAL key file {}: {e}", path.display()),
33+
})?;
34+
35+
if symlink_meta.file_type().is_symlink() {
36+
return Err(WalError::EncryptionError {
37+
detail: format!(
38+
"WAL key file {} is a symlink, which is not permitted \
39+
(path traversal / TOCTOU risk)",
40+
path.display()
41+
),
42+
});
43+
}
44+
45+
if !symlink_meta.is_file() {
46+
return Err(WalError::EncryptionError {
47+
detail: format!("WAL key file {} is not a regular file", path.display()),
48+
});
49+
}
50+
51+
#[cfg(unix)]
52+
{
53+
use std::os::unix::fs::MetadataExt as _;
54+
55+
let mode = symlink_meta.mode();
56+
if mode & 0o077 != 0 {
57+
return Err(WalError::EncryptionError {
58+
detail: format!(
59+
"WAL key file {} has insecure permissions: 0o{:03o} \
60+
(must be 0o400 or 0o600 — no group or world access)",
61+
path.display(),
62+
mode & 0o777,
63+
),
64+
});
65+
}
66+
67+
let file_uid = symlink_meta.uid();
68+
// SAFETY: geteuid() is always safe to call; it has no preconditions.
69+
let process_uid = unsafe { libc::geteuid() };
70+
if file_uid != process_uid {
71+
return Err(WalError::EncryptionError {
72+
detail: format!(
73+
"WAL key file {} is owned by UID {} but process runs as UID {} \
74+
— key files must be owned by the server process user",
75+
path.display(),
76+
file_uid,
77+
process_uid,
78+
),
79+
});
80+
}
81+
}
82+
83+
// On Windows: ACL-based permission enforcement is not implemented.
84+
// The symlink check above still applies on all platforms.
85+
86+
Ok(())
87+
}
88+
2489
/// AES-256-GCM key with a random per-lifetime epoch for nonce disambiguation.
2590
///
2691
/// The epoch is generated randomly at construction time. Each WAL lifetime
@@ -81,7 +146,14 @@ impl WalEncryptionKey {
81146
}
82147

83148
/// Load key from a file (must contain exactly 32 bytes).
149+
///
150+
/// Before reading, the file is checked for:
151+
/// - No symlinks (TOCTOU / path-traversal risk).
152+
/// - Regular file (not a device, FIFO, or directory).
153+
/// - Unix: no group or world access bits (`mode & 0o077 == 0`).
154+
/// - Unix: file owner matches the current process UID.
84155
pub fn from_file(path: &std::path::Path) -> Result<Self> {
156+
check_key_file_wal(path)?;
85157
let key_bytes = std::fs::read(path).map_err(WalError::Io)?;
86158
if key_bytes.len() != 32 {
87159
return Err(WalError::EncryptionError {
@@ -91,7 +163,7 @@ impl WalEncryptionKey {
91163
),
92164
});
93165
}
94-
let mut key_arr = [0u8; 32];
166+
let mut key_arr = zeroize::Zeroizing::new([0u8; 32]);
95167
key_arr.copy_from_slice(&key_bytes);
96168
Self::from_bytes(&key_arr)
97169
}
@@ -381,6 +453,90 @@ mod tests {
381453
}
382454

383455
#[test]
456+
#[cfg(unix)]
457+
fn from_file_0o600_accepted() {
458+
use std::io::Write as _;
459+
use std::os::unix::fs::PermissionsExt as _;
460+
461+
let dir = tempfile::tempdir().unwrap();
462+
let path = dir.path().join("key.bin");
463+
let mut f = std::fs::File::create(&path).unwrap();
464+
f.write_all(&[0x42u8; 32]).unwrap();
465+
drop(f);
466+
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)).unwrap();
467+
468+
WalEncryptionKey::from_file(&path).expect("0o600 key file must be accepted");
469+
}
470+
471+
#[test]
472+
#[cfg(unix)]
473+
fn from_file_0o400_accepted() {
474+
use std::io::Write as _;
475+
use std::os::unix::fs::PermissionsExt as _;
476+
477+
let dir = tempfile::tempdir().unwrap();
478+
let path = dir.path().join("key.bin");
479+
let mut f = std::fs::File::create(&path).unwrap();
480+
f.write_all(&[0x42u8; 32]).unwrap();
481+
drop(f);
482+
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o400)).unwrap();
483+
484+
WalEncryptionKey::from_file(&path).expect("0o400 key file must be accepted");
485+
}
486+
487+
#[test]
488+
#[cfg(unix)]
489+
fn from_file_0o644_rejected() {
490+
use std::io::Write as _;
491+
use std::os::unix::fs::PermissionsExt as _;
492+
493+
let dir = tempfile::tempdir().unwrap();
494+
let path = dir.path().join("key.bin");
495+
let mut f = std::fs::File::create(&path).unwrap();
496+
f.write_all(&[0x42u8; 32]).unwrap();
497+
drop(f);
498+
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)).unwrap();
499+
500+
let err = match WalEncryptionKey::from_file(&path) {
501+
Ok(_) => panic!("expected insecure-permissions error, got Ok"),
502+
Err(e) => e,
503+
};
504+
let detail = format!("{err:?}");
505+
assert!(
506+
detail.contains("insecure") || detail.contains("644"),
507+
"expected insecure-permissions error, got: {detail}"
508+
);
509+
}
510+
511+
#[test]
512+
#[cfg(unix)]
513+
fn from_file_symlink_rejected() {
514+
use std::io::Write as _;
515+
use std::os::unix::fs::PermissionsExt as _;
516+
517+
let dir = tempfile::tempdir().unwrap();
518+
let target = dir.path().join("target.bin");
519+
let mut f = std::fs::File::create(&target).unwrap();
520+
f.write_all(&[0x42u8; 32]).unwrap();
521+
drop(f);
522+
std::fs::set_permissions(&target, std::fs::Permissions::from_mode(0o600)).unwrap();
523+
524+
let link = dir.path().join("link.bin");
525+
std::os::unix::fs::symlink(&target, &link).unwrap();
526+
527+
let err = match WalEncryptionKey::from_file(&link) {
528+
Ok(_) => panic!("expected symlink rejection, got Ok"),
529+
Err(e) => e,
530+
};
531+
let detail = format!("{err:?}");
532+
assert!(
533+
detail.contains("symlink"),
534+
"expected symlink rejection, got: {detail}"
535+
);
536+
}
537+
538+
#[test]
539+
#[cfg(unix)]
384540
fn same_lsn_different_wal_lifetimes_produce_different_ciphertext() {
385541
// Simulate two WAL lifetimes: same key bytes, same LSN=1, but
386542
// separate WalEncryptionKey instances (each gets a fresh random epoch).

0 commit comments

Comments
 (0)