crypto: Upload signatures after verification is done.

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();
+                            }
+                        }
                     }
                 }
             }

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(())
     }
 

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();
 

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<BTreeMap<UserId, BTreeMap<String, Value>>, SignatureError> {
+    ) -> Result<SignatureUploadRequest, SignatureError> {
-        self.user_signing_key
+        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,

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<ToDeviceRequest> for OutgoingRequests {
     }
 }
 
+impl From<SignatureUploadRequest> 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)]

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? {
+                        match s.mark_as_done().await? {
-                            self.outgoing_to_device_messages.insert(
+                            VerificationResult::Ok => (),
-                                r.txn_id,
+                            VerificationResult::Cancel(r) => {
-                                OutgoingRequest {
+                                self.outgoing_to_device_messages.insert(
-                                    request_id: r.txn_id,
+                                    r.txn_id,
-                                    request: Arc::new(r.into()),
+                                    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);
 

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 {

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<Option<ToDeviceRequest>, CryptoStoreError> {
+    pub async fn confirm(
+        &self,
+    ) -> Result<(Option<ToDeviceRequest>, Option<SignatureUploadRequest>), 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 {
+        let mac_request = content
-            // Pass on the signature upload request here as well.
+            .map(|c| self.content_to_request(AnyToDeviceEventContent::KeyVerificationMac(c)));
-            self.mark_as_done().await?
-        } else {
-            None
-        };
 
-        if cancel.is_some() {
+        if done {
-            Ok(cancel)
+            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| {
+            Ok((mac_request, None))
-                let content = AnyToDeviceEventContent::KeyVerificationMac(c);
-                self.content_to_request(content)
-            }))
         }
     }
 
-    pub(crate) async fn mark_as_done(&self) -> Result<Option<ToDeviceRequest>, CryptoStoreError> {
+    pub(crate) async fn mark_as_done(&self) -> Result<VerificationResult, CryptoStoreError> {
         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 {
+            let identity_signature_request = if let Some(i) = identity {
-                changes.identities.changed.push(i);
+                // 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);