diff --git a/bindings/matrix-sdk-crypto-ffi/src/lib.rs b/bindings/matrix-sdk-crypto-ffi/src/lib.rs index 2de7ad695..6e6f1dd15 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/lib.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/lib.rs @@ -843,6 +843,7 @@ impl From for RustRoomSettings { Self { algorithm: value.algorithm.into(), only_allow_trusted_devices: value.only_allow_trusted_devices, + ..RustRoomSettings::default() } } } diff --git a/crates/matrix-sdk-crypto/CHANGELOG.md b/crates/matrix-sdk-crypto/CHANGELOG.md index 795365522..0c4bc7327 100644 --- a/crates/matrix-sdk-crypto/CHANGELOG.md +++ b/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)) diff --git a/crates/matrix-sdk-crypto/src/error.rs b/crates/matrix-sdk-crypto/src/error.rs index a1a10be62..31cbf557b 100644 --- a/crates/matrix-sdk-crypto/src/error.rs +++ b/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), +} diff --git a/crates/matrix-sdk-crypto/src/lib.rs b/crates/matrix-sdk-crypto/src/lib.rs index e926d844e..d3febf9c5 100644 --- a/crates/matrix-sdk-crypto/src/lib.rs +++ b/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, diff --git a/crates/matrix-sdk-crypto/src/machine.rs b/crates/matrix-sdk-crypto/src/machine.rs index 1ddd64e55..fff7865bd 100644 --- a/crates/matrix-sdk-crypto/src/machine.rs +++ b/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> { + // 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(); + } } diff --git a/crates/matrix-sdk-crypto/src/store/integration_tests.rs b/crates/matrix-sdk-crypto/src/store/integration_tests.rs index 73905a4a6..9373300ef 100644 --- a/crates/matrix-sdk-crypto/src/store/integration_tests.rs +++ b/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"); diff --git a/crates/matrix-sdk-crypto/src/store/memorystore.rs b/crates/matrix-sdk-crypto/src/store/memorystore.rs index 8d7cdbb01..f75b2155f 100644 --- a/crates/matrix-sdk-crypto/src/store/memorystore.rs +++ b/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>>, backup_keys: RwLock, next_batch_token: RwLock>, + room_settings: StdRwLock>, } 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> { - warn!("Method not implemented"); - Ok(None) + async fn get_room_settings(&self, room_id: &RoomId) -> Result> { + Ok(self.room_settings.read().unwrap().get(room_id).cloned()) } async fn get_custom_value(&self, key: &str) -> Result>> { diff --git a/crates/matrix-sdk-crypto/src/store/mod.rs b/crates/matrix-sdk-crypto/src/store/mod.rs index b79c908cd..21aa40506 100644 --- a/crates/matrix-sdk-crypto/src/store/mod.rs +++ b/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, + + /// The maximum number of messages an encryption session should be used for, + /// before it is rotated. + pub session_rotation_period_messages: Option, } 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, } } }