fix: purge keychain credentials on profile delete, atomic owner-only secret writes, zeroize derived keys
- Secrets::delete_profile removes every profile credential kind from the OS keychain as well as the vault (previously only the vault was purged, so keychain passwords/tokens outlived the profile). - write_private writes to a sibling temp file and renames it into place, so a crash mid-write cannot truncate the vault or an export, and a pre-existing world-readable file is replaced rather than reused. - Argon2-derived vault/export keys are Zeroizing and wiped on drop. - Recordings (which capture typed input), JSON exports and --data-dir directories are created owner-only.
This commit is contained in:
Generated
+1
@@ -2843,6 +2843,7 @@ dependencies = [
|
||||
"ureq",
|
||||
"uuid",
|
||||
"webpki-roots",
|
||||
"zeroize",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -82,8 +82,7 @@ fn main() -> eframe::Result {
|
||||
let config = match data_dir {
|
||||
Some(d) => {
|
||||
let c = AppConfig::in_dir(d.into());
|
||||
std::fs::create_dir_all(&c.vault_dir).ok();
|
||||
Ok(c)
|
||||
nexus_core::config::create_private_dir(&c.data_dir).and_then(|_| nexus_core::config::create_private_dir(&c.vault_dir)).map(|_| c)
|
||||
}
|
||||
None => AppConfig::load(),
|
||||
};
|
||||
|
||||
@@ -210,7 +210,12 @@ impl Transfer {
|
||||
.add_filter("JSON", &["json"])
|
||||
.save_file()
|
||||
{
|
||||
match s.svc.export_json(*include_secrets).map_err(|e| e.to_string()).and_then(|j| std::fs::write(&path, j).map_err(|e| e.to_string())) {
|
||||
match s
|
||||
.svc
|
||||
.export_json(*include_secrets)
|
||||
.map_err(|e| e.to_string())
|
||||
.and_then(|j| nexus_core::secrets::write_private(&path, j.as_bytes()).map_err(|e| e.to_string()))
|
||||
{
|
||||
Ok(()) => {
|
||||
s.success(format!("Exported {} profile(s) to {}", s.profiles.len(), path.display()));
|
||||
close = true;
|
||||
|
||||
@@ -21,6 +21,7 @@ tokio = { version = "1.53.2", features = ["full"] }
|
||||
sha2 = "0.11.0"
|
||||
hmac = "0.13.0"
|
||||
sha1 = "0.11.0"
|
||||
zeroize = "1"
|
||||
portable-pty = "0.9.0"
|
||||
serialport = { version = "4", default-features = false }
|
||||
des = "0.9"
|
||||
|
||||
@@ -51,7 +51,8 @@ impl AppConfig {
|
||||
}
|
||||
}
|
||||
|
||||
fn create_private_dir(d: &Path) -> std::io::Result<()> {
|
||||
/// Creates `d` (and parents) readable only by the owner.
|
||||
pub fn create_private_dir(d: &Path) -> std::io::Result<()> {
|
||||
std::fs::create_dir_all(d)?;
|
||||
#[cfg(unix)]
|
||||
std::fs::set_permissions(d, std::os::unix::fs::PermissionsExt::from_mode(0o700))?;
|
||||
|
||||
@@ -41,7 +41,7 @@ pub fn encrypt_and_export(profiles: &[Profile], password: &str, path: &Path) ->
|
||||
let salt = random_bytes::<32>();
|
||||
let nonce = random_bytes::<12>();
|
||||
let key = derive_key(password, &salt)?;
|
||||
let ct = Aes256Gcm::new((&key).into()).encrypt(Nonce::from_slice(&nonce), plain.as_slice()).map_err(|e| e.to_string())?;
|
||||
let ct = Aes256Gcm::new((&*key).into()).encrypt(Nonce::from_slice(&nonce), plain.as_slice()).map_err(|e| e.to_string())?;
|
||||
let mut out = vec![VERSION];
|
||||
out.extend(salt);
|
||||
out.extend(nonce);
|
||||
@@ -64,7 +64,7 @@ pub fn decrypt_and_import(path: &Path, password: &str) -> Result<Vec<Profile>> {
|
||||
return Err(format!("exportimport: unsupported format version {}", data[0]).into());
|
||||
}
|
||||
let key = derive_key(password, &data[1..33])?;
|
||||
let plain = Aes256Gcm::new((&key).into())
|
||||
let plain = Aes256Gcm::new((&*key).into())
|
||||
.decrypt(Nonce::from_slice(&data[33..45]), &data[45..])
|
||||
.map_err(|_| "exportimport: decryption failed — wrong password or corrupt file")?;
|
||||
let bundle: Bundle = serde_json::from_slice(&plain).map_err(|e| format!("exportimport: invalid bundle JSON: {e}"))?;
|
||||
|
||||
@@ -116,7 +116,12 @@ impl Recorder {
|
||||
fn with_limit(dir: &Path, id: &str, title: &str, cols: i64, rows: i64, max_bytes: u64) -> Result<Recorder> {
|
||||
mkdir_private(dir).map_err(|e| format!("recording: failed to create directory: {e}"))?;
|
||||
let path = dir.join(format!("{id}.cast"));
|
||||
let mut f = File::create(&path).map_err(|e| format!("recording: failed to create file: {e}"))?;
|
||||
// Recordings capture typed input (passwords included): owner-only, like the vault.
|
||||
let mut opts = std::fs::OpenOptions::new();
|
||||
opts.write(true).create(true).truncate(true);
|
||||
#[cfg(unix)]
|
||||
std::os::unix::fs::OpenOptionsExt::mode(&mut opts, 0o600);
|
||||
let mut f = opts.open(&path).map_err(|e| format!("recording: failed to create file: {e}"))?;
|
||||
let start = Instant::now();
|
||||
let hdr = Header {
|
||||
version: 2,
|
||||
@@ -391,6 +396,11 @@ mod tests {
|
||||
assert_eq!(rec.event_count(), 4);
|
||||
rec.close().unwrap();
|
||||
rec.close().unwrap(); // double close is a no-op
|
||||
#[cfg(unix)]
|
||||
{
|
||||
let mode = std::os::unix::fs::PermissionsExt::mode(&std::fs::metadata(dir.join("test-session-001.cast")).unwrap().permissions());
|
||||
assert_eq!(mode & 0o777, 0o600);
|
||||
}
|
||||
let text = std::fs::read_to_string(dir.join("test-session-001.cast")).unwrap();
|
||||
let lines: Vec<&str> = text.trim().lines().collect();
|
||||
assert_eq!(lines.len(), 5);
|
||||
|
||||
+78
-16
@@ -11,16 +11,32 @@ use serde::{Deserialize, Serialize};
|
||||
use std::collections::BTreeMap;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Mutex;
|
||||
use zeroize::Zeroizing;
|
||||
|
||||
const VAULT_FILE: &str = "vault.json";
|
||||
const MAX_VAULT_BYTES: u64 = 10 * 1024 * 1024;
|
||||
|
||||
/// Argon2id with the Go parameters: t=3, m=128 MiB, p=4, 32-byte key.
|
||||
pub fn derive_key(password: &str, salt: &[u8]) -> Result<[u8; 32]> {
|
||||
/// Every credential kind the session manager stores under `profile:<id>:<kind>`.
|
||||
pub const PROFILE_SECRET_KINDS: [&str; 11] = [
|
||||
"password",
|
||||
"passphrase",
|
||||
"totp",
|
||||
"github-token",
|
||||
"vnc-password",
|
||||
"vnc-ssh-password",
|
||||
"vnc-ssh-passphrase",
|
||||
"irc-server-password",
|
||||
"irc-nickserv-password",
|
||||
"irc-ssh-password",
|
||||
"irc-ssh-passphrase",
|
||||
];
|
||||
|
||||
/// Argon2id with the Go parameters: t=3, m=128 MiB, p=4, 32-byte key. The key is wiped on drop.
|
||||
pub fn derive_key(password: &str, salt: &[u8]) -> Result<Zeroizing<[u8; 32]>> {
|
||||
let params = argon2::Params::new(128 * 1024, 3, 4, Some(32)).map_err(|e| e.to_string())?;
|
||||
let mut key = [0u8; 32];
|
||||
let mut key = Zeroizing::new([0u8; 32]);
|
||||
argon2::Argon2::new(argon2::Algorithm::Argon2id, argon2::Version::V0x13, params)
|
||||
.hash_password_into(password.as_bytes(), salt, &mut key)
|
||||
.hash_password_into(password.as_bytes(), salt, &mut *key)
|
||||
.map_err(|e| e.to_string())?;
|
||||
Ok(key)
|
||||
}
|
||||
@@ -54,16 +70,10 @@ struct VaultFile {
|
||||
}
|
||||
|
||||
struct Unlocked {
|
||||
key: [u8; 32],
|
||||
key: Zeroizing<[u8; 32]>,
|
||||
vault: VaultFile,
|
||||
}
|
||||
|
||||
impl Drop for Unlocked {
|
||||
fn drop(&mut self) {
|
||||
self.key.fill(0);
|
||||
}
|
||||
}
|
||||
|
||||
pub struct Vault {
|
||||
dir: PathBuf,
|
||||
state: Mutex<Option<Unlocked>>,
|
||||
@@ -155,17 +165,27 @@ impl Vault {
|
||||
}
|
||||
}
|
||||
|
||||
/// Writes a file readable only by the owner.
|
||||
/// Writes a file readable only by the owner, atomically: the data goes to a sibling temp file
|
||||
/// that replaces `path` only once it is fully written and synced, so a crash can't leave a
|
||||
/// truncated vault or export behind (and an existing file's wider mode is never kept).
|
||||
pub fn write_private(path: &std::path::Path, data: &[u8]) -> Result<()> {
|
||||
use std::io::Write;
|
||||
let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default();
|
||||
let tmp = path.with_file_name(format!(".{name}.tmp-{}", std::process::id()));
|
||||
let mut opts = std::fs::OpenOptions::new();
|
||||
opts.write(true).create(true).truncate(true);
|
||||
#[cfg(unix)]
|
||||
std::os::unix::fs::OpenOptionsExt::mode(&mut opts, 0o600);
|
||||
let mut f = opts.open(path)?;
|
||||
f.write_all(data)?;
|
||||
f.sync_all()?;
|
||||
Ok(())
|
||||
let written = (|| {
|
||||
let mut f = opts.open(&tmp)?;
|
||||
f.write_all(data)?;
|
||||
f.sync_all()?;
|
||||
std::fs::rename(&tmp, path)
|
||||
})();
|
||||
if written.is_err() {
|
||||
let _ = std::fs::remove_file(&tmp);
|
||||
}
|
||||
Ok(written?)
|
||||
}
|
||||
|
||||
pub const KEYCHAIN_SERVICE: &str = "NexusTerminal";
|
||||
@@ -270,7 +290,13 @@ impl Secrets {
|
||||
let _ = self.vault.delete(key);
|
||||
}
|
||||
|
||||
/// Removes every credential saved for a profile from both stores.
|
||||
pub fn delete_profile(&self, profile_id: &str) {
|
||||
if self.keychain_ok {
|
||||
for kind in PROFILE_SECRET_KINDS {
|
||||
self.keychain.delete(&format!("profile:{profile_id}:{kind}"));
|
||||
}
|
||||
}
|
||||
let _ = self.vault.delete_prefix(&format!("profile:{profile_id}:"));
|
||||
}
|
||||
}
|
||||
@@ -303,4 +329,40 @@ mod tests {
|
||||
assert_eq!(v.get("a").unwrap(), "1");
|
||||
std::fs::remove_dir_all(dir).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn private_writes_are_atomic_and_owner_only() {
|
||||
let dir = std::env::temp_dir().join(format!("nexus-wp-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let path = dir.join("secret.bin");
|
||||
// A pre-existing world-readable file is replaced, not reused.
|
||||
std::fs::write(&path, b"old").unwrap();
|
||||
#[cfg(unix)]
|
||||
std::fs::set_permissions(&path, std::os::unix::fs::PermissionsExt::from_mode(0o644)).unwrap();
|
||||
write_private(&path, b"new").unwrap();
|
||||
assert_eq!(std::fs::read(&path).unwrap(), b"new");
|
||||
#[cfg(unix)]
|
||||
assert_eq!(std::os::unix::fs::PermissionsExt::mode(&std::fs::metadata(&path).unwrap().permissions()) & 0o777, 0o600);
|
||||
// No temp file is left behind, on success or when the target directory is unwritable.
|
||||
assert_eq!(std::fs::read_dir(&dir).unwrap().count(), 1);
|
||||
assert!(write_private(&dir.join("missing").join("x"), b"x").is_err());
|
||||
std::fs::remove_dir_all(dir).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deleting_a_profile_clears_every_vault_kind() {
|
||||
let dir = std::env::temp_dir().join(format!("nexus-delprof-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
let s = Secrets { vault: Vault::new(dir.clone()), keychain: Keychain::new("NexusTerminalTest", None), keychain_ok: false };
|
||||
s.vault.initialise("pw").unwrap();
|
||||
for kind in PROFILE_SECRET_KINDS {
|
||||
s.vault.set(&format!("profile:p1:{kind}"), "v").unwrap();
|
||||
}
|
||||
s.vault.set("profile:p2:password", "keep").unwrap();
|
||||
s.delete_profile("p1");
|
||||
assert!(PROFILE_SECRET_KINDS.iter().all(|k| s.get(&format!("profile:p1:{k}")).is_none()));
|
||||
assert_eq!(s.get("profile:p2:password").as_deref(), Some("keep"));
|
||||
std::fs::remove_dir_all(dir).unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user