Fix/crypto audit hardening

src-tauri/Cargo.toml

@@ -70,6 +70,7 @@ matrix-sdk = { version = "0.18", default-features = false, features = [
 # verification, so the engine needs it for parity with the wasm backend.
 matrix-sdk-crypto = { version = "0.18", features = [
   "qrcode",
+  "automatic-room-key-forwarding",
   "experimental-push-secrets",
 ], optional = true }
 # `bundled` compiles SQLite from source. Android's NDK ships no libsqlite3 to link

src-tauri/src/matrix_crypto/backup.rs

@@ -66,10 +66,11 @@ fn exportable_keys(sessions: Vec<BackedUpSession>) -> (Vec<ExportedRoomKey>, usi
     (exported, skipped)
 }
 
-fn import_result(result: RoomKeyImportResult) -> Value {
+fn import_result(result: RoomKeyImportResult, skipped: usize) -> Value {
     json!({
         "importedCount": result.imported_count,
         "totalCount": result.total_count,
+        "skippedCount": skipped,
         "keys": result.keys,
     })
 }
@@ -185,7 +186,7 @@ async fn handle(machine: &OlmMachine, method: &str, args: &Value) -> Result<Opti
                 .import_room_keys(exported, Some(&backup_version), ignore_progress)
                 .await
                 .map_err(|e| format!("importBackedUpRoomKeys failed: {e}"))?;
-            import_result(result)
+            import_result(result, skipped)
         }
         "importExportedRoomKeys" => {
             let keys: Vec<ExportedRoomKey> = serde_json::from_str(&str_arg(args, method, "keys")?)
@@ -195,7 +196,7 @@ async fn handle(machine: &OlmMachine, method: &str, args: &Value) -> Result<Opti
                 .import_exported_room_keys(keys, ignore_progress)
                 .await
                 .map_err(|e| format!("importExportedRoomKeys failed: {e}"))?;
-            import_result(result)
+            import_result(result, 0)
         }
         "exportRoomKeys" => {
             let keys = machine
@@ -217,6 +218,7 @@ async fn handle(machine: &OlmMachine, method: &str, args: &Value) -> Result<Opti
 
 #[cfg(test)]
 mod tests {
+    use matrix_sdk_crypto::store::types::BackupDecryptionKey;
     use serde_json::json;
 
     use super::*;
@@ -270,6 +272,111 @@ mod tests {
         assert_eq!(parsed[0].session_id, "session-1");
     }
 
+    async fn machine() -> OlmMachine {
+        let user: &matrix_sdk::ruma::UserId = "@backup:example.org".try_into().unwrap();
+        OlmMachine::new(user, "BACKUPDEV".into()).await
+    }
+
+    #[tokio::test]
+    async fn a_saved_decryption_key_reads_back_in_the_wasm_shape() {
+        let machine = machine().await;
+        let key = BackupDecryptionKey::new().to_base64();
+
+        let saved = invoke(
+            &machine,
+            "saveBackupDecryptionKey",
+            &json!({ "decryptionKey": key, "version": "7" }),
+        )
+        .await
+        .unwrap()
+        .unwrap();
+        assert_eq!(saved, Value::Null);
+
+        let keys = invoke(&machine, "getBackupKeys", &json!({}))
+            .await
+            .unwrap()
+            .unwrap();
+        assert_eq!(keys["className"], "BackupKeys");
+        assert_eq!(keys["backupVersion"], "7");
+        assert_eq!(keys["decryptionKeyBase64"], key);
+    }
+
+    #[tokio::test]
+    async fn an_empty_store_reports_nulls_rather_than_erroring() {
+        let machine = machine().await;
+
+        let keys = invoke(&machine, "getBackupKeys", &json!({}))
+            .await
+            .unwrap()
+            .unwrap();
+        assert!(keys["backupVersion"].is_null());
+        assert!(keys["decryptionKeyBase64"].is_null());
+
+        assert_eq!(
+            invoke(&machine, "isBackupEnabled", &json!({}))
+                .await
+                .unwrap()
+                .unwrap(),
+            json!(false)
+        );
+        assert_eq!(
+            invoke(&machine, "backupVersion", &json!({}))
+                .await
+                .unwrap()
+                .unwrap(),
+            Value::Null
+        );
+    }
+
+    #[tokio::test]
+    async fn enabling_a_backup_makes_its_version_readable() {
+        let machine = machine().await;
+        let public = BackupDecryptionKey::new()
+            .megolm_v1_public_key()
+            .to_base64();
+
+        invoke(
+            &machine,
+            "enableBackupV1",
+            &json!({ "publicKeyBase64": public, "version": "3" }),
+        )
+        .await
+        .unwrap()
+        .unwrap();
+
+        assert_eq!(
+            invoke(&machine, "isBackupEnabled", &json!({}))
+                .await
+                .unwrap()
+                .unwrap(),
+            json!(true)
+        );
+        assert_eq!(
+            invoke(&machine, "backupVersion", &json!({}))
+                .await
+                .unwrap()
+                .unwrap(),
+            json!("3")
+        );
+    }
+
+    #[tokio::test]
+    async fn a_malformed_decryption_key_is_an_error_that_does_not_leak_it() {
+        let machine = machine().await;
+
+        let result = invoke(
+            &machine,
+            "saveBackupDecryptionKey",
+            &json!({ "decryptionKey": "notBase64", "version": "1" }),
+        )
+        .await;
+
+        let Some(Err(message)) = result else {
+            panic!("expected an error");
+        };
+        assert!(!message.contains("notBase64"), "{message}");
+    }
+
     const VALID_CURVE_KEY: &str = "KyHFkVuB9MFbEkiCw+idNHKbiM8r3cWpNNPdyHkFeHY";
 
     fn valid_session_key() -> String {

src-tauri/src/matrix_crypto/cross_signing.rs

@@ -39,11 +39,21 @@ pub async fn invoke(
         "importSecretsBundle" => import_secrets_bundle(machine, args).await,
 
         "pushSecretToVerifiedDevices" => push_secret(machine, args).await,
+        "requestMissingSecretsIfNeeded" => request_missing_secrets(machine).await,
 
         _ => return None,
     })
 }
 
+async fn request_missing_secrets(machine: &OlmMachine) -> Result<Value, String> {
+    let requested = machine
+        .query_missing_secrets_from_other_sessions()
+        .await
+        .map_err(|e| format!("requestMissingSecretsIfNeeded failed: {e}"))?;
+
+    Ok(Value::Bool(requested))
+}
+
 async fn bootstrap(machine: &OlmMachine, args: &Value) -> Result<Value, String> {
     let reset = args
         .get("reset")

src-tauri/src/matrix_crypto/devices.rs

@@ -22,7 +22,7 @@ fn device_id(args: &Value, method: &str) -> Result<OwnedDeviceId, String> {
 fn timeout(args: &Value) -> Option<Duration> {
     args.get("timeoutSecs")
         .and_then(Value::as_f64)
-        .map(Duration::from_secs_f64)
+        .and_then(|secs| Duration::try_from_secs_f64(secs).ok())
 }
 
 fn signatures_json(signatures: &Signatures, method: &str) -> Result<Value, String> {
@@ -195,7 +195,7 @@ fn query_keys_for_users(machine: &OlmMachine, args: &Value, method: &str) -> Res
 
 async fn verify_device(machine: &OlmMachine, args: &Value, method: &str) -> Result<Value, String> {
     let Some(device) = device_for(machine, args, method).await? else {
-        return Ok(Value::Null);
+        return Err(format!("{method}: unknown device"));
     };
     let request = device
         .verify()
@@ -214,7 +214,7 @@ async fn set_local_trust(
         .and_then(Value::as_i64)
         .ok_or_else(|| format!("{method}: missing numeric argument `trustState`"))?;
     let Some(device) = device_for(machine, args, method).await? else {
-        return Ok(Value::Null);
+        return Err(format!("{method}: unknown device"));
     };
     device
         .set_local_trust(LocalTrust::from(trust))
@@ -327,3 +327,56 @@ pub async fn invoke(
         _ => return None,
     })
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{invoke, timeout};
+    use matrix_sdk_crypto::OlmMachine;
+    use serde_json::json;
+    use std::time::Duration;
+
+    async fn machine() -> OlmMachine {
+        let user: &matrix_sdk::ruma::UserId = "@me:example.org".try_into().unwrap();
+        OlmMachine::new(user, "MYDEVICE".into()).await
+    }
+
+    #[tokio::test]
+    async fn verifying_an_unknown_device_is_an_error() {
+        let machine = machine().await;
+        let args = json!({ "userId": "@me:example.org", "deviceId": "NOSUCHDEVICE" });
+
+        let result = invoke(&machine, "device.verify", &args).await;
+
+        assert!(matches!(result, Some(Err(_))));
+    }
+
+    #[tokio::test]
+    async fn trusting_an_unknown_device_is_an_error() {
+        let machine = machine().await;
+        let args = json!({
+            "userId": "@me:example.org",
+            "deviceId": "NOSUCHDEVICE",
+            "trustState": 1,
+        });
+
+        let result = invoke(&machine, "device.setLocalTrust", &args).await;
+
+        assert!(matches!(result, Some(Err(_))));
+    }
+
+    #[test]
+    fn an_unusable_timeout_is_ignored_rather_than_panicking() {
+        for secs in [-1.0, f64::NAN, f64::INFINITY, 1e300] {
+            assert_eq!(timeout(&json!({ "timeoutSecs": secs })), None);
+        }
+    }
+
+    #[test]
+    fn a_usable_timeout_is_kept() {
+        assert_eq!(
+            timeout(&json!({ "timeoutSecs": 10.0 })),
+            Some(Duration::from_secs(10))
+        );
+        assert_eq!(timeout(&json!({})), None);
+    }
+}

src-tauri/src/matrix_crypto/dispatch.rs

@@ -21,7 +21,7 @@ use serde_json::{json, Value};
 use matrix_sdk::deserialized_responses::{DeviceLinkProblem, VerificationLevel};
 use matrix_sdk_crypto::MegolmError;
 
-use super::args::{caller_decryption_settings, decryption_settings, room_id, str_arg};
+use super::args::{caller_decryption_settings, room_id, str_arg};
 use super::requests::{mark_request_sent, outgoing_requests};
 use super::wasm_enums::processed_to_device_event_type;
 
@@ -76,7 +76,7 @@ enum ToDeviceEncryptionInfoSnapshot {
 fn processed_to_device_event_json(
     event: &ProcessedToDeviceEvent,
     verification_request: Option<Value>,
-) -> Result<Value, String> {
+) -> Option<Value> {
     let raw_event = event.as_raw().json().get();
 
     let snapshot = match event {
@@ -88,10 +88,10 @@ fn processed_to_device_event_json(
                     curve25519_public_key_base64,
                 } => curve25519_public_key_base64.as_str(),
                 _ => {
-                    return Err(
-                        "receiveSyncChanges: decrypted to-device event did not use Olm v1"
-                            .to_owned(),
-                    )
+                    log::warn!(
+                        "Dropping incoming to-device event with unrecognised encryption_info"
+                    );
+                    return None;
                 }
             };
 
@@ -132,12 +132,11 @@ fn processed_to_device_event_json(
         }
     };
 
-    let mut value = serde_json::to_value(snapshot)
-        .map_err(|e| format!("receiveSyncChanges: failed to serialize processed event: {e}"))?;
+    let mut value = serde_json::to_value(snapshot).ok()?;
     if let Some(request) = verification_request {
         value["verificationRequest"] = request;
     }
-    Ok(value)
+    Some(value)
 }
 
 mod decryption_error_code {
@@ -270,44 +269,44 @@ pub async fn invoke(machine: &OlmMachine, method: &str, args: Value) -> Result<V
                 })
                 .unwrap_or_default();
 
-            let fallback_keys: Vec<OneTimeKeyAlgorithm> = args
+            let fallback_keys: Option<Vec<OneTimeKeyAlgorithm>> = args
                 .get("unusedFallbackKeys")
                 .and_then(Value::as_array)
                 .map(|keys| {
                     keys.iter()
                         .filter_map(Value::as_str)
                         .map(OneTimeKeyAlgorithm::from)
                         .collect()
-                })
-                .unwrap_or_default();
+                });
 
             let (processed, _room_keys) = machine
                 .receive_sync_changes(
                     EncryptionSyncChanges {
                         to_device_events,
                         changed_devices: &device_lists,
                         one_time_keys_counts: &key_counts,
-                        unused_fallback_keys: Some(&fallback_keys),
+                        unused_fallback_keys: fallback_keys.as_deref(),
                         next_batch_token: args
                             .get("nextBatchToken")
                             .and_then(Value::as_str)
                             .map(str::to_owned),
                     },
-                    &decryption_settings(),
+                    &caller_decryption_settings(&args),
                 )
                 .await
                 .map_err(|e| format!("receiveSyncChanges failed: {e}"))?;
 
-            processed
-                .iter()
-                .map(|event| {
-                    processed_to_device_event_json(
-                        event,
-                        verification_request_snapshot(machine, event),
-                    )
-                })
-                .collect::<Result<Vec<_>, _>>()
-                .map(Value::Array)
+            Ok(Value::Array(
+                processed
+                    .iter()
+                    .filter_map(|event| {
+                        processed_to_device_event_json(
+                            event,
+                            verification_request_snapshot(machine, event),
+                        )
+                    })
+                    .collect(),
+            ))
         }
 
         "outgoingRequests" => outgoing_requests(machine).await,

src-tauri/src/matrix_crypto/events.rs

@@ -59,11 +59,14 @@ pub fn spawn(
             let (app, account) = (app.clone(), account.clone());
             async move {
                 while let Some(update) = room_keys.next().await {
-                    // Lagging drops updates rather than ending the stream; js-sdk
-                    // recovers on the next key or a retry, so keep listening.
-                    if let Ok(keys) = update {
-                        let payload = Value::Array(keys.iter().map(room_key_json).collect());
-                        emit(app.clone(), ROOM_KEYS_RECEIVED, account.clone(), payload);
+                    match update {
+                        Ok(keys) => {
+                            let payload = Value::Array(keys.iter().map(room_key_json).collect());
+                            emit(app.clone(), ROOM_KEYS_RECEIVED, account.clone(), payload);
+                        }
+                        Err(error) => {
+                            log::warn!("room-key stream lagged, {error}; some events may stay undecryptable until restart");
+                        }
                     }
                 }
             }