Fix the serialization of outbound group sessions in the SQLite store

The SQLite crypto store uses rmp_serde to serialize all the data we're
going to store. This works nicely for most things, one exception to this
is the OutboundGroupSession type.

The OutboundGroupSession type stores to-device requests to ensure that
the session doesn't get used before it is shared with the whole group
and to ensure that the to-device requests get restored if the
session gets restored after an application restart.

The to-device requests type critically contain `Raw<AnyToDeviceEvent>`,
the `Raw` type here being the serde_json::Raw type. rmp_serde seems to
serialize this just fine, but later deserialization fails.

We're avoiding the issue by using serde_json to serialize the
OutboundGroupSession.

crates/matrix-sdk-sqlite/Cargo.toml

@@ -33,6 +33,7 @@ rmp-serde = "1.1.1"
 ruma = { workspace = true }
 rusqlite = "0.28.0"
 serde = { workspace = true }
+serde_json = { workspace = true }
 thiserror = { workspace = true }
 tokio = { workspace = true, features = ["fs"] }
 tracing = { workspace = true }
@@ -45,7 +46,6 @@ matrix-sdk-base = { path = "../matrix-sdk-base", features = ["testing"] }
 matrix-sdk-crypto = { path = "../matrix-sdk-crypto", features = ["testing"] }
 matrix-sdk-test = { path = "../../testing/matrix-sdk-test" }
 once_cell = { workspace = true }
-serde_json = { workspace = true }
 tempfile = "3.3.0"
 tokio = { workspace = true, features = ["rt-multi-thread", "macros"] }
 tracing-subscriber = { version = "0.3.16", features = ["env-filter"] }

crates/matrix-sdk-sqlite/migrations/004_drop_outbound_group_sessions.sql

@@ -0,0 +1,5 @@
+-- Outbound group sessions in some cases might want to persist a
+-- Raw<AnyToDeviceEvent>, this does not work with MessagePack. Since it's fine
+-- to rotate outbound group sessions let's force them to be rotated instead of
+-- trying to salvage anything that was persisted.
+DELETE FROM "outbound_group_session";

crates/matrix-sdk-sqlite/src/crypto_store.rs

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 use std::{
+    borrow::Cow,
     collections::HashMap,
     fmt,
     path::{Path, PathBuf},
@@ -111,28 +112,45 @@ impl SqliteCryptoStore {
         })
     }
 
-    fn serialize_value(&self, value: &impl Serialize) -> Result<Vec<u8>> {
-        let serialized = rmp_serde::to_vec_named(value)?;
-
+    fn encode_value(&self, value: Vec<u8>) -> Result<Vec<u8>> {
         if let Some(key) = &self.store_cipher {
-            let encrypted = key.encrypt_value_data(serialized)?;
+            let encrypted = key.encrypt_value_data(value)?;
             Ok(rmp_serde::to_vec_named(&encrypted)?)
         } else {
-            Ok(serialized)
+            Ok(value)
         }
     }
 
-    fn deserialize_value<T: DeserializeOwned>(&self, value: &[u8]) -> Result<T> {
+    fn decode_value<'a>(&self, value: &'a [u8]) -> Result<Cow<'a, [u8]>> {
         if let Some(key) = &self.store_cipher {
             let encrypted = rmp_serde::from_slice(value)?;
             let decrypted = key.decrypt_value_data(encrypted)?;
-
-            Ok(rmp_serde::from_slice(&decrypted)?)
+            Ok(Cow::Owned(decrypted))
         } else {
-            Ok(rmp_serde::from_slice(value)?)
+            Ok(Cow::Borrowed(value))
         }
     }
 
+    fn serialize_json(&self, value: &impl Serialize) -> Result<Vec<u8>> {
+        let serialized = serde_json::to_vec(value)?;
+        self.encode_value(serialized)
+    }
+
+    fn deserialize_json<T: DeserializeOwned>(&self, data: &[u8]) -> Result<T> {
+        let decoded = self.decode_value(data)?;
+        Ok(serde_json::from_slice(&decoded)?)
+    }
+
+    fn serialize_value(&self, value: &impl Serialize) -> Result<Vec<u8>> {
+        let serialized = rmp_serde::to_vec_named(value)?;
+        self.encode_value(serialized)
+    }
+
+    fn deserialize_value<T: DeserializeOwned>(&self, value: &[u8]) -> Result<T> {
+        let decoded = self.decode_value(value)?;
+        Ok(rmp_serde::from_slice(&decoded)?)
+    }
+
     fn deserialize_pickled_inbound_group_session(
         &self,
         value: &[u8],
@@ -171,7 +189,7 @@ impl SqliteCryptoStore {
     }
 }
 
-const DATABASE_VERSION: u8 = 3;
+const DATABASE_VERSION: u8 = 4;
 
 async fn run_migrations(conn: &SqliteConn) -> rusqlite::Result<()> {
     let kv_exists = conn
@@ -227,6 +245,13 @@ async fn run_migrations(conn: &SqliteConn) -> rusqlite::Result<()> {
         .await?;
     }
 
+    if version < 4 {
+        conn.with_transaction(|txn| {
+            txn.execute_batch(include_str!("../migrations/004_drop_outbound_group_sessions.sql"))
+        })
+        .await?;
+    }
+
     conn.set_kv("version", vec![DATABASE_VERSION]).await?;
 
     Ok(())
@@ -702,7 +727,7 @@ impl CryptoStore for SqliteCryptoStore {
                 }
 
                 for (room_id, pickle) in &outbound_session_changes {
-                    let serialized_session = this.serialize_value(&pickle)?;
+                    let serialized_session = this.serialize_json(&pickle)?;
                     txn.set_outbound_group_session(room_id, &serialized_session)?;
                 }
 
@@ -847,7 +872,7 @@ impl CryptoStore for SqliteCryptoStore {
 
         let account_info = self.get_account_info().ok_or(Error::AccountUnset)?;
 
-        let pickle = self.deserialize_value(&value)?;
+        let pickle = self.deserialize_json(&value)?;
         let session = OutboundGroupSession::from_pickle(
             account_info.device_id,
             account_info.identity_keys,

crates/matrix-sdk-sqlite/src/error.rs

@@ -62,6 +62,8 @@ pub enum Error {
     #[error(transparent)]
     Decode(rmp_serde::decode::Error),
     #[error(transparent)]
+    Json(#[from] serde_json::Error),
+    #[error(transparent)]
     Encryption(matrix_sdk_store_encryption::Error),
     #[error("can't save/load sessions or group sessions in the store before an account is stored")]
     AccountUnset,