fix: Store MlsGroup state in keystore (openmls#1364)

openmls/Cargo.toml

@@ -12,41 +12,41 @@ readme = "../README.md"
 [dependencies]
 openmls_traits = { version = "0.1.0", path = "../traits" }
 serde = { version = "^1.0", features = ["derive"] }
-serde_json = "^1.0"
 log = { version = "0.4", features = ["std"] }
 tls_codec = { workspace = true }
 rayon = "^1.5.0"
 thiserror = "^1.0"
 backtrace = "0.3"
 # Only required for tests.
 rand = { version = "0.8", optional = true }
+serde_json = { version = "1.0", optional = true }
 # Crypto backends required for KAT and testing - "test-utils" feature
 itertools = { version = "0.10", optional = true }
 openmls_rust_crypto = { version = "0.1.0", path = "../openmls_rust_crypto", optional = true }
 openmls_evercrypt = { version = "0.1.0", path = "../evercrypt_backend", optional = true }
-openmls_basic_credential = { version = "0.1.0", path = "../basic_credential", optional = true }
+openmls_basic_credential = { version = "0.1.0", path = "../basic_credential", optional = true, features = ["clonable", "test-utils"] }
 rstest = { version = "^0.16", optional = true }
 rstest_reuse = { version = "0.4", optional = true }
 
 [features]
 default = []
 crypto-subtle = [] # Enable subtle crypto APIs that have to be used with care.
 test-utils = [
-    "itertools",
-    "openmls_rust_crypto",
-    "rand",
-    "rstest",
-    "rstest_reuse",
-    "openmls_basic_credential",
-    "openmls_basic_credential/clonable",
-    "openmls_basic_credential/test-utils",
+    "dep:serde_json",
+    "dep:itertools",
+    "dep:openmls_rust_crypto",
+    "dep:rand",
+    "dep:rstest",
+    "dep:rstest_reuse",
+    "dep:openmls_basic_credential",
 ]
-evercrypt = ["openmls_evercrypt"] # Evercrypt needs to be enabled individually
+evercrypt = ["dep:openmls_evercrypt"] # Evercrypt needs to be enabled individually
 crypto-debug = [] # ☣️ Enable logging of sensitive cryptographic information
 content-debug = [] # ☣️ Enable logging of sensitive message content
 
 [dev-dependencies]
 backtrace = "0.3"
+
 criterion = "^0.4"
 hex = { version = "0.4", features = ["serde"] }
 itertools = "0.10"

openmls/src/group/mls_group/mod.rs

@@ -16,7 +16,6 @@ use crate::{
     treesync::{node::leaf_node::LeafNode, RatchetTree},
 };
 use openmls_traits::{key_store::OpenMlsKeyStore, types::Ciphersuite, OpenMlsCryptoProvider};
-use std::io::{Error, Read, Write};
 
 // Private
 mod application;
@@ -26,7 +25,6 @@ mod updates;
 
 use config::*;
 use errors::*;
-use ser::*;
 
 // Crate
 pub(crate) mod config;
@@ -297,17 +295,19 @@ impl MlsGroup {
     // === Load & save ===
 
     /// Loads the state from persisted state.
-    pub fn load<R: Read>(reader: R) -> Result<MlsGroup, Error> {
-        // TODO #245: Remove this once we have a proper serialization format
-        #[allow(deprecated)]
-        let serialized_mls_group: SerializedMlsGroup = serde_json::from_reader(reader)?;
-        Ok(serialized_mls_group.into_mls_group())
+    pub fn load(group_id: &GroupId, backend: &impl OpenMlsCryptoProvider) -> Option<MlsGroup> {
+        backend.key_store().read(group_id.as_slice())
     }
 
     /// Persists the state.
-    pub fn save<W: Write>(&mut self, writer: &mut W) -> Result<(), Error> {
-        let serialized_mls_group = serde_json::to_string_pretty(self)?;
-        writer.write_all(&serialized_mls_group.into_bytes())?;
+    pub fn save<KeyStore: OpenMlsKeyStore>(
+        &mut self,
+        backend: &impl OpenMlsCryptoProvider<KeyStoreProvider = KeyStore>,
+    ) -> Result<(), KeyStore::Error> {
+        backend
+            .key_store()
+            .store(self.group_id().as_slice(), &*self)?;
+
         self.state_changed = InnerState::Persisted;
         Ok(())
     }

openmls/src/group/mls_group/ser.rs

@@ -4,6 +4,7 @@
 use super::*;
 use crate::schedule::psk::store::ResumptionPskStore;
 
+use openmls_traits::key_store::{MlsEntity, MlsEntityId};
 use serde::{
     ser::{SerializeStruct, Serializer},
     Deserialize, Serialize,
@@ -25,9 +26,9 @@ pub struct SerializedMlsGroup {
     group_state: MlsGroupState,
 }
 
-impl SerializedMlsGroup {
-    /// Helper method that converts the SerializedMlsGroup to MlsGroup.
-    pub fn into_mls_group(self) -> MlsGroup {
+#[allow(clippy::from_over_into)]
+impl Into<MlsGroup> for SerializedMlsGroup {
+    fn into(self) -> MlsGroup {
         MlsGroup {
             mls_group_config: self.mls_group_config,
             group: self.group,
@@ -40,6 +41,10 @@ impl SerializedMlsGroup {
     }
 }
 
+impl MlsEntity for MlsGroup {
+    const ID: MlsEntityId = MlsEntityId::GroupState;
+}
+
 impl Serialize for MlsGroup {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
@@ -56,3 +61,13 @@ impl Serialize for MlsGroup {
         state.end()
     }
 }
+
+impl<'de> Deserialize<'de> for MlsGroup {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        let sgroup = SerializedMlsGroup::deserialize(deserializer)?;
+        Ok(sgroup.into())
+    }
+}

openmls/src/group/mls_group/test_mls_group.rs

@@ -29,23 +29,20 @@ fn test_mls_group_persistence(ciphersuite: Ciphersuite, backend: &impl OpenMlsCr
         backend,
         &alice_signer,
         &mls_group_config,
-        group_id,
+        group_id.clone(),
         alice_credential_with_key,
     )
     .expect("An unexpected error occurred.");
 
     // Check the internal state has changed
     assert_eq!(alice_group.state_changed(), InnerState::Changed);
 
-    let mut file_out = tempfile::NamedTempFile::new().expect("Could not create file");
     alice_group
-        .save(&mut file_out)
+        .save(backend)
         .expect("Could not write group state to file");
 
-    let file_in = file_out
-        .reopen()
-        .expect("Error re-opening serialized group state file");
-    let alice_group_deserialized = MlsGroup::load(file_in).expect("Could not deserialize MlsGroup");
+    let alice_group_deserialized =
+        MlsGroup::load(&group_id, backend).expect("Could not deserialize MlsGroup");
 
     assert_eq!(
         (

openmls/tests/book_code.rs

@@ -1,6 +1,3 @@
-use std::fs::File;
-
-use lazy_static::lazy_static;
 use openmls::{
     prelude::{config::CryptoConfig, *},
     test_utils::*,
@@ -10,11 +7,6 @@ use openmls_basic_credential::SignatureKeyPair;
 use openmls_rust_crypto::OpenMlsRustCrypto;
 use openmls_traits::{signatures::Signer, types::SignatureScheme, OpenMlsCryptoProvider};
 
-lazy_static! {
-    static ref TEMP_DIR: tempfile::TempDir =
-        tempfile::tempdir().expect("Error creating temp directory");
-}
-
 #[test]
 fn create_backend_rust_crypto() {
     // ANCHOR: create_backend_rust_crypto
@@ -192,6 +184,8 @@ fn book_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoProvide
         let _ignore_mut_warning = &mut alice_group;
     }
 
+    let group_id = alice_group.group_id().clone();
+
     // === Alice adds Bob ===
     // ANCHOR: alice_adds_bob
     let (mls_message_out, welcome, group_info) = alice_group
@@ -1255,27 +1249,14 @@ fn book_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoProvide
     assert_eq!(bob_group.state_changed(), InnerState::Changed);
     //save(&mut bob_group);
 
-    let name = bytes_to_hex(
-        bob_group
-            .own_leaf_node()
-            .unwrap()
-            .signature_key()
-            .as_slice(),
-    )
-    .to_lowercase();
-    let path = TEMP_DIR
-        .path()
-        .join(format!("test_mls_group_{}.json", &name));
-    let out_file = &mut File::create(path.clone()).expect("Could not create file");
     bob_group
-        .save(out_file)
+        .save(backend)
         .expect("Could not write group state to file");
 
     // Check that the state flag gets reset when saving
     assert_eq!(bob_group.state_changed(), InnerState::Persisted);
 
-    let file = File::open(path).expect("Could not open file");
-    let bob_group = MlsGroup::load(file).expect("Could not load group from file");
+    let bob_group = MlsGroup::load(&group_id, backend).expect("Could not load group from file");
 
     // Make sure the state is still the same
     assert_eq!(

openmls/tests/test_mls_group.rs

@@ -4,14 +4,7 @@ use openmls::{
     *,
 };
 
-use lazy_static::lazy_static;
 use openmls_traits::{key_store::OpenMlsKeyStore, signatures::Signer, OpenMlsCryptoProvider};
-use std::fs::File;
-
-lazy_static! {
-    static ref TEMP_DIR: tempfile::TempDir =
-        tempfile::tempdir().expect("Error creating temp directory");
-}
 
 fn generate_key_package<KeyStore: OpenMlsKeyStore>(
     ciphersuite: Ciphersuite,
@@ -96,7 +89,7 @@ fn mls_group_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr
             backend,
             &alice_signer,
             &mls_group_config,
-            group_id,
+            group_id.clone(),
             alice_credential.clone(),
         )
         .expect("An unexpected error occurred.");
@@ -905,27 +898,14 @@ fn mls_group_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr
         assert_eq!(bob_group.state_changed(), InnerState::Changed);
         //save(&mut bob_group);
 
-        let name = bytes_to_hex(
-            bob_group
-                .own_leaf_node()
-                .unwrap()
-                .signature_key()
-                .as_slice(),
-        )
-        .to_lowercase();
-        let path = TEMP_DIR
-            .path()
-            .join(format!("test_mls_group_{}.json", &name));
-        let out_file = &mut File::create(path.clone()).expect("Could not create file");
         bob_group
-            .save(out_file)
+            .save(backend)
             .expect("Could not write group state to file");
 
         // Check that the state flag gets reset when saving
         assert_eq!(bob_group.state_changed(), InnerState::Persisted);
 
-        let file = File::open(path).expect("Could not open file");
-        let bob_group = MlsGroup::load(file).expect("Could not load group from file");
+        let bob_group = MlsGroup::load(&group_id, backend).expect("Could not load group from file");
 
         // Make sure the state is still the same
         assert_eq!(

traits/src/key_store.rs

@@ -7,6 +7,7 @@ pub enum MlsEntityId {
     KeyPackage,
     PskBundle,
     EncryptionKeyPair,
+    GroupState,
 }
 
 /// To implement by any struct owned by openmls aiming to be persisted in [OpenMlsKeyStore]