diff --git a/crates/matrix-sdk-common/src/deserialized_responses.rs b/crates/matrix-sdk-common/src/deserialized_responses.rs index 0474a281a85..49c93c456bf 100644 --- a/crates/matrix-sdk-common/src/deserialized_responses.rs +++ b/crates/matrix-sdk-common/src/deserialized_responses.rs @@ -1200,6 +1200,13 @@ pub enum WithheldCode { #[ruma_enum(rename = "m.no_olm")] NoOlm, + /// Normally used when sharing history, per [MSC4268]: indicates + /// that the session was not marked as "shared_history". + /// + /// [MSC4268]: https://github.com/matrix-org/matrix-spec-proposals/pull/4268 + #[ruma_enum(rename = "io.element.msc4268.history_not_shared", alias = "m.history_not_shared")] + HistoryNotShared, + #[doc(hidden)] _Custom(PrivOwnedStr), } @@ -1212,6 +1219,7 @@ impl fmt::Display for WithheldCode { WithheldCode::Unauthorised => "You are not authorised to read the message.", WithheldCode::Unavailable => "The requested key was not found.", WithheldCode::NoOlm => "Unable to establish a secure channel.", + WithheldCode::HistoryNotShared => "The sender disabled sharing encrypted history.", _ => self.as_str(), }; diff --git a/crates/matrix-sdk-crypto/CHANGELOG.md b/crates/matrix-sdk-crypto/CHANGELOG.md index 4a236111ba5..eb0f6882ed3 100644 --- a/crates/matrix-sdk-crypto/CHANGELOG.md +++ b/crates/matrix-sdk-crypto/CHANGELOG.md @@ -8,6 +8,9 @@ All notable changes to this project will be documented in this file. ### Features +- Use new withheld code in key bundles for sessions not marked as + `shared_history`. + ([#5807](https://github.com/matrix-org/matrix-rust-sdk/pull/5807) - Improve feedback support for shared history when downloading room key bundles. ([#5737](https://github.com/matrix-org/matrix-rust-sdk/pull/5737)) - Add `RoomKeyWithheldEntry` enum, wrapping either a received to-device `m.room_key.withheld` event or diff --git a/crates/matrix-sdk-crypto/src/machine/mod.rs b/crates/matrix-sdk-crypto/src/machine/mod.rs index 6cdc1364d41..bbb3ec31fdf 100644 --- a/crates/matrix-sdk-crypto/src/machine/mod.rs +++ b/crates/matrix-sdk-crypto/src/machine/mod.rs @@ -897,7 +897,7 @@ impl OlmMachine { // This function is only ever called by add_room_key via // handle_decrypted_to_device_event, so sender, sender_key, and algorithm are // already recorded. - fields(room_id = ? content.room_id, session_id, message_index) + fields(room_id = ? content.room_id, session_id, message_index, shared_history = content.shared_history) )] async fn handle_key( &self, diff --git a/crates/matrix-sdk-crypto/src/store/mod.rs b/crates/matrix-sdk-crypto/src/store/mod.rs index 98586a0c5b7..c19dd5a22b4 100644 --- a/crates/matrix-sdk-crypto/src/store/mod.rs +++ b/crates/matrix-sdk-crypto/src/store/mod.rs @@ -1600,7 +1600,7 @@ impl Store { } else { bundle.withheld.push(RoomKeyWithheldContent::new( session.algorithm().to_owned(), - WithheldCode::Unauthorised, + WithheldCode::HistoryNotShared, session.room_id().to_owned(), session.session_id().to_owned(), session.sender_key().to_owned(), @@ -1717,6 +1717,8 @@ impl Store { bundle_info: &StoredRoomKeyBundleData, bundle: &RoomKeyBundle, ) -> Result<(), CryptoStoreError> { + let mut session_id_to_withheld_code_map = BTreeMap::new(); + let mut changes = Changes::default(); for withheld in &bundle.withheld { let (room_id, session_id) = match withheld { @@ -1744,9 +1746,17 @@ impl Store { content: withheld.to_owned(), }, ); + session_id_to_withheld_code_map.insert(session_id, withheld.withheld_code()); } + self.save_changes(changes).await?; + info!( + room_id = ?bundle_info.bundle_data.room_id, + ?session_id_to_withheld_code_map, + "Successfully imported withheld info from room key bundle", + ); + Ok(()) } } @@ -2144,11 +2154,11 @@ mod tests { RoomKeyWithheldEntry { #[cfg(not(feature = "experimental-algorithms"))] content: RoomKeyWithheldContent::MegolmV1AesSha2( - MegolmV1AesSha2WithheldContent::Unauthorised(_) + MegolmV1AesSha2WithheldContent::HistoryNotShared(_) ), #[cfg(feature = "experimental-algorithms")] content: RoomKeyWithheldContent::MegolmV2AesSha2( - MegolmV1AesSha2WithheldContent::Unauthorised(_) + MegolmV1AesSha2WithheldContent::HistoryNotShared(_) ), .. } diff --git a/crates/matrix-sdk-crypto/src/store/snapshots/matrix_sdk_crypto__store__tests__build_room_key_bundle.snap b/crates/matrix-sdk-crypto/src/store/snapshots/matrix_sdk_crypto__store__tests__build_room_key_bundle.snap index cdc15086a1b..d212f2c25d5 100644 --- a/crates/matrix-sdk-crypto/src/store/snapshots/matrix_sdk_crypto__store__tests__build_room_key_bundle.snap +++ b/crates/matrix-sdk-crypto/src/store/snapshots/matrix_sdk_crypto__store__tests__build_room_key_bundle.snap @@ -28,9 +28,9 @@ expression: bundle "withheld": [ { "algorithm": "[algorithm]", - "code": "m.unauthorised", + "code": "io.element.msc4268.history_not_shared", "from_device": "BOB", - "reason": "You are not authorised to read the message.", + "reason": "The sender disabled sharing encrypted history.", "room_id": "!room1:localhost", "sender_key": "[alice curve key]", "session_id": "lpRzTgD3Nook/Wk62Fm9ECWGnKYZgeCwO1Y+uuPJz/I" diff --git a/crates/matrix-sdk-crypto/src/types/events/room_key_withheld.rs b/crates/matrix-sdk-crypto/src/types/events/room_key_withheld.rs index d63e74ad837..c071dcec847 100644 --- a/crates/matrix-sdk-crypto/src/types/events/room_key_withheld.rs +++ b/crates/matrix-sdk-crypto/src/types/events/room_key_withheld.rs @@ -65,7 +65,8 @@ macro_rules! construct_withheld_content { WithheldCode::Blacklisted | WithheldCode::Unverified | WithheldCode::Unauthorised - | WithheldCode::Unavailable => { + | WithheldCode::Unavailable + | WithheldCode::HistoryNotShared => { let content = CommonWithheldCodeContent { $room_id, $session_id, @@ -84,7 +85,9 @@ macro_rules! construct_withheld_content { .into(), )) } - _ => unreachable!("Can't create an unknown withheld code content"), + WithheldCode::_Custom(_) => { + unreachable!("Can't create an unknown withheld code content") + } } }; } @@ -180,6 +183,9 @@ pub enum MegolmV1AesSha2WithheldContent { Unauthorised(Box), /// The `m.unavailable` variant of the withheld code content. Unavailable(Box), + /// The `m.history_not_shared` variant of the withheld code content (cf + /// [MSC4268](https://github.com/matrix-org/matrix-spec-proposals/pull/4268)). + HistoryNotShared(Box), /// The `m.no_olm` variant of the withheld code content. NoOlm(Box), } @@ -231,7 +237,10 @@ impl MegolmV1AesSha2WithheldContent { MegolmV1AesSha2WithheldContent::BlackListed(content) | MegolmV1AesSha2WithheldContent::Unverified(content) | MegolmV1AesSha2WithheldContent::Unauthorised(content) - | MegolmV1AesSha2WithheldContent::Unavailable(content) => Some(&content.session_id), + | MegolmV1AesSha2WithheldContent::Unavailable(content) + | MegolmV1AesSha2WithheldContent::HistoryNotShared(content) => { + Some(&content.session_id) + } MegolmV1AesSha2WithheldContent::NoOlm(_) => None, } } @@ -242,7 +251,8 @@ impl MegolmV1AesSha2WithheldContent { MegolmV1AesSha2WithheldContent::BlackListed(content) | MegolmV1AesSha2WithheldContent::Unverified(content) | MegolmV1AesSha2WithheldContent::Unauthorised(content) - | MegolmV1AesSha2WithheldContent::Unavailable(content) => Some(&content.room_id), + | MegolmV1AesSha2WithheldContent::Unavailable(content) + | MegolmV1AesSha2WithheldContent::HistoryNotShared(content) => Some(&content.room_id), MegolmV1AesSha2WithheldContent::NoOlm(_) => None, } } @@ -254,6 +264,7 @@ impl MegolmV1AesSha2WithheldContent { MegolmV1AesSha2WithheldContent::Unverified(_) => WithheldCode::Unverified, MegolmV1AesSha2WithheldContent::Unauthorised(_) => WithheldCode::Unauthorised, MegolmV1AesSha2WithheldContent::Unavailable(_) => WithheldCode::Unavailable, + MegolmV1AesSha2WithheldContent::HistoryNotShared(_) => WithheldCode::HistoryNotShared, MegolmV1AesSha2WithheldContent::NoOlm(_) => WithheldCode::NoOlm, } } @@ -266,7 +277,10 @@ impl MegolmV1AesSha2WithheldContent { WithheldCode::Unverified => Self::Unverified(content), WithheldCode::Unauthorised => Self::Unauthorised(content), WithheldCode::Unavailable => Self::Unavailable(content), - _ => unreachable!("This constructor requires one of the common withheld codes"), + WithheldCode::HistoryNotShared => Self::HistoryNotShared(content), + WithheldCode::NoOlm | WithheldCode::_Custom(_) => { + unreachable!("This constructor requires one of the common withheld codes") + } } } } @@ -349,14 +363,15 @@ impl TryFrom for RoomKeyWithheldContent { WithheldCode::Blacklisted | WithheldCode::Unverified | WithheldCode::Unauthorised - | WithheldCode::Unavailable => { + | WithheldCode::Unavailable + | WithheldCode::HistoryNotShared => { let content: CommonWithheldCodeContent = serde_json::from_value(value.other)?; Self::MegolmV1AesSha2(MegolmV1AesSha2WithheldContent::from_code_and_content( value.code, content, )) } - _ => unknown(value)?, + WithheldCode::_Custom(_) => unknown(value)?, }, #[cfg(feature = "experimental-algorithms")] EventEncryptionAlgorithm::MegolmV2AesSha2 => match value.code { @@ -367,14 +382,15 @@ impl TryFrom for RoomKeyWithheldContent { WithheldCode::Blacklisted | WithheldCode::Unverified | WithheldCode::Unauthorised - | WithheldCode::Unavailable => { + | WithheldCode::Unavailable + | WithheldCode::HistoryNotShared => { let content: CommonWithheldCodeContent = serde_json::from_value(value.other)?; Self::MegolmV1AesSha2(MegolmV1AesSha2WithheldContent::from_code_and_content( value.code, content, )) } - _ => unknown(value)?, + WithheldCode::_Custom(_) => unknown(value)?, }, _ => unknown(value)?, }) @@ -397,7 +413,8 @@ impl Serialize for RoomKeyWithheldContent { MegolmV1AesSha2WithheldContent::BlackListed(content) | MegolmV1AesSha2WithheldContent::Unverified(content) | MegolmV1AesSha2WithheldContent::Unauthorised(content) - | MegolmV1AesSha2WithheldContent::Unavailable(content) => WithheldHelper { + | MegolmV1AesSha2WithheldContent::Unavailable(content) + | MegolmV1AesSha2WithheldContent::HistoryNotShared(content) => WithheldHelper { algorithm, code, reason, @@ -420,7 +437,8 @@ impl Serialize for RoomKeyWithheldContent { MegolmV1AesSha2WithheldContent::BlackListed(content) | MegolmV1AesSha2WithheldContent::Unverified(content) | MegolmV1AesSha2WithheldContent::Unauthorised(content) - | MegolmV1AesSha2WithheldContent::Unavailable(content) => WithheldHelper { + | MegolmV1AesSha2WithheldContent::Unavailable(content) + | MegolmV1AesSha2WithheldContent::HistoryNotShared(content) => WithheldHelper { algorithm, code, reason, @@ -534,6 +552,7 @@ pub(super) mod tests { WithheldCode::Blacklisted, WithheldCode::Unauthorised, WithheldCode::Unavailable, + WithheldCode::HistoryNotShared, ]; for code in codes { let json = json(&code); diff --git a/crates/matrix-sdk-crypto/src/types/events/utd_cause.rs b/crates/matrix-sdk-crypto/src/types/events/utd_cause.rs index 42be623f858..1ca78d7bd0f 100644 --- a/crates/matrix-sdk-crypto/src/types/events/utd_cause.rs +++ b/crates/matrix-sdk-crypto/src/types/events/utd_cause.rs @@ -150,6 +150,7 @@ impl UtdCause { WithheldCode::Blacklisted | WithheldCode::Unauthorised | WithheldCode::Unavailable + | WithheldCode::HistoryNotShared | WithheldCode::NoOlm | WithheldCode::_Custom(_) => UtdCause::WithheldBySender, } diff --git a/testing/matrix-sdk-integration-testing/src/tests/e2ee/shared_history.rs b/testing/matrix-sdk-integration-testing/src/tests/e2ee/shared_history.rs index 18b985c6ee1..f05b966e19b 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/e2ee/shared_history.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/e2ee/shared_history.rs @@ -5,19 +5,26 @@ use assert_matches2::{assert_let, assert_matches}; use assign::assign; use futures::{FutureExt, StreamExt, future, pin_mut}; use matrix_sdk::{ - assert_decrypted_message_eq, assert_next_with_timeout, + Room, assert_decrypted_message_eq, assert_next_with_timeout, deserialized_responses::TimelineEventKind, encryption::EncryptionSettings, + room::power_levels::RoomPowerLevelChanges, ruma::{ + EventId, api::client::{ room::create_room::v3::{Request as CreateRoomRequest, RoomPreset}, uiaa::Password, }, - events::room::message::RoomMessageEventContent, + events::room::{ + history_visibility::{HistoryVisibility, RoomHistoryVisibilityEventContent}, + message::RoomMessageEventContent, + }, }, timeout::timeout, }; -use matrix_sdk_common::deserialized_responses::ProcessedToDeviceEvent; +use matrix_sdk_common::deserialized_responses::{ + ProcessedToDeviceEvent, UnableToDecryptReason::MissingMegolmSession, WithheldCode, +}; use matrix_sdk_ui::sync_service::SyncService; use similar_asserts::assert_eq; use tracing::{Instrument, info}; @@ -360,3 +367,173 @@ async fn test_history_share_on_invite_pin_violation() -> Result<()> { Ok(()) } + +/// Test history sharing where some sessions are withheld. +/// +/// In this scenario we have three separate users: +/// +/// 1. Alice and Bob share a room, where the history visibility is set to +/// "shared". +/// 2. Bob sends a message. This will be "shareable". +/// 3. Alice changes the history viz to "joined". +/// 4. Alice changes the history viz back to "shared", but Bob doesn't (yet) +/// receive the memo. +/// 5. Bob sends a second message; the key is "unshareable" because Bob still +/// thinks the history viz is "joined". +/// 6. Bob syncs, and sends a third message; the key is now "shareable". +/// 7. Alice invites Charlie. +/// 8. Charlie joins the room. He should see Bob's first message; the second +/// should have an appropriate withheld code from Alice; the third should be +/// decryptable. +/// +/// This tests correct "withheld" code handling. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn test_transitive_history_share_with_withhelds() -> Result<()> { + let alice_span = tracing::info_span!("alice"); + let bob_span = tracing::info_span!("bob"); + let charlie_span = tracing::info_span!("charlie"); + + let alice = create_encryption_enabled_client("alice").instrument(alice_span.clone()).await?; + let bob = create_encryption_enabled_client("bob").instrument(bob_span.clone()).await?; + let charlie = + create_encryption_enabled_client("charlie").instrument(charlie_span.clone()).await?; + + // 1. Alice creates a room, and enables encryption + let alice_room = alice + .create_room(assign!(CreateRoomRequest::new(), { + preset: Some(RoomPreset::PublicChat), + })) + .instrument(alice_span.clone()) + .await?; + alice_room.enable_encryption().instrument(alice_span.clone()).await?; + // Allow regular users to send invites + alice.sync_once().instrument(alice_span.clone()).await?; + alice_room + .apply_power_level_changes(RoomPowerLevelChanges { invite: Some(0), ..Default::default() }) + .instrument(alice_span.clone()) + .await + .expect("Should be able to set power levels"); + + info!(room_id = ?alice_room.room_id(), "Alice has created and enabled encryption in the room"); + + // ... and invites Bob to the room + alice_room.invite_user_by_id(bob.user_id().unwrap()).instrument(alice_span.clone()).await?; + + // Bob joins + bob.sync_once().instrument(bob_span.clone()).await?; + + let bob_room = bob + .join_room_by_id(alice_room.room_id()) + .instrument(bob_span.clone()) + .await + .expect("Bob should be able to accept the invitation from Alice"); + + // 2. Bob sends a message, which Alice should receive + let assert_event_received = async |room: &Room, event_id: &EventId, expected_content: &str| { + let event = room.event(event_id, None).await.unwrap_or_else(|err| { + panic!("Should receive Bob's event with content '{expected_content}': {err:?}") + }); + assert_decrypted_message_eq!( + event, + expected_content, + "The decrypted event should match the message Bob has sent" + ); + }; + + let bob_send_test_event = async |event_content: &str| { + let bob_event_id = bob_room + .send(RoomMessageEventContent::text_plain(event_content)) + .into_future() + .instrument(bob_span.clone()) + .await + .expect("We should be able to send a message to the room") + .event_id; + + alice + .sync_once() + .instrument(alice_span.clone()) + .await + .expect("Alice should be able to sync"); + + assert_event_received(&alice_room, &bob_event_id, event_content).await; + + bob_event_id + }; + + let event_id_1 = bob_send_test_event("Event 1").await; + + // 3. Alice changes the history visibility to "joined" + alice_room + .send_state_event(RoomHistoryVisibilityEventContent::new(HistoryVisibility::Joined)) + .into_future() + .instrument(alice_span.clone()) + .await?; + bob.sync_once().instrument(bob_span.clone()).await?; + assert_eq!(bob_room.history_visibility(), Some(HistoryVisibility::Joined)); + + // 4. Alice changes the history visibility back to "shared", but Bob doesn't + // know about it. + alice_room + .send_state_event(RoomHistoryVisibilityEventContent::new(HistoryVisibility::Shared)) + .into_future() + .instrument(alice_span.clone()) + .await?; + + // 5. Bob sends a second message; the key is "unshareable" because Bob still + // thinks the history viz is "joined". + assert_eq!(bob_room.history_visibility(), Some(HistoryVisibility::Joined)); + let event_id_2 = bob_send_test_event("Event 2").await; + + // 6. Bob syncs, and sends a third message; the key is now "shareable". + bob.sync_once().instrument(bob_span.clone()).await?; + assert_eq!(bob_room.history_visibility(), Some(HistoryVisibility::Shared)); + let event_id_3 = bob_send_test_event("Event 3").await; + + // 7. Alice invites Charlie. + alice_room.invite_user_by_id(charlie.user_id().unwrap()).instrument(alice_span.clone()).await?; + + // Workaround for https://github.com/matrix-org/matrix-rust-sdk/issues/5770: Charlie needs a copy of + // Alice's identity. + charlie + .encryption() + .request_user_identity(alice.user_id().unwrap()) + .instrument(charlie_span.clone()) + .await?; + + // 8. Charlie joins the room + charlie.sync_once().instrument(charlie_span.clone()).await?; + let charlie_room = charlie + .join_room_by_id(alice_room.room_id()) + .instrument(charlie_span.clone()) + .await + .expect("Charlie should be able to accept the invitation from Alice"); + + // Events 1 and 3 should be decryptable; 2 should be "history not shared". + assert_event_received(&charlie_room, &event_id_1, "Event 1").await; + assert_event_received(&charlie_room, &event_id_3, "Event 3").await; + let event = charlie_room.event(&event_id_2, None).await.expect("Should receive Bob's event 2"); + assert_let!(TimelineEventKind::UnableToDecrypt { utd_info, .. } = event.kind); + assert_eq!( + utd_info.reason, + MissingMegolmSession { withheld_code: Some(WithheldCode::HistoryNotShared) } + ); + + Ok(()) +} + +async fn create_encryption_enabled_client(username: &str) -> Result { + let encryption_settings = + EncryptionSettings { auto_enable_cross_signing: true, ..Default::default() }; + + let client = SyncTokenAwareClient::new( + TestClientBuilder::new(username) + .use_sqlite() + .encryption_settings(encryption_settings) + .enable_share_history_on_invite(true) + .build() + .await?, + ); + + client.encryption().wait_for_e2ee_initialization_tasks().await; + Ok(client) +}