code cleanup and use hex for secret key encoding

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",

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"]

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<String>,
-	pub secret_key: Protected<String>, // base64 encoded string that is required along with the master password
+	pub secret_key: Protected<String>, // 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<String>, // base64 encoded string that is required along with the master password
+	pub secret_key: Protected<String>, // hex encoded string that is required along with the master password
 	pub updated_keystore: Vec<StoredKey>,
 }
 
 /// The `KeyManager` functions should be used for all key-related management.
 impl KeyManager {
+	fn format_secret_key(salt: &[u8; 16]) -> Protected<String> {
+		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<String>` to a `Protected<Vec<u8>>`
-	#[allow(clippy::unused_self)]
-	#[allow(clippy::needless_pass_by_value)]
-	fn convert_master_password_string(
-		&self,
-		master_password: Protected<String>,
-	) -> Protected<Vec<u8>> {
-		Protected::new(master_password.expose().as_bytes().to_vec())
-	}
-
-	/// Used internally to convert from a base64-encoded `Protected<String>` to a `Protected<[u8; SALT_LEN]>` in a secretive manner.
+	/// Used internally to convert from a hex-encoded `Protected<String>` 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<String>,
-	) -> 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<String>) -> 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<Vec<StoredKey>> {
 		// 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