From 10cc8a28bf8bc14768b31b6ef5cde3a76dcee7cf Mon Sep 17 00:00:00 2001 From: brxken128 <77554505+brxken128@users.noreply.github.com> Date: Wed, 7 Dec 2022 15:00:14 +0000 Subject: [PATCH] code cleanup and use `hex` for secret key encoding --- Cargo.lock | 2 +- crates/crypto/Cargo.toml | 2 +- crates/crypto/src/keys/keymanager.rs | 58 +++++++++++++++------------- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 80b51cf1c..c1276a14b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5208,9 +5208,9 @@ dependencies = [ "aead", "aes-gcm", "argon2", - "base64 0.13.1", "chacha20poly1305", "dashmap", + "hex", "rand 0.8.5", "rand_chacha 0.3.1", "rspc", diff --git a/crates/crypto/Cargo.toml b/crates/crypto/Cargo.toml index 7ca87a828..c11abdbc1 100644 --- a/crates/crypto/Cargo.toml +++ b/crates/crypto/Cargo.toml @@ -38,7 +38,7 @@ dashmap = "5.4.0" rspc = { workspace = true, optional = true } specta = { workspace = true, optional = true } -base64 = "0.13.1" +hex = "0.4.3" [features] rspc = ["dep:rspc", "dep:specta"] diff --git a/crates/crypto/src/keys/keymanager.rs b/crates/crypto/src/keys/keymanager.rs index 7127d7778..38e5e737f 100644 --- a/crates/crypto/src/keys/keymanager.rs +++ b/crates/crypto/src/keys/keymanager.rs @@ -112,20 +112,37 @@ pub struct KeyManager { pub struct OnboardingBundle { pub verification_key: StoredKey, // nil UUID key that is only ever used for verifying the master password is correct pub master_password: Protected, - pub secret_key: Protected, // base64 encoded string that is required along with the master password + pub secret_key: Protected, // hex encoded string that is required along with the master password } pub struct MasterPasswordChangeBundle { pub verification_key: StoredKey, // nil UUID key that is only ever used for verifying the master password is correct - pub secret_key: Protected, // base64 encoded string that is required along with the master password + pub secret_key: Protected, // hex encoded string that is required along with the master password pub updated_keystore: Vec, } /// The `KeyManager` functions should be used for all key-related management. impl KeyManager { + fn format_secret_key(salt: &[u8; 16]) -> Protected { + let hex_string: String = hex::encode_upper(salt) + .chars() + .enumerate() + .map(|(i, c)| { + if (i + 1) % 8 == 0 && i != 31 { + c.to_string() + "-" + } else { + c.to_string() + } + }) + .into_iter() + .collect(); + + Protected::new(hex_string) + } + /// This should be used to generate everything for the user during onboarding. /// - /// This will create a master password (a 7-word diceware passphrase), and a secret key (16 bytes, encoded in base64) + /// This will create a master password (a 7-word diceware passphrase), and a secret key (16 bytes, hex encoded) /// /// It will also generate a verification key, which should be written to the database. #[allow(clippy::needless_pass_by_value)] @@ -172,7 +189,7 @@ impl KeyManager { key: Vec::new(), }; - let secret_key = Protected::new(base64::encode(salt)); + let secret_key = Self::format_secret_key(&salt); let onboarding_bundle = OnboardingBundle { verification_key, @@ -385,7 +402,7 @@ impl KeyManager { key: Vec::new(), }; - let secret_key = Protected::new(base64::encode(salt)); + let secret_key = Self::format_secret_key(&salt); let mpc_bundle = MasterPasswordChangeBundle { verification_key, @@ -401,26 +418,15 @@ impl KeyManager { Ok(mpc_bundle) } - /// Used internally to convert a `Protected` to a `Protected>` - #[allow(clippy::unused_self)] - #[allow(clippy::needless_pass_by_value)] - fn convert_master_password_string( - &self, - master_password: Protected, - ) -> Protected> { - Protected::new(master_password.expose().as_bytes().to_vec()) - } - - /// Used internally to convert from a base64-encoded `Protected` to a `Protected<[u8; SALT_LEN]>` in a secretive manner. + /// Used internally to convert from a hex-encoded `Protected` to a `Protected<[u8; SALT_LEN]>` in a secretive manner. /// /// If the secret key is wrong (not base64 or not the correct length), a filler secret key will be inserted secretly. - #[allow(clippy::unused_self)] #[allow(clippy::needless_pass_by_value)] - fn convert_secret_key_string( - &self, - secret_key: Protected, - ) -> Protected<[u8; SALT_LEN]> { - let secret_key = if let Ok(secret_key) = base64::decode(secret_key.expose()) { + fn convert_secret_key_string(secret_key: Protected) -> Protected<[u8; SALT_LEN]> { + let mut secret_key_clean = secret_key.expose().clone(); + secret_key_clean.retain(|c| c != '-'); + + let secret_key = if let Ok(secret_key) = hex::decode(secret_key_clean) { secret_key } else { Vec::new() @@ -446,8 +452,8 @@ impl KeyManager { stored_keys: &[StoredKey], // from the backup ) -> Result> { // this backup should contain a verification key, which will tell us the algorithm+hashing algorithm - let master_password = self.convert_master_password_string(master_password); - let secret_key = self.convert_secret_key_string(secret_key); + let master_password = Protected::new(master_password.expose().as_bytes().to_vec()); + let secret_key = Self::convert_secret_key_string(secret_key); let mut verification_key = None; @@ -530,8 +536,8 @@ impl KeyManager { Some(k) => Ok(k.clone()), None => Err(Error::NoVerificationKey), }?; - let master_password = self.convert_master_password_string(master_password); - let secret_key = self.convert_secret_key_string(secret_key); + let master_password = Protected::new(master_password.expose().as_bytes().to_vec()); + let secret_key = Self::convert_secret_key_string(secret_key); let hashed_master_password = verification_key .hashing_algorithm