Stores: Remove StoreCipher::{en,de}crypt_value_[base64_]typed (#3638)

* Use `IndexeddbSerializer` more widely in test code

reuse `IndexeddbSerializer::maybe_encrypt_value` instead of re-inventing it.

* Rewrite `StoreCipher::decrypt_value_base64_typed`

Instead of un-base64-ing and calling `decrypt_value_typed` (which deserializes
the result`), call `decrypt_value_base64_data` (which un-base64s before
decrypting but does not deserialize the result), then deserialize.

This makes it more symmetrical with `encrypt_value_base64_typed`, and helps me
get rid of `decrypt_value_typed` (which is barely used.)

* Fix docs on `StoreCipher::encrypt_value_base64_typed`

looks like they got C&Ped from `encrypt_value_typed`.

* Inline `StoreCipher::{encrypt,decrypt}_value_typed`

Each of these are quite simple, are only used in two places, and their
existence melts my brain.

* Rewrite `IndexeddbSerializer::maybe_{encrypt,decrypt}_value`

... to use `en/decrypt_value_base64_data` instead of
`en/decrypt_value_base64_typed`.

We have to have the de/serialization code for the unencrypted case anyway, so
using the higher-level method isn't helping us much.

* Remove unused `StoreCipher::{en,de}crypt_value_base64_typed`

Outside of tests, these things are totally unused.

Author: richvdh

crates/matrix-sdk-indexeddb/src/crypto_store/indexeddb_serializer.rs

@@ -25,6 +25,7 @@ use matrix_sdk_store_encryption::{EncryptedValueBase64, StoreCipher};
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
 use wasm_bindgen::JsValue;
 use web_sys::IdbKeyRange;
+use zeroize::Zeroizing;
 
 use crate::{safe_encode::SafeEncode, IndexeddbCryptoStoreError};
 
@@ -145,18 +146,29 @@ impl IndexeddbSerializer {
         }
     }
 
-    /// Encode an InboundGroupSession for storage as a value in indexeddb.
+    /// Encode an object for storage as a value in indexeddb.
+    ///
+    /// First serializes the object as JSON bytes.
+    ///
+    /// Then, if a cipher is set, encrypts the JSON with a nonce into binary
+    /// blobs, and base64-encodes the blobs.
+    ///
+    /// If no cipher is set, just base64-encodes the JSON bytes.
+    ///
+    /// Finally, returns an object encapsulating the result.
     pub fn maybe_encrypt_value(
         &self,
         value: PickledInboundGroupSession,
     ) -> Result<MaybeEncrypted, CryptoStoreError> {
+        // First serialize the object as JSON.
+        let serialized = serde_json::to_vec(&value).map_err(CryptoStoreError::backend)?;
+
+        // Then either encrypt the JSON, or just base64-encode it.
         Ok(match &self.store_cipher {
             Some(cipher) => MaybeEncrypted::Encrypted(
-                cipher.encrypt_value_base64_typed(&value).map_err(CryptoStoreError::backend)?,
-            ),
-            None => MaybeEncrypted::Unencrypted(
-                BASE64.encode(serde_json::to_vec(&value).map_err(CryptoStoreError::backend)?),
+                cipher.encrypt_value_base64_data(serialized).map_err(CryptoStoreError::backend)?,
             ),
+            None => MaybeEncrypted::Unencrypted(BASE64.encode(serialized)),
         })
     }
 
@@ -198,16 +210,19 @@ impl IndexeddbSerializer {
         &self,
         value: MaybeEncrypted,
     ) -> Result<PickledInboundGroupSession, CryptoStoreError> {
-        match (&self.store_cipher, value) {
+        // First extract the plaintext JSON, either by decrypting or un-base64-ing.
+        let plaintext = Zeroizing::new(match (&self.store_cipher, value) {
             (Some(cipher), MaybeEncrypted::Encrypted(enc)) => {
-                cipher.decrypt_value_base64_typed(enc).map_err(CryptoStoreError::backend)
+                cipher.decrypt_value_base64_data(enc).map_err(CryptoStoreError::backend)?
             }
             (None, MaybeEncrypted::Unencrypted(unc)) => {
-                Ok(serde_json::from_slice(&BASE64.decode(unc).map_err(CryptoStoreError::backend)?)
-                    .map_err(CryptoStoreError::backend)?)
+                BASE64.decode(unc).map_err(CryptoStoreError::backend)?
             }
 
-            _ => Err(CryptoStoreError::UnpicklingError),
-        }
+            _ => return Err(CryptoStoreError::UnpicklingError),
+        });
+
+        // Then deserialize the JSON.
+        Ok(serde_json::from_slice(&plaintext)?)
     }
 }

crates/matrix-sdk-indexeddb/src/crypto_store/migrations/mod.rs

@@ -235,18 +235,16 @@ mod tests {
     /// Make lots of sessions and see how long it takes to count them in v10
     #[async_test]
     async fn count_lots_of_sessions_v10() {
-        let cipher = Arc::new(StoreCipher::new().unwrap());
-        let serializer = IndexeddbSerializer::new(Some(cipher.clone()));
+        let serializer = IndexeddbSerializer::new(Some(Arc::new(StoreCipher::new().unwrap())));
+
         // Session keys are slow to create, so make one upfront and use it for every
         // session
         let session_key = create_session_key();
 
         // Create lots of InboundGroupSessionIndexedDbObject objects
         let mut objects = Vec::with_capacity(NUM_RECORDS_FOR_PERF);
         for i in 0..NUM_RECORDS_FOR_PERF {
-            objects.push(
-                create_inbound_group_sessions3_record(i, &session_key, &cipher, &serializer).await,
-            );
+            objects.push(create_inbound_group_sessions3_record(i, &session_key, &serializer).await);
         }
 
         // Create a DB with an inbound_group_sessions3 store
@@ -334,15 +332,13 @@ mod tests {
     async fn create_inbound_group_sessions3_record(
         i: usize,
         session_key: &SessionKey,
-        cipher: &Arc<StoreCipher>,
         serializer: &IndexeddbSerializer,
     ) -> (JsValue, JsValue) {
         let session = create_inbound_group_session(i, session_key);
         let pickled_session = session.pickle().await;
+
         let session_dbo = InboundGroupSessionIndexedDbObject {
-            pickled_session: MaybeEncrypted::Encrypted(
-                cipher.encrypt_value_base64_typed(&pickled_session).unwrap(),
-            ),
+            pickled_session: serializer.maybe_encrypt_value(pickled_session).unwrap(),
             needs_backup: false,
             backed_up_to: -1,
         };

crates/matrix-sdk-indexeddb/src/state_store/mod.rs

@@ -150,7 +150,10 @@ pub use keys::ALL_STORES;
 /// Encrypt (if needs be) then JSON-serialize a value.
 fn serialize_value(store_cipher: Option<&StoreCipher>, event: &impl Serialize) -> Result<JsValue> {
     Ok(match store_cipher {
-        Some(cipher) => JsValue::from_serde(&cipher.encrypt_value_typed(event)?)?,
+        Some(cipher) => {
+            let data = serde_json::to_vec(event)?;
+            JsValue::from_serde(&cipher.encrypt_value_data(data)?)?
+        }
         None => JsValue::from_serde(event)?,
     })
 }
@@ -161,7 +164,13 @@ fn deserialize_value<T: DeserializeOwned>(
     event: &JsValue,
 ) -> Result<T> {
     match store_cipher {
-        Some(cipher) => Ok(cipher.decrypt_value_typed(event.into_serde()?)?),
+        Some(cipher) => {
+            use zeroize::Zeroize;
+            let mut plaintext = cipher.decrypt_value_data(event.into_serde()?)?;
+            let ret = serde_json::from_slice(&plaintext);
+            plaintext.zeroize();
+            Ok(ret?)
+        }
         None => Ok(event.into_serde()?),
     }
 }

crates/matrix-sdk-store-encryption/src/lib.rs

@@ -418,45 +418,8 @@ impl StoreCipher {
     /// # anyhow::Ok(()) };
     /// ```
     pub fn encrypt_value(&self, value: &impl Serialize) -> Result<Vec<u8>, Error> {
-        Ok(serde_json::to_vec(&self.encrypt_value_typed(value)?)?)
-    }
-
-    /// Encrypt a value before it is inserted into the key/value store.
-    ///
-    /// A value can be decrypted using the
-    /// [`StoreCipher::decrypt_value_typed()`] method. This is the lower
-    /// level function to `encrypt_value`, but returns the
-    /// full `EncryptdValue`-type
-    ///
-    /// # Arguments
-    ///
-    /// * `value` - A value that should be encrypted, any value that implements
-    ///   `Serialize` can be given to this method. The value will be serialized
-    ///   as json before it is encrypted.
-    ///
-    ///
-    /// # Examples
-    ///
-    /// ```no_run
-    /// # let example = || {
-    /// use matrix_sdk_store_encryption::StoreCipher;
-    /// use serde_json::{json, value::Value};
-    ///
-    /// let store_cipher = StoreCipher::new()?;
-    ///
-    /// let value = json!({
-    ///     "some": "data",
-    /// });
-    ///
-    /// let encrypted = store_cipher.encrypt_value_typed(&value)?;
-    /// let decrypted: Value = store_cipher.decrypt_value_typed(encrypted)?;
-    ///
-    /// assert_eq!(value, decrypted);
-    /// # anyhow::Ok(()) };
-    /// ```
-    pub fn encrypt_value_typed(&self, value: &impl Serialize) -> Result<EncryptedValue, Error> {
         let data = serde_json::to_vec(value)?;
-        self.encrypt_value_data(data)
+        Ok(serde_json::to_vec(&self.encrypt_value_data(data)?)?)
     }
 
     /// Encrypt some data before it is inserted into the key/value store.
@@ -497,47 +460,6 @@ impl StoreCipher {
         Ok(EncryptedValue { version: VERSION, ciphertext, nonce })
     }
 
-    /// Encrypt a value before it is inserted into the key/value store.
-    ///
-    /// A value can be decrypted using the
-    /// [`StoreCipher::decrypt_value_typed()`] method. This is the lower
-    /// level function to `encrypt_value`, but returns the
-    /// full `EncryptdValue`-type
-    ///
-    /// # Arguments
-    ///
-    /// * `value` - A value that should be encrypted, any value that implements
-    ///   `Serialize` can be given to this method. The value will be serialized
-    ///   as json before it is encrypted.
-    ///
-    ///
-    /// # Examples
-    ///
-    /// ```no_run
-    /// # let example = || {
-    /// use matrix_sdk_store_encryption::StoreCipher;
-    /// use serde_json::{json, value::Value};
-    ///
-    /// let store_cipher = StoreCipher::new()?;
-    ///
-    /// let value = json!({
-    ///     "some": "data",
-    /// });
-    ///
-    /// let encrypted = store_cipher.encrypt_value_typed(&value)?;
-    /// let decrypted: Value = store_cipher.decrypt_value_typed(encrypted)?;
-    ///
-    /// assert_eq!(value, decrypted);
-    /// # anyhow::Ok(()) };
-    /// ```
-    pub fn encrypt_value_base64_typed(
-        &self,
-        value: &impl Serialize,
-    ) -> Result<EncryptedValueBase64, Error> {
-        let data = serde_json::to_vec(value)?;
-        self.encrypt_value_base64_data(data)
-    }
-
     /// Encrypt some data before it is inserted into the key/value store,
     /// using base64 for arrays of integers.
     ///
@@ -603,89 +525,12 @@ impl StoreCipher {
     /// ```
     pub fn decrypt_value<T: DeserializeOwned>(&self, value: &[u8]) -> Result<T, Error> {
         let value: EncryptedValue = serde_json::from_slice(value)?;
-        self.decrypt_value_typed(value)
-    }
-
-    /// Decrypt a value after it was fetched from the key/value store.
-    ///
-    /// A value can be encrypted using the
-    /// [`StoreCipher::encrypt_value_typed()`] method. Lower level method to
-    /// [`StoreCipher::decrypt_value_typed()`]
-    ///
-    /// # Arguments
-    ///
-    /// * `value` - The EncryptedValue of a value that should be decrypted.
-    ///
-    /// The method will deserialize the decrypted value into the expected type.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// # let example = || {
-    /// use matrix_sdk_store_encryption::StoreCipher;
-    /// use serde_json::{json, value::Value};
-    ///
-    /// let store_cipher = StoreCipher::new()?;
-    ///
-    /// let value = json!({
-    ///     "some": "data",
-    /// });
-    ///
-    /// let encrypted = store_cipher.encrypt_value_typed(&value)?;
-    /// let decrypted: Value = store_cipher.decrypt_value_typed(encrypted)?;
-    ///
-    /// assert_eq!(value, decrypted);
-    /// # anyhow::Ok(()) };
-    /// ```
-    pub fn decrypt_value_typed<T: DeserializeOwned>(
-        &self,
-        value: EncryptedValue,
-    ) -> Result<T, Error> {
         let mut plaintext = self.decrypt_value_data(value)?;
         let ret = serde_json::from_slice(&plaintext);
         plaintext.zeroize();
         Ok(ret?)
     }
 
-    /// Decrypt a base64-encoded value after it was fetched from the key/value
-    /// store.
-    ///
-    /// A value can be encrypted using the
-    /// [`StoreCipher::encrypt_value_base64_typed()`] method.
-    ///
-    /// # Arguments
-    ///
-    /// * `value` - The EncryptedValueBase64 of a value that should be
-    ///   decrypted.
-    ///
-    /// The method will deserialize the decrypted value into the expected type.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// # let example = || {
-    /// use matrix_sdk_store_encryption::StoreCipher;
-    /// use serde_json::{json, value::Value};
-    ///
-    /// let store_cipher = StoreCipher::new()?;
-    ///
-    /// let value = json!({
-    ///     "some": "data",
-    /// });
-    ///
-    /// let encrypted = store_cipher.encrypt_value_base64_typed(&value)?;
-    /// let decrypted: Value = store_cipher.decrypt_value_base64_typed(encrypted)?;
-    ///
-    /// assert_eq!(value, decrypted);
-    /// # anyhow::Ok(()) };
-    /// ```
-    pub fn decrypt_value_base64_typed<T: DeserializeOwned>(
-        &self,
-        value: EncryptedValueBase64,
-    ) -> Result<T, Error> {
-        self.decrypt_value_typed(value.try_into()?)
-    }
-
     /// Decrypt a base64-encoded value after it was fetched from the key/value
     /// store.
     ///
@@ -1100,8 +945,11 @@ mod tests {
 
         let store_cipher = StoreCipher::new()?;
 
-        let encrypted = store_cipher.encrypt_value_base64_typed(&event)?;
-        let decrypted: Value = store_cipher.decrypt_value_base64_typed(encrypted)?;
+        let data = serde_json::to_vec(&event)?;
+        let encrypted = store_cipher.encrypt_value_base64_data(data)?;
+
+        let plaintext = store_cipher.decrypt_value_base64_data(encrypted)?;
+        let decrypted: Value = serde_json::from_slice(&plaintext)?;
 
         assert_eq!(event, decrypted);