mlock sensitive memory

11 files changed, 263 insertions, 93 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 15e8451..dbc0448 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -661,6 +661,15 @@ dependencies = [
 ]
 
 [[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"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -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",
@@ -1063,6 +1074,18 @@ 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"
 source = "registry+https://github.com/rust-lang/crates.io-index"
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<u8>, Vec<u8>)> {
+) -> Result<crate::locked::Keys> {
     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<String>,
-    priv_key: Option<(Vec<u8>, Vec<u8>)>,
+    priv_key: Option<rbw::locked::Keys>,
 
     // these should be in a state file
     iterations: Option<u32>,
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<Vec<u8>> {
+    pub fn decrypt(&self, keys: &crate::locked::Keys) -> Result<Vec<u8>> {
+        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<crate::locked::Vec> {
+        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<aes::Aes256, block_modes::block_padding::Pkcs7>,
+    > {
         if self.ty != 2 {
             unimplemented!()
         }
 
         if let Some(mac) = &self.mac {
-            let mut digest = hmac::Hmac::<sha2::Sha256>::new_varkey(mac_key)
-                .map_err(|_| Error::InvalidMacKey)?;
+            let mut digest =
+                hmac::Hmac::<sha2::Sha256>::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<u8> },
 
-    #[snafu(display(
-        "failed to parse pinentry output ({:?}): {}",
-        out,
-        source
-    ))]
-    FailedToParsePinentryUtf8 {
-        out: Vec<u8>,
-        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<u8>,
-    pub mac_key: Vec<u8>,
-    pub master_password_hash: Vec<u8>,
+    pub keys: crate::locked::Keys,
+    pub master_password_hash: crate::locked::PasswordHash,
 }
 
 impl Identity {
-    pub fn new(email: &str, password: &str, iterations: u32) -> Result<Self> {
-        let mut key = vec![0_u8; 32];
+    pub fn new(
+        email: &str,
+        password: &crate::locked::Password,
+        iterations: u32,
+    ) -> Result<Self> {
+        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::<hmac::Hmac<sha2::Sha256>>(
-            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::<hmac::Hmac<sha2::Sha256>>(
-            &key,
-            password.as_bytes(),
+            enc_key,
+            password.password(),
             1,
-            &mut hash,
+            hash.data_mut(),
         );
 
-        let hkdf = hkdf::Hkdf::<sha2::Sha256>::from_prk(&key)
+        let hkdf = hkdf::Hkdf::<sha2::Sha256>::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<arrayvec::ArrayVec<[u8; 4096]>>,
+    _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<Item = u8>) {
+        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<String> {
+) -> Result<crate::locked::Password> {
     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<usize> {
+    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");
+        });
+    }
 }