From 7c82a24ac4bd3f2527823de0d0f0174326e2316a Mon Sep 17 00:00:00 2001 From: Michael Hollister Date: Mon, 2 Oct 2023 01:04:24 -0500 Subject: [PATCH 1/7] Implemented sharing room keys for past messages (MSC3061) Signed-off-by: Michael Hollister --- bindings/matrix-sdk-crypto-ffi/src/machine.rs | 33 ++++++ .../src/gossiping/machine.rs | 109 +++++++++++++++--- crates/matrix-sdk-crypto/src/machine.rs | 105 ++++++++++++++++- .../src/olm/group_sessions/inbound.rs | 28 ++++- .../src/olm/group_sessions/mod.rs | 13 +++ .../src/olm/group_sessions/outbound.rs | 5 + .../src/types/events/forwarded_room_key.rs | 10 ++ .../src/types/events/room_key.rs | 17 ++- .../src/types/events/to_device.rs | 3 +- crates/matrix-sdk/src/room/mod.rs | 24 ++++ .../tests/integration/room/joined.rs | 1 + 11 files changed, 321 insertions(+), 27 deletions(-) diff --git a/bindings/matrix-sdk-crypto-ffi/src/machine.rs b/bindings/matrix-sdk-crypto-ffi/src/machine.rs index 48531facb62..763bc7538d3 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/machine.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/machine.rs @@ -749,6 +749,39 @@ impl OlmMachine { Ok(requests.into_iter().map(|r| r.as_ref().into()).collect()) } + /// Shares historical room keys used in previous sessions with the list of + /// users for the given room. + /// + /// After the request was sent out and a successful response was received + /// the response body should be passed back to the state machine using the + /// [mark_request_as_sent()](Self::mark_request_as_sent) method. + /// + /// This method should be called after users have been invited to the room. + /// + /// # Arguments + /// + /// * `room_id` - The unique id of the room of which previous room keys + /// will be sent out. + /// + /// * `users` - The list of users which are considered to be members of the + /// room and should receive previous room keys. + #[cfg(feature = "automatic-room-key-forwarding")] + pub fn share_room_history_keys( + &self, + room_id: String, + users: Vec, + ) -> Result, CryptoStoreError> { + let users: Vec = + users.into_iter().filter_map(|u| UserId::parse(u).ok()).collect(); + + let room_id = RoomId::parse(room_id)?; + let requests = self.runtime.block_on( + self.inner.share_room_history_keys(&room_id, users.iter().map(Deref::deref)), + )?; + + Ok(requests.into_iter().map(|r| r.as_ref().into()).collect()) + } + /// Encrypt the given event with the given type and content for the given /// room. /// diff --git a/crates/matrix-sdk-crypto/src/gossiping/machine.rs b/crates/matrix-sdk-crypto/src/gossiping/machine.rs index 2b0d2e2b633..b27df3139cc 100644 --- a/crates/matrix-sdk-crypto/src/gossiping/machine.rs +++ b/crates/matrix-sdk-crypto/src/gossiping/machine.rs @@ -31,8 +31,11 @@ use std::{ use ruma::{ api::client::keys::claim_keys::v3::Request as KeysClaimRequest, - events::secret::request::{ - RequestAction, SecretName, ToDeviceSecretRequestEvent as SecretRequestEvent, + events::{ + room::history_visibility::HistoryVisibility, + secret::request::{ + RequestAction, SecretName, ToDeviceSecretRequestEvent as SecretRequestEvent, + }, }, DeviceId, DeviceKeyAlgorithm, OwnedDeviceId, OwnedTransactionId, OwnedUserId, RoomId, TransactionId, UserId, @@ -175,8 +178,14 @@ impl GossipMachine { // out using a wildcard instead of a specific device as a recipient. // // Check if we're the sender of this request event and ignore it if - // so. - if event.sender() == self.user_id() && event.requesting_device_id() == self.device_id() { + // so. However if the request is a room key request, we should allow it + // since one of our other devices could be forwarding a room key to us. + // This could happen when one device is unable to decrypt an event and + // requests the key from another device. + if event.sender() == self.user_id() + && event.requesting_device_id() == self.device_id() + && matches!(event, RequestEvent::Secret(_)) + { trace!("Received a secret request event from ourselves, ignoring") } else { let request_info = event.to_request_info(); @@ -345,7 +354,7 @@ impl GossipMachine { /// `/keys/claim` request to be sent out and retry once the 1-to-1 Olm /// session has been established. #[cfg(feature = "automatic-room-key-forwarding")] - async fn try_to_forward_room_key( + pub async fn try_to_forward_room_key( &self, event: &RoomKeyRequestEvent, device: Device, @@ -589,7 +598,22 @@ impl GossipMachine { // information is recorded there. } else if let Some(outbound) = outbound_session { match outbound.is_shared_with(device) { - ShareState::Shared(message_index) => Ok(Some(message_index)), + ShareState::Shared(message_index) => { + if let Some(history) = session.history_visibility.as_ref() { + match history { + HistoryVisibility::Shared | HistoryVisibility::WorldReadable => { + Ok(None) + } + + HistoryVisibility::Invited + | HistoryVisibility::Joined + | HistoryVisibility::_Custom(_) => Ok(Some(message_index)), + _ => Ok(Some(message_index)), + } + } else { + Ok(Some(message_index)) + } + } ShareState::SharedButChangedSenderKey => Err(KeyForwardDecision::ChangedSenderKey), ShareState::NotShared => Err(KeyForwardDecision::OutboundSessionNotShared), } @@ -965,6 +989,8 @@ impl GossipMachine { sender_key: Curve25519PublicKey, event: &DecryptedForwardedRoomKeyEvent, ) -> Result, CryptoStoreError> { + use vodozemac::megolm::SessionKey; + let Some(info) = event.room_key_info() else { warn!( sender_key = sender_key.to_base64(), @@ -977,15 +1003,56 @@ impl GossipMachine { let Some(request) = self.inner.store.get_secret_request_by_info(&info.clone().into()).await? else { - warn!( - sender_key = ?sender_key, - room_id = ?info.room_id(), - session_id = info.session_id(), - sender_key = ?sender_key, - algorithm = ?info.algorithm(), - "Received a forwarded room key that we didn't request", - ); - return Ok(None); + // We did not request this key, so determine if this key was + // forwarded as a result from a room invite + let (room_id, session_key, shared_history) = match &event.content { + ForwardedRoomKeyContent::MegolmV1AesSha2(c) => { + (&c.room_id, &c.session_key, c.shared_history) + } + #[cfg(feature = "experimental-algorithms")] + ForwardedRoomKeyContent::MegolmV2AesSha2(c) => { + (&c.room_id, &c.session_key, c.shared_history) + } + ForwardedRoomKeyContent::Unknown(_) => { + warn!( + sender_key = ?sender_key, + room_id = ?info.room_id(), + session_id = info.session_id(), + sender_key = ?sender_key, + algorithm = ?info.algorithm(), + "Received an unknown forwarded room key that we didn't request", + ); + + return Ok(None); + } + }; + + if shared_history { + // Content does not indicate level of history visibility, so + // set it to least permissive for shared history + let visibility = Some(HistoryVisibility::Shared); + let session_key = SessionKey::from(session_key); + + return Ok(Some(InboundGroupSession::new( + sender_key, + event.keys.ed25519, + room_id, + &session_key, + event.content.algorithm(), + visibility, + )?)); + } else { + warn!( + sender_key = ?sender_key, + room_id = ?info.room_id(), + session_id = info.session_id(), + sender_key = ?sender_key, + algorithm = ?info.algorithm(), + "Received a forwarded room key that we didn't request", + ); + + return Ok(None); + } }; if self.should_accept_forward(&request, sender_key).await? { @@ -1464,6 +1531,8 @@ mod tests { #[async_test] #[cfg(feature = "automatic-room-key-forwarding")] async fn should_share_key_test() { + use ruma::events::room::history_visibility::HistoryVisibility; + let machine = get_machine().await; let account = account(); @@ -1527,8 +1596,14 @@ mod tests { .await; machine.should_share_key(&bob_device, &inbound).await.unwrap(); - let (other_outbound, other_inbound) = - account.create_group_session_pair_with_defaults(room_id()).await; + let encryption_settings = EncryptionSettings { + history_visibility: HistoryVisibility::Invited, + ..Default::default() + }; + let (other_outbound, other_inbound) = account + .create_group_session_pair(room_id(), encryption_settings) + .await + .expect("Can't create group session pair"); // But we don't share some other session that doesn't match our outbound // session. diff --git a/crates/matrix-sdk-crypto/src/machine.rs b/crates/matrix-sdk-crypto/src/machine.rs index e8e771d2ae0..ada0e0b9236 100644 --- a/crates/matrix-sdk-crypto/src/machine.rs +++ b/crates/matrix-sdk-crypto/src/machine.rs @@ -36,7 +36,8 @@ use ruma::{ }, assign, events::{ - secret::request::SecretName, AnyMessageLikeEvent, AnyToDeviceEvent, MessageLikeEventContent, + room::history_visibility::HistoryVisibility, secret::request::SecretName, + AnyMessageLikeEvent, AnyToDeviceEvent, MessageLikeEventContent, }, serde::Raw, DeviceId, DeviceKeyAlgorithm, OwnedDeviceId, OwnedDeviceKeyId, OwnedTransactionId, OwnedUserId, @@ -680,13 +681,18 @@ impl OlmMachine { event: &DecryptedRoomKeyEvent, content: &MegolmV1AesSha2Content, ) -> OlmResult> { + // Content does not indicate level of history visibility, so + // set it to least permissive for shared history if true + let visibility = + if content.shared_history { Some(HistoryVisibility::Shared) } else { None }; + let session = InboundGroupSession::new( sender_key, event.keys.ed25519, &content.room_id, &content.session_key, event.content.algorithm(), - None, + visibility, ); match session { @@ -865,6 +871,101 @@ impl OlmMachine { self.inner.group_session_manager.share_room_key(room_id, users, encryption_settings).await } + /// Get to-device requests to share room keys that are flagged with + /// shared_history with users in a room. + /// + /// # Arguments + /// + /// `room_id` - The room id of the room where the room key will be + /// used. + /// + /// `users` - The list of users that should receive the room keys. + #[cfg(feature = "automatic-room-key-forwarding")] + pub async fn share_room_history_keys( + &self, + room_id: &RoomId, + users: impl Iterator, + ) -> OlmResult>> { + use crate::types::events::room_key_request::{ + MegolmV1AesSha2Content, RequestedKeyInfo, RoomKeyRequestContent, RoomKeyRequestEvent, + }; + let mut requests = Vec::new(); + + if let Some(outbound) = self.store().get_outbound_group_session(room_id).await? { + if matches!( + outbound.settings().history_visibility, + HistoryVisibility::Shared | HistoryVisibility::WorldReadable + ) { + let all_sessions = self.store().get_inbound_group_sessions().await?; + let room_sessions: Vec = + all_sessions.into_iter().filter(|s| s.room_id == room_id).collect(); + + let mut devices = Vec::new(); + for user_id in users { + let user_devices = self.store().get_user_devices_filtered(user_id).await?; + + let valid_devices: Vec = user_devices + .devices() + .filter(|d| { + d.supports_olm() + && !(d.is_blacklisted() + || (outbound.settings().only_allow_trusted_devices + && !d.is_verified())) + }) + .collect(); + + devices.extend(valid_devices); + } + + for session in &room_sessions { + if let Some(history) = session.history_visibility.as_ref().clone() { + if matches!( + history, + HistoryVisibility::Shared | HistoryVisibility::WorldReadable + ) { + for device in &devices { + let info = MegolmV1AesSha2Content { + room_id: room_id.to_owned(), + sender_key: device.curve25519_key().unwrap(), + session_id: session.session_id().to_string(), + }; + let content = RoomKeyRequestContent::new_request( + RequestedKeyInfo::MegolmV1AesSha2(info), + device.device_id().to_owned(), + ruma::TransactionId::new(), + ); + let event = + RoomKeyRequestEvent::new(device.user_id().to_owned(), content); + + match self + .inner + .key_request_machine + .try_to_forward_room_key( + &event, + device.to_owned(), + session, + None, + ) + .await + { + Ok(_) => {} + Err(e) => warn!( + "Error forwarding room keys to device {:?}: {e}", + device.device_id() + ), + } + } + } + } + } + + requests.extend(outbound.pending_requests()); + } + } + + Ok(requests) + } + /// Receive an unencrypted verification event. /// /// This method can be used to pass verification events that are happening diff --git a/crates/matrix-sdk-crypto/src/olm/group_sessions/inbound.rs b/crates/matrix-sdk-crypto/src/olm/group_sessions/inbound.rs index 2edd99876dc..fd5452fad0b 100644 --- a/crates/matrix-sdk-crypto/src/olm/group_sessions/inbound.rs +++ b/crates/matrix-sdk-crypto/src/olm/group_sessions/inbound.rs @@ -142,7 +142,7 @@ pub struct InboundGroupSession { /// The history visibility of the room at the time when the room key was /// created. - history_visibility: Arc>, + pub history_visibility: Arc>, /// Was this room key backed up to the server. backed_up: Arc, @@ -227,6 +227,7 @@ impl InboundGroupSession { forwarding_curve25519_key_chain: vec![], session_key: backup.session_key, sender_claimed_keys: backup.sender_claimed_keys, + shared_history: backup.shared_history, }) } @@ -292,6 +293,12 @@ impl InboundGroupSession { let session_key = self.inner.lock().await.export_at(message_index).expect("Can't export session"); + let shared_history = if let Some(history) = self.history_visibility.as_ref().clone() { + matches!(history, HistoryVisibility::Shared | HistoryVisibility::WorldReadable) + } else { + false + }; + ExportedRoomKey { algorithm: self.algorithm().to_owned(), room_id: self.room_id().to_owned(), @@ -300,6 +307,7 @@ impl InboundGroupSession { forwarding_curve25519_key_chain: vec![], sender_claimed_keys: (*self.creator_info.signing_keys).clone(), session_key, + shared_history, } } @@ -521,6 +529,10 @@ impl TryFrom<&ExportedRoomKey> for InboundGroupSession { let config = OutboundGroupSession::session_config(&key.algorithm)?; let session = InnerSession::import(&key.session_key, config); let first_known_index = session.first_known_index(); + // Content does not indicate level of history visibility, so + // set it to least permissive for shared history if true + let history_visibility = + if key.shared_history { Some(HistoryVisibility::Shared) } else { None }; Ok(InboundGroupSession { inner: Mutex::new(session).into(), @@ -529,7 +541,7 @@ impl TryFrom<&ExportedRoomKey> for InboundGroupSession { curve25519_key: key.sender_key, signing_keys: key.sender_claimed_keys.to_owned().into(), }, - history_visibility: None.into(), + history_visibility: history_visibility.into(), first_known_index, room_id: key.room_id.to_owned(), imported: true, @@ -544,6 +556,10 @@ impl From<&ForwardedMegolmV1AesSha2Content> for InboundGroupSession { let session = InnerSession::import(&value.session_key, SessionConfig::version_1()); let session_id = session.session_id().into(); let first_known_index = session.first_known_index(); + // Content does not indicate level of history visibility, so + // set it to least permissive for shared history if true + let history_visibility = + if value.shared_history { Some(HistoryVisibility::Shared) } else { None }; InboundGroupSession { inner: Mutex::new(session).into(), @@ -556,7 +572,7 @@ impl From<&ForwardedMegolmV1AesSha2Content> for InboundGroupSession { )]) .into(), }, - history_visibility: None.into(), + history_visibility: history_visibility.into(), first_known_index, room_id: value.room_id.to_owned(), imported: true, @@ -571,6 +587,10 @@ impl From<&ForwardedMegolmV2AesSha2Content> for InboundGroupSession { let session = InnerSession::import(&value.session_key, SessionConfig::version_2()); let session_id = session.session_id().into(); let first_known_index = session.first_known_index(); + // Content does not indicate level of history visibility, so + // set it to least permissive for shared history if true + let history_visibility = + if value.shared_history { Some(HistoryVisibility::Shared) } else { None }; InboundGroupSession { inner: Mutex::new(session).into(), @@ -579,7 +599,7 @@ impl From<&ForwardedMegolmV2AesSha2Content> for InboundGroupSession { curve25519_key: value.claimed_sender_key, signing_keys: value.claimed_signing_keys.to_owned().into(), }, - history_visibility: None.into(), + history_visibility: history_visibility.into(), first_known_index, room_id: value.room_id.to_owned(), imported: true, diff --git a/crates/matrix-sdk-crypto/src/olm/group_sessions/mod.rs b/crates/matrix-sdk-crypto/src/olm/group_sessions/mod.rs index 0a1484ec959..e4a28048f04 100644 --- a/crates/matrix-sdk-crypto/src/olm/group_sessions/mod.rs +++ b/crates/matrix-sdk-crypto/src/olm/group_sessions/mod.rs @@ -95,6 +95,10 @@ pub struct ExportedRoomKey { serialize_with = "serialize_curve_key_vec" )] pub forwarding_curve25519_key_chain: Vec, + + /// Used to mark as having been used for shared history + #[serde(default)] + pub shared_history: bool, } /// A backed up version of an `InboundGroupSession` @@ -119,6 +123,10 @@ pub struct BackedUpRoomKey { /// Chain of Curve25519 keys through which this session was forwarded, via /// m.forwarded_room_key events. pub forwarding_curve25519_key_chain: Vec, + + /// Used to mark as having been used for shared history + #[serde(default)] + pub shared_history: bool, } impl TryFrom for ForwardedRoomKeyContent { @@ -151,6 +159,7 @@ impl TryFrom for ForwardedRoomKeyContent { forwarding_curve25519_key_chain: room_key .forwarding_curve25519_key_chain .clone(), + shared_history: room_key.shared_history, other: Default::default(), } .into(), @@ -168,6 +177,7 @@ impl TryFrom for ForwardedRoomKeyContent { session_key: room_key.session_key, claimed_sender_key: room_key.sender_key, claimed_signing_keys: room_key.sender_claimed_keys, + shared_history: room_key.shared_history, other: Default::default(), } .into(), @@ -186,6 +196,7 @@ impl From for BackedUpRoomKey { session_key: k.session_key, sender_claimed_keys: k.sender_claimed_keys, forwarding_curve25519_key_chain: k.forwarding_curve25519_key_chain, + shared_history: k.shared_history, } } } @@ -211,6 +222,7 @@ impl TryFrom for ExportedRoomKey { sender_claimed_keys, sender_key: content.claimed_sender_key, session_key: content.session_key, + shared_history: content.shared_history, }) } #[cfg(feature = "experimental-algorithms")] @@ -222,6 +234,7 @@ impl TryFrom for ExportedRoomKey { sender_claimed_keys: content.claimed_signing_keys, sender_key: content.claimed_sender_key, session_key: content.session_key, + shared_history: content.shared_history, }), ForwardedRoomKeyContent::Unknown(c) => Err(SessionExportError::Algorithm(c.algorithm)), } diff --git a/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs b/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs index a363b2bbc64..413d9dfda09 100644 --- a/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs +++ b/crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs @@ -483,12 +483,17 @@ impl OutboundGroupSession { pub(crate) async fn as_content(&self) -> RoomKeyContent { let session_key = self.session_key().await; + let shared_history = matches!( + self.settings.history_visibility, + HistoryVisibility::Shared | HistoryVisibility::WorldReadable + ); RoomKeyContent::MegolmV1AesSha2( MegolmV1AesSha2RoomKeyContent::new( self.room_id().to_owned(), self.session_id().to_owned(), session_key, + shared_history, ) .into(), ) diff --git a/crates/matrix-sdk-crypto/src/types/events/forwarded_room_key.rs b/crates/matrix-sdk-crypto/src/types/events/forwarded_room_key.rs index ab15f9a59d7..42100f8f102 100644 --- a/crates/matrix-sdk-crypto/src/types/events/forwarded_room_key.rs +++ b/crates/matrix-sdk-crypto/src/types/events/forwarded_room_key.rs @@ -127,6 +127,10 @@ pub struct ForwardedMegolmV1AesSha2Content { )] pub claimed_ed25519_key: Ed25519PublicKey, + /// Used to mark as having been used for shared history + #[serde(default, rename = "org.matrix.msc3061.shared_history")] + pub shared_history: bool, + #[serde(flatten)] pub(crate) other: BTreeMap, } @@ -162,6 +166,10 @@ pub struct ForwardedMegolmV2AesSha2Content { #[serde(default)] pub claimed_signing_keys: SigningKeys, + /// Used to mark as having been used for shared history + #[serde(default, rename = "org.matrix.msc3061.shared_history")] + pub shared_history: bool, + #[serde(flatten)] pub(crate) other: BTreeMap, } @@ -184,6 +192,7 @@ impl std::fmt::Debug for ForwardedMegolmV1AesSha2Content { .field("forwarding_curve25519_key_chain", &self.forwarding_curve25519_key_chain) .field("claimed_sender_key", &self.claimed_sender_key) .field("claimed_ed25519_key", &self.claimed_ed25519_key) + .field("shared_history", &self.shared_history) .finish_non_exhaustive() } } @@ -195,6 +204,7 @@ impl std::fmt::Debug for ForwardedMegolmV2AesSha2Content { .field("session_id", &self.session_id) .field("claimed_sender_key", &self.claimed_sender_key) .field("sender_claimed_keys", &self.claimed_signing_keys) + .field("shared_history", &self.shared_history) .finish_non_exhaustive() } } diff --git a/crates/matrix-sdk-crypto/src/types/events/room_key.rs b/crates/matrix-sdk-crypto/src/types/events/room_key.rs index dc0608bbc20..8864c04c50c 100644 --- a/crates/matrix-sdk-crypto/src/types/events/room_key.rs +++ b/crates/matrix-sdk-crypto/src/types/events/room_key.rs @@ -74,6 +74,7 @@ impl RoomKeyContent { pub room_id: &'a RoomId, pub session_id: &'a str, pub session_key: &'a str, + pub shared_history: bool, #[serde(flatten)] other: &'a BTreeMap, } @@ -83,6 +84,7 @@ impl RoomKeyContent { room_id: &content.room_id, session_id: &content.session_id, session_key: "", + shared_history: content.shared_history, other: &content.other, }; @@ -113,6 +115,9 @@ pub struct MegolmV1AesSha2Content { /// /// [`InboundGroupSession`]: vodozemac::megolm::InboundGroupSession pub session_key: SessionKey, + /// Used to mark as having been used for shared history + #[serde(default, rename = "org.matrix.msc3061.shared_history")] + pub shared_history: bool, /// Any other, custom and non-specced fields of the content. #[serde(flatten)] other: BTreeMap, @@ -120,8 +125,13 @@ pub struct MegolmV1AesSha2Content { impl MegolmV1AesSha2Content { /// Create a new `m.megolm.v1.aes-sha2` `m.room_key` content. - pub fn new(room_id: OwnedRoomId, session_id: String, session_key: SessionKey) -> Self { - Self { room_id, session_id, session_key, other: Default::default() } + pub fn new( + room_id: OwnedRoomId, + session_id: String, + session_key: SessionKey, + shared_history: bool, + ) -> Self { + Self { room_id, session_id, session_key, shared_history, other: Default::default() } } } @@ -223,7 +233,8 @@ pub(super) mod tests { tino//CDQENtcKuEt0I9s0+Kk4YSH310Szse2RQ+vjple31\ QrCexmqfFJzkR/BJ5ogJHrPBQL0LgsPyglIbMTLg7qygIaY\ U5Fe2QdKMH7nTZPNIRHh1RaMfHVETAUJBax88EWZBoifk80\ - gdHUwHSgMk77vCc2a5KHKLDA" + gdHUwHSgMk77vCc2a5KHKLDA", + "org.matrix.msc3061.shared_history": true }, "type": "m.room_key", "m.custom.top": "something custom in the top", diff --git a/crates/matrix-sdk-crypto/src/types/events/to_device.rs b/crates/matrix-sdk-crypto/src/types/events/to_device.rs index f9f1db83ed6..97fa537c585 100644 --- a/crates/matrix-sdk-crypto/src/types/events/to_device.rs +++ b/crates/matrix-sdk-crypto/src/types/events/to_device.rs @@ -474,7 +474,8 @@ mod tests { 3LqBjD21sULYEO5YTKdpMVhi9i6ZSZhdvZvp//tzRpDT7wpWVWI\ 00Y3EPEjmpm/HfZ4MMAKpk+tzJVuuvfAcHBZgpnxBGzYOc/DAqa\ pK7Tk3t3QJ1UMSD94HfAqlb1JF5QBPwoh0fOvD8pJdanB8zxz05\ - tKFdR73/vo2Q/zE3" + tKFdR73/vo2Q/zE3", + "org.matrix.msc3061.shared_history": true }, "type": "m.forwarded_room_key" }) diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index 4ec6039e8af..7ce08e31eff 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -986,6 +986,27 @@ impl Room { let request = invite_user::v3::Request::new(self.room_id().to_owned(), recipient); self.client.send(request, None).await?; + // MSC3061: Forward past room keys to invitee + #[cfg(feature = "e2e-encryption")] + if self.is_encrypted().await? + && matches!( + self.history_visibility(), + HistoryVisibility::Shared | HistoryVisibility::WorldReadable + ) + { + match self.client.olm_machine().await.as_ref() { + Some(olm) => { + // Get the most up-to-date device list before forwarding room keys + let (req_id, request) = olm.query_keys_for_users(std::iter::once(user_id)); + self.client.keys_query(&req_id, request.device_keys).await?; + + olm.share_room_history_keys(self.room_id(), std::iter::once(user_id)).await?; + self.client.send_outgoing_requests().await?; + } + None => panic!("Olm machine wasn't started"), + } + } + Ok(()) } @@ -1000,6 +1021,9 @@ impl Room { let request = invite_user::v3::Request::new(self.room_id().to_owned(), recipient); self.client.send(request, None).await?; + // MSC3061: 3PID invites cannot forward past room keys since UserId + // lookup and invite is handled by the homeserver, not the client. + Ok(()) } diff --git a/crates/matrix-sdk/tests/integration/room/joined.rs b/crates/matrix-sdk/tests/integration/room/joined.rs index 6f8a134edc2..e2b1f237e6f 100644 --- a/crates/matrix-sdk/tests/integration/room/joined.rs +++ b/crates/matrix-sdk/tests/integration/room/joined.rs @@ -37,6 +37,7 @@ async fn invite_user_by_id() { .await; mock_sync(&server, &*test_json::SYNC, None).await; + mock_encryption_state(&server, false).await; let sync_settings = SyncSettings::new().timeout(Duration::from_millis(3000)); From 85c954e18ca51854c6444c727149dbd2d0fcacde Mon Sep 17 00:00:00 2001 From: Michael Hollister Date: Mon, 2 Oct 2023 01:46:32 -0500 Subject: [PATCH 2/7] Fixed bindings generation Signed-off-by: Michael Hollister --- bindings/matrix-sdk-crypto-ffi/src/machine.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/bindings/matrix-sdk-crypto-ffi/src/machine.rs b/bindings/matrix-sdk-crypto-ffi/src/machine.rs index 763bc7538d3..8aa9cee5fb5 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/machine.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/machine.rs @@ -765,7 +765,6 @@ impl OlmMachine { /// /// * `users` - The list of users which are considered to be members of the /// room and should receive previous room keys. - #[cfg(feature = "automatic-room-key-forwarding")] pub fn share_room_history_keys( &self, room_id: String, From d113e689c75d0d6b56e76ab70aa650337be901e1 Mon Sep 17 00:00:00 2001 From: Michael Hollister Date: Mon, 2 Oct 2023 15:25:35 -0500 Subject: [PATCH 3/7] Fixed missing feature check Signed-off-by: Michael Hollister --- crates/matrix-sdk/src/room/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index 7ce08e31eff..f5e77453cef 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -987,7 +987,7 @@ impl Room { self.client.send(request, None).await?; // MSC3061: Forward past room keys to invitee - #[cfg(feature = "e2e-encryption")] + #[cfg(all(feature = "e2e-encryption", feature = "automatic-room-key-forwarding"))] if self.is_encrypted().await? && matches!( self.history_visibility(), From 7df0399ce2d0e8796bd871e41a2595fd049f4b7f Mon Sep 17 00:00:00 2001 From: Michael Hollister Date: Tue, 3 Oct 2023 21:18:20 -0500 Subject: [PATCH 4/7] Changed how InboundGroupSession is created Signed-off-by: Michael Hollister --- .../src/gossiping/machine.rs | 49 ++++++++----------- 1 file changed, 20 insertions(+), 29 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/gossiping/machine.rs b/crates/matrix-sdk-crypto/src/gossiping/machine.rs index b27df3139cc..5a4191edfb6 100644 --- a/crates/matrix-sdk-crypto/src/gossiping/machine.rs +++ b/crates/matrix-sdk-crypto/src/gossiping/machine.rs @@ -989,8 +989,6 @@ impl GossipMachine { sender_key: Curve25519PublicKey, event: &DecryptedForwardedRoomKeyEvent, ) -> Result, CryptoStoreError> { - use vodozemac::megolm::SessionKey; - let Some(info) = event.room_key_info() else { warn!( sender_key = sender_key.to_base64(), @@ -1005,13 +1003,13 @@ impl GossipMachine { else { // We did not request this key, so determine if this key was // forwarded as a result from a room invite - let (room_id, session_key, shared_history) = match &event.content { + let session = match &event.content { ForwardedRoomKeyContent::MegolmV1AesSha2(c) => { - (&c.room_id, &c.session_key, c.shared_history) + InboundGroupSession::from(c.as_ref()) } #[cfg(feature = "experimental-algorithms")] ForwardedRoomKeyContent::MegolmV2AesSha2(c) => { - (&c.room_id, &c.session_key, c.shared_history) + InboundGroupSession::from(c.as_ref()) } ForwardedRoomKeyContent::Unknown(_) => { warn!( @@ -1027,32 +1025,25 @@ impl GossipMachine { } }; - if shared_history { - // Content does not indicate level of history visibility, so - // set it to least permissive for shared history - let visibility = Some(HistoryVisibility::Shared); - let session_key = SessionKey::from(session_key); - - return Ok(Some(InboundGroupSession::new( - sender_key, - event.keys.ed25519, - room_id, - &session_key, - event.content.algorithm(), + if let Some(visibility) = session.history_visibility.as_ref() { + if matches!( visibility, - )?)); - } else { - warn!( - sender_key = ?sender_key, - room_id = ?info.room_id(), - session_id = info.session_id(), - sender_key = ?sender_key, - algorithm = ?info.algorithm(), - "Received a forwarded room key that we didn't request", - ); - - return Ok(None); + HistoryVisibility::Shared | HistoryVisibility::WorldReadable + ) { + return Ok(Some(session)); + } } + + warn!( + sender_key = ?sender_key, + room_id = ?info.room_id(), + session_id = info.session_id(), + sender_key = ?sender_key, + algorithm = ?info.algorithm(), + "Received a forwarded room key that we didn't request", + ); + + return Ok(None); }; if self.should_accept_forward(&request, sender_key).await? { From ada3e828a15cdaff0a9e6747cede3da814e74e7e Mon Sep 17 00:00:00 2001 From: Michael Hollister Date: Fri, 13 Oct 2023 18:09:29 -0500 Subject: [PATCH 5/7] Updated to offical ruma crate Signed-off-by: Michael Hollister --- Cargo.lock | 8 ++++---- Cargo.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7aaa792ba1d..953f2e9dd49 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4882,9 +4882,9 @@ dependencies = [ [[package]] name = "ruma" -version = "0.9.0" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85c63fe7f06396db1480c40cf44a57ad4359847cf6c6b3cdaa67507a00a79bb9" +checksum = "f067a50962653d837e5ead753c3c33129aaeaaa25a2e9b4868cdc8e5af3edd4a" dependencies = [ "assign", "js_int", @@ -4949,9 +4949,9 @@ dependencies = [ [[package]] name = "ruma-events" -version = "0.27.4" +version = "0.27.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1784ded2966b990151d7d30a4533b737e1100ca9856178a999f3763a615a1efe" +checksum = "e44bf8dca3bd734c175784faddeb14b1780a504f34683ac899331a141b8dfa55" dependencies = [ "as_variant", "indexmap 2.0.2", diff --git a/Cargo.toml b/Cargo.toml index 96e44753bb4..13a9a188acb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,7 @@ futures-executor = "0.3.21" futures-util = { version = "0.3.26", default-features = false, features = ["alloc"] } http = "0.2.6" itertools = "0.11.0" -ruma = { version = "0.9.0", features = ["client-api-c", "compat-upload-signatures", "compat-user-id", "compat-arbitrary-length-ids"] } +ruma = { version = "0.9.1", features = ["client-api-c", "compat-upload-signatures", "compat-user-id", "compat-arbitrary-length-ids", "unstable-msc3061"] } ruma-common = "0.12.0" once_cell = "1.16.0" serde = "1.0.151" From 7758e61ce293f9ed02da93f9159314d1ebdfbdfe Mon Sep 17 00:00:00 2001 From: Michael Hollister Date: Thu, 19 Oct 2023 23:20:04 -0500 Subject: [PATCH 6/7] Fixed key forwarding for devices without olm session Signed-off-by: Michael Hollister --- .../src/gossiping/machine.rs | 94 +++++++++++++++++-- crates/matrix-sdk-crypto/src/machine.rs | 3 +- 2 files changed, 90 insertions(+), 7 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/gossiping/machine.rs b/crates/matrix-sdk-crypto/src/gossiping/machine.rs index 5a4191edfb6..b7b4a84f05e 100644 --- a/crates/matrix-sdk-crypto/src/gossiping/machine.rs +++ b/crates/matrix-sdk-crypto/src/gossiping/machine.rs @@ -76,6 +76,12 @@ pub(crate) struct GossipMachineInner { wait_queue: WaitQueue, users_for_key_claim: Arc>>>, room_key_forwarding_enabled: AtomicBool, + /// A map from the device id to a set of session_id strings for previous + /// `InboundGroupSession`s. Used when an olm session is not established + /// with a user's device on invite, in which specific keys are flagged to + /// allow for sharing with the device when the olm session is established. + #[cfg(feature = "automatic-room-key-forwarding")] + pending_room_key_forwarding_devices: Arc>>>, } impl GossipMachine { @@ -97,6 +103,8 @@ impl GossipMachine { wait_queue: WaitQueue::new(), users_for_key_claim, room_key_forwarding_enabled, + #[cfg(feature = "automatic-room-key-forwarding")] + pending_room_key_forwarding_devices: Default::default(), }), } } @@ -360,7 +368,9 @@ impl GossipMachine { device: Device, session: &InboundGroupSession, message_index: Option, + update_outbound_devices: bool, ) -> OlmResult> { + use crate::olm::{ShareInfo, ShareState}; info!(?message_index, "Serving a room key request",); match self.forward_room_key(session, &device, message_index).await { @@ -369,6 +379,49 @@ impl GossipMachine { info!( "Key request is missing an Olm session, putting the request in the wait queue", ); + + // In the case when we are inviting a user that has a device + // with a missing session, we need to ensure that we allow the + // forwarded key to be in the outbound session's + // `shared_with_set` when the olm session is established + if update_outbound_devices { + self.inner + .pending_room_key_forwarding_devices + .write() + .unwrap() + .entry(device.device_id().to_owned()) + .or_default() + .insert(session.session_id().to_string()); + + let outbound_session = + self.inner.outbound_group_sessions.get_or_load(session.room_id()).await; + + if let Some(outbound) = outbound_session { + match outbound.is_shared_with(&device) { + ShareState::NotShared => { + let share_info = ShareInfo::new_shared( + device.curve25519_key().unwrap(), + outbound.message_index().await, + ); + outbound + .shared_with_set + .write() + .unwrap() + .entry(device.user_id().to_owned()) + .or_default() + .insert(device.device_id().to_owned(), share_info); + + self.inner.outbound_group_sessions.insert(outbound.clone()); + + let mut changes = Changes::default(); + changes.outbound_group_sessions.push(outbound); + self.inner.store.save_changes(changes).await?; + } + ShareState::SharedButChangedSenderKey | ShareState::Shared(_) => {} + } + } + } + self.handle_key_share_without_session(device, event.to_owned().into()); Ok(None) @@ -406,7 +459,7 @@ impl GossipMachine { match self.should_share_key(&device, session).await { Ok(message_index) => { - self.try_to_forward_room_key(event, device, session, message_index).await + self.try_to_forward_room_key(event, device, session, message_index, false).await } Err(e) => { if let KeyForwardDecision::ChangedSenderKey = e { @@ -582,11 +635,40 @@ impl GossipMachine { use super::KeyForwardDecision; use crate::olm::ShareState; - let outbound_session = self - .inner - .outbound_group_sessions - .get_with_id(session.room_id(), session.session_id()) - .await; + // We don't store prior `OutboundGroupSession` objects in the store. + // Because of this, we check if the key that is about to be shared has + // been flagged as a forwarded key from a result of a user invite. + // If so, we must use the current outbound session since it has the + // latest `shared_with_set` device map. + let mut use_current_session = false; + { + let mut pending_device_keys = + self.inner.pending_room_key_forwarding_devices.write().unwrap(); + + if pending_device_keys + .get(device.device_id()) + .is_some_and(|s| s.contains(session.session_id())) + { + pending_device_keys.entry(device.device_id().to_owned()).and_modify(|s| { + s.remove(session.session_id()); + }); + + if pending_device_keys.get(device.device_id()).is_some_and(|s| s.is_empty()) { + pending_device_keys.remove(device.device_id()); + } + + use_current_session = true; + } + } + + let outbound_session = if use_current_session { + self.inner.outbound_group_sessions.get_or_load(session.room_id()).await + } else { + self.inner + .outbound_group_sessions + .get_with_id(session.room_id(), session.session_id()) + .await + }; // If this is our own, verified device, we share the entire session from the // earliest known index. diff --git a/crates/matrix-sdk-crypto/src/machine.rs b/crates/matrix-sdk-crypto/src/machine.rs index ada0e0b9236..b11bb2e7153 100644 --- a/crates/matrix-sdk-crypto/src/machine.rs +++ b/crates/matrix-sdk-crypto/src/machine.rs @@ -926,7 +926,7 @@ impl OlmMachine { for device in &devices { let info = MegolmV1AesSha2Content { room_id: room_id.to_owned(), - sender_key: device.curve25519_key().unwrap(), + sender_key: session.sender_key(), session_id: session.session_id().to_string(), }; let content = RoomKeyRequestContent::new_request( @@ -945,6 +945,7 @@ impl OlmMachine { device.to_owned(), session, None, + true, ) .await { From 68fd193a782f04b05079b3aeaae064331f1d3b17 Mon Sep 17 00:00:00 2001 From: Michael Hollister Date: Fri, 20 Oct 2023 22:57:50 -0500 Subject: [PATCH 7/7] Fixed key forwarding when user identity does not exist in store Signed-off-by: Michael Hollister --- crates/matrix-sdk-crypto/src/olm/account.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/olm/account.rs b/crates/matrix-sdk-crypto/src/olm/account.rs index 1c1c9e007ac..b48bec699b1 100644 --- a/crates/matrix-sdk-crypto/src/olm/account.rs +++ b/crates/matrix-sdk-crypto/src/olm/account.rs @@ -1325,11 +1325,14 @@ impl Account { ) .into()) } else { - // If this event is an `m.room_key` event, defer the check for the - // Ed25519 key of the sender until we decrypt room events. This - // ensures that we receive the room key even if we don't have access - // to the device. - if !matches!(*event, AnyDecryptedOlmEvent::RoomKey(_)) { + // If this event is an `m.room_key` or `m.forwarded_room_key` event, + // defer the check for the Ed25519 key of the sender until we decrypt + // room events. This ensures that we receive the room key even if we + // don't have access to the device. + if !matches!( + *event, + AnyDecryptedOlmEvent::RoomKey(_) | AnyDecryptedOlmEvent::ForwardedRoomKey(_) + ) { let Some(device) = store.get_device_from_curve_key(event.sender(), sender_key).await? else {