From 56d47b757da04bdb4414e350e6438a93242f53c8 Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Wed, 8 Apr 2020 03:45:45 -0400 Subject: mlock sensitive memory --- Cargo.lock | 23 +++++++++++ Cargo.toml | 2 + src/actions.rs | 14 +++---- src/api.rs | 4 +- src/bin/agent.rs | 15 ++++--- src/cipherstring.rs | 44 +++++++++++++++------ src/error.rs | 15 ++----- src/identity.rs | 45 ++++++++++++--------- src/lib.rs | 4 ++ src/locked.rs | 80 ++++++++++++++++++++++++++++++++++++++ src/pinentry.rs | 110 ++++++++++++++++++++++++++++++++++++---------------- 11 files changed, 263 insertions(+), 93 deletions(-) create mode 100644 src/locked.rs diff --git a/Cargo.lock b/Cargo.lock index 15e8451..dbc0448 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -660,6 +660,15 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "mach" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "86dd2487cdfea56def77b88438a2c915fb45113c5319bfe7e14306ca4cd0b0e1" +dependencies = [ + "libc", +] + [[package]] name = "matches" version = "0.1.8" @@ -1007,6 +1016,7 @@ version = "0.1.0" dependencies = [ "aes", "anyhow", + "arrayvec", "base64 0.11.0", "block-modes", "clap", @@ -1018,6 +1028,7 @@ dependencies = [ "log", "nix", "pbkdf2", + "region", "reqwest", "serde", "serde_json", @@ -1062,6 +1073,18 @@ version = "0.6.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7fe5bd57d1d7414c6b5ed48563a2c855d995ff777729dcd91c369ec7fea395ae" +[[package]] +name = "region" +version = "2.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "448e868c6e4cfddfa49b6a72c95906c04e8547465e9536575b95c70a4044f856" +dependencies = [ + "bitflags", + "libc", + "mach", + "winapi 0.3.8", +] + [[package]] name = "remove_dir_all" version = "0.5.2" diff --git a/Cargo.toml b/Cargo.toml index fb4a51c..a93bd0a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,6 +7,7 @@ edition = "2018" [dependencies] aes = "*" anyhow = "*" +arrayvec = "*" base64 = "*" block-modes = "*" clap = "*" @@ -18,6 +19,7 @@ hmac = "*" log = "*" nix = "*" pbkdf2 = "*" +region = "*" reqwest = { version = "*", features = ["blocking", "json"] } serde = { version = "*", features = ["derive"] } serde_json = "*" diff --git a/src/actions.rs b/src/actions.rs index f9b1354..0402a10 100644 --- a/src/actions.rs +++ b/src/actions.rs @@ -2,7 +2,7 @@ use crate::prelude::*; pub async fn login( email: &str, - password: &str, + password: &crate::locked::Password, ) -> Result<(String, u32, String)> { let client = crate::api::Client::new_self_hosted("https://bitwarden.tozt.net"); @@ -20,22 +20,18 @@ pub async fn login( pub async fn unlock( email: &str, - password: &str, + password: &crate::locked::Password, iterations: u32, protected_key: String, -) -> Result<(Vec, Vec)> { +) -> Result { let identity = crate::identity::Identity::new(email, password, iterations)?; let protected_key = crate::cipherstring::CipherString::new(&protected_key)?; - let master_key = - protected_key.decrypt(&identity.enc_key, &identity.mac_key)?; + let master_keys = protected_key.decrypt_locked(&identity.keys)?; - let enc_key = &master_key[0..32]; - let mac_key = &master_key[32..64]; - - Ok((enc_key.to_vec(), mac_key.to_vec())) + Ok(crate::locked::Keys::new(master_keys)) } pub async fn sync( diff --git a/src/api.rs b/src/api.rs index 9637630..fc59d25 100644 --- a/src/api.rs +++ b/src/api.rs @@ -119,12 +119,12 @@ impl Client { pub async fn login( &self, email: &str, - master_password_hash: &[u8], + master_password_hash: &crate::locked::PasswordHash, ) -> Result<(String, String, String)> { let connect_req = ConnectReq { grant_type: "password".to_string(), username: email.to_string(), - password: base64::encode(&master_password_hash), + password: base64::encode(master_password_hash.hash()), scope: "api offline_access".to_string(), client_id: "desktop".to_string(), device_type: 8, diff --git a/src/bin/agent.rs b/src/bin/agent.rs index 96de0c2..d56e5a0 100644 --- a/src/bin/agent.rs +++ b/src/bin/agent.rs @@ -45,11 +45,11 @@ async fn login( rbw::actions::login(email, &password).await.unwrap(); state.access_token = Some(access_token); state.iterations = Some(iterations); - let (enc_key, mac_key) = + let keys = rbw::actions::unlock(email, &password, iterations, protected_key) .await .unwrap(); - state.priv_key = Some((enc_key, mac_key)); + state.priv_key = Some(keys); send_response(sock, &rbw::agent::Response::Ack).await; } @@ -73,7 +73,7 @@ async fn unlock( let email = "bitwarden@tozt.net"; // XXX read from config let password = rbw::pinentry::getpin("prompt", "desc", tty).await.unwrap(); - let (enc_key, mac_key) = rbw::actions::unlock( + let keys = rbw::actions::unlock( email, &password, state.iterations.unwrap(), @@ -81,7 +81,7 @@ async fn unlock( ) .await .unwrap(); - state.priv_key = Some((enc_key, mac_key)); + state.priv_key = Some(keys); send_response(sock, &rbw::agent::Response::Ack).await; } @@ -110,12 +110,11 @@ async fn decrypt( ) { ensure_unlock(sock, state.clone()).await; let state = state.read().await; - let (enc_key, mac_key) = state.priv_key.as_ref().unwrap(); + let keys = state.priv_key.as_ref().unwrap(); let cipherstring = rbw::cipherstring::CipherString::new(cipherstring).unwrap(); let plaintext = - String::from_utf8(cipherstring.decrypt(&enc_key, &mac_key).unwrap()) - .unwrap(); + String::from_utf8(cipherstring.decrypt(keys).unwrap()).unwrap(); send_response(sock, &rbw::agent::Response::Decrypt { plaintext }).await; } @@ -151,7 +150,7 @@ struct Agent { struct State { access_token: Option, - priv_key: Option<(Vec, Vec)>, + priv_key: Option, // these should be in a state file iterations: Option, diff --git a/src/cipherstring.rs b/src/cipherstring.rs index 75edec4..9f2c261 100644 --- a/src/cipherstring.rs +++ b/src/cipherstring.rs @@ -51,32 +51,54 @@ impl CipherString { }) } - pub fn decrypt(&self, enc_key: &[u8], mac_key: &[u8]) -> Result> { + pub fn decrypt(&self, keys: &crate::locked::Keys) -> Result> { + let cipher = self.decrypt_common(keys)?; + cipher + .decrypt_vec(&self.ciphertext) + .context(crate::error::Decrypt) + } + + pub fn decrypt_locked( + &self, + keys: &crate::locked::Keys, + ) -> Result { + let mut res = crate::locked::Vec::new(); + res.extend(self.ciphertext.iter().copied()); + let cipher = self.decrypt_common(keys)?; + cipher + .decrypt(res.data_mut()) + .context(crate::error::Decrypt)?; + Ok(res) + } + + fn decrypt_common( + &self, + keys: &crate::locked::Keys, + ) -> Result< + block_modes::Cbc, + > { if self.ty != 2 { unimplemented!() } if let Some(mac) = &self.mac { - let mut digest = hmac::Hmac::::new_varkey(mac_key) - .map_err(|_| Error::InvalidMacKey)?; + let mut digest = + hmac::Hmac::::new_varkey(keys.mac_key()) + .map_err(|_| Error::InvalidMacKey)?; digest.input(&self.iv); digest.input(&self.ciphertext); let calculated_mac = digest.result().code(); - if !macs_equal(mac, &calculated_mac, mac_key)? { + if !macs_equal(mac, &calculated_mac, keys.mac_key())? { return Err(Error::InvalidMac); } } - let cipher = block_modes::Cbc::< + Ok(block_modes::Cbc::< aes::Aes256, block_modes::block_padding::Pkcs7, - >::new_var(enc_key, &self.iv) - .context(crate::error::CreateBlockMode)?; - - cipher - .decrypt_vec(&self.ciphertext) - .context(crate::error::Decrypt) + >::new_var(keys.enc_key(), &self.iv) + .context(crate::error::CreateBlockMode)?) } } diff --git a/src/error.rs b/src/error.rs index 8947fd0..8642e5e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -12,16 +12,6 @@ pub enum Error { #[snafu(display("failed to parse pinentry output ({:?})", out,))] FailedToParsePinentry { out: Vec }, - #[snafu(display( - "failed to parse pinentry output ({:?}): {}", - out, - source - ))] - FailedToParsePinentryUtf8 { - out: Vec, - source: std::string::FromUtf8Error, - }, - // no Error impl // #[snafu(display("failed to expand with hkdf: {}", source))] // HkdfExpand { source: hkdf::InvalidLength }, @@ -49,8 +39,11 @@ pub enum Error { #[snafu(display("invalid mac key"))] InvalidMacKey, + #[snafu(display("error reading pinentry output: {}", source))] + PinentryReadOutput { source: tokio::io::Error }, + #[snafu(display("error waiting for pinentry to exit: {}", source))] - ProcessWaitOutput { source: tokio::io::Error }, + PinentryWait { source: tokio::io::Error }, #[snafu(display("error making api request: {}", source))] Reqwest { source: reqwest::Error }, diff --git a/src/identity.rs b/src/identity.rs index 69294ca..1baac0f 100644 --- a/src/identity.rs +++ b/src/identity.rs @@ -2,43 +2,52 @@ use crate::prelude::*; pub struct Identity { pub email: String, - pub enc_key: Vec, - pub mac_key: Vec, - pub master_password_hash: Vec, + pub keys: crate::locked::Keys, + pub master_password_hash: crate::locked::PasswordHash, } impl Identity { - pub fn new(email: &str, password: &str, iterations: u32) -> Result { - let mut key = vec![0_u8; 32]; + pub fn new( + email: &str, + password: &crate::locked::Password, + iterations: u32, + ) -> Result { + let mut keys = crate::locked::Vec::new(); + keys.extend(std::iter::repeat(0).take(64)); + + let enc_key = &mut keys.data_mut()[0..32]; pbkdf2::pbkdf2::>( - password.as_bytes(), + password.password(), email.as_bytes(), iterations as usize, - &mut key, + enc_key, ); - let mut hash = vec![0_u8; 32]; + let mut hash = crate::locked::Vec::new(); + hash.extend(std::iter::repeat(0).take(32)); pbkdf2::pbkdf2::>( - &key, - password.as_bytes(), + enc_key, + password.password(), 1, - &mut hash, + hash.data_mut(), ); - let hkdf = hkdf::Hkdf::::from_prk(&key) + let hkdf = hkdf::Hkdf::::from_prk(enc_key) .map_err(|_| Error::HkdfFromPrk)?; - hkdf.expand(b"enc", &mut key) + hkdf.expand(b"enc", enc_key) .map_err(|_| Error::HkdfExpand)?; - let mut mac_key = vec![0_u8; 32]; - hkdf.expand(b"mac", &mut mac_key) + let mac_key = &mut keys.data_mut()[32..64]; + hkdf.expand(b"mac", mac_key) .map_err(|_| Error::HkdfExpand)?; + let keys = crate::locked::Keys::new(keys); + let master_password_hash = crate::locked::PasswordHash::new(hash); + Ok(Self { email: email.to_string(), - enc_key: key, - mac_key, - master_password_hash: hash, + keys, + master_password_hash, }) } } diff --git a/src/lib.rs b/src/lib.rs index 996098e..340de00 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,8 @@ #![warn(clippy::pedantic)] #![warn(clippy::nursery)] +#![allow(clippy::missing_const_for_fn)] #![allow(clippy::missing_errors_doc)] +#![allow(clippy::must_use_candidate)] #![allow(clippy::similar_names)] pub mod actions; @@ -10,5 +12,7 @@ pub mod cipherstring; pub mod dirs; mod error; pub mod identity; +pub mod locked; pub mod pinentry; mod prelude; +// pub mod secrets; diff --git a/src/locked.rs b/src/locked.rs new file mode 100644 index 0000000..4915232 --- /dev/null +++ b/src/locked.rs @@ -0,0 +1,80 @@ +pub struct Vec { + data: Box>, + _lock: region::LockGuard, +} + +impl Default for Vec { + fn default() -> Self { + let data = Box::new(arrayvec::ArrayVec::<[_; 4096]>::new()); + let lock = region::lock(data.as_ptr(), data.capacity()).unwrap(); + Self { data, _lock: lock } + } +} + +impl Vec { + pub fn new() -> Self { + Self::default() + } + + pub fn data(&self) -> &[u8] { + self.data.as_slice() + } + + pub fn data_mut(&mut self) -> &mut [u8] { + self.data.as_mut_slice() + } + + pub fn extend(&mut self, it: impl Iterator) { + self.data.extend(it); + } + + pub fn truncate(&mut self, len: usize) { + self.data.truncate(len); + } +} + +pub struct Password { + password: Vec, +} + +impl Password { + pub fn new(password: Vec) -> Self { + Self { password } + } + + pub fn password(&self) -> &[u8] { + self.password.data() + } +} + +pub struct Keys { + keys: Vec, +} + +impl Keys { + pub fn new(keys: Vec) -> Self { + Self { keys } + } + + pub fn enc_key(&self) -> &[u8] { + &self.keys.data()[0..32] + } + + pub fn mac_key(&self) -> &[u8] { + &self.keys.data()[32..64] + } +} + +pub struct PasswordHash { + hash: Vec, +} + +impl PasswordHash { + pub fn new(hash: Vec) -> Self { + Self { hash } + } + + pub fn hash(&self) -> &[u8] { + self.hash.data() + } +} diff --git a/src/pinentry.rs b/src/pinentry.rs index ff778e7..4a2196e 100644 --- a/src/pinentry.rs +++ b/src/pinentry.rs @@ -6,7 +6,7 @@ pub async fn getpin( prompt: &str, desc: &str, tty: Option<&str>, -) -> Result { +) -> Result { let mut opts = tokio::process::Command::new("pinentry"); let opts = opts .stdin(std::process::Stdio::piped()) @@ -17,42 +17,84 @@ pub async fn getpin( opts }; let mut child = opts.spawn().context(crate::error::Spawn)?; - { - let stdin = child.stdin.as_mut().unwrap(); - - stdin - .write_all(b"SETTITLE rbw\n") - .await - .context(crate::error::WriteStdin)?; - stdin - .write_all(format!("SETPROMPT {}\n", prompt).as_bytes()) - .await - .context(crate::error::WriteStdin)?; - stdin - .write_all(format!("SETDESC {}\n", desc).as_bytes()) - .await - .context(crate::error::WriteStdin)?; - stdin - .write_all(b"GETPIN\n") - .await - .context(crate::error::WriteStdin)?; - } + let mut stdin = child.stdin.take().unwrap(); - let out = child - .wait_with_output() + stdin + .write_all(b"SETTITLE rbw\n") + .await + .context(crate::error::WriteStdin)?; + stdin + .write_all(format!("SETPROMPT {}\n", prompt).as_bytes()) + .await + .context(crate::error::WriteStdin)?; + stdin + .write_all(format!("SETDESC {}\n", desc).as_bytes()) .await - .context(crate::error::ProcessWaitOutput)? - .stdout; - let out_str = String::from_utf8(out.clone()).context( - crate::error::FailedToParsePinentryUtf8 { out: out.clone() }, - )?; - for line in out_str.lines() { - if line.starts_with("D ") { - return Ok(line[2..line.len()].to_string()); - } else if !line.starts_with("OK") { - break; + .context(crate::error::WriteStdin)?; + stdin + .write_all(b"GETPIN\n") + .await + .context(crate::error::WriteStdin)?; + drop(stdin); + + let mut buf = crate::locked::Vec::new(); + buf.extend(std::iter::repeat(0)); + let len = + read_password(buf.data_mut(), child.stdout.as_mut().unwrap()).await?; + buf.truncate(len); + + child.await.context(crate::error::PinentryWait)?; + + Ok(crate::locked::Password::new(buf)) +} + +async fn read_password< + R: tokio::io::AsyncRead + tokio::io::AsyncReadExt + Unpin, +>( + data: &mut [u8], + mut r: R, +) -> Result { + let mut len = 0; + loop { + let nl = data.iter().take(len).position(|c| *c == b'\n'); + if let Some(nl) = nl { + if data.starts_with(b"OK") { + data.copy_within((nl + 1).., 0); + len -= nl + 1; + } else if data.starts_with(b"D ") { + data.copy_within(2..nl, 0); + len = nl - 2; + break; + } else { + return Err(Error::FailedToParsePinentry { + out: data.to_vec(), + }); + } + } else { + let bytes = r + .read(&mut data[len..]) + .await + .context(crate::error::PinentryReadOutput)?; + len += bytes; } } - Err(Error::FailedToParsePinentry { out }) + Ok(len) +} + +#[test] +fn test_read_password() { + let good_inputs = &[ + &b"D super secret password\n"[..], + &b"OK\nOK\nOK\nD super secret password\nOK\n"[..], + &b"OK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nD super secret password\nOK\n"[..], + &b"OK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nOK\nD super secret password\nOK\n"[..], + ]; + for input in good_inputs { + let mut buf = [0; 64]; + tokio::runtime::Runtime::new().unwrap().block_on(async { + let len = read_password(&mut buf, &input[..]).await.unwrap(); + assert_eq!(&buf[0..len], b"super secret password"); + }); + } } -- cgit v1.2.3-54-g00ecf