diff --git a/bindings/matrix-sdk-crypto-ffi/src/responses.rs b/bindings/matrix-sdk-crypto-ffi/src/responses.rs index 63e4cbb57..57015daa0 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/responses.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/responses.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use http::Response; use matrix_sdk_crypto::{ - IncomingResponse, KeysBackupRequest, OutgoingRequest, + CrossSigningBootstrapRequests, IncomingResponse, KeysBackupRequest, OutgoingRequest, OutgoingVerificationRequest as SdkVerificationRequest, RoomMessageRequest, ToDeviceRequest, UploadSigningKeysRequest as RustUploadSigningKeysRequest, }; @@ -71,17 +71,23 @@ impl From for UploadSigningKeysRequest { #[derive(uniffi::Record)] pub struct BootstrapCrossSigningResult { + /// Optional request to upload some device keys alongside. + /// + /// Must be sent first if present, and marked with `mark_request_as_sent`. + pub upload_keys_request: Option, + /// Request to upload the signing keys. Must be sent second. pub upload_signing_keys_request: UploadSigningKeysRequest, + /// Request to upload the keys signatures, including for the signing keys + /// and optionally for the device keys. Must be sent last. pub upload_signature_request: SignatureUploadRequest, } -impl From<(RustUploadSigningKeysRequest, RustSignatureUploadRequest)> - for BootstrapCrossSigningResult -{ - fn from(requests: (RustUploadSigningKeysRequest, RustSignatureUploadRequest)) -> Self { +impl From for BootstrapCrossSigningResult { + fn from(requests: CrossSigningBootstrapRequests) -> Self { Self { - upload_signing_keys_request: requests.0.into(), - upload_signature_request: requests.1.into(), + upload_signing_keys_request: requests.upload_signing_keys_req.into(), + upload_keys_request: requests.upload_keys_req.map(Request::from), + upload_signature_request: requests.upload_signatures_req.into(), } } } diff --git a/crates/matrix-sdk-crypto/CHANGELOG.md b/crates/matrix-sdk-crypto/CHANGELOG.md index e035768dc..364213f4d 100644 --- a/crates/matrix-sdk-crypto/CHANGELOG.md +++ b/crates/matrix-sdk-crypto/CHANGELOG.md @@ -73,3 +73,6 @@ - Stop logging large quantities of data about the `Store` during olm decryption. + +- Change the return value of `bootstrap_cross_signing` so it returns an extra keys upload request. + The three requests must be sent in the order they appear in the return tuple. diff --git a/crates/matrix-sdk-crypto/src/lib.rs b/crates/matrix-sdk-crypto/src/lib.rs index 39a570b4a..79d366df9 100644 --- a/crates/matrix-sdk-crypto/src/lib.rs +++ b/crates/matrix-sdk-crypto/src/lib.rs @@ -81,7 +81,7 @@ pub use identities::{ Device, LocalTrust, OwnUserIdentity, ReadOnlyDevice, ReadOnlyOwnUserIdentity, ReadOnlyUserIdentities, ReadOnlyUserIdentity, UserDevices, UserIdentities, UserIdentity, }; -pub use machine::{EncryptionSyncChanges, OlmMachine}; +pub use machine::{CrossSigningBootstrapRequests, EncryptionSyncChanges, OlmMachine}; #[cfg(feature = "qrcode")] pub use matrix_sdk_qrcode; pub use olm::{Account, CrossSigningStatus, EncryptionSettings, Session}; diff --git a/crates/matrix-sdk-crypto/src/machine.rs b/crates/matrix-sdk-crypto/src/machine.rs index 11aa6ed97..f37e56eae 100644 --- a/crates/matrix-sdk-crypto/src/machine.rs +++ b/crates/matrix-sdk-crypto/src/machine.rs @@ -29,7 +29,7 @@ use ruma::{ keys::{ claim_keys::v3::{Request as KeysClaimRequest, Response as KeysClaimResponse}, get_keys::v3::Response as KeysQueryResponse, - upload_keys, + upload_keys::v3::{Request as UploadKeysRequest, Response as UploadKeysResponse}, upload_signatures::v3::Request as UploadSignaturesRequest, }, sync::sync_events::DeviceLists, @@ -375,7 +375,7 @@ impl OlmMachine { { let store_cache = self.inner.store.cache().await?; let account = store_cache.account().await?; - if let Some(r) = self.keys_for_upload(&*account).await.map(|r| OutgoingRequest { + if let Some(r) = self.keys_for_upload(&account).await.map(|r| OutgoingRequest { request_id: TransactionId::new(), request: Arc::new(r.into()), }) { @@ -482,15 +482,17 @@ impl OlmMachine { /// Create a new cross signing identity and get the upload request to push /// the new public keys to the server. /// - /// **Warning**: This will delete any existing cross signing keys that might - /// exist on the server and thus will reset the trust between all the - /// devices. + /// **Warning**: if called with `reset`, this will delete any existing cross + /// signing keys that might exist on the server and thus will reset the + /// trust between all the devices. /// /// # Returns /// - /// A pair of requests which should be sent out to the server. Once - /// sent, the responses should be passed back to the state machine using - /// [`mark_request_as_sent`]. + /// A triple of requests which should be sent out to the server, in the + /// order they appear in the return tuple. + /// + /// The first request's response, if present, should be passed back to the + /// state machine using [`mark_request_as_sent`]. /// /// These requests may require user interactive auth. /// @@ -498,15 +500,18 @@ impl OlmMachine { pub async fn bootstrap_cross_signing( &self, reset: bool, - ) -> StoreResult<(UploadSigningKeysRequest, UploadSignaturesRequest)> { + ) -> StoreResult { let mut identity = self.inner.user_identity.lock().await; - if reset || identity.is_empty().await { + let (upload_signing_keys_req, upload_signatures_req) = if reset || identity.is_empty().await + { info!("Creating new cross signing identity"); - let cache = self.inner.store.cache().await?; - let account = cache.account().await?; - let (new_identity, upload_signing_keys_req, upload_signatures_req) = - account.bootstrap_cross_signing().await; + + let (new_identity, upload_signing_keys_req, upload_signatures_req) = { + let cache = self.inner.store.cache().await?; + let account = cache.account().await?; + account.bootstrap_cross_signing().await + }; *identity = new_identity; @@ -514,24 +519,46 @@ impl OlmMachine { "Couldn't create a public version of the identity from a new private identity", ); - let changes = Changes { - identities: IdentityChanges { new: vec![public.into()], ..Default::default() }, - private_identity: Some(identity.clone()), - ..Default::default() - }; + self.store() + .save_changes(Changes { + identities: IdentityChanges { new: vec![public.into()], ..Default::default() }, + private_identity: Some(identity.clone()), + ..Default::default() + }) + .await?; - self.store().save_changes(changes).await?; - - Ok((upload_signing_keys_req, upload_signatures_req)) + (upload_signing_keys_req, upload_signatures_req) } else { info!("Trying to upload the existing cross signing identity"); let upload_signing_keys_req = identity.as_upload_request().await; - let account = self.inner.store.static_account(); + // TODO remove this expect. - let upload_signatures_req = - identity.sign_account(account).await.expect("Can't sign device keys"); - Ok((upload_signing_keys_req, upload_signatures_req)) - } + let upload_signatures_req = identity + .sign_account(self.inner.store.static_account()) + .await + .expect("Can't sign device keys"); + + (upload_signing_keys_req, upload_signatures_req) + }; + + // If there are any *device* keys to upload (i.e. the account isn't shared), + // upload them before we upload the signatures, since the signatures may + // reference keys to be uploaded. + let upload_keys_req = { + let cache = self.store().cache().await?; + let account = cache.account().await?; + if account.shared() { + None + } else { + self.keys_for_upload(&account).await.map(OutgoingRequest::from) + } + }; + + Ok(CrossSigningBootstrapRequests { + upload_signing_keys_req, + upload_keys_req, + upload_signatures_req, + }) } /// Receive a successful keys upload response. @@ -540,10 +567,7 @@ impl OlmMachine { /// /// * `response` - The keys upload response of the request that the client /// performed. - async fn receive_keys_upload_response( - &self, - response: &upload_keys::v3::Response, - ) -> OlmResult<()> { + async fn receive_keys_upload_response(&self, response: &UploadKeysResponse) -> OlmResult<()> { self.inner .store .with_transaction(|mut tr| async { @@ -623,7 +647,7 @@ impl OlmMachine { /// the [`OlmMachine`] with the [`receive_keys_upload_response`]. /// /// [`receive_keys_upload_response`]: #method.receive_keys_upload_response - async fn keys_for_upload(&self, account: &Account) -> Option { + async fn keys_for_upload(&self, account: &Account) -> Option { let (device_keys, one_time_keys, fallback_keys) = account.keys_for_upload(); if device_keys.is_none() && one_time_keys.is_empty() && fallback_keys.is_empty() { @@ -631,7 +655,7 @@ impl OlmMachine { } else { let device_keys = device_keys.map(|d| d.to_raw()); - Some(assign!(upload_keys::v3::Request::new(), { + Some(assign!(UploadKeysRequest::new(), { device_keys, one_time_keys, fallback_keys })) } @@ -2048,6 +2072,30 @@ impl OlmMachine { } } +/// A set of requests to be executed when bootstrapping cross-signing using +/// [`OlmMachine::bootstrap_cross_signing`]. +#[derive(Debug)] +pub struct CrossSigningBootstrapRequests { + /// An optional request to upload a device key. + /// + /// Should be sent first, if present. + /// + /// If present, its result must be processed back with + /// `OlmMachine::mark_request_as_sent`. + pub upload_keys_req: Option, + + /// Request to upload the cross-signing keys. + /// + /// Should be sent second. + pub upload_signing_keys_req: UploadSigningKeysRequest, + + /// Request to upload key signatures, including those for the cross-signing + /// keys, and maybe some for the optional uploaded key too. + /// + /// Should be sent last. + pub upload_signatures_req: UploadSignaturesRequest, +} + /// Data contained from a sync response and that needs to be processed by the /// OlmMachine. #[derive(Debug)] @@ -2122,7 +2170,7 @@ pub(crate) mod tests { Curve25519PublicKey, Ed25519PublicKey, }; - use super::testing::response_from_file; + use super::{testing::response_from_file, CrossSigningBootstrapRequests}; use crate::{ error::EventError, machine::{EncryptionSyncChanges, OlmMachine}, @@ -3186,10 +3234,10 @@ pub(crate) mod tests { } async fn setup_cross_signing_for_machine_test_helper(alice: &OlmMachine, bob: &OlmMachine) { - let (alice_upload_signing, _) = + let CrossSigningBootstrapRequests { upload_signing_keys_req: alice_upload_signing, .. } = alice.bootstrap_cross_signing(false).await.expect("Expect Alice x-signing key request"); - let (bob_upload_signing, _) = + let CrossSigningBootstrapRequests { upload_signing_keys_req: bob_upload_signing, .. } = bob.bootstrap_cross_signing(false).await.expect("Expect Bob x-signing key request"); let bob_device_keys = bob @@ -3239,8 +3287,11 @@ pub(crate) mod tests { } async fn sign_alice_device_for_machine_test_helper(alice: &OlmMachine, bob: &OlmMachine) { - let (upload_signing, upload_signature) = - alice.bootstrap_cross_signing(false).await.expect("Expect Alice x-signing key request"); + let CrossSigningBootstrapRequests { + upload_signing_keys_req: upload_signing, + upload_signatures_req: upload_signature, + .. + } = alice.bootstrap_cross_signing(false).await.expect("Expect Alice x-signing key request"); let mut device_keys = alice .get_device(alice.user_id(), alice.device_id(), None) @@ -3320,7 +3371,7 @@ pub(crate) mod tests { .bootstrap_cross_signing(false) .await .expect("Expect Alice x-signing key request") - .0 + .upload_signing_keys_req .user_signing_key .unwrap() .get_first_key_and_id() @@ -3342,7 +3393,7 @@ pub(crate) mod tests { .bootstrap_cross_signing(false) .await .expect("Expect Alice x-signing key request") - .0; + .upload_signing_keys_req; let json = json!({ "device_keys": { diff --git a/crates/matrix-sdk-crypto/src/requests.rs b/crates/matrix-sdk-crypto/src/requests.rs index 8426fd31f..06ba63181 100644 --- a/crates/matrix-sdk-crypto/src/requests.rs +++ b/crates/matrix-sdk-crypto/src/requests.rs @@ -283,6 +283,12 @@ impl From for OutgoingRequest { } } +impl From for OutgoingRequest { + fn from(r: KeysUploadRequest) -> Self { + Self { request_id: TransactionId::new(), request: Arc::new(r.into()) } + } +} + /// Enum over all the incoming responses we need to receive. #[derive(Debug)] pub enum IncomingResponse<'a> { diff --git a/crates/matrix-sdk/src/encryption/mod.rs b/crates/matrix-sdk/src/encryption/mod.rs index a0306a677..4d1ee663d 100644 --- a/crates/matrix-sdk/src/encryption/mod.rs +++ b/crates/matrix-sdk/src/encryption/mod.rs @@ -29,7 +29,9 @@ use futures_util::{ future::try_join, stream::{self, StreamExt}, }; -use matrix_sdk_base::crypto::{OlmMachine, OutgoingRequest, RoomMessageRequest, ToDeviceRequest}; +use matrix_sdk_base::crypto::{ + CrossSigningBootstrapRequests, OlmMachine, OutgoingRequest, RoomMessageRequest, ToDeviceRequest, +}; use ruma::{ api::client::{ backup::add_backup_keys::v3::Response as KeysBackupResponse, @@ -814,8 +816,11 @@ impl Encryption { let olm = self.client.olm_machine().await; let olm = olm.as_ref().ok_or(Error::NoOlmMachine)?; - let (upload_signing_keys_req, upload_signatures_req) = - olm.bootstrap_cross_signing(false).await?; + let CrossSigningBootstrapRequests { + upload_signing_keys_req, + upload_keys_req, + upload_signatures_req, + } = olm.bootstrap_cross_signing(false).await?; let upload_signing_keys_req = assign!(UploadSigningKeysRequest::new(), { auth: auth_data, @@ -824,6 +829,9 @@ impl Encryption { user_signing_key: upload_signing_keys_req.user_signing_key.map(|c| c.to_raw()), }); + if let Some(req) = upload_keys_req { + self.client.send_outgoing_request(req).await?; + } self.client.send(upload_signing_keys_req, None).await?; self.client.send(upload_signatures_req, None).await?; diff --git a/crates/matrix-sdk/tests/integration/client.rs b/crates/matrix-sdk/tests/integration/client.rs index 933df2c7e..55737cdef 100644 --- a/crates/matrix-sdk/tests/integration/client.rs +++ b/crates/matrix-sdk/tests/integration/client.rs @@ -547,6 +547,14 @@ async fn cross_signing_status() { .mount(&server) .await; + Mock::given(method("POST")) + .and(path("/_matrix/client/r0/keys/upload")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "one_time_key_counts": {} + }))) + .mount(&server) + .await; + let status = client .encryption() .cross_signing_status() diff --git a/crates/matrix-sdk/tests/integration/encryption/verification.rs b/crates/matrix-sdk/tests/integration/encryption/verification.rs index e626a697b..dac67c6e5 100644 --- a/crates/matrix-sdk/tests/integration/encryption/verification.rs +++ b/crates/matrix-sdk/tests/integration/encryption/verification.rs @@ -266,27 +266,26 @@ fn mock_keys_signature_upload(keys: Arc>) -> impl Fn(&Request) -> Re // Either merge signatures if an entry is already present, or insert a new // entry. let known_devices = keys.device.entry(user.clone()).or_default(); - if let Some(device_keys) = known_devices.get_mut(key_id) { - let param: DeviceKeys = serde_json::from_str(raw_key.get()).unwrap(); + let device_keys = known_devices + .get_mut(key_id) + .expect("trying to add a signature for a missing key"); - let mut existing: DeviceKeys = device_keys.deserialize().unwrap(); + let param: DeviceKeys = serde_json::from_str(raw_key.get()).unwrap(); - for (uid, sigs) in existing.signatures.iter_mut() { - if let Some(new_sigs) = param.signatures.get(uid) { - sigs.extend(new_sigs.clone()); - } + let mut existing: DeviceKeys = device_keys.deserialize().unwrap(); + + for (uid, sigs) in existing.signatures.iter_mut() { + if let Some(new_sigs) = param.signatures.get(uid) { + sigs.extend(new_sigs.clone()); } - for (uid, sigs) in param.signatures.iter() { - if !existing.signatures.contains_key(uid) { - existing.signatures.insert(uid.clone(), sigs.clone()); - } - } - - *device_keys = Raw::new(&existing).unwrap(); - } else { - tracing::warn!("adding a signature to a key that hasn't been uploaded yet, user={user}, key={key_id}"); - known_devices.insert(key_id.to_owned(), Raw::from_json(raw_key.to_owned())); } + for (uid, sigs) in param.signatures.iter() { + if !existing.signatures.contains_key(uid) { + existing.signatures.insert(uid.clone(), sigs.clone()); + } + } + + *device_keys = Raw::new(&existing).unwrap(); } } diff --git a/crates/matrix-sdk/tests/integration/matrix_auth.rs b/crates/matrix-sdk/tests/integration/matrix_auth.rs index 34b5cf7d6..2bdf86a05 100644 --- a/crates/matrix-sdk/tests/integration/matrix_auth.rs +++ b/crates/matrix-sdk/tests/integration/matrix_auth.rs @@ -341,6 +341,14 @@ async fn test_login_with_cross_signing_bootstrapping() { .mount(&server) .await; + Mock::given(method("POST")) + .and(path("/_matrix/client/r0/keys/upload")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "one_time_key_counts": {} + }))) + .mount(&server) + .await; + let num_calls = Mutex::new(0); Mock::given(method("POST")) .and(path("/_matrix/client/unstable/keys/device_signing/upload"))