Skip to content

Commit

Permalink
crypto: Implement OlmMachine::{get,set}_room_settings
Browse files Browse the repository at this point in the history
We need to make sure we guard against servers downgrading the encryption
settings, or breaking them with unsupported algorithms. It's not *entirely*
clear what the expectation is here, but legacy crypto in matrix-js-sdk blocks
any changes to the settings at all, so we'll follow suit for now.
  • Loading branch information
richvdh committed Jan 19, 2024
1 parent 5e0e752 commit 2314a74
Show file tree
Hide file tree
Showing 4 changed files with 213 additions and 8 deletions.
4 changes: 4 additions & 0 deletions crates/matrix-sdk-crypto/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# 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))
Expand Down
19 changes: 19 additions & 0 deletions crates/matrix-sdk-crypto/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
4 changes: 3 additions & 1 deletion crates/matrix-sdk-crypto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
194 changes: 187 additions & 7 deletions crates/matrix-sdk-crypto/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// limitations under the License.

use std::{
collections::{BTreeMap, HashSet},
collections::{BTreeMap, HashMap, HashSet},
sync::{Arc, RwLock as StdRwLock},
time::Duration,
};
Expand Down Expand Up @@ -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::{
Expand All @@ -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::{
Expand All @@ -86,7 +86,7 @@ use crate::{
},
ToDeviceEvents,
},
Signatures,
EventEncryptionAlgorithm, Signatures,
},
verification::{Verification, VerificationMachine, VerificationRequest},
CrossSigningKeyExport, CryptoStoreError, KeysQueryRequest, LocalTrust, ReadOnlyDevice,
Expand Down Expand Up @@ -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 get_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::get_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.
///
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -4065,4 +4148,101 @@ pub(crate) mod tests {
// The waiting should successfully complete.
wait.await.unwrap();
}

#[async_test]
async fn get_room_settings_returns_none_for_unknown_room() {
let machine = OlmMachine::new(user_id(), alice_device_id()).await;

machine
.set_room_settings(room_id!("!test:localhost"), &RoomSettings::default())
.await
.unwrap();

let settings = machine.get_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_millis(10000)),
session_rotation_period_msgs: Some(1234),
};

machine.set_room_settings(room_id, &settings).await.unwrap();
assert_eq!(machine.get_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_msgs: 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_msgs: 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_msgs: Some(100), ..Default::default() },
)
.await
.unwrap();

// Same again; should be fine.
machine
.set_room_settings(
room_id,
&RoomSettings { session_rotation_period_msgs: Some(100), ..Default::default() },
)
.await
.unwrap();
}
}

0 comments on commit 2314a74

Please sign in to comment.