`OlmMachine` constantly tries to upload keys when restoring session

[MadLittleMods]

Background

Here is how the code works:

1. The Client continually asks OlmMachine.outgoing_requests() for requests to send (including OlmMachine.keys_for_upload(...)) and translates those AnyOutgoingRequest to actual outgoing requests
2. When it gets a response, it should be calling OlmMachine.mark_request_as_sent(...) which is happening for Client.keys_upload(...)
3. This trickles down as OlmMachine.mark_request_as_sent(...) -> OlmMachine.receive_keys_upload_response(...) -> Account.receive_keys_upload_response(...) -> which calls Account.mark_as_shared(), Account.mark_keys_as_published(), Account.update_key_counts(...)
4. Then the next time the OlmMachine asks for the OlmMachine.keys_for_upload(...) request, it sees that the keys are already shared and returns None so we stop uploading more keys.

For new sessions, things work properly and Client.keys_upload(...) gets a 200 response and everything gets called as expected.

Problem:

But for existing sessions where we restore_session(...), Client.keys_upload(...) gets a 400 (keys already exists) error response from the homeserver because it's already uploaded those one-time-keys before (OTK) and mark_request_as_sent(...) never gets called and the keys are never marked as shared. So then we try the exact same thing on the next cycle of outgoing requests and get the same result every time. To be clear, we try to upload device_keys, one_time_keys, and fallback_keys continuously because nothing is marked as shared.

Synapse logs:

Shows off the 400 error response from /_matrix/client/v3/keys/upload: One time key signed_curve25519:AAAAAAAAAA0 already exists

2025-06-09 15:12:07,526 - synapse.rest.client.keys - 146 - INFO - POST-14 - asdf /keys/upload: user_id=@test-user-18-device-list-update2:my.synapse.linux.server, device_id=EYDNWNNLLA
2025-06-09 15:12:07,526 - synapse.handlers.e2e_keys - 913 - INFO - POST-14 - Updating device_keys for device 'EYDNWNNLLA' for user @test-user-18-device-list-update2:my.synapse.linux.server at 1749499927526
2025-06-09 15:12:07,531 - synapse.handlers.e2e_keys - 950 - INFO - POST-14- - Adding one_time_keys dict_keys(['signed_curve25519:AAAAAAAAAA0', 'signed_curve25519:AAAAAAAAAA4', 'signed_curve25519:AAAAAAAAAA8', 'signed_curve25519:AAAAAAAAAAA', 'signed_curve25519:AAAAAAAAAAE', 'signed_curve25519:AAAAAAAAAAI', 'signed_curve25519:AAAAAAAAAAM', 'signed_curve25519:AAAAAAAAAAQ', 'signed_curve25519:AAAAAAAAAAU', 'signed_curve25519:AAAAAAAAAAY', 'signed_curve25519:AAAAAAAAAAc', 'signed_curve25519:AAAAAAAAAAg', 'signed_curve25519:AAAAAAAAAAk', 'signed_curve25519:AAAAAAAAAAo', 'signed_curve25519:AAAAAAAAAAs', 'signed_curve25519:AAAAAAAAAAw', 'signed_curve25519:AAAAAAAAAB0', 'signed_curve25519:AAAAAAAAAB4', 'signed_curve25519:AAAAAAAAAB8', 'signed_curve25519:AAAAAAAAABA', 'signed_curve25519:AAAAAAAAABE', 'signed_curve25519:AAAAAAAAABI', 'signed_curve25519:AAAAAAAAABM', 'signed_curve25519:AAAAAAAAABQ', 'signed_curve25519:AAAAAAAAABU', 'signed_curve25519:AAAAAAAAABY', 'signed_curve25519:AAAAAAAAABc', 'signed_curve25519:AAAAAAAAABg', 'signed_curve25519:AAAAAAAAABk', 'signed_curve25519:AAAAAAAAABo', 'signed_curve25519:AAAAAAAAABs', 'signed_curve25519:AAAAAAAAABw', 'signed_curve25519:AAAAAAAAAC0', 'signed_curve25519:AAAAAAAAAC4', 'signed_curve25519:AAAAAAAAAC8', 'signed_curve25519:AAAAAAAAACA', 'signed_curve25519:AAAAAAAAACE', 'signed_curve25519:AAAAAAAAACI', 'signed_curve25519:AAAAAAAAACM', 'signed_curve25519:AAAAAAAAACQ', 'signed_curve25519:AAAAAAAAACU', 'signed_curve25519:AAAAAAAAACY', 'signed_curve25519:AAAAAAAAACc', 'signed_curve25519:AAAAAAAAACg', 'signed_curve25519:AAAAAAAAACk', 'signed_curve25519:AAAAAAAAACo', 'signed_curve25519:AAAAAAAAACs', 'signed_curve25519:AAAAAAAAACw', 'signed_curve25519:AAAAAAAAADA', 'signed_curve25519:AAAAAAAAADE']) for device 'EYDNWNNLLA' for user '@test-user-18-device-list-update2:my.synapse.linux.server' at 1749499927526
2025-06-09 15:12:07,536 - synapse.http.server - 130 - INFO - POST-14 - <XForwardedForRequest at 0x7f62fcf53890 method='POST' uri='/_matrix/client/v3/keys/upload' clientproto='HTTP/1.1' site='8008'> SynapseError: 400 - One time key signed_curve25519:AAAAAAAAAA0 already exists. Old key: {"key":"EGcRB4WrbDaRXEpHTBLmh0oygnGuWv82dzUL+uFNTTk","signatures":{"@test-user-18-device-list-update2:my.synapse.linux.server":{"ed25519:EYDNWNNLLA":"RxxsfpZpbtMdlC4UUrmTpCSHVvOXCDxd/nnkF1uRG9LAzWDciEv2WR+IGGKpTWwTmdx9LdmVU6JTNmDwYQrPDA"}}}; new key: {'key': '9lY+QpfmqmNv5+8jfb3qbgLyEhyV7SlUtp7VNm0Nc0I', 'signatures': {'@test-user-18-device-list-update2:my.synapse.linux.server': {'ed25519:EYDNWNNLLA': 'rpY9uVdqMN9GUPS50eWDidUq9foCQFvR4eB/8GSEb/qSxVHrKv1JEdsao6M0EArh/xrkx0wUmnDEB2aKgMfrBw'}}}
2025-06-09 15:12:07,537 - synapse.access.http.8008 - 508 - INFO - POST-14 - 127.0.0.1 - 8008 - {@test-user-18-device-list-update2:my.synapse.linux.server} Processed request: 0.011sec/0.000sec (0.001sec, 0.000sec) (0.000sec/0.008sec/4) 610B 400 "POST /_matrix/client/v3/keys/upload HTTP/1.1" "-" [0 dbevts]

Reproduction case

The example just uses the matrix-sdk to create a client, matrix_client.restore_session(...) if available (which means the first-run will just login/register the user), and setup a /sync loop, and keep it running until you press Enter.

main.rs

use std::time::Duration;

use anyhow::Context;
use matrix_sdk::reqwest;
use matrix_sdk::{LoopCtrl, config::SyncSettings, ruma::api::client::uiaa};
use serde::Deserialize;
use tokio_util::sync::CancellationToken;
use url::Url;

mod register;

/// Configuration for a given homeserver
#[derive(Debug, Clone, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct Homeserver {
    /// Homeserver name. Must be unique.
    ///
    /// Users on this homeserver should have MXID's like `@user:server_name`
    pub server_name: String,

    /// URL to access the client API.
    pub client_url: Url,

    /// Method to use to register new users
    pub user_registration: register::UserRegistration,

    /// Password to use for all users.
    pub user_password: String,
}

#[tokio::main]
async fn main() -> anyhow::Result<()> {
    let use_existing_sessions = true;

    let homeserver = Homeserver {
        server_name: "my.synapse.server".to_string(),
        client_url: Url::parse("http://localhost:8008/").context("Invalid client URL")?,
        user_registration: register::UserRegistration::RegistrationSharedSecret(
            "xxx".to_string(),
        ),
        user_password: "secret_password".to_string(),
    };
    let user_id = format!(
        "@test-user-matrix-rust-sdk-key-upload1:{}",
        homeserver.server_name
    );

    let http_client = reqwest::Client::builder()
        .timeout(Duration::from_secs(30))
        .build()
        .context("Failed to build HTTP client")?;

    let matrix_client = matrix_sdk::Client::builder()
        .http_client(http_client.clone())
        .homeserver_url(&homeserver.client_url)
        .build()
        .await
        .context("Couldn't create matrix client")?;

    let session_path_string = format!("sessions/{}/{}.json", homeserver.server_name, user_id);
    let session_path = std::path::Path::new(&session_path_string);

    let (new_user, new_session) =
        match (use_existing_sessions, std::fs::read_to_string(session_path)) {
            (true, Ok(serialized_session)) => {
                // This is added to avoid type inference issues: `match the size for
                // values of type `str` cannot be known at compilation time`
                let serialized_session: String = serialized_session;

                // Try restoring from a previously persisted session first
                let session: matrix_sdk::authentication::matrix::MatrixSession =
                    serde_json::from_str(&serialized_session)?;
                matrix_client
                    .restore_session(session)
                    .await
                    .context("Failed restoring session for user")?;
                println!("Restored user session");

                (false, false)
            }
            _ => {
                println!("Registering user");
                // Register a user or login if the user already exists.
                let new_user = register::login_or_register_user(
                    http_client,
                    &matrix_client,
                    homeserver.user_registration.clone(),
                    user_id,
                    homeserver.user_password.clone(),
                )
                .await
                .context("Failed registering user")?;

                (new_user, true)
            }
        };

    // Ensure the user is logged in at this point
    let user_id = matrix_client
        .user_id()
        .context("Client is not logged in")?
        .to_owned();

    // Persist the session for future runs
    if let Some(session) = matrix_client.session() {
        match session {
            // We should get a `MatrixSession` because we're using the
            // "native Matrix authentication API".
            matrix_sdk::authentication::AuthSession::Matrix(matrix_session) => {
                let serialized_session = serde_json::to_string(&matrix_session)?;
                if let Some(session_path_directory) = session_path.parent() {
                    std::fs::create_dir_all(session_path_directory)?;
                }
                std::fs::write(session_path, serialized_session)?;
            }
            matrix_sdk::authentication::AuthSession::OAuth(_oauth_session) => {
                return Err(anyhow::anyhow!(
                    "Unable to persist OAuth session (not supported and unexpected)"
                ));
            }
            // This enum is non-exhaustive so we have to handle the `_` case
            // but it's not expected.
            _ => {
                return Err(anyhow::anyhow!(
                    "Unable to persist unknown session type (not supported): {:?}",
                    session
                ));
            }
        }
    } else {
        return Err(anyhow::anyhow!(
            "Client should be logged in by this point but has no session"
        ));
    }

    let cancelation_token = CancellationToken::new();

    // We need to bootstrap cross-signing for any new login session because each new
    // login creates a new device.
    if new_session {
        println!("Bootstrapping cross signing");
        if let Err(e) = matrix_client
            .encryption()
            .bootstrap_cross_signing_if_needed(None)
            .await
        {
            if let Some(response) = e.as_uiaa_response() {
                let mut password = uiaa::Password::new(
                    user_id.clone().into(),
                    homeserver.user_password.clone().to_owned(),
                );
                password.session.clone_from(&response.session);

                matrix_client
                    .encryption()
                    .bootstrap_cross_signing_if_needed(Some(uiaa::AuthData::Password(password)))
                    .await
                    .context("Couldn't bootstrap cross signing")?;
            } else {
                return Err(e).context("Couldn't bootstrap cross signing");
            }
        }
    }

    // Spawn the sync loop in the background with a cancellation token
    //
    // We care about syncing to mimic the real-world use case of a client listening
    // for new messages.
    println!("Starting sync loop");
    let sync_handle = tokio::spawn({
        let cancelation_token = cancelation_token.child_token();
        let client = matrix_client.clone();
        async move {
            client
                .sync_with_callback(SyncSettings::default(), |_| {
                    if cancelation_token.is_cancelled() {
                        std::future::ready(LoopCtrl::Break)
                    } else {
                        std::future::ready(LoopCtrl::Continue)
                    }
                })
                .await?;

            anyhow::Ok(())
        }
    });

    println!("Press Enter to continue...");
    let mut input = String::new();
    std::io::stdin()
        .read_line(&mut input)
        .expect("Failed to read line");

    // Stop all the agents
    cancelation_token.cancel();

    // Wait for the sync loop to finish
    if let Err(err) = sync_handle.await {
        println!("Something went wrong while waiting for the sync loop: {err}");
    }

    Ok(())
}

register.rs

use anyhow::{Context, ensure};
use hmac::{Hmac, Mac};
use http::{StatusCode, header};
use matrix_sdk::reqwest;
use matrix_sdk::ruma::api::client::{
    account::{check_registration_token_validity, register::v3::Request as RegistrationRequest},
    uiaa,
};
use serde::{Deserialize, Serialize};
use sha1::Sha1;

/// Method to use to register new users
#[derive(Debug, Clone, Deserialize)]
#[serde(deny_unknown_fields)]
pub enum UserRegistration {
    /// Synapse registration token
    ///
    /// Created via the Synapse admin API: `POST /_synapse/admin/v1/registration_tokens/new`.
    /// See https://github.com/element-hq/synapse/blob/develop/docs/usage/administration/admin_api/registration_tokens.md#create-token
    #[serde(rename = "admin_registration_token")]
    AdminRegistrationToken(String),

    /// Shared-Secret Registration
    ///
    /// As configured in the Synapse homeserver configuration `registration_shared_secret`
    #[serde(rename = "registration_shared_secret")]
    RegistrationSharedSecret(String),
}

/// `GET /_synapse/admin/v1/register`
#[derive(Deserialize, Debug)]
pub struct SharedSecretRegistrationNonceResponse {
    pub nonce: String,
}

#[allow(dead_code)]
pub enum UserType {
    /// Normal user
    Normal,
    /// Special designation in Synapse (see their docs)
    Bot,
    /// Special designation in Synapse (see their docs)
    Support,
}

/// Information necessary for a `POST /_synapse/admin/v1/register` request
pub struct SharedSecretRegistrationRequest {
    /// The `registgration_shared_secret` configured in Synapse
    pub registration_shared_secret: String,
    pub nonce: String,
    pub username: String,
    pub displayname: String,
    pub password: String,
    pub admin: bool,
    pub user_type: UserType,
}

impl SharedSecretRegistrationRequest {
    /// Serialize the request body to a JSON string
    fn to_request_body(&self) -> anyhow::Result<SharedSecretRegistrationRequestBody> {
        Ok(SharedSecretRegistrationRequestBody {
            nonce: self.nonce.clone(),
            username: self.username.clone(),
            displayname: self.displayname.clone(),
            password: self.password.clone(),
            admin: self.admin,
            mac: self.generate_mac_digest_for_registration()?,
        })
    }

    /// Used for Shared-Secret Registration (`POST /_synapse/admin/v1/register`)
    ///
    /// Args:
    fn generate_mac_digest_for_registration(&self) -> anyhow::Result<String> {
        let mut mac = Hmac::<Sha1>::new_from_slice(self.registration_shared_secret.as_bytes())?;

        mac.update(self.nonce.as_bytes());
        mac.update(b"\0");
        mac.update(self.username.as_bytes());
        mac.update(b"\0");
        mac.update(self.password.as_bytes());
        mac.update(b"\0");
        mac.update(if self.admin { b"admin" } else { b"notadmin" });

        match self.user_type {
            UserType::Normal => (),
            UserType::Bot => {
                mac.update(b"\0");
                mac.update("bot".as_bytes());
            }
            UserType::Support => {
                mac.update(b"\0");
                mac.update("support".as_bytes());
            }
        };

        let result = mac.finalize();
        let code_bytes = result.into_bytes();
        Ok(hex::encode(code_bytes))
    }
}

/// Serialized request body for a `POST /_synapse/admin/v1/register` request
#[derive(Serialize, Deserialize, Debug)]
pub struct SharedSecretRegistrationRequestBody {
    pub nonce: String,
    pub username: String,
    pub displayname: String,
    pub password: String,
    pub admin: bool,
    pub mac: String,
}

/// `POST /_synapse/admin/v1/register`
#[derive(Deserialize, Debug)]
#[allow(dead_code)]
pub struct SharedSecretRegistrationResponse {
    pub access_token: String,
    pub user_id: String,
    pub home_server: String,
    pub device_id: String,
}

/// Register a user or login if the user already exists.
///
/// Returns whether the user was newly registered or not.
pub async fn login_or_register_user(
    http_client: reqwest::Client,
    matrix_client: &matrix_sdk::Client,
    user_registration_method: UserRegistration,
    user_id: String,
    password: String,
) -> anyhow::Result<bool> {
    let user_id = matrix_sdk::ruma::UserId::parse(user_id)?;
    let username = user_id.localpart().to_owned();
    let server_name = user_id.server_name().to_owned();

    // Sanity check that the matrix_client is for the correct server when possible.
    if let Some(matrix_client_server_name) = matrix_client.server() {
        ensure!(
            server_name == matrix_client_server_name.to_string(),
            "Server name in user_id ({}) doesn't match `matrix_client` server name ({})",
            user_id,
            server_name,
        );
    }

    let new_user = match user_registration_method {
        UserRegistration::AdminRegistrationToken(admin_registration_token) => {
            let request = check_registration_token_validity::v1::Request::new(
                admin_registration_token.clone(),
            );
            let is_registration_token_valid = matrix_client.send(request).await?;
            println!(
                "Registration token valid? {}",
                is_registration_token_valid.valid,
            );
            if !is_registration_token_valid.valid {
                return Err(anyhow::anyhow!(
                    "`registration_token` for homeserver ({:?}) is invalid",
                    server_name
                ));
            }

            let mut request: RegistrationRequest = RegistrationRequest::new();

            request.username = Some(username.clone());
            request.password = Some(password.to_owned());
            request.auth = Some(uiaa::AuthData::Password(uiaa::Password::new(
                uiaa::UserIdentifier::UserIdOrLocalpart(username.clone()),
                password.clone(),
            )));

            //matrix_client.send(Requ)
            match matrix_client.matrix_auth().register(request).await {
                Ok(_response) => true,
                Err(err) => {
                    if let Some(uiaa_info) = err.as_uiaa_response() {
                        let session = uiaa_info.session.clone();
                        println!("Failed registering with session {:?}: {:?}", session, err,);
                        let mut request = RegistrationRequest::new();

                        request.username = Some(username.clone());
                        request.password = Some(password.clone().to_owned());
                        let mut reg_token =
                            uiaa::RegistrationToken::new(admin_registration_token.clone());
                        reg_token.session = session.clone();
                        request.auth = Some(uiaa::AuthData::RegistrationToken(reg_token));
                        match matrix_client
                            .matrix_auth()
                            .register(request)
                            .await
                            .context("Failed to register user")
                        {
                            Ok(_response) => true,
                            Err(_err) => {
                                let mut request = RegistrationRequest::new();

                                request.username = Some(username.clone());
                                request.password = Some(password.clone().to_owned());
                                let mut password_register = uiaa::Dummy::new();
                                password_register.session = session;
                                request.auth = Some(uiaa::AuthData::Dummy(password_register));
                                match matrix_client
                                    .matrix_auth()
                                    .register(request)
                                    .await
                                    .context("Failed to register user")
                                {
                                    Ok(_response) => true,
                                    Err(_err) => {
                                        println!(
                                            "Failed registering user, attempting to login as existing user.",
                                        );
                                        matrix_client
                                            .matrix_auth()
                                            .login_username(username, &password.clone())
                                            .await
                                            .context("Failed logging in user")?;
                                        false
                                    }
                                }
                            }
                        }
                    } else {
                        println!(
                            "Failed getting session for user registration, attempting to login as existing user.",
                        );
                        matrix_client
                            .matrix_auth()
                            .login_username(username, &password.clone())
                            .await
                            .context("Failed logging in user")?;
                        false
                    }
                }
            }
        }
        UserRegistration::RegistrationSharedSecret(registration_shared_secret) => {
            let admin_register_url = matrix_client
                .homeserver()
                .join("/_synapse/admin/v1/register")?;

            let nonce_response = http_client
                .get(admin_register_url.clone())
                .send()
                .await?
                .error_for_status()?
                .json::<SharedSecretRegistrationNonceResponse>()
                .await?;

            let serialized_registration_request_body = serde_json::to_string(
                &SharedSecretRegistrationRequest {
                    registration_shared_secret,
                    nonce: nonce_response.nonce,
                    username: username.clone(),
                    displayname: username.clone(),
                    password: password.to_owned(),
                    admin: false,
                    user_type: UserType::Normal,
                }
                .to_request_body()?,
            )?;
            let registration_response = http_client
                .post(admin_register_url.clone())
                .header(header::CONTENT_TYPE, "application/json")
                .body(serialized_registration_request_body.clone())
                .send()
                .await?;

            let status = registration_response.status();
            match status {
                StatusCode::OK => {
                    let registration_response: SharedSecretRegistrationResponse =
                        registration_response.json().await?;

                    // Sanity check
                    ensure!(
                        registration_response.home_server == server_name,
                        "`home_server` ({}) in response doesn't match our expected server_name ({})",
                        registration_response.home_server,
                        server_name,
                    );
                    ensure!(
                        registration_response.user_id == user_id,
                        "`user_id` in response ({}) doesn't match expected user ID ({})",
                        registration_response.user_id,
                        user_id,
                    );

                    // Login the user for the `matrix_client`
                    matrix_client
                        .matrix_auth()
                        .login_username(username, &password.clone())
                        .await
                        .context("Failed logging in user")?;

                    // New user
                    true
                }
                StatusCode::BAD_REQUEST => {
                    let error_response: matrix_sdk::ruma::api::client::error::StandardErrorBody =
                        registration_response.json().await?;

                    match error_response.kind {
                        matrix_sdk::ruma::api::client::error::ErrorKind::UserInUse => {
                            // Login the user for the `matrix_client`
                            println!("User already exists, logging in.");
                            matrix_client
                                .matrix_auth()
                                .login_username(username, &password.clone())
                                .await
                                .context("Failed logging in user")?;

                            // Not a new user
                            false
                        }
                        _ => {
                            return Err(anyhow::anyhow!(
                                "Expected registration (POST {} with {}) to respond successfully (or `{}`) but encountered {} -> {:?}",
                                admin_register_url,
                                // Note: This does log senstive information but it is useful
                                // for debugging. We're assuming test users aren't important
                                // and/or in a test environment.
                                serialized_registration_request_body,
                                matrix_sdk::ruma::api::client::error::ErrorCode::UserInUse,
                                StatusCode::BAD_REQUEST,
                                error_response,
                            ));
                        }
                    }
                }
                status_code => {
                    return Err(anyhow::anyhow!(
                        "Expected registration (POST {} with {}) to respond successfully (or `{}`) but encountered {} -> {}",
                        admin_register_url,
                        // Note: This does log senstive information but it is useful
                        // for debugging. We're assuming test users aren't important
                        // and/or in a test environment.
                        serialized_registration_request_body,
                        matrix_sdk::ruma::api::client::error::ErrorCode::UserInUse,
                        status_code,
                        registration_response
                            .text()
                            .await
                            .unwrap_or("<unable to get response body>".to_string()),
                    ));
                }
            }
        }
    };

    Ok(new_user)
}

Cargo.toml

[package]
name = "matrix-sdk-keys-upload"
version = "0.1.0"
edition = "2024"

[dependencies]
anyhow = "1.0.86"
hex = "0.4.3"
hmac = "0.12.1"
http = "1.3.1"
matrix-sdk = { version = "0.11.0", features = ["anyhow"] }
# matrix-sdk = { path = "/home/eric/Documents/github/element/matrix-rust-sdk/crates/matrix-sdk/", features = [
#   "anyhow",
# ] }
reqwest = { version = "0.12.15", default-features = false, features = ["json"] }
serde = { version = "1.0.202", features = ["derive", "rc"] }
serde_json = "1.0.140"
sha1 = "0.10.6"
tokio = { version = "1.37.0", features = ["full"] }
tokio-util = "0.7.11"
url = "2.5.4"

Workaround

Currently, it seems like the matrix-sdk assumes you're defining a client store_config. Things appear to work once you start using one but the Rust SDK should gracefully handle key conflicts without this.

    let matrix_client = matrix_sdk::Client::builder()
        .http_client(http_client.clone())
        .homeserver_url(&homeserver.client_url)
        // Works when a client store is defined (`indexeddb_store()` and `store_config()` also available)
        .sqlite_store(client_session_store_path, None)
        .build()
        .await
        .context("Couldn't create matrix client")?;

See examples/persist_session/src/main.rs as a full example.

Definition of done

Tasks that have spawned from the discussion below:

• Update doc comments for Client::restore_session() to point out the need to also setup persistent storage on the client
• Update Restoring a client section in the encryption.md docs on to more explicitly point out the need to setup persistent storage on the client
• Detect when the persistent storage isn't configured (i.e. no keys in the crypto storage) and someone uses Client::restore_session(); make sure error is thrown when the e2e-encryption feature is enabled.
• (needs buy-in): Detect key conflict on /_matrix/client/v3/keys/upload scenarios and back-off on trying to upload keys (this is a last resort as usually we should be able to detect a mismatch between the device ID from user session/CryptoStore and throw an error then instead)
• (needs buy-in): Better API (still being discussed)

[poljar]

Currently, it seems like the matrix-sdk assumes you're defining a client store_config.

It doesn't really assume that on its own, it's how end-to-end encryption works.

Your device ID was tied to a certain set of device keys and one-time keys the first time you logged in and uploaded those keys.

This is documented here: https://matrix-org.github.io/matrix-rust-sdk/matrix_sdk/encryption/index.html#restoring-a-client.

You can't just steal an access token from another client instance once the device keys have been uploaded.

There is no way to recover from this. Well sure, we could remove all the one-time keys and upload new ones, but other devices will not want to communicate with your device.

If you don't want to persist the device keys by defining a store_config, then you can't persist the device ID and access token either.

[MadLittleMods]

I appreciate you laying out some of the background here around encryption in Matrix because I am lacking the expert details on the crypto side 🙇

I think there are a few pitfalls with how things are currently laid out.

What If I don't want end-to-end encryption?

It seems like it is only possible to disable end-to-end encryption via the e2e-encryption feature flag which means you can't mix clients which use encryption with others that don't care at all.

Feels like a feature that could be toggle-able at the per-client level.

Why do we have multiple clients?

We use multiple clients because we're using the matrix-rust-sdk in the context of a load-test where we spawn many users/agents each with their own matrix_client and have them interact with each other to simulate real-world activity against a homeserver.

To be clear, we don't have a use case for mixing e2ee and non-e2ee clients but this issue would also manifest itself for anyone trying to do so.

Missing links/context for the persist/restore cycle

As a developer working on a pre-existing project, I'm coming at this from just poking at the matrix-rust-sdk API from the Client level and the doc comments for Client::restore_session() don't mention or link to the extra details from that encryption.md doc.

If I'm lucky enough to find those docs, jumping straight to https://docs.rs/matrix-sdk/0.12.0/matrix_sdk/encryption/index.html#restoring-a-client, I will be missing the detail about needing to configure a persistent storage backend which is mentioned above and below that section:

matrix-rust-sdk/crates/matrix-sdk/src/docs/encryption.md

Lines 168 to 169 in c340a71

	2. To persist the encryption keys, you can use [`ClientBuilder::store_config`]
	or one of the `_store` methods on [`ClientBuilder`].

Or more subtly in this table:

matrix-rust-sdk/crates/matrix-sdk/src/docs/encryption.md

Lines 214 to 221 in c340a71

	## Common pitfalls
	| Failure | Cause | Fix |
	| ------------------- | ----- | ----------- |
	| No messages get encrypted or decrypted | The `e2e-encryption` feature is disabled | [Enable the feature in your `Cargo.toml` file] |
	| Messages that were decryptable aren't after a restart | Storage isn't setup to be persistent | Ensure you've activated the persistent storage backend feature, e.g. `sqlite` |
	| Messages are encrypted but can't be decrypted | The access token that the client is using is tied to another device | Clear storage to create a new device, read the [Restoring a Client] section |
	| Messages don't get encrypted but get decrypted | The `m.room.encryption` event is missing | Make sure encryption is [enabled] for the room and the event isn't [filtered] out. Otherwise it might be a deserialization bug |

By the nature of the API shape, this is easy to mess up. You have to know that the necessary keys are stored in the CryptoStore which is part of the persistent storage backend configured when creating the client, not when using Client::restore_session().

At the very least, it feels like there should be an error if someone tries to use Client::restore_session() and the keys it's expecting aren't available in the CryptoStore if we're working with that assumption anyway.

My previous expectations

I can see how my previous expectations kinda expected magic to happen but basically I assumed that we could re-use an existing login session/device and it would re-upload new keys as necessary. (this kind of thing wouldn't work at the homeserver level if element-hq/synapse#18512 was merged)

When it comes to the load-test, we don't really care whether previous messages have UTD (unable-to-decrypt) problems or not. We just want to send new messages during the load test, federate, and have them received by other clients.

As one possible alternative, perhaps we could use a seeded deterministic rng to generate keys so they are the same across runs and match what was initially uploaded for the device. I guess the client would check what keys are already uploaded and see that they are already the same.

There is no way to recover from this. Well sure, we could remove all the one-time keys and upload new ones, but other devices will not want to communicate with your device.

What mechanisms make other clients not want to communicate with a device with new keys? It seems like new one-time keys is a normal part of the life cycle anyway.

I understand existing clients may still be encrypting for previous keys. In the case of our load-test, since all clients are fresh, I assume they would pull new keys and be fine.

What about recovery keys and secret storage?

API for reference:

let recovery_key = matrix_client
    .encryption()
    .recovery()
    .enable()
    .wait_for_backups_to_upload()
    .await?;

matrix_client
    .encryption()
    .recovery()
    .recover(&recovery_key)
    .await
    .context("Failed restoring from recovery key")?;

As far as I can tell (I'm fuzzy on what exactly these keys are for), the encryption recovery mechanism currently only handles making it possible to read encrypted history (m.megolm_backup.v1) and the device cross-signing keys used for device verification.

The following secrets are retrieved by this method:

• m.cross_signing.master: The master cross-signing key.
• m.cross_signing.self_signing: The self-signing cross-signing key.
• m.cross_signing.user_signing: The user-signing cross-signing key.
• m.megolm_backup.v1: The backup recovery key.

-- https://docs.rs/matrix-sdk/0.12.0/matrix_sdk/encryption/secret_storage/struct.SecretStore.html#method.import_secrets

Can we store device_keys, one_time_keys, fallback_keys in the secret storage?

Given the secret storage seems to be a bit of a generic secret storage system in the spec, can we use it to store all of these device specific keys that are normally stored in the CryptoStore in the persistent storage backend on the client?

Then use the recovery key whenever we want to fetch all of our device-scoped keys again.

What does someone's "identity being reset" mean in Element?

Just for my own knowledge, I assume the "identity reset" language in Element corresponds device verification stuff? which is a separate topic ⏩

[poljar]

I appreciate you laying out some of the background here around encryption in Matrix because I am lacking the expert details on the crypto side 🙇

I think there are a few pitfalls with how things are currently laid out.

What If I don't want end-to-end encryption?

It seems like it is only possible to disable end-to-end encryption via the e2e-encryption feature flag which means you can't mix clients which use encryption with others that don't care at all.

Yes, this is by design. The problem with runtime flags is that they can lead to longer term UTD-s. Since room keys in the usual case encrypt around 100 room messages, it's very important to not miss or drop a room key. The room keys arrive as to-device messages.

If we allow users to turn off end-to-end encryption and processing of to-device messages we would either drop room keys, or we would need to have a maybe_e2ee_will_be_turned_on_again queue for to-device messages.

All in all, it sounds like an error prone and complicated setup for very little gain. Not many people want a Matrix client that doesn't understand end-to-end encryption, let alone one that can turn the feature off at runtime.

Missing links/context for the persist/restore cycle

As a developer working on a pre-existing project, I'm coming at this from just poking at the matrix-rust-sdk API from the Client level and the doc comments for Client::restore_session() don't mention or link to the extra details from that encryption.md doc.

Yup, that's a good idea, we should link to the encryption stuff and to the pitfals listed there.

Actually, the Client::restore_sessions() method used to have much nicer docs in general, we should add some examples there like the Matrix auth restore_session() method has: https://matrix-org.github.io/matrix-rust-sdk/matrix_sdk/authentication/matrix/struct.MatrixAuth.html#method.restore_session.

This was the main method before the introduction of OIDC.

If I'm lucky enough to find those docs, jumping straight to https://docs.rs/matrix-sdk/0.12.0/matrix_sdk/encryption/index.html#restoring-a-client, I will be missing the detail about needing to configure a persistent storage backend which is mentioned above and below that section

Yeah, we could reword things to be a bit more explicit about this.

By the nature of the API shape, this is easy to mess up. You have to know that the necessary keys are stored in the CryptoStore which is part of the persistent storage backend configured when creating the client, not when using Client::restore_session().

At the very least, it feels like there should be an error if someone tries to use Client::restore_session() and the keys it's expecting aren't available in the CryptoStore if we're working with that assumption anyway.

Well it can't know that it expects keys if they aren't persisted. If you do use the wrong device ID and there is a store configured, it will yell.

I think what is missing is some higher level API where we manage the device ID and access token for users. Though it isn't particularly easy to do this while also having generic storage support where stores could persist multiple accounts.

You need to know the user/device you want to load from the store if you support multiple accounts in a single store.

But I think a specialized single account situation could be made simpler.

My previous expectations

I can see how my previous expectations kinda expected magic to happen but basically I assumed that we could re-use an existing login session/device and it would re-upload new keys as necessary. (this kind of thing wouldn't work at the homeserver level if element-hq/synapse#18512 was merged)

When it comes to the load-test, we don't really care whether previous messages have UTD (unable-to-decrypt) problems or not. We just want to send new messages during the load test, federate, and have them received by other clients.

As one possible alternative, perhaps we could use a seeded deterministic rng to generate keys so they are the same across runs and match what was initially uploaded for the device. I guess the client would check what keys are already uploaded and see that they are already the same.

I think that a deterministic seed could be a workaround.

The problem isn't as much that the keys differ, the server complains that keys with a certain key ID have already been uploaded. The key ID is deterministic, it's a counter that starts from 0 and is base64 encoded.

Then the server checks if a (key ID, key) pair exists and if it differs.

What mechanisms make other clients not want to communicate with a device with new keys? It seems like new one-time keys is a normal part of the life cycle anyway.

Other clients will download the device keys and remember them. The device keys represent a device (more info: https://matrix-org.github.io/matrix-rust-sdk/matrix_sdk/encryption/identities/struct.Device.html). These keys contain an Ed25519 key which signs the device keys and any one-time keys the device uploads.

If the other client has remembered some device keys it won't accept a new set. On the other hand if it hasn't yet seen the device and has only seen the reuploaded variant, then the signatures of the one-time keys won't be correct.

I understand existing clients may still be encrypting for previous keys. In the case of our load-test, since all clients are fresh, I assume they would pull new keys and be fine.

I think you're going to hit the invalid signatures on the one-time keys scenario then.

What about recovery keys and secret storage?

API for reference:

let recovery_key = matrix_client
.encryption()
.recovery()
.enable()
.wait_for_backups_to_upload()
.await?;`

matrix_client
.encryption()
.recovery()
.recover(&recovery_key)
.await
.context("Failed restoring from recovery key")?;

As far as I can tell (I'm fuzzy on what exactly these keys are for), the encryption recovery mechanism currently only handles making it possible to read encrypted history (m.megolm_backup.v1) and the device cross-signing keys used for device verification.

The following secrets are retrieved by this method:

• m.cross_signing.master: The master cross-signing key.
• m.cross_signing.self_signing: The self-signing cross-signing key.
• m.cross_signing.user_signing: The user-signing cross-signing key.
• m.megolm_backup.v1: The backup recovery key.

-- https://docs.rs/matrix-sdk/0.12.0/matrix_sdk/encryption/secret_storage/struct.SecretStore.html#method.import_secrets

There are two types of identities in the Matrix world, we already covered devices so I won't repeat that here. The other type are users or user identities which are backed by the cross-signing keys (https://matrix-org.github.io/matrix-rust-sdk/matrix_sdk/encryption/identities/struct.UserIdentity.html).

The user identities are a bit abstract and don't exist on a single client, like device keys do.

The user identities sign devices which establishes some form of web-of trust scheme for a given user. User identities verify each other and the implicitly trust the other user's devices.

Since these don't necessarily exist on a single device we put them into secret storage so new devices can fetch them from there.

Can we store device_keys, one_time_keys, fallback_keys in the secret storage?

Theoretically yes, but there's no support for this, what you are asking for are basically dehydrated devices.

Given the secret storage seems to be a bit of a generic secret storage system in the spec, can we use it to store all of these device specific keys that are normally stored in the CryptoStore in the persistent storage backend on the client?

Then use the recovery key whenever we want to fetch all of our device-scoped keys again.

What does someone's "identity being reset" mean in Element?

Just for my own knowledge, I assume the "identity reset" language in Element corresponds device verification stuff? which is a separate topic ⏩

If it's from EX, then it's about creating a new user identity, i.e. cross-signing keys. It also deletes secret storage and the backup. It basically gets you into a new initial state.

[MadLittleMods]

@poljar Thanks for taking the time to go over this and supply some great nuggets of knowledge!

I've added a Definition of done section to the issue description outlining some of the tasks mentioned in the discussion here so far. Feel free to edit and push back with better ideas (some things were added a bit proactively without your direct buy-in).

Better errors

By the nature of the API shape, this is easy to mess up. You have to know that the necessary keys are stored in the CryptoStore which is part of the persistent storage backend configured when creating the client, not when using Client::restore_session().

At the very least, it feels like there should be an error if someone tries to use Client::restore_session() and the keys it's expecting aren't available in the CryptoStore if we're working with that assumption anyway.

Well it can't know that it expects keys if they aren't persisted. If you do use the wrong device ID and there is a store configured, it will yell.

Confirmed that when someone has persistent storage configured and uses Client::restore_session(), the matrix-rust-sdk detects the wrong device ID:

Error: Failed restoring session for user

Caused by:
    0: failed to read or write to the crypto store the account in the store doesn't match the account in the constructor: expected @test-user:server:VGPAWUXCGQ, got @test-user:server:WRONG_DEVICE_ID
    1: the account in the store doesn't match the account in the constructor: expected @test-user:server:VGPAWUXCGQ, got @test-user:server:WRONG_DEVICE_ID

The matrix-rust-sdk also detects if you have existing data in your persistent storage and try to login or register:

Error: Failed logging in user

Caused by:
    0: failed to read or write to the crypto store the account in the store doesn't match the account in the constructor: expected @test-user:server:MCOTIOYKXS, got @test-user:server:VYAQYSMCXU
    1: the account in the store doesn't match the account in the constructor: expected @test-user:server:MCOTIOYKXS, got @test-user:server:VYAQYSMCXU

Missing: We should also be able to detect when the persistent storage isn't configured (i.e. no keys in the crypto storage) and someone uses Client::restore_session(). Currently this isn't caught and would have saved me in my use case. This check would only happen if the e2e-encryption feature is enabled.

I think this covers all of the cases where a Matrix client would be left spinning trying to upload keys forever. If there are more, we should try to plug those holes. The only other cases I can think of involve tampering of the storage which as you mention, we can't tell if the expected keys are expected if we have nothing persisted to compare against.

Perhaps, there should also be some sort of conflict detection in any case so we back-off on trying to upload new keys which conflict. But maybe the best we could do is that /_matrix/client/v3/keys/upload -> 400 Bad Request triggers us to fetch the existing keys to compare and find the conflict. Feels like ideally, the spec should use 409 Conflict and/or a specific errcode for this scenario to make it more obvious.

Better API

To-preface everything: I think better errors would go a long way to solving this and preventing people from running with flawed setups.

I think what is missing is some higher level API where we manage the device ID and access token for users. Though it isn't particularly easy to do this while also having generic storage support where stores could persist multiple accounts.

You need to know the user/device you want to load from the store if you support multiple accounts in a single store.

But I think a specialized single account situation could be made simpler.

I'm not sure and can't picture what you have in mind. (I don't expect you to lay it out if you don't want to)

---

Overall, in terms of API feel, it seems like persisting Client::session() and using Client::restore_session(session) should be enough to handle getting my client in a usable state from before.

I understand it currently has a much more limited scope right now but perhaps it should be re-thought to also include device-related keys. At-least device_keys which don't change ever after logging in.

Things get a bit more tricky with the one_time_keys and fallback_keys that evolve over time and is what the persistent storage backend is currently handling. If I'm talking strictly for my own use case, having them stored in the default memory store is fine for me. The only missing piece here is removing/replacing the existing one_time_keys/fallback_keys when the client starts up and uploading new ones. Even the spec allows the client to discard keys: "A device that is trying to store too many private keys may discard keys starting with the oldest." (source). I'm not sure how this part of the matrix-rust-sdk API would look exactly or when keys are generated for the CryptoStore to make this easy. As a dumb-simple potential solution, we could have Client::restore_session(session, replace_otk_and_fallback_keys=true) but this forces everyone to think about this which isn't great (because the answer is false for every normal client) so it's not something I'm leaning towards.

As a little aside, configuring persistent storage on the client, also forces me into having an event and state cache which I don't care about in my use case. Although this could may be argued that I should use my own custom implementation of the persistent storage which is fine and understandable 👍

Can we store device_keys, one_time_keys, fallback_keys in the secret storage?

Theoretically yes, but there's no support for this, what you are asking for are basically dehydrated devices.

Good shout on the pre-existing proposed patterns for this kind of thing!

MSC3814: Dehydrated devices with SSSS

User identity reset

The user identities sign devices which establishes some form of web-of trust scheme for a given user. User identities verify each other and the implicitly trust the other user's devices.

If it's from EX, then it's about creating a new user identity, i.e. cross-signing keys. It also deletes secret storage and the backup. It basically gets you into a new initial state.

MSC4161: Crypto terminology for non-technical users - also helped clarify this better for me.

[poljar]

Missing: We should also be able to detect when the persistent storage isn't configured (i.e. no keys in the crypto storage) and someone uses Client::restore_session(). Currently this isn't caught and would have saved me in my use case. This check would only happen if the e2e-encryption feature is enabled.

The reason why this isn't caught is that it has some legitimate uses, i.e. you can steal your access token from element and do various things as long as you don't try to upload new E2EE-related keys.

This check would only happen if the e2e-encryption feature is enabled.

Right, that could work, good thinking.

Perhaps, there should also be some sort of conflict detection in any case so we back-off on trying to upload new keys which conflict. But maybe the best we could do is that /_matrix/client/v3/keys/upload -> 400 Bad Request triggers us to fetch the existing keys to compare and find the conflict. Feels like ideally, the spec should use 409 Conflict and/or a specific errcode for this scenario to make it more obvious.

We don't do this because we had some serious bugs that would cause this scenario and we didn't want any self-healing to be implemented as this would make it harder to figure out what's going wrong.

I'm not sure and can't picture what you have in mind. (I don't expect you to lay it out if you don't want to)

Something akin to a restore_from_storage() function that would attempt to get the Session from the configured store.

As a little aside, configuring persistent storage on the client, also forces me into having an event and state cache which I don't care about in my use case.

No, you can configure the individual persistent stores separately: https://matrix-org.github.io/matrix-rust-sdk/matrix_sdk/struct.ClientBuilder.html#method.store_config

Although this could may be argued that I should use my own custom implementation of the persistent storage which is fine and understandable 👍

I think that's exactly what you are looking for. My advice would be to implement a custom crypto store that only persists the Olm Account. Combining this with the restore_from_storage() functionality would lead to a pretty simple API for you use-case.

[MadLittleMods]

Better errors and docs

Glad that it seems we've landed on a plan to prevent this particular situation (better docs and errors) 👍

Key conflict detection

We don't do this because we had some serious bugs that would cause this scenario and we didn't want any self-healing to be implemented as this would make it harder to figure out what's going wrong.

I don't want to add self-healing but I would like to prevent an infinite loop of requests and provide feedback that the client is not operating correctly in some fashion. The feedback could be panicking or something better/more appropriate if possible.

Custom persistent storage backend

My advice would be to implement a custom crypto store that only persists the Olm Account

Implementing a custom persistent storage backend (specifically CryptoStore) that saves out matrix_sdk::crypto::olm::Account::pickle() -> matrix_sdk::crypto::olm::PickledAccount sounds like the right track to minimize things.

Per you advice:

you can configure the individual persistent stores separately: https://matrix-org.github.io/matrix-rust-sdk/matrix_sdk/struct.ClientBuilder.html#method.store_config

I think using the CryptoStore on its own gets us pretty far anyway 👍

let matrix_client_persistent_store_config =
    matrix_sdk::config::StoreConfig::new("main".to_string()).crypto_store(
        matrix_sdk::SqliteCryptoStore::open_with_config(
            matrix_sdk::SqliteStoreConfig::new(client_session_store_directory_path)
                .passphrase(None),
        )
        .await?,
    );

let matrix_client_builder = Client::builder()
    .http_client(http_client.clone())
    .homeserver_url(&tenant.client_url)
    .store_config(matrix_client_persistent_store_config);

Better API

Overall, any solution that separates the user session (what's currently stored from Client::session()) and crypto keys (what's currently stored in the CryptoStore) is going to be non-obvious to use and realize that you need both.

I don't know how to cleanly solve the problem of immutable session data (user_id, access_token, device_id, device_keys) vs mutable session data that changes over time (one_time_keys, fallback_keys).

At the very least, it should simply be one call where you provide both: restore_from_storage(session, store_config).

And then we need a way to prevent people from using Client::restore_session(session) whether that be deprecating, removing the API when the e2e-encryption feature is enabled, etc.