crypto: Propagate the we_started info to the SAS verification

master
Damir Jelić 2021-06-25 12:51:45 +02:00
parent 55690ddd54
commit d4e80883dc
3 changed files with 54 additions and 24 deletions

View File

@ -72,6 +72,7 @@ impl VerificationMachine {
self.store.clone(), self.store.clone(),
identity, identity,
None, None,
true,
); );
let request = match content { let request = match content {
@ -325,6 +326,7 @@ impl VerificationMachine {
device, device,
identity, identity,
false, false,
false,
) { ) {
Ok(sas) => { Ok(sas) => {
self.verifications.insert_sas(sas); self.verifications.insert_sas(sas);
@ -461,6 +463,7 @@ mod test {
bob_store, bob_store,
None, None,
None, None,
true,
); );
machine machine

View File

@ -414,7 +414,7 @@ impl VerificationRequest {
let inner = self.inner.lock().unwrap().clone(); let inner = self.inner.lock().unwrap().clone();
if let InnerRequest::Ready(s) = inner { if let InnerRequest::Ready(s) = inner {
s.receive_start(sender, content).await?; s.receive_start(sender, content, self.we_started).await?;
} else { } else {
warn!( warn!(
sender = sender.as_str(), sender = sender.as_str(),
@ -454,6 +454,7 @@ impl VerificationRequest {
s.store.clone(), s.store.clone(),
s.account.clone(), s.account.clone(),
s.private_cross_signing_identity.clone(), s.private_cross_signing_identity.clone(),
self.we_started,
) )
.await? .await?
{ {
@ -565,9 +566,10 @@ impl InnerRequest {
content: &StartContent, content: &StartContent,
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
other_identity: Option<UserIdentities>, other_identity: Option<UserIdentities>,
we_started: bool,
) -> Result<Option<Sas>, OutgoingContent> { ) -> Result<Option<Sas>, OutgoingContent> {
if let InnerRequest::Ready(s) = self { if let InnerRequest::Ready(s) = self {
Ok(Some(s.to_started_sas(content, other_device, other_identity)?)) Ok(Some(s.to_started_sas(content, other_device, other_identity, we_started)?))
} else { } else {
Ok(None) Ok(None)
} }
@ -762,6 +764,7 @@ impl RequestState<Ready> {
content: &StartContent<'a>, content: &StartContent<'a>,
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
other_identity: Option<UserIdentities>, other_identity: Option<UserIdentities>,
we_started: bool,
) -> Result<Sas, OutgoingContent> { ) -> Result<Sas, OutgoingContent> {
Sas::from_start_event( Sas::from_start_event(
(&*self.flow_id).to_owned(), (&*self.flow_id).to_owned(),
@ -772,6 +775,7 @@ impl RequestState<Ready> {
other_device, other_device,
other_identity, other_identity,
true, true,
we_started,
) )
} }
@ -868,6 +872,7 @@ impl RequestState<Ready> {
&self, &self,
sender: &UserId, sender: &UserId,
content: &StartContent<'_>, content: &StartContent<'_>,
we_started: bool,
) -> Result<(), CryptoStoreError> { ) -> Result<(), CryptoStoreError> {
info!( info!(
sender = sender.as_str(), sender = sender.as_str(),
@ -890,29 +895,31 @@ impl RequestState<Ready> {
let identity = self.store.get_user_identity(sender).await?; let identity = self.store.get_user_identity(sender).await?;
match content.method() { match content.method() {
StartMethod::SasV1(_) => match self.to_started_sas(content, device.clone(), identity) { StartMethod::SasV1(_) => {
// TODO check if there is already a SAS verification, i.e. we match self.to_started_sas(content, device.clone(), identity, we_started) {
// already started one before the other side tried to do the // TODO check if there is already a SAS verification, i.e. we
// same; ignore it if we did and we're the lexicographically // already started one before the other side tried to do the
// smaller user ID, otherwise auto-accept the newly started one. // same; ignore it if we did and we're the lexicographically
Ok(s) => { // smaller user ID, otherwise auto-accept the newly started one.
info!("Started a new SAS verification."); Ok(s) => {
self.verification_cache.insert_sas(s); info!("Started a new SAS verification.");
self.verification_cache.insert_sas(s);
}
Err(c) => {
warn!(
user_id = device.user_id().as_str(),
device_id = device.device_id().as_str(),
content =? c,
"Can't start key verification, canceling.",
);
self.verification_cache.queue_up_content(
device.user_id(),
device.device_id(),
c,
)
}
} }
Err(c) => { }
warn!(
user_id = device.user_id().as_str(),
device_id = device.device_id().as_str(),
content =? c,
"Can't start key verification, canceling.",
);
self.verification_cache.queue_up_content(
device.user_id(),
device.device_id(),
c,
)
}
},
StartMethod::ReciprocateV1(_) => { StartMethod::ReciprocateV1(_) => {
if let Some(qr_verification) = if let Some(qr_verification) =
self.verification_cache.get_qr(sender, content.flow_id()) self.verification_cache.get_qr(sender, content.flow_id())
@ -941,6 +948,7 @@ impl RequestState<Ready> {
store: Arc<dyn CryptoStore>, store: Arc<dyn CryptoStore>,
account: ReadOnlyAccount, account: ReadOnlyAccount,
private_identity: PrivateCrossSigningIdentity, private_identity: PrivateCrossSigningIdentity,
we_started: bool,
) -> Result<Option<(Sas, OutgoingContent)>, CryptoStoreError> { ) -> Result<Option<(Sas, OutgoingContent)>, CryptoStoreError> {
if !self.state.their_methods.contains(&VerificationMethod::SasV1) { if !self.state.their_methods.contains(&VerificationMethod::SasV1) {
return Ok(None); return Ok(None);
@ -972,6 +980,7 @@ impl RequestState<Ready> {
store, store,
other_identity, other_identity,
Some(t.to_owned()), Some(t.to_owned()),
we_started,
); );
(sas, content) (sas, content)
} }
@ -984,6 +993,7 @@ impl RequestState<Ready> {
device, device,
store, store,
other_identity, other_identity,
we_started,
); );
(sas, content) (sas, content)
} }

View File

@ -55,6 +55,7 @@ pub struct Sas {
account: ReadOnlyAccount, account: ReadOnlyAccount,
identities_being_verified: IdentitiesBeingVerified, identities_being_verified: IdentitiesBeingVerified,
flow_id: Arc<FlowId>, flow_id: Arc<FlowId>,
we_started: bool,
} }
impl Sas { impl Sas {
@ -114,6 +115,11 @@ impl Sas {
self.inner.lock().unwrap().cancel_code() self.inner.lock().unwrap().cancel_code()
} }
/// Did we initiate the verification flow.
pub fn we_started(&self) -> bool {
self.we_started
}
#[cfg(test)] #[cfg(test)]
#[allow(dead_code)] #[allow(dead_code)]
pub(crate) fn set_creation_time(&self, time: Instant) { pub(crate) fn set_creation_time(&self, time: Instant) {
@ -127,6 +133,7 @@ impl Sas {
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
store: Arc<dyn CryptoStore>, store: Arc<dyn CryptoStore>,
other_identity: Option<UserIdentities>, other_identity: Option<UserIdentities>,
we_started: bool,
) -> Sas { ) -> Sas {
let flow_id = inner_sas.verification_flow_id(); let flow_id = inner_sas.verification_flow_id();
@ -142,6 +149,7 @@ impl Sas {
account, account,
identities_being_verified: identities, identities_being_verified: identities,
flow_id, flow_id,
we_started,
} }
} }
@ -162,6 +170,7 @@ impl Sas {
store: Arc<dyn CryptoStore>, store: Arc<dyn CryptoStore>,
other_identity: Option<UserIdentities>, other_identity: Option<UserIdentities>,
transaction_id: Option<String>, transaction_id: Option<String>,
we_started: bool,
) -> (Sas, OutgoingContent) { ) -> (Sas, OutgoingContent) {
let (inner, content) = InnerSas::start( let (inner, content) = InnerSas::start(
account.clone(), account.clone(),
@ -178,6 +187,7 @@ impl Sas {
other_device, other_device,
store, store,
other_identity, other_identity,
we_started,
), ),
content, content,
) )
@ -193,6 +203,7 @@ impl Sas {
/// ///
/// Returns the new `Sas` object and a `StartEventContent` that needs to be /// Returns the new `Sas` object and a `StartEventContent` that needs to be
/// sent out through the server to the other device. /// sent out through the server to the other device.
#[allow(clippy::too_many_arguments)]
pub(crate) fn start_in_room( pub(crate) fn start_in_room(
flow_id: EventId, flow_id: EventId,
room_id: RoomId, room_id: RoomId,
@ -201,6 +212,7 @@ impl Sas {
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
store: Arc<dyn CryptoStore>, store: Arc<dyn CryptoStore>,
other_identity: Option<UserIdentities>, other_identity: Option<UserIdentities>,
we_started: bool,
) -> (Sas, OutgoingContent) { ) -> (Sas, OutgoingContent) {
let (inner, content) = InnerSas::start_in_room( let (inner, content) = InnerSas::start_in_room(
flow_id, flow_id,
@ -218,6 +230,7 @@ impl Sas {
other_device, other_device,
store, store,
other_identity, other_identity,
we_started,
), ),
content, content,
) )
@ -243,6 +256,7 @@ impl Sas {
other_device: ReadOnlyDevice, other_device: ReadOnlyDevice,
other_identity: Option<UserIdentities>, other_identity: Option<UserIdentities>,
started_from_request: bool, started_from_request: bool,
we_started: bool,
) -> Result<Sas, OutgoingContent> { ) -> Result<Sas, OutgoingContent> {
let inner = InnerSas::from_start_event( let inner = InnerSas::from_start_event(
account.clone(), account.clone(),
@ -260,6 +274,7 @@ impl Sas {
other_device, other_device,
store, store,
other_identity, other_identity,
we_started,
)) ))
} }
@ -568,6 +583,7 @@ mod test {
alice_store, alice_store,
None, None,
None, None,
true,
); );
let flow_id = alice.flow_id().to_owned(); let flow_id = alice.flow_id().to_owned();
@ -582,6 +598,7 @@ mod test {
alice_device, alice_device,
None, None,
false, false,
false,
) )
.unwrap(); .unwrap();