diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index 2f592d73..274ee528 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -1616,6 +1616,15 @@ impl Client { .unwrap(); } } + OutgoingRequests::SignatureUpload(request) => { + // TODO remove this unwrap. + if let Ok(resp) = self.send(request.clone()).await { + self.base_client + .mark_request_as_sent(&r.request_id(), &resp) + .await + .unwrap(); + } + } } } } diff --git a/matrix_sdk/src/sas.rs b/matrix_sdk/src/sas.rs index 4f885f70..3db1df47 100644 --- a/matrix_sdk/src/sas.rs +++ b/matrix_sdk/src/sas.rs @@ -38,13 +38,19 @@ impl Sas { /// Confirm that the short auth strings match on both sides. pub async fn confirm(&self) -> Result<()> { - if let Some(req) = self.inner.confirm().await? { + let (to_device, signature) = self.inner.confirm().await?; + + if let Some(req) = to_device { let txn_id_string = req.txn_id_string(); let request = ToDeviceRequest::new(req.event_type, &txn_id_string, req.messages); self.http_client.send(request).await?; } + if let Some(s) = signature { + self.http_client.send(s).await?; + } + Ok(()) } diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 3eccab14..e727e067 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -351,6 +351,9 @@ impl OlmMachine { IncomingResponse::SigningKeysUpload(_) => { self.receive_cross_signing_upload_response().await?; } + IncomingResponse::SignatureUpload(_) => { + self.verification_machine.mark_request_as_sent(request_id); + } }; Ok(()) @@ -1785,6 +1788,7 @@ pub(crate) mod test { .confirm() .await .unwrap() + .0 .map(|r| request_to_event(bob.user_id(), &r)) .unwrap(); alice.handle_verification_event(&mut event).await; @@ -1796,6 +1800,7 @@ pub(crate) mod test { .confirm() .await .unwrap() + .0 .map(|r| request_to_event(alice.user_id(), &r)) .unwrap(); diff --git a/matrix_sdk_crypto/src/olm/signing/mod.rs b/matrix_sdk_crypto/src/olm/signing/mod.rs index a0cf353b..99bfcae9 100644 --- a/matrix_sdk_crypto/src/olm/signing/mod.rs +++ b/matrix_sdk_crypto/src/olm/signing/mod.rs @@ -14,9 +14,8 @@ mod pk_signing; -use matrix_sdk_common::encryption::DeviceKeys; use serde::{Deserialize, Serialize}; -use serde_json::{Error as JsonError, Value}; +use serde_json::Error as JsonError; use std::{ collections::BTreeMap, sync::{ @@ -27,7 +26,8 @@ use std::{ use matrix_sdk_common::{ api::r0::keys::{upload_signatures::Request as SignatureUploadRequest, KeyUsage}, - identifiers::UserId, + encryption::DeviceKeys, + identifiers::{DeviceKeyAlgorithm, DeviceKeyId, UserId}, locks::Mutex, }; @@ -108,14 +108,17 @@ impl PrivateCrossSigningIdentity { pub(crate) async fn sign_user( &self, user_identity: &UserIdentity, - ) -> Result>, SignatureError> { - self.user_signing_key + ) -> Result { + let signed_keys = self + .user_signing_key .lock() .await .as_ref() .ok_or(SignatureError::MissingSigningKey)? .sign_user(&user_identity) - .await + .await?; + + Ok(SignatureUploadRequest { signed_keys }) } /// Sign the given device keys with this identity. @@ -190,7 +193,11 @@ impl PrivateCrossSigningIdentity { .signatures .entry(account.user_id().to_owned()) .or_insert_with(BTreeMap::new) - .insert(format!("ed25519:{}", account.device_id()), signature); + .insert( + DeviceKeyId::from_parts(DeviceKeyAlgorithm::Ed25519, account.device_id()) + .to_string(), + signature, + ); let master = MasterSigning { inner: master, diff --git a/matrix_sdk_crypto/src/requests.rs b/matrix_sdk_crypto/src/requests.rs index 5b70c744..fb619018 100644 --- a/matrix_sdk_crypto/src/requests.rs +++ b/matrix_sdk_crypto/src/requests.rs @@ -20,6 +20,9 @@ use matrix_sdk_common::{ claim_keys::Response as KeysClaimResponse, get_keys::Response as KeysQueryResponse, upload_keys::{Request as KeysUploadRequest, Response as KeysUploadResponse}, + upload_signatures::{ + Request as SignatureUploadRequest, Response as SignatureUploadResponse, + }, upload_signing_keys::Response as SigningKeysUploadResponse, CrossSigningKey, }, @@ -114,6 +117,9 @@ pub enum OutgoingRequests { /// things, the main use is key requests/forwards and interactive device /// verification. ToDeviceRequest(ToDeviceRequest), + /// Signature upload request, this request is used after a successful device + /// or user verification is done. + SignatureUpload(SignatureUploadRequest), } #[cfg(test)] @@ -144,6 +150,12 @@ impl From for OutgoingRequests { } } +impl From for OutgoingRequests { + fn from(request: SignatureUploadRequest) -> Self { + OutgoingRequests::SignatureUpload(request) + } +} + /// Enum over all the incoming responses we need to receive. #[derive(Debug)] pub enum IncomingResponse<'a> { @@ -161,6 +173,9 @@ pub enum IncomingResponse<'a> { /// The cross signing keys upload response, marking our private cross /// signing identity as shared. SigningKeysUpload(&'a SigningKeysUploadResponse), + /// The cross signing keys upload response, marking our private cross + /// signing identity as shared. + SignatureUpload(&'a SignatureUploadResponse), } impl<'a> From<&'a KeysUploadResponse> for IncomingResponse<'a> { @@ -187,6 +202,12 @@ impl<'a> From<&'a KeysClaimResponse> for IncomingResponse<'a> { } } +impl<'a> From<&'a SignatureUploadResponse> for IncomingResponse<'a> { + fn from(response: &'a SignatureUploadResponse) -> Self { + IncomingResponse::SignatureUpload(response) + } +} + /// Outgoing request type, holds the unique ID of the request and the actual /// request. #[derive(Debug, Clone)] diff --git a/matrix_sdk_crypto/src/verification/machine.rs b/matrix_sdk_crypto/src/verification/machine.rs index d9725520..c3ba5e88 100644 --- a/matrix_sdk_crypto/src/verification/machine.rs +++ b/matrix_sdk_crypto/src/verification/machine.rs @@ -25,7 +25,7 @@ use matrix_sdk_common::{ uuid::Uuid, }; -use super::sas::{content_to_request, Sas}; +use super::sas::{content_to_request, Sas, VerificationResult}; use crate::{ olm::PrivateCrossSigningIdentity, requests::{OutgoingRequest, ToDeviceRequest}, @@ -206,14 +206,28 @@ impl VerificationMachine { self.receive_event_helper(&s, event); if s.is_done() { - if let Some(r) = s.mark_as_done().await? { - self.outgoing_to_device_messages.insert( - r.txn_id, - OutgoingRequest { - request_id: r.txn_id, - request: Arc::new(r.into()), - }, - ); + match s.mark_as_done().await? { + VerificationResult::Ok => (), + VerificationResult::Cancel(r) => { + self.outgoing_to_device_messages.insert( + r.txn_id, + OutgoingRequest { + request_id: r.txn_id, + request: Arc::new(r.into()), + }, + ); + } + VerificationResult::SignatureUpload(r) => { + let request_id = Uuid::new_v4(); + + self.outgoing_to_device_messages.insert( + request_id, + OutgoingRequest { + request_id, + request: Arc::new(r.into()), + }, + ); + } } } }; @@ -352,13 +366,13 @@ mod test { let mut event = wrap_any_to_device_content( alice.user_id(), - get_content_from_request(&alice.confirm().await.unwrap().unwrap()), + get_content_from_request(&alice.confirm().await.unwrap().0.unwrap()), ); bob.receive_event(&mut event); let mut event = wrap_any_to_device_content( bob.user_id(), - get_content_from_request(&bob.confirm().await.unwrap().unwrap()), + get_content_from_request(&bob.confirm().await.unwrap().0.unwrap()), ); alice.receive_event(&mut event); diff --git a/matrix_sdk_crypto/src/verification/mod.rs b/matrix_sdk_crypto/src/verification/mod.rs index 7083c760..d93dc8ec 100644 --- a/matrix_sdk_crypto/src/verification/mod.rs +++ b/matrix_sdk_crypto/src/verification/mod.rs @@ -16,7 +16,7 @@ mod machine; mod sas; pub use machine::VerificationMachine; -pub use sas::Sas; +pub use sas::{Sas, VerificationResult}; #[cfg(test)] pub(crate) mod test { diff --git a/matrix_sdk_crypto/src/verification/sas/mod.rs b/matrix_sdk_crypto/src/verification/sas/mod.rs index b978b780..0d1c3a9c 100644 --- a/matrix_sdk_crypto/src/verification/sas/mod.rs +++ b/matrix_sdk_crypto/src/verification/sas/mod.rs @@ -19,9 +19,10 @@ mod sas_state; use std::time::Instant; use std::sync::{Arc, Mutex}; -use tracing::{info, trace, warn}; +use tracing::{error, info, trace, warn}; use matrix_sdk_common::{ + api::r0::keys::upload_signatures::Request as SignatureUploadRequest, events::{ key::verification::{ accept::AcceptEventContent, cancel::CancelCode, mac::MacEventContent, @@ -33,6 +34,7 @@ use matrix_sdk_common::{ }; use crate::{ + error::SignatureError, identities::{LocalTrust, ReadOnlyDevice, UserIdentities}, olm::PrivateCrossSigningIdentity, store::{Changes, CryptoStore, CryptoStoreError, DeviceChanges}, @@ -44,6 +46,17 @@ use sas_state::{ Accepted, Canceled, Confirmed, Created, Done, KeyReceived, MacReceived, SasState, Started, }; +#[derive(Debug)] +/// A result of a verification flow. +pub enum VerificationResult { + /// The verification succeeded, nothing needs to be done. + Ok, + /// The verification was canceled. + Cancel(ToDeviceRequest), + /// The verification is done and has signatures that need to be uploaded. + SignatureUpload(SignatureUploadRequest), +} + #[derive(Clone, Debug)] /// Short authentication string object. pub struct Sas { @@ -185,7 +198,9 @@ impl Sas { /// Does nothing if we're not in a state where we can confirm the short auth /// string, otherwise returns a `MacEventContent` that needs to be sent to /// the server. - pub async fn confirm(&self) -> Result, CryptoStoreError> { + pub async fn confirm( + &self, + ) -> Result<(Option, Option), CryptoStoreError> { let (content, done) = { let mut guard = self.inner.lock().unwrap(); let sas: InnerSas = (*guard).clone(); @@ -195,27 +210,52 @@ impl Sas { (content, guard.is_done()) }; - let cancel = if done { - // Pass on the signature upload request here as well. - self.mark_as_done().await? - } else { - None - }; + let mac_request = content + .map(|c| self.content_to_request(AnyToDeviceEventContent::KeyVerificationMac(c))); - if cancel.is_some() { - Ok(cancel) + if done { + match self.mark_as_done().await? { + VerificationResult::Cancel(r) => Ok((Some(r), None)), + VerificationResult::Ok => Ok((mac_request, None)), + VerificationResult::SignatureUpload(r) => Ok((mac_request, Some(r))), + } } else { - Ok(content.map(|c| { - let content = AnyToDeviceEventContent::KeyVerificationMac(c); - self.content_to_request(content) - })) + Ok((mac_request, None)) } } - pub(crate) async fn mark_as_done(&self) -> Result, CryptoStoreError> { + pub(crate) async fn mark_as_done(&self) -> Result { if let Some(device) = self.mark_device_as_verified().await? { let identity = self.mark_identity_as_verified().await?; + // We only sign devices of our own user here. + let signature_request = if device.user_id() == self.user_id() { + match self.private_identity.sign_device(&device).await { + Ok(r) => Some(r), + Err(SignatureError::MissingSigningKey) => { + warn!( + "Can't sign the device keys for {} {}, \ + no private user signing key found", + device.user_id(), + device.device_id(), + ); + + None + } + Err(e) => { + error!( + "Error signing device keys for {} {} {:?}", + device.user_id(), + device.device_id(), + e + ); + None + } + } + } else { + None + }; + let mut changes = Changes { devices: DeviceChanges { changed: vec![device], @@ -224,14 +264,68 @@ impl Sas { ..Default::default() }; - if let Some(i) = identity { - changes.identities.changed.push(i); - } + let identity_signature_request = if let Some(i) = identity { + // We only sign other users here. + let request = if let Some(i) = i.other() { + // Signing can fail if the user signing key is missing. + match self.private_identity.sign_user(&i).await { + Ok(r) => Some(r), + Err(SignatureError::MissingSigningKey) => { + warn!( + "Can't sign the public cross signing keys for {}, \ + no private user signing key found", + i.user_id() + ); + None + } + Err(e) => { + error!( + "Error signing the public cross signing keys for {} {:?}", + i.user_id(), + e + ); + None + } + } + } else { + None + }; + changes.identities.changed.push(i); + + request + } else { + None + }; + + // If there are two signature upload requests, merge them. Otherwise + // use the one we have or None. + // + // Realistically at most one reuqest will be used but let's make + // this future proof. + let merged_request = if let Some(mut r) = signature_request { + if let Some(user_request) = identity_signature_request { + r.signed_keys.extend(user_request.signed_keys); + Some(r) + } else { + Some(r) + } + } else if let Some(r) = identity_signature_request { + Some(r) + } else { + None + }; + + // TODO store the request as well. self.store.save_changes(changes).await?; - Ok(None) + Ok(merged_request + .map(|r| VerificationResult::SignatureUpload(r)) + .unwrap_or(VerificationResult::Ok)) } else { - Ok(self.cancel()) + Ok(self + .cancel() + .map(|r| VerificationResult::Cancel(r)) + .unwrap_or(VerificationResult::Ok)) } } @@ -857,13 +951,13 @@ mod test { let mut event = wrap_any_to_device_content( alice.user_id(), - get_content_from_request(&alice.confirm().await.unwrap().unwrap()), + get_content_from_request(&alice.confirm().await.unwrap().0.unwrap()), ); bob.receive_event(&mut event); let mut event = wrap_any_to_device_content( bob.user_id(), - get_content_from_request(&bob.confirm().await.unwrap().unwrap()), + get_content_from_request(&bob.confirm().await.unwrap().0.unwrap()), ); alice.receive_event(&mut event);