mirror/matrix-rust-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-rust-sdk.git synced 2026-05-16 03:55:42 -04:00
Code Issues Packages Projects Releases Wiki Activity

Fix handling of SAS start events once we have shown a QR code (#2242)

Browse Source 
Fixes #2237

Also a couple of other cleanups while I'm in the area.
This commit is contained in:
Richard van der Hoff
2023-07-07 17:55:13 +01:00
committed by GitHub
parent fa4c1ef00b
commit 0bf99d5c73
4 changed files with 265 additions and 165 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
bindings/matrix-sdk-crypto-js/src/verification.rs
View File

@@ -1116,6 +1116,8 @@ impl VerificationRequest {
/// Generate a QR code that can be used by another client to start
/// a QR code based verification.
///
/// Returns a `Qr`.
#[cfg(feature = "qrcode")]
#[wasm_bindgen(js_name = "generateQrCode")]
pub fn generate_qr_code(&self) -> Promise {

2
crates/matrix-sdk-crypto/CHANGELOG.md
View File

@@ -36,3 +36,5 @@
- For verification-via-emojis, return the word "Aeroplane" rather than
"Airplane", for consistency with the Matrix spec.
- Fix handling of SAS verification start events once we have shown a QR code.

39
crates/matrix-sdk-crypto/src/verification/qrcode.rs
View File

@@ -95,13 +95,26 @@ pub enum ScanError {
#[derive(Debug, Clone)]
pub enum QrVerificationState {
/// The QR verification has been started.
///
/// We have received the other device's details (from the
/// `m.key.verification.request` or `m.key.verification.ready`) and
/// established the shared secret, so can
/// display the QR code.
///
/// Note that despite the name of this state, we have not yet sent or
/// received an `m.key.verification.start` message.
Started,
/// The QR verification has been scanned by the other side.
Scanned,
/// The scanning of the QR code has been confirmed by us.
/// We have confirmed the other side's scan of the QR code.
Confirmed,
/// We have successfully scanned the QR code and are able to send a
/// reciprocation event.
///
/// Call `QrVerification::reciprocate` to build the reciprocation message.
///
/// Note that, despite the name of this state, we have not necessarily
/// yet sent the `m.reciprocate.v1` message.
Reciprocated,
/// The verification process has been successfully concluded.
Done {
@@ -676,11 +689,35 @@ impl QrVerification {
#[derive(Debug, Clone)]
enum InnerState {
/// We have received the other device's details (from the
/// `m.key.verification.request` or `m.key.verification.ready`) and
/// established the shared secret, so can
/// display the QR code.
Created(QrState<Created>),
/// The other side has scanned our QR code and sent an
/// `m.key.verification.start` message with `method: m.reciprocate.v1` with
/// matching shared secret.
Scanned(QrState<Scanned>),
/// Our user has confirmed that the other device scanned successfully. We
/// have sent an `m.key.verification.done`.
Confirmed(QrState<Confirmed>),
/// We have scanned the other side's QR code and are able to send a
/// `m.key.verification.start` message with `method: m.reciprocate.v1`.
///
/// Call `QrVerification::reciprocate` to build the start message.
///
/// Note that, despite the name of this state, we have not necessarily
/// yet sent the `m.reciprocate.v1` message.
Reciprocated(QrState<Reciprocated>),
/// Verification complete: we have received an `m.key.verification.done`
/// from the other side.
Done(QrState<Done>),
/// Verification cancelled or failed.
Cancelled(QrState<Cancelled>),
}

387
crates/matrix-sdk-crypto/src/verification/requests.rs
View File

@@ -42,7 +42,7 @@ use tracing::debug;
use tracing::{info, trace, warn};
#[cfg(feature = "qrcode")]
use super::qrcode::{QrVerification, ScanError};
use super::qrcode::{QrVerification, QrVerificationState, ScanError};
use super::{
cache::VerificationCache,
event_enums::{
@@ -1379,30 +1379,51 @@ async fn receive_start<T: Clone>(
we_started,
) {
Ok(new) => {
if let Some(Verification::SasV1(_old)) =
request_state.verification_cache.get(sender, request_state.flow_id.as_str())
{
// If there is already a SAS verification, i.e. we already started one
// before the other side tried to do the same; ignore it if we did and
// we're the lexicographically smaller user ID (or device ID if equal).
use std::cmp::Ordering;
if !matches!(
(sender.cmp(own_user_id), device.device_id().cmp(own_device_id)),
(Ordering::Greater, _) | (Ordering::Equal, Ordering::Greater)
) {
info!(
"Started a new SAS verification, replacing an already started one."
);
request_state.verification_cache.replace_sas(new.to_owned());
Ok(Some(state.to_transitioned(request_state, new.into())))
} else {
Ok(None)
let old_verification = request_state
.verification_cache
.get(sender, request_state.flow_id.as_str());
match old_verification {
Some(Verification::SasV1(_old)) => {
// If there is already a SAS verification, i.e. we already started one
// before the other side tried to do the same; ignore it if we did and
// we're the lexicographically smaller user ID (or device ID if equal).
use std::cmp::Ordering;
if !matches!(
(sender.cmp(own_user_id), device.device_id().cmp(own_device_id)),
(Ordering::Greater, _) | (Ordering::Equal, Ordering::Greater)
) {
info!("Started a new SAS verification, replacing an already started one.");
request_state.verification_cache.replace_sas(new.to_owned());
Ok(Some(state.to_transitioned(request_state, new.into())))
} else {
info!("Ignored incoming SAS verification from lexicographically larger user/device ID.");
Ok(None)
}
}
#[cfg(feature = "qrcode")]
Some(Verification::QrV1(old)) => {
// If there is already a QR verification, our ability to transition to
// SAS depends on how far we got through the QR flow.
if let QrVerificationState::Started = old.state() {
// it is legit to transition from QR display to SAS
info!("Transitioned from QR display to SAS");
request_state.verification_cache.replace_sas(new.to_owned());
Ok(Some(state.to_transitioned(request_state, new.into())))
} else {
// otherwise, we've either scanned their QR code, or they have
// scanned ours -- i.e., an `m.key.verification.start` with method
// `m.reciprocate.v1` has already been sent/received and, per the
// spec, it is too late to switch to SAS.
warn!(qr_state = ?old.state(), "Invalid transition from QR to SAS");
request_state.verification_cache.insert_sas(new.to_owned());
Ok(Some(state.to_transitioned(request_state, new.into())))
}
}
None => {
info!("Started a new SAS verification.");
request_state.verification_cache.insert_sas(new.to_owned());
Ok(Some(state.to_transitioned(request_state, new.into())))
}
} else {
info!("Started a new SAS verification.");
request_state.verification_cache.insert_sas(new.to_owned());
Ok(Some(state.to_transitioned(request_state, new.into())))
}
}
Err(c) => {
@@ -1614,9 +1635,10 @@ mod tests {
#[cfg(feature = "qrcode")]
use matrix_sdk_qrcode::QrVerificationData;
use matrix_sdk_test::async_test;
#[cfg(feature = "qrcode")]
use ruma::events::key::verification::VerificationMethod;
use ruma::{event_id, room_id, to_device::DeviceIdOrAllDevices};
use ruma::{
event_id, events::key::verification::VerificationMethod, room_id,
to_device::DeviceIdOrAllDevices, UserId,
};
use super::VerificationRequest;
use crate::{
@@ -1626,7 +1648,7 @@ mod tests {
CancelContent, OutgoingContent, ReadyContent, RequestContent, StartContent,
},
test::{alice_id, bob_id, setup_stores},
FlowId, Verification,
FlowId, Verification, VerificationStore,
},
OutgoingVerificationRequest, ReadOnlyDevice, VerificationRequestState,
};
@@ -1689,29 +1711,9 @@ mod tests {
let (alice_store, bob_store) = setup_stores().await;
let bob_device = ReadOnlyDevice::from_account(&bob_store.account).await;
let flow_id = FlowId::ToDevice("TEST_FLOW_ID".into());
let bob_request = VerificationRequest::new(
VerificationCache::new(),
bob_store,
flow_id,
alice_id(),
vec![],
None,
);
let request = bob_request.request_to_device();
let content: OutgoingContent = request.try_into().unwrap();
let content = RequestContent::try_from(&content).unwrap();
let flow_id = bob_request.flow_id().to_owned();
let alice_request = VerificationRequest::from_request(
VerificationCache::new(),
alice_store,
bob_id(),
flow_id,
&content,
);
// 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);
let outgoing_request = alice_request.cancel().unwrap();
@@ -1772,13 +1774,7 @@ mod tests {
&(&content).into(),
);
let content: OutgoingContent = alice_request.accept().unwrap().try_into().unwrap();
let content = ReadyContent::try_from(&content).unwrap();
bob_request.receive_ready(alice_id(), &content);
assert!(bob_request.is_ready());
assert!(alice_request.is_ready());
do_accept_request(&alice_request, &bob_request, None);
let (bob_sas, request) = bob_request.start_sas().await.unwrap().unwrap();
@@ -1807,37 +1803,10 @@ mod tests {
let (alice_store, bob_store) = setup_stores().await;
let bob_device = ReadOnlyDevice::from_account(&bob_store.account).await;
let flow_id = FlowId::ToDevice("TEST_FLOW_ID".into());
let bob_request = VerificationRequest::new(
VerificationCache::new(),
bob_store,
flow_id,
alice_id(),
vec![],
None,
);
let request = bob_request.request_to_device();
let content: OutgoingContent = request.try_into().unwrap();
let content = RequestContent::try_from(&content).unwrap();
let flow_id = bob_request.flow_id().to_owned();
let alice_request = VerificationRequest::from_request(
VerificationCache::new(),
alice_store,
bob_id(),
flow_id,
&content,
);
let content: OutgoingContent = alice_request.accept().unwrap().try_into().unwrap();
let content = ReadyContent::try_from(&content).unwrap();
bob_request.receive_ready(alice_id(), &content);
assert!(bob_request.is_ready());
assert!(alice_request.is_ready());
// 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);
do_accept_request(&alice_request, &bob_request, None);
let (bob_sas, request) = bob_request.start_sas().await.unwrap().unwrap();
@@ -1868,44 +1837,19 @@ mod tests {
async fn can_scan_another_qr_after_creating_mine() {
let (alice_store, bob_store) = setup_stores().await;
let flow_id = FlowId::ToDevice("TEST_FLOW_ID".into());
// We setup the initial verification request
let bob_request = VerificationRequest::new(
VerificationCache::new(),
bob_store,
flow_id.clone(),
// Set up the pair of verification requests
let bob_request = build_test_request(
&bob_store,
alice_id(),
vec![],
Some(vec![VerificationMethod::QrCodeScanV1, VerificationMethod::QrCodeShowV1]),
);
let request = bob_request.request_to_device();
let content: OutgoingContent = request.try_into().unwrap();
let content = RequestContent::try_from(&content).unwrap();
let alice_request = VerificationRequest::from_request(
VerificationCache::new(),
alice_store,
bob_id(),
flow_id,
&content,
let alice_request = build_incoming_verification_request(&alice_store, &bob_request);
do_accept_request(
&alice_request,
&bob_request,
Some(vec![VerificationMethod::QrCodeScanV1, VerificationMethod::QrCodeShowV1]),
);
let content: OutgoingContent = alice_request
.accept_with_methods(vec![
VerificationMethod::QrCodeScanV1,
VerificationMethod::QrCodeShowV1,
])
.unwrap()
.try_into()
.unwrap();
let content = ReadyContent::try_from(&content).unwrap();
bob_request.receive_ready(alice_id(), &content);
assert!(bob_request.is_ready());
assert!(alice_request.is_ready());
// Each side can start its own QR verification flow by generating QR code
let alice_verification = alice_request.generate_qr_code().await.unwrap();
let bob_verification = bob_request.generate_qr_code().await.unwrap();
@@ -1945,47 +1889,10 @@ mod tests {
async fn can_start_sas_after_generating_qr_code() {
let (alice_store, bob_store) = setup_stores().await;
let flow_id = FlowId::ToDevice("TEST_FLOW_ID".into());
// We setup the initial verification request
let bob_request = VerificationRequest::new(
VerificationCache::new(),
bob_store,
flow_id.clone(),
alice_id(),
vec![],
Some(vec![
VerificationMethod::QrCodeScanV1,
VerificationMethod::QrCodeShowV1,
VerificationMethod::SasV1,
]),
);
let request = bob_request.request_to_device();
let content: OutgoingContent = request.try_into().unwrap();
let content = RequestContent::try_from(&content).unwrap();
let alice_request = VerificationRequest::from_request(
VerificationCache::new(),
alice_store,
bob_id(),
flow_id,
&content,
);
let content: OutgoingContent = alice_request
.accept_with_methods(vec![
VerificationMethod::QrCodeScanV1,
VerificationMethod::QrCodeShowV1,
])
.unwrap()
.try_into()
.unwrap();
let content = ReadyContent::try_from(&content).unwrap();
bob_request.receive_ready(alice_id(), &content);
assert!(bob_request.is_ready());
assert!(alice_request.is_ready());
// 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);
do_accept_request(&alice_request, &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();
@@ -2001,11 +1908,163 @@ mod tests {
// Alice can now start SAS verification flow instead of QR without cancelling
// the request
let (sas, _) = alice_request.start_sas().await.unwrap().unwrap();
let (sas, request) = alice_request.start_sas().await.unwrap().unwrap();
assert_matches!(
alice_request.state(),
VerificationRequestState::Transitioned { verification: Verification::SasV1(_) }
);
assert!(!sas.is_cancelled());
// Bob receives the SAS start
let content: OutgoingContent = request.try_into().unwrap();
let content = StartContent::try_from(&content).unwrap();
bob_request.receive_start(alice_id(), &content).await.unwrap();
// Bob should now have transitioned to SAS...
let bob_sas = assert_matches!(
bob_request.state(),
VerificationRequestState::Transitioned { verification: Verification::SasV1(sas) } => sas
);
// ... and, more to the point, it should not be cancelled.
assert!(!bob_sas.is_cancelled());
}
#[async_test]
#[cfg(feature = "qrcode")]
async fn start_sas_after_scan_cancels_request() {
let (alice_store, bob_store) = setup_stores().await;
// 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);
do_accept_request(&alice_request, &bob_request, Some(all_methods()));
// Bob generates a QR code
let bob_verification = bob_request.generate_qr_code().await.unwrap().unwrap();
assert_matches!(
bob_request.state(),
VerificationRequestState::Transitioned { verification: Verification::QrV1(_) }
);
// Now Alice scans Bob's code
let bob_qr_code = bob_verification.to_bytes().unwrap();
let bob_qr_code = QrVerificationData::from_bytes(bob_qr_code).unwrap();
let _ = alice_request.scan_qr_code(bob_qr_code).await.unwrap().unwrap();
let alice_qr = assert_matches!(
alice_request.state(),
VerificationRequestState::Transitioned {
verification: Verification::QrV1(v)
} => v
);
assert!(alice_qr.reciprocated());
// But Bob wants to do an SAS verification!
let (_, request) = bob_request.start_sas().await.unwrap().unwrap();
assert_matches!(
bob_request.state(),
VerificationRequestState::Transitioned { verification: Verification::SasV1(_) }
);
// Alice receives the SAS start
let content: OutgoingContent = request.try_into().unwrap();
let content = StartContent::try_from(&content).unwrap();
alice_request.receive_start(bob_id(), &content).await.unwrap();
// ... which should mean that her Qr is cancelled
assert!(alice_qr.is_cancelled());
// and she should now have a *cancelled* SAS verification
let alice_sas = assert_matches!(
alice_request.state(),
VerificationRequestState::Transitioned { verification: Verification::SasV1(sas) } => sas
);
assert!(alice_sas.is_cancelled());
}
/// Build an outgoing Verification request
///
/// # Arguments
///
/// * `verification_store` - The `VerificationStore` for the user making the
/// request.
/// * `other_user_id` - The ID of the user we want to verify
/// * `methods` - A list of `VerificationMethods` to say we support. If
/// `None`, will use the default list.
fn build_test_request(
verification_store: &VerificationStore,
other_user_id: &UserId,
methods: Option<Vec<VerificationMethod>>,
) -> VerificationRequest {
VerificationRequest::new(
VerificationCache::new(),
verification_store.clone(),
FlowId::ToDevice("TEST_FLOW_ID".into()),
other_user_id,
vec![],
methods,
)
}
/// Given an outgoing `VerificationRequest`, create an incoming
/// `VerificationRequest` for the other side.
///
/// Tells the outgoing request to generate an `m.key.verification.request`
/// to-device message, and uses it to build a new request for the incoming
/// side.
fn build_incoming_verification_request(
verification_store: &VerificationStore,
outgoing_request: &VerificationRequest,
) -> VerificationRequest {
let request = outgoing_request.request_to_device();
let content: OutgoingContent = request.try_into().unwrap();
let content = RequestContent::try_from(&content).unwrap();
VerificationRequest::from_request(
VerificationCache::new(),
verification_store.clone(),
outgoing_request.own_user_id(),
outgoing_request.flow_id().clone(),
&content,
)
}
/// Have a `VerificationRequest` generate an `m.key.verification.ready` and
/// feed it into another `VerificationRequest`.
///
/// # Arguments
///
/// * `accepting_request` - The request which should send the acceptance.
/// * `initiating_request` - The request which initiated the flow -- i.e.,
/// the one that should *receive* the acceptance.
/// * `methods` - The list of methods to say we support. If `None`, the
/// default list of methods will be used.
fn do_accept_request(
accepting_request: &VerificationRequest,
initiating_request: &VerificationRequest,
methods: Option<Vec<VerificationMethod>>,
) {
let request = match methods {
Some(methods) => accepting_request.accept_with_methods(methods),
None => accepting_request.accept(),
};
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);
assert!(initiating_request.is_ready());
assert!(accepting_request.is_ready());
}
/// Get a list of all the verification methods, including those used for QR
/// codes.
#[cfg(feature = "qrcode")]
fn all_methods() -> Vec<VerificationMethod> {
vec![
VerificationMethod::SasV1,
VerificationMethod::QrCodeScanV1,
VerificationMethod::QrCodeShowV1,
VerificationMethod::ReciprocateV1,
]
}
}