[PM-26534] Remove non-generic wrapper for PasswordProtectedKeyEnvelope (#488)

## 🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-26534

## 📔 Objective

The initial implementation assumed that the struct of of the
PasswordProtectedKeyEnvelope in crypto had to be generic, because the
key context is generic w.r.t. the keys used, and the keys are only then
provided by core. Because uniffy / wasm-bindgen can't handle generics, I
added a non-generic wrapper struct in core, leaving us with two
different "PasswordProtectedKeyEnvelopes".

This understanding was not correct. You don't actually need to make the
struct generic, having just two specific functions on the impl generic
to the key id's suffices, allowing us to drop the "non generic wrapper"
entirely, vastly simplifing our code here. The changes are mostly
removing code and moving the uniffi mapping to the crypto crate.

## ⏰ Reminders before review

- Contributor guidelines followed
- All formatters and local linters executed and passed
- Written new unit and / or integration tests where applicable
- Protected functional changes with optionality (feature flags)
- Used internationalization (i18n) for all UI strings
- CI builds passed
- Communicated to DevOps any deployment requirements
- Updated any necessary documentation (Confluence, contributing docs) or
informed the documentation
  team

## 🦮 Reviewer guidelines

<!-- Suggested interactions but feel free to use (or not) as you desire!
-->

- 👍 (`:+1:`) or similar for great changes
- 📝 (`:memo:`) or ℹ️ (`:information_source:`) for notes or general info
- ❓ (`:question:`) for questions
- 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry
that's not quite a confirmed
  issue and could potentially benefit from discussion
- 🎨 (`:art:`) for suggestions / improvements
- ❌ (`:x:`) or ⚠️ (`:warning:`) for more significant problems or
concerns needing attention
- 🌱 (`:seedling:`) or ♻️ (`:recycle:`) for future improvements or
indications of technical debt
- ⛏ (`:pick:`) for minor or nitpick changes

Author: quexten

crates/bitwarden-core/src/client/internal.rs

@@ -4,7 +4,10 @@ use bitwarden_crypto::KeyStore;
 #[cfg(any(feature = "internal", feature = "secrets"))]
 use bitwarden_crypto::SymmetricCryptoKey;
 #[cfg(feature = "internal")]
-use bitwarden_crypto::{CryptoError, EncString, Kdf, MasterKey, PinKey, UnsignedSharedKey};
+use bitwarden_crypto::{
+    CryptoError, EncString, Kdf, MasterKey, PinKey, UnsignedSharedKey,
+    safe::PasswordProtectedKeyEnvelope,
+};
 #[cfg(feature = "internal")]
 use bitwarden_state::registry::StateRegistry;
 use chrono::Utc;
@@ -26,8 +29,7 @@ use crate::{
     },
     error::NotAuthenticatedError,
     key_management::{
-        MasterPasswordUnlockData, PasswordProtectedKeyEnvelope, SecurityState, SignedSecurityState,
-        crypto::InitUserCryptoRequest,
+        MasterPasswordUnlockData, SecurityState, SignedSecurityState, crypto::InitUserCryptoRequest,
     },
 };
 

crates/bitwarden-core/src/key_management/crypto.rs

@@ -10,7 +10,8 @@ use bitwarden_crypto::{
     AsymmetricCryptoKey, CoseSerializable, CryptoError, EncString, Kdf, KeyDecryptable,
     KeyEncryptable, MasterKey, Pkcs8PrivateKeyBytes, PrimitiveEncryptable, SignatureAlgorithm,
     SignedPublicKey, SigningKey, SpkiPublicKeyBytes, SymmetricCryptoKey, UnsignedSharedKey,
-    UserKey, dangerous_get_v2_rotated_account_keys, safe::PasswordProtectedKeyEnvelopeError,
+    UserKey, dangerous_get_v2_rotated_account_keys,
+    safe::{PasswordProtectedKeyEnvelope, PasswordProtectedKeyEnvelopeError},
 };
 use bitwarden_encoding::B64;
 use bitwarden_error::bitwarden_error;
@@ -26,7 +27,6 @@ use crate::{
     key_management::{
         AsymmetricKeyId, SecurityState, SignedSecurityState, SigningKeyId, SymmetricKeyId,
         master_password::{MasterPasswordAuthenticationData, MasterPasswordUnlockData},
-        non_generic_wrappers::PasswordProtectedKeyEnvelope,
     },
 };
 
@@ -429,12 +429,7 @@ pub(super) fn enroll_pin(
     let key_store = client.internal.get_key_store();
     let mut ctx = key_store.context_mut();
 
-    let key_envelope =
-        PasswordProtectedKeyEnvelope(bitwarden_crypto::safe::PasswordProtectedKeyEnvelope::seal(
-            SymmetricKeyId::User,
-            &pin,
-            &ctx,
-        )?);
+    let key_envelope = PasswordProtectedKeyEnvelope::seal(SymmetricKeyId::User, &pin, &ctx)?;
     let encrypted_pin = pin.encrypt(&mut ctx, SymmetricKeyId::User)?;
     Ok(EnrollPinResponse {
         pin_protected_user_key_envelope: key_envelope,

crates/bitwarden-core/src/key_management/crypto_client.rs

@@ -1,3 +1,5 @@
+#[cfg(feature = "wasm")]
+use bitwarden_crypto::safe::PasswordProtectedKeyEnvelope;
 use bitwarden_crypto::{CryptoError, Decryptable, Kdf};
 #[cfg(feature = "internal")]
 use bitwarden_crypto::{EncString, UnsignedSharedKey};
@@ -10,8 +12,6 @@ use super::crypto::{
     MakeKeyPairResponse, VerifyAsymmetricKeysRequest, VerifyAsymmetricKeysResponse,
     derive_key_connector, make_key_pair, verify_asymmetric_keys,
 };
-#[cfg(any(feature = "wasm", test))]
-use crate::key_management::PasswordProtectedKeyEnvelope;
 #[cfg(feature = "internal")]
 use crate::key_management::{
     SymmetricKeyId,

crates/bitwarden-core/src/key_management/mod.rs

@@ -26,10 +26,6 @@ pub use master_password::MasterPasswordError;
 #[cfg(feature = "internal")]
 pub(crate) use master_password::{MasterPasswordAuthenticationData, MasterPasswordUnlockData};
 #[cfg(feature = "internal")]
-mod non_generic_wrappers;
-#[cfg(feature = "internal")]
-pub(crate) use non_generic_wrappers::*;
-#[cfg(feature = "internal")]
 mod security_state;
 #[cfg(feature = "internal")]
 pub use security_state::{SecurityState, SignedSecurityState};

crates/bitwarden-core/src/key_management/non_generic_wrappers.rs

@@ -2,12 +2,14 @@
 
 use std::{num::NonZeroU32, str::FromStr};
 
+use bitwarden_crypto::safe;
 use bitwarden_uniffi_error::convert_result;
 use uuid::Uuid;
 
-use crate::key_management::{PasswordProtectedKeyEnvelope, SignedSecurityState};
+use crate::key_management::SignedSecurityState;
 
 uniffi::use_remote_type!(bitwarden_crypto::NonZeroU32);
+uniffi::use_remote_type!(bitwarden_crypto::safe::PasswordProtectedKeyEnvelope);
 
 type DateTime = chrono::DateTime<chrono::Utc>;
 uniffi::custom_type!(DateTime, std::time::SystemTime, { remote });
@@ -33,10 +35,3 @@ uniffi::custom_type!(SignedSecurityState, String, {
     },
     lower: |obj| obj.into(),
 });
-
-uniffi::custom_type!(PasswordProtectedKeyEnvelope, String, {
-    remote,
-    try_lift: |val| convert_result(bitwarden_crypto::safe::PasswordProtectedKeyEnvelope::from_str(&val)
-        .map(PasswordProtectedKeyEnvelope)),
-    lower: |obj| obj.0.into(),
-});

crates/bitwarden-core/src/uniffi_support.rs

@@ -34,12 +34,11 @@ fn main() {
     ctx.clear_local();
 
     // Load the envelope from disk and unseal it with the PIN, and store it in the context.
-    let deserialized: PasswordProtectedKeyEnvelope<ExampleIds> =
-        PasswordProtectedKeyEnvelope::try_from(
-            disk.load("vault_key_envelope")
-                .expect("Loading from disk should work"),
-        )
-        .expect("Deserializing envelope should work");
+    let deserialized: PasswordProtectedKeyEnvelope = PasswordProtectedKeyEnvelope::try_from(
+        disk.load("vault_key_envelope")
+            .expect("Loading from disk should work"),
+    )
+    .expect("Deserializing envelope should work");
     deserialized
         .unseal(pin, &mut ctx)
         .expect("Unsealing should work");

crates/bitwarden-crypto/examples/protect_key_with_password.rs

@@ -13,7 +13,7 @@
 //! single recipient's unprotected headers. The output from the KDF - "envelope key", is used to
 //! wrap the symmetric key, that is sealed by the envelope.
 
-use std::{marker::PhantomData, num::TryFromIntError, str::FromStr};
+use std::{num::TryFromIntError, str::FromStr};
 
 use argon2::Params;
 use bitwarden_encoding::{B64, FromStrVisitor};
@@ -22,6 +22,8 @@ use coset::{CborSerializable, CoseError, Header, HeaderBuilder};
 use rand::RngCore;
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
+#[cfg(feature = "wasm")]
+use wasm_bindgen::convert::FromWasmAbi;
 
 use crate::{
     BitwardenLegacyKeyBytes, ContentFormat, CoseKeyBytes, EncodedSymmetricKey, KeyIds,
@@ -47,17 +49,16 @@ const ENVELOPE_ARGON2_OUTPUT_KEY_SIZE: usize = 32;
 /// be provided.
 ///
 /// Internally, Argon2 is used as the KDF and XChaCha20-Poly1305 is used to encrypt the key.
-pub struct PasswordProtectedKeyEnvelope<Ids: KeyIds> {
-    _phantom: PhantomData<Ids>,
+pub struct PasswordProtectedKeyEnvelope {
     cose_encrypt: coset::CoseEncrypt,
 }
 
-impl<Ids: KeyIds> PasswordProtectedKeyEnvelope<Ids> {
+impl PasswordProtectedKeyEnvelope {
     /// Seals a symmetric key with a password, using the current default KDF parameters and a random
     /// salt.
     ///
     /// This should never fail, except for memory allocation error, when running the KDF.
-    pub fn seal(
+    pub fn seal<Ids: KeyIds>(
         key_to_seal: Ids::Symmetric,
         password: &str,
         ctx: &KeyStoreContext<Ids>,
@@ -129,15 +130,12 @@ impl<Ids: KeyIds> PasswordProtectedKeyEnvelope<Ids> {
             .build();
         cose_encrypt.unprotected.iv = nonce.into();
 
-        Ok(PasswordProtectedKeyEnvelope {
-            _phantom: PhantomData,
-            cose_encrypt,
-        })
+        Ok(PasswordProtectedKeyEnvelope { cose_encrypt })
     }
 
     /// Unseals a symmetric key from the password-protected envelope, and stores it in the key store
     /// context.
-    pub fn unseal(
+    pub fn unseal<Ids: KeyIds>(
         &self,
         password: &str,
         ctx: &mut KeyStoreContext<Ids>,
@@ -225,36 +223,33 @@ impl<Ids: KeyIds> PasswordProtectedKeyEnvelope<Ids> {
     }
 }
 
-impl<Ids: KeyIds> From<&PasswordProtectedKeyEnvelope<Ids>> for Vec<u8> {
-    fn from(val: &PasswordProtectedKeyEnvelope<Ids>) -> Self {
+impl From<&PasswordProtectedKeyEnvelope> for Vec<u8> {
+    fn from(val: &PasswordProtectedKeyEnvelope) -> Self {
         val.cose_encrypt
             .clone()
             .to_vec()
             .expect("Serialization to cose should not fail")
     }
 }
 
-impl<Ids: KeyIds> TryFrom<&Vec<u8>> for PasswordProtectedKeyEnvelope<Ids> {
+impl TryFrom<&Vec<u8>> for PasswordProtectedKeyEnvelope {
     type Error = CoseError;
 
     fn try_from(value: &Vec<u8>) -> Result<Self, Self::Error> {
         let cose_encrypt = coset::CoseEncrypt::from_slice(value)?;
-        Ok(PasswordProtectedKeyEnvelope {
-            _phantom: PhantomData,
-            cose_encrypt,
-        })
+        Ok(PasswordProtectedKeyEnvelope { cose_encrypt })
     }
 }
 
-impl<Ids: KeyIds> std::fmt::Debug for PasswordProtectedKeyEnvelope<Ids> {
+impl std::fmt::Debug for PasswordProtectedKeyEnvelope {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("PasswordProtectedKeyEnvelope")
             .field("cose_encrypt", &self.cose_encrypt)
             .finish()
     }
 }
 
-impl<Ids: KeyIds> FromStr for PasswordProtectedKeyEnvelope<Ids> {
+impl FromStr for PasswordProtectedKeyEnvelope {
     type Err = PasswordProtectedKeyEnvelopeError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
@@ -271,14 +266,14 @@ impl<Ids: KeyIds> FromStr for PasswordProtectedKeyEnvelope<Ids> {
     }
 }
 
-impl<Ids: KeyIds> From<PasswordProtectedKeyEnvelope<Ids>> for String {
-    fn from(val: PasswordProtectedKeyEnvelope<Ids>) -> Self {
+impl From<PasswordProtectedKeyEnvelope> for String {
+    fn from(val: PasswordProtectedKeyEnvelope) -> Self {
         let serialized: Vec<u8> = (&val).into();
         B64::from(serialized).to_string()
     }
 }
 
-impl<'de, Ids: KeyIds> Deserialize<'de> for PasswordProtectedKeyEnvelope<Ids> {
+impl<'de> Deserialize<'de> for PasswordProtectedKeyEnvelope {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: serde::Deserializer<'de>,
@@ -287,7 +282,7 @@ impl<'de, Ids: KeyIds> Deserialize<'de> for PasswordProtectedKeyEnvelope<Ids> {
     }
 }
 
-impl<Ids: KeyIds> Serialize for PasswordProtectedKeyEnvelope<Ids> {
+impl Serialize for PasswordProtectedKeyEnvelope {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
         S: serde::Serializer,
@@ -443,6 +438,30 @@ impl From<TryFromIntError> for PasswordProtectedKeyEnvelopeError {
     }
 }
 
+#[cfg(feature = "wasm")]
+#[wasm_bindgen::prelude::wasm_bindgen(typescript_custom_section)]
+const TS_CUSTOM_TYPES: &'static str = r#"
+export type PasswordProtectedKeyEnvelope = Tagged<string, "PasswordProtectedKeyEnvelope">;
+"#;
+
+#[cfg(feature = "wasm")]
+impl wasm_bindgen::describe::WasmDescribe for PasswordProtectedKeyEnvelope {
+    fn describe() {
+        <String as wasm_bindgen::describe::WasmDescribe>::describe();
+    }
+}
+
+#[cfg(feature = "wasm")]
+impl FromWasmAbi for PasswordProtectedKeyEnvelope {
+    type Abi = <String as FromWasmAbi>::Abi;
+
+    unsafe fn from_abi(abi: Self::Abi) -> Self {
+        use wasm_bindgen::UnwrapThrowExt;
+        let string = unsafe { String::from_abi(abi) };
+        PasswordProtectedKeyEnvelope::from_str(&string).unwrap_throw()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -543,7 +562,7 @@ mod tests {
         let serialized: Vec<u8> = (&envelope).into();
 
         // Unseal the key from the envelope
-        let deserialized: PasswordProtectedKeyEnvelope<TestIds> =
+        let deserialized: PasswordProtectedKeyEnvelope =
             PasswordProtectedKeyEnvelope::try_from(&serialized).unwrap();
         let key = deserialized.unseal(password, &mut ctx).unwrap();
 
@@ -574,7 +593,7 @@ mod tests {
         let serialized: Vec<u8> = (&envelope).into();
 
         // Unseal the key from the envelope
-        let deserialized: PasswordProtectedKeyEnvelope<TestIds> =
+        let deserialized: PasswordProtectedKeyEnvelope =
             PasswordProtectedKeyEnvelope::try_from(&serialized).unwrap();
         let key = deserialized.unseal(password, &mut ctx).unwrap();
 
@@ -599,7 +618,7 @@ mod tests {
         let new_password = "new_test_password";
 
         // Seal the key with a password
-        let envelope: PasswordProtectedKeyEnvelope<TestIds> =
+        let envelope: PasswordProtectedKeyEnvelope =
             PasswordProtectedKeyEnvelope::seal_ref(&key, password).expect("Sealing should work");
 
         // Reseal
@@ -627,7 +646,7 @@ mod tests {
         let envelope = PasswordProtectedKeyEnvelope::seal(test_key, password, &ctx).unwrap();
 
         // Attempt to unseal with the wrong password
-        let deserialized: PasswordProtectedKeyEnvelope<TestIds> =
+        let deserialized: PasswordProtectedKeyEnvelope =
             PasswordProtectedKeyEnvelope::try_from(&(&envelope).into()).unwrap();
         assert!(matches!(
             deserialized.unseal(wrong_password, &mut ctx),

crates/bitwarden-crypto/src/safe/password_protected_key_envelope.rs

@@ -2,7 +2,9 @@ use std::{num::NonZeroU32, str::FromStr};
 
 use bitwarden_uniffi_error::convert_result;
 
-use crate::{CryptoError, EncString, SignedPublicKey, UnsignedSharedKey};
+use crate::{
+    CryptoError, EncString, SignedPublicKey, UnsignedSharedKey, safe::PasswordProtectedKeyEnvelope,
+};
 
 uniffi::custom_type!(NonZeroU32, u32, {
     remote,
@@ -32,3 +34,9 @@ uniffi::custom_type!(SignedPublicKey, String, {
     },
     lower: |obj| obj.into(),
 });
+
+uniffi::custom_type!(PasswordProtectedKeyEnvelope, String, {
+    remote,
+    try_lift: |val| convert_result(PasswordProtectedKeyEnvelope::from_str(&val)),
+    lower: |obj| obj.into(),
+});