Merge branch 'main' into sm/update-ruby-readme

.vscode/settings.json

@@ -21,6 +21,7 @@
     "totp",
     "uniffi",
     "wordlist",
+    "Zeroize",
     "zxcvbn"
   ]
 }

README.md

@@ -140,5 +140,24 @@ VALUES
   );
 ```
 
+## Developer tools
+
+This project recommends the use of certain developer tools, and also includes configurations for
+them to make developers lives easier. The use of these tools is optional and they might require a
+separate installation step.
+
+The list of developer tools is:
+
+- `Visual Studio Code`: We provide a recommended extension list which should show under the
+  `Extensions` tab when opening this project with the editor. We also offer a few launch settings
+  and tasks to build and run the SDK
+- `bacon`: This is a CLI background code checker. We provide a configuration file with some of the
+  most common tasks to run (`check`, `clippy`, `test`, `doc` - run `bacon -l` to see them all). This
+  tool needs to be installed separately by running `cargo install bacon --locked`.
+- `nexttest`: This is a new and faster test runner, capable of running tests in parallel and with a
+  much nicer output compared to `cargo test`. This tool needs to be installed separately by running
+  `cargo install cargo-nextest --locked`. It can be manually run using
+  `cargo nextest run --all-features`
+
 [secrets-manager]: https://bitwarden.com/products/secrets-manager/
 [bws-help]: https://bitwarden.com/help/secrets-manager-cli/

bacon.toml

@@ -0,0 +1,78 @@
+# This is a configuration file for the bacon tool
+#
+# Bacon repository: https://github.com/Canop/bacon
+# Complete help on configuration: https://dystroy.org/bacon/config/
+# You can also check bacon's own bacon.toml file
+#  as an example: https://github.com/Canop/bacon/blob/main/bacon.toml
+
+default_job = "check"
+
+[jobs.check]
+command = ["cargo", "check", "--color", "always"]
+need_stdout = false
+
+[jobs.check-all]
+command = ["cargo", "check", "--all-targets", "--color", "always"]
+need_stdout = false
+
+[jobs.clippy]
+command = ["cargo", "clippy", "--all-targets", "--color", "always"]
+need_stdout = false
+
+[jobs.test]
+command = [
+    "cargo",
+    "test",
+    "--all-features",
+    "--color",
+    "always",
+    "--",
+    "--color",
+    "always",         # see https://github.com/Canop/bacon/issues/124
+]
+need_stdout = true
+
+[jobs.doc]
+command = ["cargo", "doc", "--color", "always", "--no-deps"]
+need_stdout = false
+
+# If the doc compiles, then it opens in your browser and bacon switches
+# to the previous job
+[jobs.doc-open]
+command = ["cargo", "doc", "--color", "always", "--no-deps", "--open"]
+need_stdout = false
+on_success = "back"                                                    # so that we don't open the browser at each change
+
+[jobs.doc-internal]
+command = [
+    "cargo",
+    "doc",
+    "--color",
+    "always",
+    "--no-deps",
+    "--all-features",
+    "--document-private-items",
+]
+need_stdout = false
+
+[jobs.doc-internal-open]
+command = [
+    "cargo",
+    "doc",
+    "--color",
+    "always",
+    "--no-deps",
+    "--all-features",
+    "--document-private-items",
+    "--open",
+]
+allow_warnings = true
+need_stdout = false
+on_success = "job:doc-internal"
+
+# You may define here keybindings that would be specific to
+# a project, for example a shortcut to launch a specific job.
+# Shortcuts to internal functions (scrolling, toggling, etc.)
+# should go in your personal global prefs.toml file instead.
+[keybindings]
+# alt-m = "job:my-job"

crates/bitwarden-crypto/src/keys/master_key.rs

@@ -5,8 +5,14 @@ use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
 use super::utils::{derive_kdf_key, stretch_kdf_key};
-use crate::{util, CryptoError, EncString, KeyDecryptable, Result, SymmetricCryptoKey, UserKey};
+use crate::{
+    util, CryptoError, EncString, KeyDecryptable, Result, SensitiveVec, SymmetricCryptoKey, UserKey,
+};
 
+/// Key Derivation Function for Bitwarden Account
+///
+/// In Bitwarden accounts can use multiple KDFs to derive their master key from their password. This
+/// Enum represents all the possible KDFs.
 #[derive(Serialize, Deserialize, Debug, JsonSchema, Clone)]
 #[serde(rename_all = "camelCase", deny_unknown_fields)]
 #[cfg_attr(feature = "mobile", derive(uniffi::Enum))]
@@ -22,22 +28,27 @@ pub enum Kdf {
 }
 
 impl Default for Kdf {
+    /// Default KDF for new accounts.
     fn default() -> Self {
         Kdf::PBKDF2 {
             iterations: default_pbkdf2_iterations(),
         }
     }
 }
 
+/// Default PBKDF2 iterations
 pub fn default_pbkdf2_iterations() -> NonZeroU32 {
     NonZeroU32::new(600_000).expect("Non-zero number")
 }
+/// Default Argon2 iterations
 pub fn default_argon2_iterations() -> NonZeroU32 {
     NonZeroU32::new(3).expect("Non-zero number")
 }
+/// Default Argon2 memory
 pub fn default_argon2_memory() -> NonZeroU32 {
     NonZeroU32::new(64).expect("Non-zero number")
 }
+/// Default Argon2 parallelism
 pub fn default_argon2_parallelism() -> NonZeroU32 {
     NonZeroU32::new(4).expect("Non-zero number")
 }
@@ -60,13 +71,17 @@ impl MasterKey {
     }
 
     /// Derives a users master key from their password, email and KDF.
-    pub fn derive(password: &[u8], email: &[u8], kdf: &Kdf) -> Result<Self> {
-        derive_kdf_key(password, email, kdf).map(Self)
+    pub fn derive(password: &SensitiveVec, email: &[u8], kdf: &Kdf) -> Result<Self> {
+        derive_kdf_key(password.expose(), email, kdf).map(Self)
     }
 
     /// Derive the master key hash, used for local and remote password validation.
-    pub fn derive_master_key_hash(&self, password: &[u8], purpose: HashPurpose) -> Result<String> {
-        let hash = util::pbkdf2(&self.0.key, password, purpose as u32);
+    pub fn derive_master_key_hash(
+        &self,
+        password: &SensitiveVec,
+        purpose: HashPurpose,
+    ) -> Result<String> {
+        let hash = util::pbkdf2(&self.0.key, password.expose(), purpose as u32);
 
         Ok(STANDARD.encode(hash))
     }
@@ -115,13 +130,15 @@ mod tests {
     use rand::SeedableRng;
 
     use super::{make_user_key, HashPurpose, Kdf, MasterKey};
-    use crate::{keys::symmetric_crypto_key::derive_symmetric_key, SymmetricCryptoKey};
+    use crate::{
+        keys::symmetric_crypto_key::derive_symmetric_key, SensitiveVec, SymmetricCryptoKey,
+    };
 
     #[test]
     fn test_master_key_derive_pbkdf2() {
         let master_key = MasterKey::derive(
-            &b"67t9b5g67$%Dh89n"[..],
-            "test_key".as_bytes(),
+            &SensitiveVec::test(b"67t9b5g67$%Dh89n"),
+            b"test_key",
             &Kdf::PBKDF2 {
                 iterations: NonZeroU32::new(10000).unwrap(),
             },
@@ -141,8 +158,8 @@ mod tests {
     #[test]
     fn test_master_key_derive_argon2() {
         let master_key = MasterKey::derive(
-            &b"67t9b5g67$%Dh89n"[..],
-            "test_key".as_bytes(),
+            &SensitiveVec::test(b"67t9b5g67$%Dh89n"),
+            b"test_key",
             &Kdf::Argon2id {
                 iterations: NonZeroU32::new(4).unwrap(),
                 memory: NonZeroU32::new(32).unwrap(),
@@ -163,38 +180,38 @@ mod tests {
 
     #[test]
     fn test_password_hash_pbkdf2() {
-        let password = "asdfasdf".as_bytes();
-        let salt = "test_salt".as_bytes();
+        let password = SensitiveVec::test(b"asdfasdf");
+        let salt = b"test_salt";
         let kdf = Kdf::PBKDF2 {
             iterations: NonZeroU32::new(100_000).unwrap(),
         };
 
-        let master_key = MasterKey::derive(password, salt, &kdf).unwrap();
+        let master_key = MasterKey::derive(&password, salt, &kdf).unwrap();
 
         assert_eq!(
             "ZF6HjxUTSyBHsC+HXSOhZoXN+UuMnygV5YkWXCY4VmM=",
             master_key
-                .derive_master_key_hash(password, HashPurpose::ServerAuthorization)
+                .derive_master_key_hash(&password, HashPurpose::ServerAuthorization)
                 .unwrap(),
         );
     }
 
     #[test]
     fn test_password_hash_argon2id() {
-        let password = "asdfasdf".as_bytes();
-        let salt = "test_salt".as_bytes();
+        let password = SensitiveVec::test(b"asdfasdf");
+        let salt = b"test_salt";
         let kdf = Kdf::Argon2id {
             iterations: NonZeroU32::new(4).unwrap(),
             memory: NonZeroU32::new(32).unwrap(),
             parallelism: NonZeroU32::new(2).unwrap(),
         };
 
-        let master_key = MasterKey::derive(password, salt, &kdf).unwrap();
+        let master_key = MasterKey::derive(&password, salt, &kdf).unwrap();
 
         assert_eq!(
             "PR6UjYmjmppTYcdyTiNbAhPJuQQOmynKbdEl1oyi/iQ=",
             master_key
-                .derive_master_key_hash(password, HashPurpose::ServerAuthorization)
+                .derive_master_key_hash(&password, HashPurpose::ServerAuthorization)
                 .unwrap(),
         );
     }

crates/bitwarden-crypto/src/keys/shareable_key.rs

@@ -3,47 +3,58 @@ use std::pin::Pin;
 use aes::cipher::typenum::U64;
 use generic_array::GenericArray;
 use hmac::Mac;
+use zeroize::Zeroize;
 
 use crate::{
     keys::SymmetricCryptoKey,
     util::{hkdf_expand, PbkdfSha256Hmac},
+    Sensitive,
 };
 
 /// Derive a shareable key using hkdf from secret and name.
 ///
 /// A specialized variant of this function was called `CryptoService.makeSendKey` in the Bitwarden
 /// `clients` repository.
 pub fn derive_shareable_key(
-    secret: [u8; 16],
+    secret: Sensitive<[u8; 16]>,
     name: &str,
     info: Option<&str>,
 ) -> SymmetricCryptoKey {
     // Because all inputs are fixed size, we can unwrap all errors here without issue
-
-    // TODO: Are these the final `key` and `info` parameters or should we change them? I followed
-    // the pattern used for sends
-    let res = PbkdfSha256Hmac::new_from_slice(format!("bitwarden-{}", name).as_bytes())
+    let mut res = PbkdfSha256Hmac::new_from_slice(format!("bitwarden-{}", name).as_bytes())
         .expect("hmac new_from_slice should not fail")
-        .chain_update(secret)
+        .chain_update(secret.expose())
         .finalize()
         .into_bytes();
 
     let mut key: Pin<Box<GenericArray<u8, U64>>> =
         hkdf_expand(&res, info).expect("Input is a valid size");
 
+    // Zeroize the temporary buffer
+    res.zeroize();
+
     SymmetricCryptoKey::try_from(key.as_mut_slice()).expect("Key is a valid size")
 }
 
 #[cfg(test)]
 mod tests {
     use super::derive_shareable_key;
+    use crate::Sensitive;
 
     #[test]
     fn test_derive_shareable_key() {
-        let key = derive_shareable_key(*b"&/$%F1a895g67HlX", "test_key", None);
+        let key = derive_shareable_key(
+            Sensitive::new(Box::new(*b"&/$%F1a895g67HlX")),
+            "test_key",
+            None,
+        );
         assert_eq!(key.to_base64().expose(), "4PV6+PcmF2w7YHRatvyMcVQtI7zvCyssv/wFWmzjiH6Iv9altjmDkuBD1aagLVaLezbthbSe+ktR+U6qswxNnQ==");
 
-        let key = derive_shareable_key(*b"67t9b5g67$%Dh89n", "test_key", Some("test"));
+        let key = derive_shareable_key(
+            Sensitive::new(Box::new(*b"67t9b5g67$%Dh89n")),
+            "test_key",
+            Some("test"),
+        );
         assert_eq!(key.to_base64().expose(), "F9jVQmrACGx9VUPjuzfMYDjr726JtL300Y3Yg+VYUnVQtQ1s8oImJ5xtp1KALC9h2nav04++1LDW4iFD+infng==");
     }
 }

crates/bitwarden-crypto/src/keys/symmetric_crypto_key.rs

@@ -134,9 +134,9 @@ impl std::fmt::Debug for SymmetricCryptoKey {
 
 #[cfg(test)]
 pub fn derive_symmetric_key(name: &str) -> SymmetricCryptoKey {
-    use crate::{derive_shareable_key, generate_random_bytes};
+    use crate::{derive_shareable_key, generate_random_bytes, Sensitive};
 
-    let secret: [u8; 16] = generate_random_bytes();
+    let secret: Sensitive<[u8; 16]> = generate_random_bytes();
     derive_shareable_key(secret, name, None)
 }