Merge pull request #3042 from matrix-org/rav/room_settings

crypto: Implement `OlmMachine::{set_,}room_settings`

bindings/matrix-sdk-crypto-ffi/src/lib.rs

@@ -843,6 +843,7 @@ impl From<RoomSettings> for RustRoomSettings {
         Self {
             algorithm: value.algorithm.into(),
             only_allow_trusted_devices: value.only_allow_trusted_devices,
+            ..RustRoomSettings::default()
         }
     }
 }

crates/matrix-sdk-crypto/CHANGELOG.md

@@ -1,5 +1,13 @@
 # UNRELEASED
 
+- Expose new methods `OlmMachine::set_room_settings` and
+  `OlmMachine::get_room_settings`.
+  ([#3042](https://github.com/matrix-org/matrix-rust-sdk/pull/3042))
+
+- Add new properties `session_rotation_period` and
+  `session_rotation_period_msgs` to `store::RoomSettings`.
+  ([#3042](https://github.com/matrix-org/matrix-rust-sdk/pull/3042))
+
 - Fix bug which caused `SecretStorageKey` to incorrectly reject secret storage
   keys whose metadata lacked check fields.
   ([#3046](https://github.com/matrix-org/matrix-rust-sdk/pull/3046))

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

@@ -289,3 +289,22 @@ pub enum SessionCreationError {
     #[error(transparent)]
     InboundCreation(#[from] vodozemac::olm::SessionCreationError),
 }
+
+/// Errors that can be returned by
+/// [`crate::machine::OlmMachine::set_room_settings`].
+#[derive(Debug, Error)]
+pub enum SetRoomSettingsError {
+    /// The changes are rejected because they conflict with the previous
+    /// settings for this room.
+    #[error("the new settings would cause a downgrade of encryption security")]
+    EncryptionDowngrade,
+
+    /// The changes are rejected because we would be unable to use them to
+    /// encrypt events.
+    #[error("the new settings are invalid")]
+    InvalidSettings,
+
+    /// The store ran into an error.
+    #[error(transparent)]
+    Store(#[from] CryptoStoreError),
+}

crates/matrix-sdk-crypto/src/lib.rs

@@ -70,7 +70,9 @@ impl RoomKeyImportResult {
     }
 }
 
-pub use error::{EventError, MegolmError, OlmError, SessionCreationError, SignatureError};
+pub use error::{
+    EventError, MegolmError, OlmError, SessionCreationError, SetRoomSettingsError, SignatureError,
+};
 pub use file_encryption::{
     decrypt_room_key_export, encrypt_room_key_export, AttachmentDecryptor, AttachmentEncryptor,
     DecryptorError, KeyExportError, MediaEncryptionInfo,

crates/matrix-sdk-crypto/src/machine.rs

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 use std::{
-    collections::{BTreeMap, HashSet},
+    collections::{BTreeMap, HashMap, HashSet},
     sync::{Arc, RwLock as StdRwLock},
     time::Duration,
 };
@@ -58,7 +58,7 @@ use vodozemac::{
 use crate::{
     backups::{BackupMachine, MegolmV1BackupKey},
     dehydrated_devices::{DehydratedDevices, DehydrationError},
-    error::{EventError, MegolmError, MegolmResult, OlmError, OlmResult},
+    error::{EventError, MegolmError, MegolmResult, OlmError, OlmResult, SetRoomSettingsError},
     gossiping::GossipMachine,
     identities::{user::UserIdentities, Device, IdentityManager, UserDevices},
     olm::{
@@ -70,8 +70,8 @@ use crate::{
     session_manager::{GroupSessionManager, SessionManager},
     store::{
         Changes, CryptoStoreWrapper, DeviceChanges, IdentityChanges, IntoCryptoStore, MemoryStore,
-        PendingChanges, Result as StoreResult, RoomKeyInfo, SecretImportError, Store, StoreCache,
-        StoreTransaction,
+        PendingChanges, Result as StoreResult, RoomKeyInfo, RoomSettings, SecretImportError, Store,
+        StoreCache, StoreTransaction,
     },
     types::{
         events::{
@@ -86,7 +86,7 @@ use crate::{
             },
             ToDeviceEvents,
         },
-        Signatures,
+        EventEncryptionAlgorithm, Signatures,
     },
     verification::{Verification, VerificationMachine, VerificationRequest},
     CrossSigningKeyExport, CryptoStoreError, KeysQueryRequest, LocalTrust, ReadOnlyDevice,
@@ -2043,6 +2043,89 @@ impl OlmMachine {
         DehydratedDevices { inner: self.to_owned() }
     }
 
+    /// Get the stored encryption settings for the given room, such as the
+    /// encryption algorithm or whether to encrypt only for trusted devices.
+    ///
+    /// These settings can be modified via [`OlmMachine::set_room_settings`].
+    pub async fn room_settings(&self, room_id: &RoomId) -> StoreResult<Option<RoomSettings>> {
+        // There's not much to do here: it's just exposed for symmetry with
+        // `set_room_settings`.
+        self.inner.store.get_room_settings(room_id).await
+    }
+
+    /// Store encryption settings for the given room.
+    ///
+    /// This method checks if the new settings are "safe" -- ie, that they do
+    /// not represent a downgrade in encryption security from any previous
+    /// settings. Attempts to downgrade security will result in a
+    /// [`SetRoomSettingsError::EncryptionDowngrade`].
+    ///
+    /// If the settings are valid, they will be persisted to the crypto store.
+    /// These settings are not used directly by this library, but the saved
+    /// settings can be retrieved via [`OlmMachine::room_settings`].
+    pub async fn set_room_settings(
+        &self,
+        room_id: &RoomId,
+        new_settings: &RoomSettings,
+    ) -> Result<(), SetRoomSettingsError> {
+        let store = &self.inner.store;
+
+        // We want to make sure that we do not race against a second concurrent call to
+        // `set_room_settings`. By way of an easy way to do so, we start a
+        // StoreTransaction. There's no need to commit() it: we're just using it as a
+        // lock guard.
+        let _store_transaction = store.transaction().await;
+
+        let old_settings = store.get_room_settings(room_id).await?;
+
+        // We want to make sure that the change to the room settings does not represent
+        // a downgrade in security. The [E2EE implementation guide] recommends:
+        //
+        //  > This flag should **not** be cleared if a later `m.room.encryption` event
+        //  > changes the configuration.
+        //
+        // (However, it doesn't really address how to handle changes to the rotation
+        // parameters, etc.) For now at least, we are very conservative here:
+        // any new settings are rejected if they differ from the existing settings.
+        // merit improvement (cf https://github.com/element-hq/element-meta/issues/69).
+        //
+        // [E2EE implementation guide]: https://matrix.org/docs/matrix-concepts/end-to-end-encryption/#handling-an-m-room-encryption-state-event
+        if let Some(old_settings) = old_settings {
+            if old_settings != *new_settings {
+                return Err(SetRoomSettingsError::EncryptionDowngrade);
+            } else {
+                // nothing to do here
+                return Ok(());
+            }
+        }
+
+        // Make sure that the new settings are valid
+        match new_settings.algorithm {
+            EventEncryptionAlgorithm::MegolmV1AesSha2 => (),
+
+            #[cfg(feature = "experimental-algorithms")]
+            EventEncryptionAlgorithm::MegolmV2AesSha2 => (),
+
+            _ => {
+                warn!(
+                    ?room_id,
+                    "Rejecting invalid encryption algorithm {}", new_settings.algorithm
+                );
+                return Err(SetRoomSettingsError::InvalidSettings);
+            }
+        }
+
+        // The new settings are acceptable, so let's save them.
+        store
+            .save_changes(Changes {
+                room_settings: HashMap::from([(room_id.to_owned(), new_settings.clone())]),
+                ..Default::default()
+            })
+            .await?;
+
+        Ok(())
+    }
+
     #[cfg(any(feature = "testing", test))]
     /// Returns whether this `OlmMachine` is the same another one.
     ///
@@ -2161,10 +2244,10 @@ pub(crate) mod tests {
 
     use super::{testing::response_from_file, CrossSigningBootstrapRequests};
     use crate::{
-        error::EventError,
+        error::{EventError, SetRoomSettingsError},
         machine::{EncryptionSyncChanges, OlmMachine},
         olm::{InboundGroupSession, OutboundGroupSession, VerifyJson},
-        store::Changes,
+        store::{Changes, RoomSettings},
         types::{
             events::{
                 room::encrypted::{EncryptedToDeviceEvent, ToDeviceEncryptedEventContent},
@@ -4065,4 +4148,98 @@ pub(crate) mod tests {
         // The waiting should successfully complete.
         wait.await.unwrap();
     }
+
+    #[async_test]
+    async fn room_settings_returns_none_for_unknown_room() {
+        let machine = OlmMachine::new(user_id(), alice_device_id()).await;
+        let settings = machine.room_settings(room_id!("!test2:localhost")).await.unwrap();
+        assert!(settings.is_none());
+    }
+
+    #[async_test]
+    async fn stores_and_returns_room_settings() {
+        let machine = OlmMachine::new(user_id(), alice_device_id()).await;
+        let room_id = room_id!("!test:localhost");
+
+        let settings = RoomSettings {
+            algorithm: EventEncryptionAlgorithm::MegolmV1AesSha2,
+            only_allow_trusted_devices: true,
+            session_rotation_period: Some(Duration::from_secs(10)),
+            session_rotation_period_messages: Some(1234),
+        };
+
+        machine.set_room_settings(room_id, &settings).await.unwrap();
+        assert_eq!(machine.room_settings(room_id).await.unwrap(), Some(settings));
+    }
+
+    #[async_test]
+    async fn set_room_settings_rejects_invalid_algorithms() {
+        let machine = OlmMachine::new(user_id(), alice_device_id()).await;
+        let room_id = room_id!("!test:localhost");
+
+        let err = machine
+            .set_room_settings(
+                room_id,
+                &RoomSettings {
+                    algorithm: EventEncryptionAlgorithm::OlmV1Curve25519AesSha2,
+                    ..Default::default()
+                },
+            )
+            .await
+            .unwrap_err();
+        assert_matches!(err, SetRoomSettingsError::InvalidSettings)
+    }
+
+    #[async_test]
+    async fn set_room_settings_rejects_changes() {
+        let machine = OlmMachine::new(user_id(), alice_device_id()).await;
+        let room_id = room_id!("!test:localhost");
+
+        // Initial settings
+        machine
+            .set_room_settings(
+                room_id,
+                &RoomSettings { session_rotation_period_messages: Some(100), ..Default::default() },
+            )
+            .await
+            .unwrap();
+
+        // Now, modifying the settings should be rejected
+        let err = machine
+            .set_room_settings(
+                room_id,
+                &RoomSettings {
+                    session_rotation_period_messages: Some(1000),
+                    ..Default::default()
+                },
+            )
+            .await
+            .unwrap_err();
+
+        assert_matches!(err, SetRoomSettingsError::EncryptionDowngrade);
+    }
+
+    #[async_test]
+    async fn set_room_settings_accepts_noop_changes() {
+        let machine = OlmMachine::new(user_id(), alice_device_id()).await;
+        let room_id = room_id!("!test:localhost");
+
+        // Initial settings
+        machine
+            .set_room_settings(
+                room_id,
+                &RoomSettings { session_rotation_period_messages: Some(100), ..Default::default() },
+            )
+            .await
+            .unwrap();
+
+        // Same again; should be fine.
+        machine
+            .set_room_settings(
+                room_id,
+                &RoomSettings { session_rotation_period_messages: Some(100), ..Default::default() },
+            )
+            .await
+            .unwrap();
+    }
 }

crates/matrix-sdk-crypto/src/store/integration_tests.rs

@@ -3,6 +3,7 @@
 macro_rules! cryptostore_integration_tests {
     () => {
         mod cryptostore_integration_tests {
+            use std::time::Duration;
             use std::collections::{BTreeMap, HashMap};
 
             use assert_matches::assert_matches;
@@ -855,12 +856,15 @@ macro_rules! cryptostore_integration_tests {
                 let settings_1 = RoomSettings {
                     algorithm: EventEncryptionAlgorithm::MegolmV1AesSha2,
                     only_allow_trusted_devices: true,
+                    session_rotation_period: Some(Duration::from_secs(10)),
+                    session_rotation_period_messages: Some(123),
                 };
 
                 let room_2 = room_id!("!test_2:localhost");
                 let settings_2 = RoomSettings {
                     algorithm: EventEncryptionAlgorithm::OlmV1Curve25519AesSha2,
                     only_allow_trusted_devices: false,
+                    ..Default::default()
                 };
 
                 let room_3 = room_id!("!test_3:localhost");

crates/matrix-sdk-crypto/src/store/memorystore.rs

@@ -25,7 +25,6 @@ use ruma::{
     OwnedUserId, RoomId, TransactionId, UserId,
 };
 use tokio::sync::{Mutex, RwLock};
-use tracing::warn;
 
 use super::{
     caches::{DeviceStore, GroupSessionStore, SessionStore},
@@ -66,6 +65,7 @@ pub struct MemoryStore {
     secret_inbox: StdRwLock<HashMap<String, Vec<GossippedSecret>>>,
     backup_keys: RwLock<BackupKeys>,
     next_batch_token: RwLock<Option<String>>,
+    room_settings: StdRwLock<HashMap<OwnedRoomId, RoomSettings>>,
 }
 
 impl Default for MemoryStore {
@@ -85,6 +85,7 @@ impl Default for MemoryStore {
             backup_keys: Default::default(),
             secret_inbox: Default::default(),
             next_batch_token: Default::default(),
+            room_settings: Default::default(),
         }
     }
 }
@@ -213,6 +214,11 @@ impl CryptoStore for MemoryStore {
             *self.next_batch_token.write().await = Some(next_batch_token);
         }
 
+        if !changes.room_settings.is_empty() {
+            let mut settings = self.room_settings.write().unwrap();
+            settings.extend(changes.room_settings);
+        }
+
         Ok(())
     }
 
@@ -393,9 +399,8 @@ impl CryptoStore for MemoryStore {
         Ok(())
     }
 
-    async fn get_room_settings(&self, _room_id: &RoomId) -> Result<Option<RoomSettings>> {
-        warn!("Method not implemented");
-        Ok(None)
+    async fn get_room_settings(&self, room_id: &RoomId) -> Result<Option<RoomSettings>> {
+        Ok(self.room_settings.read().unwrap().get(room_id).cloned())
     }
 
     async fn get_custom_value(&self, key: &str) -> Result<Option<Vec<u8>>> {

crates/matrix-sdk-crypto/src/store/mod.rs

@@ -833,9 +833,18 @@ pub(crate) enum UserKeyQueryResult {
 pub struct RoomSettings {
     /// The encryption algorithm that should be used in the room.
     pub algorithm: EventEncryptionAlgorithm,
+
     /// Should untrusted devices receive the room key, or should they be
     /// excluded from the conversation.
     pub only_allow_trusted_devices: bool,
+
+    /// The maximum time an encryption session should be used for, before it is
+    /// rotated.
+    pub session_rotation_period: Option<Duration>,
+
+    /// The maximum number of messages an encryption session should be used for,
+    /// before it is rotated.
+    pub session_rotation_period_messages: Option<usize>,
 }
 
 impl Default for RoomSettings {
@@ -843,6 +852,8 @@ impl Default for RoomSettings {
         Self {
             algorithm: EventEncryptionAlgorithm::MegolmV1AesSha2,
             only_allow_trusted_devices: false,
+            session_rotation_period: None,
+            session_rotation_period_messages: None,
         }
     }
 }