From 784c7459fe69c94e61e7f5ca07de5b855b7bccfa Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 18 Jan 2024 14:25:50 +0100 Subject: [PATCH] refactor(bootstrap): init cross-signing with other e2e initial task (#3024) In existing code the bootstraping is done in different places: - in `bootstrap_cross_signing_if_needed` - or in `run_intialization_tasks` And both are based on the same EncryptionSettings. `bootstrap_cross_signing_if_needed` was done by `LoginBuilder` but `run_intialization_tasks` by `MatrixAuth`. I propose to move all that under `run_intialization_tasks` that has already support to do it in background, and to call it in one place only: `MatrixAuth` Also Fixes https://github.com/matrix-org/matrix-rust-sdk/issues/2763 ## Notes - Some tests have been updated to properly `wait_for_e2ee_initialization_tasks` - `bootstrap_cross_signing_if_needed` might require re-authentication. Which I suppose was the original reason to have that in a seperate place. But given that the need to re-auth will soon be deprecated for bootstrap (when this [MSC](https://github.com/matrix-org/matrix-spec-proposals/pull/3967) will land); so for now we pass the authentication info to `run_intialization_tasks` if any. --- * refactor(bootstrap): init cross-signing with other e2e initial task * fix compilation warning for NoEncryption feature set * Doc and Formatting: Improve doc and formatting * Fix missing import with encryption feature flag --- crates/matrix-sdk/src/encryption/mod.rs | 31 +++++++- .../src/matrix_auth/login_builder.rs | 25 ++----- crates/matrix-sdk/src/matrix_auth/mod.rs | 74 ++++++++++++------- crates/matrix-sdk/src/oidc/mod.rs | 9 +-- .../tests/integration/matrix_auth.rs | 6 ++ 5 files changed, 90 insertions(+), 55 deletions(-) diff --git a/crates/matrix-sdk/src/encryption/mod.rs b/crates/matrix-sdk/src/encryption/mod.rs index fdd0196a5..94b77672a 100644 --- a/crates/matrix-sdk/src/encryption/mod.rs +++ b/crates/matrix-sdk/src/encryption/mod.rs @@ -1224,15 +1224,40 @@ impl Encryption { Ok(olm_machine.uploaded_key_count().await?) } - /// Enables event listeners for the E2EE support. + /// Bootstrap encryption and enables event listeners for the E2EE support. /// - /// For now enables only listeners for backups. Should be called once we + /// Based on the `EncryptionSettings`, this call might: + /// - Bootstrap cross-signing if needed (POST `/device_signing/upload`) + /// - Create a key backup if needed (POST `/room_keys/version`) + /// - Create a secret storage if needed (PUT `/account_data/{type}`) + /// + /// As part of this process, and if needed, the current device keys would be + /// uploaded to the server, new account data would be added, and cross + /// signing keys and signatures might be uploaded. + /// + /// Should be called once we /// created a [`OlmMachine`], i.e. after logging in. - pub(crate) async fn run_initialization_tasks(&self) -> Result<()> { + /// + /// # Arguments + /// + /// * `auth_data` - Some requests may require re-authentication. To prevent + /// the user from having to re-enter their password (or use other methods), + /// we can provide the authentication data here. This is necessary for + /// uploading cross-signing keys. However, please note that there is a + /// proposal (MSC3967) to remove this requirement, which would allow for + /// the initial upload of cross-signing keys without authentication, + /// rendering this parameter obsolete. + pub(crate) async fn run_initialization_tasks(&self, auth_data: Option) -> Result<()> { let mut tasks = self.client.inner.tasks.lock().unwrap(); let this = self.clone(); tasks.setup_e2ee = Some(spawn(async move { + if this.settings().auto_enable_cross_signing { + if let Err(e) = this.bootstrap_cross_signing_if_needed(auth_data).await { + error!("Couldn't bootstrap cross signing {e:?}"); + } + } + if let Err(e) = this.backups().setup_and_resume().await { error!("Couldn't setup and resume backups {e:?}"); } diff --git a/crates/matrix-sdk/src/matrix_auth/login_builder.rs b/crates/matrix-sdk/src/matrix_auth/login_builder.rs index 98a5ce8d4..23fad46ac 100644 --- a/crates/matrix-sdk/src/matrix_auth/login_builder.rs +++ b/crates/matrix-sdk/src/matrix_auth/login_builder.rs @@ -183,24 +183,13 @@ impl LoginBuilder { }); let response = client.send(request, Some(RequestConfig::short_retry())).await?; - self.auth.receive_login_response(&response).await?; - - // This may block login for a while, but the user asked for it! - // TODO: (#2763) put this into a background task. - #[cfg(feature = "e2e-encryption")] - { - use ruma::api::client::uiaa::{AuthData, Password}; - - let auth_data = match login_info { - login::v3::LoginInfo::Password(p) => { - Some(AuthData::Password(Password::new(p.identifier, p.password))) - } - // Other methods can't be immediately translated to an auth. - _ => None, - }; - - client.post_login_cross_signing(auth_data).await; - } + self.auth + .receive_login_response( + &response, + #[cfg(feature = "e2e-encryption")] + Some(login_info), + ) + .await?; Ok(response) } diff --git a/crates/matrix-sdk/src/matrix_auth/mod.rs b/crates/matrix-sdk/src/matrix_auth/mod.rs index 7846bae42..f9fafb6e2 100644 --- a/crates/matrix-sdk/src/matrix_auth/mod.rs +++ b/crates/matrix-sdk/src/matrix_auth/mod.rs @@ -24,8 +24,6 @@ use eyeball::SharedObservable; use futures_core::Stream; use futures_util::StreamExt; use matrix_sdk_base::SessionMeta; -#[cfg(feature = "e2e-encryption")] -use ruma::api::client::uiaa::{AuthData as RumaUiaaAuthData, Password as RumaUiaaPassword}; use ruma::{ api::{ client::{ @@ -547,19 +545,27 @@ impl MatrixAuth { pub async fn register(&self, request: register::v3::Request) -> Result { let homeserver = self.client.homeserver(); info!("Registering to {homeserver}"); + #[cfg(feature = "e2e-encryption")] - let auth_data = match (&request.username, &request.password) { - (Some(u), Some(p)) => Some(RumaUiaaAuthData::Password(RumaUiaaPassword::new( - UserIdentifier::UserIdOrLocalpart(u.clone()), - p.clone(), - ))), + let login_info = match (&request.username, &request.password) { + (Some(u), Some(p)) => Some(ruma::api::client::session::login::v3::LoginInfo::Password( + ruma::api::client::session::login::v3::Password::new( + UserIdentifier::UserIdOrLocalpart(u.into()), + p.clone(), + ), + )), _ => None, }; + let response = self.client.send(request, None).await?; if let Some(session) = MatrixSession::from_register_response(&response) { - let _ = self.set_session(session).await; - #[cfg(feature = "e2e-encryption")] - self.client.post_login_cross_signing(auth_data).await; + let _ = self + .set_session( + session, + #[cfg(feature = "e2e-encryption")] + login_info, + ) + .await; } Ok(response) } @@ -819,7 +825,12 @@ impl MatrixAuth { #[instrument(skip_all)] pub async fn restore_session(&self, session: MatrixSession) -> Result<()> { debug!("Restoring Matrix auth session"); - self.set_session(session).await?; + self.set_session( + session, + #[cfg(feature = "e2e-encryption")] + None, + ) + .await?; debug!("Done restoring Matrix auth session"); Ok(()) } @@ -833,38 +844,47 @@ impl MatrixAuth { pub(crate) async fn receive_login_response( &self, response: &login::v3::Response, + #[cfg(feature = "e2e-encryption")] login_info: Option, ) -> Result<()> { self.client.maybe_update_login_well_known(response.well_known.as_ref()); - self.set_session(response.into()).await?; + self.set_session( + response.into(), + #[cfg(feature = "e2e-encryption")] + login_info, + ) + .await?; Ok(()) } - async fn set_session(&self, session: MatrixSession) -> Result<()> { + async fn set_session( + &self, + session: MatrixSession, + #[cfg(feature = "e2e-encryption")] login_info: Option, + ) -> Result<()> { self.set_session_tokens(session.tokens); self.client.set_session_meta(session.meta).await?; #[cfg(feature = "e2e-encryption")] - self.client.encryption().run_initialization_tasks().await?; + { + use ruma::api::client::uiaa::{AuthData, Password}; + + let auth_data = match login_info { + Some(login::v3::LoginInfo::Password(p)) => { + Some(AuthData::Password(Password::new(p.identifier, p.password))) + } + // Other methods can't be immediately translated to an auth. + _ => None, + }; + + self.client.encryption().run_initialization_tasks(auth_data).await?; + } Ok(()) } } -// Internal client helpers -impl Client { - #[cfg(feature = "e2e-encryption")] - pub(crate) async fn post_login_cross_signing(&self, auth_data: Option) { - let encryption = self.encryption(); - if encryption.settings().auto_enable_cross_signing { - if let Err(err) = encryption.bootstrap_cross_signing_if_needed(auth_data).await { - tracing::warn!("cross-signing bootstrapping failed: {err}"); - } - } - } -} - /// A user session using the native Matrix authentication API. /// /// # Examples diff --git a/crates/matrix-sdk/src/oidc/mod.rs b/crates/matrix-sdk/src/oidc/mod.rs index dbbc40538..3e153c9b3 100644 --- a/crates/matrix-sdk/src/oidc/mod.rs +++ b/crates/matrix-sdk/src/oidc/mod.rs @@ -756,7 +756,7 @@ impl Oidc { } #[cfg(feature = "e2e-encryption")] - self.client.encryption().run_initialization_tasks().await?; + self.client.encryption().run_initialization_tasks(None).await?; Ok(()) } @@ -920,11 +920,6 @@ impl Oidc { // Enable the cross-process lock for refreshes, if needs be. self.deferred_enable_cross_process_refresh_lock().await?; - // Bootstrap cross signing, if needs be. - // TODO: (#2763) put this into a background task. - #[cfg(feature = "e2e-encryption")] - self.client.post_login_cross_signing(None).await; - if let Some(cross_process_manager) = self.ctx().cross_process_token_refresh_manager.get() { if let Some(tokens) = self.session_tokens() { let mut cross_process_guard = cross_process_manager @@ -950,7 +945,7 @@ impl Oidc { } #[cfg(feature = "e2e-encryption")] - self.client.encryption().run_initialization_tasks().await?; + self.client.encryption().run_initialization_tasks(None).await?; Ok(()) } diff --git a/crates/matrix-sdk/tests/integration/matrix_auth.rs b/crates/matrix-sdk/tests/integration/matrix_auth.rs index e426aca72..8917fcbf1 100644 --- a/crates/matrix-sdk/tests/integration/matrix_auth.rs +++ b/crates/matrix-sdk/tests/integration/matrix_auth.rs @@ -452,6 +452,8 @@ async fn test_login_with_cross_signing_bootstrapping() { assert!(client.logged_in(), "Client should be logged in"); assert!(auth.logged_in(), "Client should be logged in with the MatrixAuth API"); + client.encryption().wait_for_e2ee_initialization_tasks().await; + let me = client.user_id().expect("we are now logged in"); let own_identity = client.encryption().get_user_identity(me).await.expect("succeeds").expect("is present"); @@ -503,6 +505,8 @@ async fn test_login_with_cross_signing_bootstrapping() { assert!(client.logged_in(), "Client should be logged in"); assert!(auth.logged_in(), "Client should be logged in with the MatrixAuth API"); + client.encryption().wait_for_e2ee_initialization_tasks().await; + let me = client.user_id().expect("we are now logged in"); let own_identity = client.encryption().get_user_identity(me).await.expect("succeeds").expect("is present"); @@ -579,6 +583,8 @@ async fn test_login_doesnt_fail_if_cross_signing_bootstrapping_failed() { let me = client.user_id().expect("we are now logged in"); + client.encryption().wait_for_e2ee_initialization_tasks().await; + let own_identity = client.encryption().get_user_identity(me).await.expect("succeeds"); let identity = own_identity.expect("created local default identity"); assert!(identity.is_verified());