security: fix Bitwarden password exposure and ChaCha nonce hardening
Issue 12: pass bw unlock password via stdin (--passwordstdin) instead of as a CLI arg, preventing it from appearing in /proc/<pid>/cmdline or ps output. Issue 11: replace Nonce::default() (all-zero) with the first 12 bytes of the 16-byte random payload nonce. The key-nonce pair was already unique per message (HKDF salt), but using an explicit non-zero nonce removes the latent risk of accidental key reuse and makes the safety invariant self-documenting. Closes #11, closes #12
This commit is contained in:
@@ -55,7 +55,10 @@ impl Decryptor {
|
|||||||
|
|
||||||
let payload_key = derive_payload_key(&file_key, &nonce_bytes)?;
|
let payload_key = derive_payload_key(&file_key, &nonce_bytes)?;
|
||||||
let cipher = ChaCha20Poly1305::new(Key::from_slice(&payload_key));
|
let cipher = ChaCha20Poly1305::new(Key::from_slice(&payload_key));
|
||||||
let counter_nonce = Nonce::default();
|
// Mirror encrypt.rs: use the first 12 bytes of the 16-byte random nonce.
|
||||||
|
let counter_nonce = Nonce::from(
|
||||||
|
<[u8; 12]>::try_from(&nonce_bytes[..12]).expect("slice is exactly 12 bytes"),
|
||||||
|
);
|
||||||
let plaintext = cipher
|
let plaintext = cipher
|
||||||
.decrypt(&counter_nonce, payload_ciphertext)
|
.decrypt(&counter_nonce, payload_ciphertext)
|
||||||
.map_err(|_| AgeError::CryptoError("payload decryption failed".to_string()))?;
|
.map_err(|_| AgeError::CryptoError("payload decryption failed".to_string()))?;
|
||||||
|
|||||||
@@ -82,7 +82,13 @@ impl Encryptor {
|
|||||||
let payload_key = derive_payload_key(&file_key, &nonce_bytes)?;
|
let payload_key = derive_payload_key(&file_key, &nonce_bytes)?;
|
||||||
|
|
||||||
let cipher = ChaCha20Poly1305::new(Key::from_slice(&payload_key));
|
let cipher = ChaCha20Poly1305::new(Key::from_slice(&payload_key));
|
||||||
let counter_nonce = Nonce::default(); // [0u8; 12]
|
// Use the first 12 bytes of the 16-byte random nonce as the ChaCha nonce.
|
||||||
|
// The payload key is already unique per message (derived via HKDF with the full
|
||||||
|
// 16-byte nonce as salt), so this is safe — but using a non-zero nonce makes
|
||||||
|
// the invariant explicit and removes the latent risk of accidental key reuse.
|
||||||
|
let counter_nonce = Nonce::from(
|
||||||
|
<[u8; 12]>::try_from(&nonce_bytes[..12]).expect("slice is exactly 12 bytes"),
|
||||||
|
);
|
||||||
let ciphertext = cipher
|
let ciphertext = cipher
|
||||||
.encrypt(&counter_nonce, plaintext)
|
.encrypt(&counter_nonce, plaintext)
|
||||||
.map_err(|_| AgeError::CryptoError("payload encryption failed".to_string()))?;
|
.map_err(|_| AgeError::CryptoError("payload encryption failed".to_string()))?;
|
||||||
|
|||||||
@@ -2,7 +2,8 @@ use crate::error::AgeError;
|
|||||||
use crate::identities::Identity;
|
use crate::identities::Identity;
|
||||||
use crate::identities::x25519::X25519Identity;
|
use crate::identities::x25519::X25519Identity;
|
||||||
use crate::sources::IdentitySource;
|
use crate::sources::IdentitySource;
|
||||||
use std::process::Command;
|
use std::io::Write;
|
||||||
|
use std::process::{Command, Stdio};
|
||||||
|
|
||||||
pub struct BitwardenSource {
|
pub struct BitwardenSource {
|
||||||
pub item_name: String,
|
pub item_name: String,
|
||||||
@@ -37,13 +38,27 @@ impl BitwardenSource {
|
|||||||
source: anyhow::anyhow!("could not read password: {e}"),
|
source: anyhow::anyhow!("could not read password: {e}"),
|
||||||
}
|
}
|
||||||
})?;
|
})?;
|
||||||
let output = Command::new("bw")
|
// Pass password via stdin to avoid it appearing in the process list.
|
||||||
.args(["unlock", "--raw", &password])
|
let mut child = Command::new("bw")
|
||||||
.output()
|
.args(["unlock", "--raw", "--passwordstdin"])
|
||||||
|
.stdin(Stdio::piped())
|
||||||
|
.stdout(Stdio::piped())
|
||||||
|
.stderr(Stdio::piped())
|
||||||
|
.spawn()
|
||||||
.map_err(|e| AgeError::SourceError {
|
.map_err(|e| AgeError::SourceError {
|
||||||
name: self.name().to_string(),
|
name: self.name().to_string(),
|
||||||
source: anyhow::anyhow!("bw unlock spawn: {e}"),
|
source: anyhow::anyhow!("bw unlock spawn: {e}"),
|
||||||
})?;
|
})?;
|
||||||
|
if let Some(mut stdin) = child.stdin.take() {
|
||||||
|
stdin.write_all(password.as_bytes()).map_err(|e| AgeError::SourceError {
|
||||||
|
name: self.name().to_string(),
|
||||||
|
source: anyhow::anyhow!("bw unlock stdin write: {e}"),
|
||||||
|
})?;
|
||||||
|
}
|
||||||
|
let output = child.wait_with_output().map_err(|e| AgeError::SourceError {
|
||||||
|
name: self.name().to_string(),
|
||||||
|
source: anyhow::anyhow!("bw unlock wait: {e}"),
|
||||||
|
})?;
|
||||||
if !output.status.success() {
|
if !output.status.success() {
|
||||||
return Err(AgeError::SourceError {
|
return Err(AgeError::SourceError {
|
||||||
name: self.name().to_string(),
|
name: self.name().to_string(),
|
||||||
|
|||||||
Reference in New Issue
Block a user