Parent: #108
Problem
crates/identity/src/lib.rs:130-150 (load_or_generate) does:
let identity = Self::generate();
if let Some(parent) = path.parent() {
fs::create_dir_all(parent).map_err(...)?;
}
fs::write(path, identity.to_bytes()).map_err(...)?;
- Not atomic:
fs::write truncates-then-writes. A crash mid-write leaves a corrupted or empty key file.
- No permissions set: on Unix the default
umask typically makes the file 0644, world-readable. On a multi-user system anyone can read the secret key.
- No permissions validated on load: if an existing key file is
0644 (or worse, 0666), load_or_generate happily uses it.
Fix
- Write to a temp file in the same directory and
fs::rename into place. rename is atomic on the same filesystem.
- Open the file with
create_new(true) to catch race conditions on concurrent startup.
- On Unix, set the file mode to
0600 via std::os::unix::fs::PermissionsExt::set_mode.
- On load, check the mode is
0600 (or at least no world or group read/write bits) and refuse otherwise.
- Return a new error variant like
IdentityError::InsecurePermissions { path, mode }.
Suggested code
pub fn load_or_generate(path: impl AsRef<Path>) -> Result<Self, IdentityError> {
let path = path.as_ref();
if let Ok(bytes) = fs::read(path) {
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let perms = fs::metadata(path)?.permissions();
if perms.mode() & 0o077 != 0 {
return Err(IdentityError::InsecurePermissions {
path: path.display().to_string(),
mode: perms.mode(),
});
}
}
return Self::from_bytes(&bytes)
.ok_or_else(|| IdentityError::Other("invalid key file".into()));
}
if let Some(parent) = path.parent() {
fs::create_dir_all(parent)?;
}
let identity = Self::generate();
let tmp = path.with_extension("tmp");
{
let mut file = OpenOptions::new()
.write(true)
.create_new(true)
.open(&tmp)?;
file.write_all(&identity.to_bytes())?;
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
file.set_permissions(fs::Permissions::from_mode(0o600))?;
}
}
fs::rename(&tmp, path)?;
Ok(identity)
}
Test
- Test that
load_or_generate on a non-existent path produces a file with mode 0600 on Unix.
- Test that loading an existing
0644 file returns InsecurePermissions.
- Test that a crash mid-write (simulate with a process wrapper or
fs::write interruption) doesn't leave a corrupted key file.
Platform notes
- Windows: ACLs are different. Use
fs::set_permissions(path, fs::Permissions::from(...)) carefully or gate the perms check with #[cfg(unix)].
- WASM: the identity file is loaded from IndexedDB / localStorage, not the filesystem. Make sure this code is gated with
#[cfg(not(target_arch = "wasm32"))].
Parent: #108
Problem
crates/identity/src/lib.rs:130-150(load_or_generate) does:fs::writetruncates-then-writes. A crash mid-write leaves a corrupted or empty key file.umasktypically makes the file0644, world-readable. On a multi-user system anyone can read the secret key.0644(or worse,0666),load_or_generatehappily uses it.Fix
fs::renameinto place.renameis atomic on the same filesystem.create_new(true)to catch race conditions on concurrent startup.0600viastd::os::unix::fs::PermissionsExt::set_mode.0600(or at least no world or group read/write bits) and refuse otherwise.IdentityError::InsecurePermissions { path, mode }.Suggested code
Test
load_or_generateon a non-existent path produces a file with mode0600on Unix.0644file returnsInsecurePermissions.fs::writeinterruption) doesn't leave a corrupted key file.Platform notes
fs::set_permissions(path, fs::Permissions::from(...))carefully or gate the perms check with#[cfg(unix)].#[cfg(not(target_arch = "wasm32"))].