Skip to content

Commit

Permalink
refactor(bootstrap): init cross-signing with other e2e initial task (#…
Browse files Browse the repository at this point in the history
…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 #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](matrix-org/matrix-spec-proposals#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
  • Loading branch information
BillCarsonFr authored Jan 18, 2024
1 parent 18065cb commit 784c745
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 54 deletions.
31 changes: 28 additions & 3 deletions crates/matrix-sdk/src/encryption/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<AuthData>) -> 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:?}");
}
Expand Down
25 changes: 7 additions & 18 deletions crates/matrix-sdk/src/matrix_auth/login_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
72 changes: 46 additions & 26 deletions crates/matrix-sdk/src/matrix_auth/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -547,19 +545,27 @@ impl MatrixAuth {
pub async fn register(&self, request: register::v3::Request) -> Result<register::v3::Response> {
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)
}
Expand Down Expand Up @@ -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(())
}
Expand All @@ -833,35 +844,44 @@ impl MatrixAuth {
pub(crate) async fn receive_login_response(
&self,
response: &login::v3::Response,
#[cfg(feature = "e2e-encryption")] login_info: Option<login::v3::LoginInfo>,
) -> 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<login::v3::LoginInfo>,
) -> 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};

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

// Internal client helpers
impl Client {
#[cfg(feature = "e2e-encryption")]
pub(crate) async fn post_login_cross_signing(&self, auth_data: Option<RumaUiaaAuthData>) {
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}");
}
self.client.encryption().run_initialization_tasks(auth_data).await?;
}

Ok(())
}
}

Expand Down
9 changes: 2 additions & 7 deletions crates/matrix-sdk/src/oidc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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
Expand All @@ -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(())
}
Expand Down
6 changes: 6 additions & 0 deletions crates/matrix-sdk/tests/integration/matrix_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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());
Expand Down

0 comments on commit 784c745

Please sign in to comment.