diff --git a/bindings/matrix-sdk-crypto-ffi/src/verification.rs b/bindings/matrix-sdk-crypto-ffi/src/verification.rs index e8c31c4a3a8..c522a1ffa16 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/verification.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/verification.rs @@ -752,7 +752,7 @@ impl VerificationRequest { RustVerificationRequestState::Ready { their_methods, our_methods, - other_device_id: _, + other_device_data: _, } => VerificationRequestState::Ready { their_methods: their_methods.iter().map(|m| m.to_string()).collect(), our_methods: our_methods.iter().map(|m| m.to_string()).collect(), diff --git a/crates/matrix-sdk-crypto/src/verification/machine.rs b/crates/matrix-sdk-crypto/src/verification/machine.rs index 864800f1a97..892fa255183 100644 --- a/crates/matrix-sdk-crypto/src/verification/machine.rs +++ b/crates/matrix-sdk-crypto/src/verification/machine.rs @@ -412,7 +412,13 @@ impl VerificationMachine { }; if request.flow_id() == &flow_id { - request.receive_ready(event.sender(), c); + if let Some(device_data) = + self.store.get_device(event.sender(), c.from_device()).await? + { + request.receive_ready(event.sender(), c, device_data); + } else { + warn!("Could not retrieve the data for the accepting device, ignoring it"); + } } else { flow_id_mismatch(); } diff --git a/crates/matrix-sdk-crypto/src/verification/requests.rs b/crates/matrix-sdk-crypto/src/verification/requests.rs index 9c1c4e202be..7b8d7a27338 100644 --- a/crates/matrix-sdk-crypto/src/verification/requests.rs +++ b/crates/matrix-sdk-crypto/src/verification/requests.rs @@ -90,9 +90,9 @@ pub enum VerificationRequestState { /// The verification methods supported by the us. our_methods: Vec, - /// The device ID of the device that responded to the verification + /// The device data of the device that responded to the verification /// request. - other_device_id: OwnedDeviceId, + other_device_data: DeviceData, }, /// The verification request has transitioned into a concrete verification /// flow. For example it transitioned into the emoji based SAS @@ -121,7 +121,7 @@ impl From<&InnerRequest> for VerificationRequestState { InnerRequest::Ready(s) => Self::Ready { their_methods: s.state.their_methods.to_owned(), our_methods: s.state.our_methods.to_owned(), - other_device_id: s.state.other_device_id.to_owned(), + other_device_data: s.state.other_device_data.to_owned(), }, InnerRequest::Transitioned(s) => { Self::Transitioned { verification: s.state.verification.to_owned() } @@ -282,8 +282,10 @@ impl VerificationRequest { pub fn other_device_id(&self) -> Option { match &*self.inner.read() { InnerRequest::Requested(r) => Some(r.state.other_device_data.device_id().to_owned()), - InnerRequest::Ready(r) => Some(r.state.other_device_id.to_owned()), - InnerRequest::Transitioned(r) => Some(r.state.ready.other_device_id.to_owned()), + InnerRequest::Ready(r) => Some(r.state.other_device_data.device_id().to_owned()), + InnerRequest::Transitioned(r) => { + Some(r.state.ready.other_device_data.device_id().to_owned()) + } InnerRequest::Created(_) | InnerRequest::Passive(_) | InnerRequest::Done(_) @@ -666,12 +668,18 @@ impl VerificationRequest { Some(ToDeviceRequest::for_recipients(recipient, recip_devices, &c, TransactionId::new())) } - pub(crate) fn receive_ready(&self, sender: &UserId, content: &ReadyContent<'_>) { + pub(crate) fn receive_ready( + &self, + sender: &UserId, + content: &ReadyContent<'_>, + from_device_data: DeviceData, + ) { let mut guard = self.inner.write(); match &*guard { InnerRequest::Created(s) => { - let new_value = InnerRequest::Ready(s.clone().into_ready(sender, content)); + let new_value = + InnerRequest::Ready(s.clone().into_ready(sender, content, from_device_data)); ObservableWriteGuard::set(&mut guard, new_value); if let Some(request) = @@ -900,11 +908,11 @@ impl InnerRequest { DeviceIdOrAllDevices::DeviceId(r.state.other_device_data.device_id().to_owned()) } InnerRequest::Ready(r) => { - DeviceIdOrAllDevices::DeviceId(r.state.other_device_id.to_owned()) - } - InnerRequest::Transitioned(r) => { - DeviceIdOrAllDevices::DeviceId(r.state.ready.other_device_id.to_owned()) + DeviceIdOrAllDevices::DeviceId(r.state.other_device_data.device_id().to_owned()) } + InnerRequest::Transitioned(r) => DeviceIdOrAllDevices::DeviceId( + r.state.ready.other_device_data.device_id().to_owned(), + ), InnerRequest::Passive(_) => DeviceIdOrAllDevices::AllDevices, InnerRequest::Done(_) => DeviceIdOrAllDevices::AllDevices, InnerRequest::Cancelled(_) => DeviceIdOrAllDevices::AllDevices, @@ -1042,7 +1050,12 @@ impl RequestState { } } - fn into_ready(self, _sender: &UserId, content: &ReadyContent<'_>) -> RequestState { + fn into_ready( + self, + _sender: &UserId, + content: &ReadyContent<'_>, + from_device_data: DeviceData, + ) -> RequestState { // TODO check the flow id, and that the methods match what we suggested. RequestState { flow_id: self.flow_id, @@ -1052,7 +1065,7 @@ impl RequestState { state: Ready { their_methods: content.methods().to_owned(), our_methods: self.state.our_methods, - other_device_id: content.from_device().into(), + other_device_data: from_device_data, }, } } @@ -1115,7 +1128,7 @@ impl RequestState { state: Ready { their_methods: self.state.their_methods, our_methods: methods.clone(), - other_device_id: self.state.other_device_data.device_id().to_owned(), + other_device_data: self.state.other_device_data, }, }; @@ -1153,8 +1166,9 @@ struct Ready { /// The verification methods supported by the us. pub our_methods: Vec, - /// The device ID of the device that responded to the verification request. - pub other_device_id: OwnedDeviceId, + /// The device data of the device that responded to the verification + /// request. + pub other_device_data: DeviceData, } #[cfg(feature = "qrcode")] @@ -1168,7 +1182,7 @@ async fn scan_qr_code( let verification = QrVerification::from_scan( request_state.store.to_owned(), request_state.other_user_id.to_owned(), - state.other_device_id.to_owned(), + state.other_device_data.device_id().to_owned(), request_state.flow_id.as_ref().to_owned(), data, we_started, @@ -1207,21 +1221,7 @@ async fn generate_qr_code( return Ok(None); } - let Some(device) = request_state - .store - .get_device(&request_state.other_user_id, &state.other_device_id) - .await? - else { - warn!( - user_id = ?request_state.other_user_id, - device_id = ?state.other_device_id, - "Can't create a QR code, the device that accepted the \ - verification doesn't exist" - ); - return Ok(None); - }; - - let identities = request_state.store.get_identities(device).await?; + let identities = request_state.store.get_identities(state.other_device_data.clone()).await?; let verification = if let Some(identity) = &identities.identity_being_verified { match &identity { @@ -1240,7 +1240,7 @@ async fn generate_qr_code( } else { warn!( user_id = ?request_state.other_user_id, - device_id = ?state.other_device_id, + device_id = ?state.other_device_data.device_id(), "Can't create a QR code, the other device \ doesn't have a valid device key" ); @@ -1259,7 +1259,7 @@ async fn generate_qr_code( } else { warn!( user_id = ?request_state.other_user_id, - device_id = ?state.other_device_id, + device_id = ?state.other_device_data.device_id(), "Can't create a QR code, our cross signing identity \ doesn't contain a valid master key" ); @@ -1288,7 +1288,7 @@ async fn generate_qr_code( } else { warn!( user_id = ?request_state.other_user_id, - device_id = ?state.other_device_id, + device_id = ?state.other_device_data.device_id(), "Can't create a QR code, we don't trust our own \ master key" ); @@ -1297,7 +1297,7 @@ async fn generate_qr_code( } else { warn!( user_id = ?request_state.other_user_id, - device_id = ?state.other_device_id, + device_id = ?state.other_device_data.device_id(), "Can't create a QR code, the user's identity \ doesn't have a valid master key" ); @@ -1308,7 +1308,7 @@ async fn generate_qr_code( } else { warn!( user_id = ?request_state.other_user_id, - device_id = ?state.other_device_id, + device_id = ?state.other_device_data.device_id(), "Can't create a QR code, the user doesn't have a valid cross \ signing identity." ); @@ -1478,22 +1478,7 @@ async fn start_sas( return Ok(None); } - // TODO signal why starting the sas flow doesn't work? - let Some(device) = request_state - .store - .get_device(&request_state.other_user_id, &state.other_device_id) - .await? - else { - warn!( - user_id = ?request_state.other_user_id, - device_id = ?state.other_device_id, - "Can't start the SAS verification flow, the device that \ - accepted the verification doesn't exist" - ); - return Ok(None); - }; - - let identities = request_state.store.get_identities(device).await?; + let identities = request_state.store.get_identities(state.other_device_data.clone()).await?; let (state, sas, content) = match request_state.flow_id.as_ref() { FlowId::ToDevice(t) => { @@ -1653,7 +1638,10 @@ mod tests { let event_id = event_id!("$1234localhost").to_owned(); let room_id = room_id!("!test:localhost").to_owned(); - let (_alice, alice_store, _bob, bob_store) = setup_stores().await; + let (alice, alice_store, bob, bob_store) = setup_stores().await; + + let alice_device_data = DeviceData::from_account(&alice); + let bob_device_data = DeviceData::from_account(&bob); let content = VerificationRequest::request( &bob_store.account.user_id, @@ -1662,12 +1650,6 @@ mod tests { None, ); - let device_data = alice_store - .get_device(&bob_store.account.user_id, &bob_store.account.device_id) - .await - .unwrap() - .expect("Missing device data"); - let flow_id = FlowId::InRoom(room_id, event_id); let bob_request = VerificationRequest::new( @@ -1690,7 +1672,7 @@ mod tests { bob_id(), flow_id, &(&content).into(), - device_data, + bob_device_data, ); assert_matches!(alice_request.state(), VerificationRequestState::Requested { .. }); @@ -1698,7 +1680,7 @@ mod tests { let content: OutgoingContent = alice_request.accept().unwrap().try_into().unwrap(); let content = ReadyContent::try_from(&content).unwrap(); - bob_request.receive_ready(alice_id(), &content); + bob_request.receive_ready(alice_id(), &content, alice_device_data); assert_matches!(bob_request.state(), VerificationRequestState::Ready { .. }); assert_matches!(alice_request.state(), VerificationRequestState::Ready { .. }); @@ -1748,8 +1730,10 @@ mod tests { let event_id = event_id!("$1234localhost"); let room_id = room_id!("!test:localhost"); - let (_alice, alice_store, bob, bob_store) = setup_stores().await; - let bob_device = DeviceData::from_account(&bob); + let (alice, alice_store, bob, bob_store) = setup_stores().await; + + let alice_device_data = DeviceData::from_account(&alice); + let bob_device_data = DeviceData::from_account(&bob); let content = VerificationRequest::request( &bob_store.account.user_id, @@ -1758,12 +1742,6 @@ mod tests { None, ); - let device_data = alice_store - .get_device(&bob_store.account.user_id, &bob_store.account.device_id) - .await - .unwrap() - .expect("Missing device data"); - let flow_id = FlowId::from((room_id, event_id)); let bob_request = VerificationRequest::new( @@ -1782,19 +1760,19 @@ mod tests { bob_id(), flow_id, &(&content).into(), - device_data, + bob_device_data.clone(), ); - do_accept_request(&alice_request, &bob_request, None); + do_accept_request(&alice_request, alice_device_data, &bob_request, None); let (bob_sas, request) = bob_request.start_sas().await.unwrap().unwrap(); let content: OutgoingContent = request.try_into().unwrap(); let content = StartContent::try_from(&content).unwrap(); let flow_id = content.flow_id().to_owned(); - alice_request.receive_start(bob_device.user_id(), &content).await.unwrap(); + alice_request.receive_start(bob_device_data.user_id(), &content).await.unwrap(); let alice_sas = - alice_request.verification_cache.get_sas(bob_device.user_id(), &flow_id).unwrap(); + alice_request.verification_cache.get_sas(bob_device_data.user_id(), &flow_id).unwrap(); assert_matches!( alice_request.state(), @@ -1811,22 +1789,24 @@ mod tests { #[async_test] async fn test_requesting_until_sas_to_device() { - let (_alice, alice_store, bob, bob_store) = setup_stores().await; - let bob_device = DeviceData::from_account(&bob); + let (alice, alice_store, bob, bob_store) = setup_stores().await; + + let alice_device_data = DeviceData::from_account(&alice); + let bob_device_data = DeviceData::from_account(&bob); // Set up the pair of verification requests let bob_request = build_test_request(&bob_store, alice_id(), None); let alice_request = build_incoming_verification_request(&alice_store, &bob_request).await; - do_accept_request(&alice_request, &bob_request, None); + do_accept_request(&alice_request, alice_device_data, &bob_request, None); let (bob_sas, request) = bob_request.start_sas().await.unwrap().unwrap(); let content: OutgoingContent = request.try_into().unwrap(); let content = StartContent::try_from(&content).unwrap(); let flow_id = content.flow_id().to_owned(); - alice_request.receive_start(bob_device.user_id(), &content).await.unwrap(); + alice_request.receive_start(bob_device_data.user_id(), &content).await.unwrap(); let alice_sas = - alice_request.verification_cache.get_sas(bob_device.user_id(), &flow_id).unwrap(); + alice_request.verification_cache.get_sas(bob_device_data.user_id(), &flow_id).unwrap(); assert_matches!( alice_request.state(), @@ -1846,7 +1826,9 @@ mod tests { #[async_test] #[cfg(feature = "qrcode")] async fn test_can_scan_another_qr_after_creating_mine() { - let (_alice, alice_store, _bob, bob_store) = setup_stores().await; + let (alice, alice_store, _bob, bob_store) = setup_stores().await; + + let alice_device_data = DeviceData::from_account(&alice); // Set up the pair of verification requests let bob_request = build_test_request( @@ -1857,6 +1839,7 @@ mod tests { let alice_request = build_incoming_verification_request(&alice_store, &bob_request).await; do_accept_request( &alice_request, + alice_device_data, &bob_request, Some(vec![VerificationMethod::QrCodeScanV1, VerificationMethod::QrCodeShowV1]), ); @@ -1897,12 +1880,14 @@ mod tests { #[async_test] #[cfg(feature = "qrcode")] async fn test_can_start_sas_after_generating_qr_code() { - let (_alice, alice_store, _bob, bob_store) = setup_stores().await; + let (alice, alice_store, _bob, bob_store) = setup_stores().await; + + let alice_device_data = DeviceData::from_account(&alice); // Set up the pair of verification requests let bob_request = build_test_request(&bob_store, alice_id(), Some(all_methods())); let alice_request = build_incoming_verification_request(&alice_store, &bob_request).await; - do_accept_request(&alice_request, &bob_request, Some(all_methods())); + do_accept_request(&alice_request, alice_device_data, &bob_request, Some(all_methods())); // Each side can start its own QR verification flow by generating QR code let alice_verification = alice_request.generate_qr_code().await.unwrap(); @@ -1942,12 +1927,14 @@ mod tests { #[async_test] #[cfg(feature = "qrcode")] async fn test_start_sas_after_scan_cancels_request() { - let (_alice, alice_store, _bob, bob_store) = setup_stores().await; + let (alice, alice_store, _bob, bob_store) = setup_stores().await; + + let alice_device_data = DeviceData::from_account(&alice); // Set up the pair of verification requests let bob_request = build_test_request(&bob_store, alice_id(), Some(all_methods())); let alice_request = build_incoming_verification_request(&alice_store, &bob_request).await; - do_accept_request(&alice_request, &bob_request, Some(all_methods())); + do_accept_request(&alice_request, alice_device_data, &bob_request, Some(all_methods())); // Bob generates a QR code let bob_verification = bob_request.generate_qr_code().await.unwrap().unwrap(); @@ -2057,6 +2044,7 @@ mod tests { /// default list of methods will be used. fn do_accept_request( accepting_request: &VerificationRequest, + accepting_device_data: DeviceData, initiating_request: &VerificationRequest, methods: Option>, ) { @@ -2066,7 +2054,11 @@ mod tests { }; let content: OutgoingContent = request.unwrap().try_into().unwrap(); let content = ReadyContent::try_from(&content).unwrap(); - initiating_request.receive_ready(accepting_request.own_user_id(), &content); + initiating_request.receive_ready( + accepting_request.own_user_id(), + &content, + accepting_device_data, + ); assert!(initiating_request.is_ready()); assert!(accepting_request.is_ready()); diff --git a/crates/matrix-sdk/src/encryption/verification/requests.rs b/crates/matrix-sdk/src/encryption/verification/requests.rs index 0e6fb35f510..3892826d2ee 100644 --- a/crates/matrix-sdk/src/encryption/verification/requests.rs +++ b/crates/matrix-sdk/src/encryption/verification/requests.rs @@ -16,7 +16,7 @@ use futures_util::{Stream, StreamExt}; use matrix_sdk_base::crypto::{ CancelInfo, DeviceData, VerificationRequest as BaseVerificationRequest, }; -use ruma::{events::key::verification::VerificationMethod, OwnedDeviceId, RoomId}; +use ruma::{events::key::verification::VerificationMethod, RoomId}; #[cfg(feature = "qrcode")] use super::{QrVerification, QrVerificationData}; @@ -55,9 +55,9 @@ pub enum VerificationRequestState { /// The verification methods supported by the us. our_methods: Vec, - /// The device ID of the device that responded to the verification + /// The device data of the device that responded to the verification /// request. - other_device_id: OwnedDeviceId, + other_device_data: DeviceData, }, /// The verification request has transitioned into a concrete verification /// flow. For example it transitioned into the emoji based SAS @@ -223,8 +223,8 @@ impl VerificationRequest { Requested { their_methods, other_device_data } => { VerificationRequestState::Requested { their_methods, other_device_data } } - Ready { their_methods, our_methods, other_device_id } => { - VerificationRequestState::Ready { their_methods, our_methods, other_device_id } + Ready { their_methods, our_methods, other_device_data } => { + VerificationRequestState::Ready { their_methods, our_methods, other_device_data } } Transitioned { verification } => VerificationRequestState::Transitioned { verification: match verification {