CVE-2024-34353: Insufficient Information (NVD-CWE-noinfo)
value: The matrix-sdk-crypto crate, part of the Matrix Rust SDK project, is an implementation of a Matrix end-to-end encryption state machine in Rust. In Matrix, the server-side `key backup` stores encrypted copies of Matrix message keys. This facilitates key sharing between a user's devices and provides a redundant copy in case all devices are lost. The key backup uses asymmetric
cryptography, with each server-side key backup assigned a unique public-private key pair. Due to a logic bug introduced in commit 71136e44c03c79f80d6d1a2446673bc4d53a2067, matrix-sdk-crypto version 0.7.0 will sometimes log the private part of the backup key pair to Rust debug logs (using the `tracing` crate). This issue has been resolved in matrix-sdk-crypto version 0.7.1. No known workarounds are available.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-05-14T15:38Z
Repository: https://github.com/matrix-org/matrix-rust-sdk
Language: Rust
Files changed: crates/matrix-sdk-crypto/src/backups/mod.rs, crates/matrix-sdk-crypto/src/machine.rs
Commit date: 2024-05-13 12:31:26 +0200
Commit message: fix(crypto): Avoid incorrect usage of private backup key

This fixes instances of key backup corruption and prevents inadvertently
logging the private backup key to the logs.

Diff: diff --git a/crates/matrix-sdk-crypto/src/backups/mod.rs b/crates/matrix-sdk-crypto/src/backups/mod.rs
index cdefd5218..d42cf0049 100644
--- a/crates/matrix-sdk-crypto/src/backups/mod.rs
+++ b/crates/matrix-sdk-crypto/src/backups/mod.rs
@@ -624,12 +624,14 @@ mod tests {
     use matrix_sdk_test::async_test;
     use ruma::{device_id, room_id, user_id, CanonicalJsonValue, DeviceId, RoomId, UserId};
     use serde_json::json;
 
     use crate::{
-        olm::BackedUpRoomKey, store::BackupDecryptionKey, types::RoomKeyBackupInfo, OlmError,
-        OlmMachine,
+        olm::BackedUpRoomKey,
+        store::{BackupDecryptionKey, Changes, CryptoStore, MemoryStore},
+        types::RoomKeyBackupInfo,
+        OlmError, OlmMachine,
     };
 
     fn room_key() -> BackedUpRoomKey {
         let json = json!({
             "algorithm": "m.megolm.v1.aes-sha2",
@@ -848,6 +850,35 @@ mod tests {
 
         let result = backup_machine.verify_backup(backup_info, false).await.unwrap();
 
         assert!(result.trusted());
     }
+
+    #[async_test]
+    async fn test_fix_backup_key_mismatch() {
+        let store = MemoryStore::new();
+
+        let backup_decryption_key = BackupDecryptionKey::new().unwrap();
+
+        store
+            .save_changes(Changes {
+                backup_decryption_key: Some(backup_decryption_key.clone()),
+                backup_version: Some("1".to_owned()),
+                ..Default::default()
+            })
+            .await
+            .unwrap();
+
+        // Create the machine using `with_store` and without a call to enable_backup_v1,
+        // like regenerate_olm would do
+        let alice = OlmMachine::with_store(alice_id(), alice_device_id(), store).await.unwrap();
+
+        let binding = alice.backup_machine().backup_key.read().await;
+        let machine_backup_key = binding.as_ref().unwrap();
+
+        assert_eq!(
+            machine_backup_key.to_base64(),
+            backup_decryption_key.megolm_v1_public_key().to_base64(),
+            "The OlmMachine loaded the wrong backup key."
+        );
+    }
 }
diff --git a/crates/matrix-sdk-crypto/src/machine.rs b/crates/matrix-sdk-crypto/src/machine.rs
index 12995e3cd..72a7e2bca 100644
--- a/crates/matrix-sdk-crypto/src/machine.rs
+++ b/crates/matrix-sdk-crypto/src/machine.rs
@@ -311,17 +311,20 @@ impl OlmMachine {
                 debug!("Creating an empty cross signing identity stub");
                 PrivateCrossSigningIdentity::empty(user_id)
             }
         };
 
+        // FIXME: This is a workaround for `regenerate_olm` clearing the backup
+        // state. Ideally, backups should not get automatically enabled since
+        // the `OlmMachine` doesn't get enough info from the homeserver for this
+        // to work reliably.
         let saved_keys = store.load_backup_keys().await?;
         let maybe_backup_key = saved_keys.decryption_key.and_then(|k| {
             if let Some(version) = saved_keys.backup_version {
-                MegolmV1BackupKey::from_base64(&k.to_base64()).ok().map(|k| {
-                    k.set_version(version);
-                    k
-                })
+                let megolm_v1_backup_key = k.megolm_v1_public_key();
+                megolm_v1_backup_key.set_version(version);
+                Some(megolm_v1_backup_key)
             } else {
                 None
             }
         });
 
@@ -2167,12 +2170,14 @@ pub(crate) mod tests {
 
     use super::{testing::response_from_file, CrossSigningBootstrapRequests};
     use crate::{
         error::EventError,
         machine::{EncryptionSyncChanges, OlmMachine},
-        olm::{InboundGroupSession, OutboundGroupSession, VerifyJson},
-        store::Changes,
+        olm::{
+            BackedUpRoomKey, ExportedRoomKey, InboundGroupSession, OutboundGroupSession, VerifyJson,
+        },
+        store::{BackupDecryptionKey, Changes, CryptoStore, MemoryStore, RoomSettings},
         types::{
             events::{
                 room::encrypted::{EncryptedToDeviceEvent, ToDeviceEncryptedEventContent},
                 room_key_withheld::{RoomKeyWithheldContent, WithheldCode},
                 ToDeviceEvent,
@@ -4069,6 +4074,67 @@ pub(crate) mod tests {
         }
 
         // The waiting should successfully complete.
         wait.await.unwrap();
     }
+
+    #[async_test]
+    async fn test_fix_incorrect_usage_of_backup_key_causing_decryption_errors() {
+        let store = MemoryStore::new();
+
+        let backup_decryption_key = BackupDecryptionKey::new().unwrap();
+
+        store
+            .save_changes(Changes {
+                backup_decryption_key: Some(backup_decryption_key.clone()),
+                backup_version: Some("1".to_owned()),
+                ..Default::default()
+            })
+            .await
+            .unwrap();
+
+        // Some valid key data
+        let data = json!({
+           "algorithm": "m.megolm.v1.aes-sha2",
+           "room_id": "!room:id",
+           "sender_key": "FOvlmz18LLI3k/llCpqRoKT90+gFF8YhuL+v1YBXHlw",
+           "session_id": "/2K+V777vipCxPZ0gpY9qcpz1DYaXwuMRIu0UEP0Wa0",
+           "session_key": "AQAAAAAclzWVMeWBKH+B/WMowa3rb4ma3jEl6n5W4GCs9ue65CruzD3ihX+85pZ9hsV9Bf6fvhjp76WNRajoJYX0UIt7aosjmu0i+H+07hEQ0zqTKpVoSH0ykJ6stAMhdr6Q4uW5crBmdTTBIsqmoWsNJZKKoE2+ldYrZ1lrFeaJbjBIY/9ivle++74qQsT2dIKWPanKc9Q2Gl8LjESLtFBD9Fmt",
+           "sender_claimed_keys": {
+               "ed25519": "F4P7f1Z0RjbiZMgHk1xBCG3KC4/Ng9PmxLJ4hQ13sHA"
+           },
+           "forwarding_curve25519_key_chain": ["DBPC2zr6c9qimo9YRFK3RVr0Two/I6ODb9mbsToZN3Q", "bBc/qzZFOOKshMMT+i4gjS/gWPDoKfGmETs9yfw9430"]
+        });
+
+        let backed_up_room_key: BackedUpRoomKey = serde_json::from_value(data).unwrap();
+
+        // Create the machine using `with_store` and without a call to enable_backup_v1,
+        // like regenerate_olm would do
+        let alice = OlmMachine::with_store(user_id(), alice_device_id(), store).await.unwrap();
+
+        let exported_key = ExportedRoomKey::from_backed_up_room_key(
+            room_id!("!room:id").to_owned(),
+            "/2K+V777vipCxPZ0gpY9qcpz1DYaXwuMRIu0UEP0Wa0".into(),
+            backed_up_room_key,
+        );
+
+        alice.store().import_exported_room_keys(vec![exported_key], |_, _| {}).await.unwrap();
+
+        let (_, request) = alice.backup_machine().backup().await.unwrap().unwrap();
+
+        let key_backup_data = request.rooms[&room_id!("!room:id").to_owned()]
+            .sessions
+            .get("/2K+V777vipCxPZ0gpY9qcpz1DYaXwuMRIu0UEP0Wa0")
+            .unwrap()
+            .deserialize()
+            .unwrap();
+
+        let ephemeral = key_backup_data.session_data.ephemeral.encode();
+        let ciphertext = key_backup_data.session_data.ciphertext.encode();
+        let mac = key_backup_data.session_data.mac.encode();
+
+        // Prior to the fix for GHSA-9ggc-845v-gcgv, this would produce a `Mac(MacError)`
+        backup_decryption_key
+            .decrypt_v1(&ephemeral, &mac, &ciphertext)
+            .expect("The backed up key should be decrypted successfully");
+    }
 }

Vulnerable code:         olm::BackedUpRoomKey, store::BackupDecryptionKey, types::RoomKeyBackupInfo, OlmError,
        OlmMachine,
                MegolmV1BackupKey::from_base64(&k.to_base64()).ok().map(|k| {
                    k.set_version(version);
                    k
                })
        olm::{InboundGroupSession, OutboundGroupSession, VerifyJson},
        store::Changes,
Fixed code:         olm::BackedUpRoomKey,
        store::{BackupDecryptionKey, Changes, CryptoStore, MemoryStore},
        types::RoomKeyBackupInfo,
        OlmError, OlmMachine,

    #[async_test]
    async fn test_fix_backup_key_mismatch() {
        let store = MemoryStore::new();

        let backup_decryption_key = BackupDecryptionKey::new().unwrap();

        store
            .save_changes(Changes {
                backup_decryption_key: Some(backup_decryption_key.clone()),
                backup_version: Some("1".to_owned()),
                ..Default::default()
            })
            .await
            .unwrap();

        // Create the machine using `with_store` and without a call to enable_backup_v1,
        // like regenerate_olm would do
        let alice = OlmMachine::with_store(alice_id(), alice_device_id(), store).await.unwrap();

        let binding = alice.backup_machine().backup_key.read().await;
        let machine_backup_key = binding.as_ref().unwrap();

        assert_eq!(
            machine_backup_key.to_base64(),
            backup_decryption_key.megolm_v1_public_key().to_base64(),
            "The OlmMachine loaded the wrong backup key."
        );
    }
        // FIXME: This is a workaround for `regenerate_olm` clearing the backup
        // state. Ideally, backups should not get automatically enabled since
        // the `OlmMachine` doesn't get enough info from the homeserver for this
        // to work reliably.
                let megolm_v1_backup_key = k.megolm_v1_public_key();
                megolm_v1_backup_key.set_version(version);
                Some(megolm_v1_backup_key)
        olm::{
            BackedUpRoomKey, ExportedRoomKey, InboundGroupSession, OutboundGroupSession, VerifyJson,
        },
        store::{BackupDecryptionKey, Changes, CryptoStore, MemoryStore, RoomSettings},

    #[async_test]
    async fn test_fix_incorrect_usage_of_backup_key_causing_decryption_errors() {
        let store = MemoryStore::new();

        let backup_decryption_key = BackupDecryptionKey::new().unwrap();

        store
            .save_changes(Changes {
                backup_decryption_key: Some(backup_decryption_key.clone()),
                backup_version: Some("1".to_owned()),
                ..Default::default()
            })
            .await
            .unwrap();

        // Some valid key data
        let data = json!({
           "algorithm": "m.megolm.v1.aes-sha2",
           "room_id": "!room:id",
           "sender_key": "FOvlmz18LLI3k/llCpqRoKT90+gFF8YhuL+v1YBXHlw",
           "session_id": "/2K+V777vipCxPZ0gpY9qcpz1DYaXwuMRIu0UEP0Wa0",
           "session_key": "AQAAAAAclzWVMeWBKH+B/WMowa3rb4ma3jEl6n5W4GCs9ue65CruzD3ihX+85pZ9hsV9Bf6fvhjp76WNRajoJYX0UIt7aosjmu0i+H+07hEQ0zqTKpVoSH0ykJ6stAMhdr6Q4uW5crBmdTTBIsqmoWsNJZKKoE2+ldYrZ1lrFeaJbjBIY/9ivle++74qQsT2dIKWPanKc9Q2Gl8LjESLtFBD9Fmt",
           "sender_claimed_keys": {
               "ed25519": "F4P7f1Z0RjbiZMgHk1xBCG3KC4/Ng9PmxLJ4hQ13sHA"
           },
           "forwarding_curve25519_key_chain": ["DBPC2zr6c9qimo9YRFK3RVr0Two/I6ODb9mbsToZN3Q", "bBc/qzZFOOKshMMT+i4gjS/gWPDoKfGmETs9yfw9430"]
        });

        let backed_up_room_key: BackedUpRoomKey = serde_json::from_value(data).unwrap();

        // Create the machine using `with_store` and without a call to enable_backup_v1,
        // like regenerate_olm would do
        let alice = OlmMachine::with_store(user_id(), alice_device_id(), store).await.unwrap();

        let exported_key = ExportedRoomKey::from_backed_up_room_key(
            room_id!("!room:id").to_owned(),
            "/2K+V777vipCxPZ0gpY9qcpz1DYaXwuMRIu0UEP0Wa0".into(),
            backed_up_room_key,
        );

        alice.store().import_exported_room_keys(vec![exported_key], |_, _| {}).await.unwrap();

        let (_, request) = alice.backup_machine().backup().await.unwrap().unwrap();

        let key_backup_data = request.rooms[&room_id!("!room:id").to_owned()]
            .sessions
            .get("/2K+V777vipCxPZ0gpY9qcpz1DYaXwuMRIu0UEP0Wa0")
            .unwrap()
            .deserialize()
            .unwrap();

        let ephemeral = key_backup_data.session_data.ephemeral.encode();
        let ciphertext = key_backup_data.session_data.ciphertext.encode();
        let mac = key_backup_data.session_data.mac.encode();

        // Prior to the fix for GHSA-9ggc-845v-gcgv, this would produce a `Mac(MacError)`
        backup_decryption_key
            .decrypt_v1(&ephemeral, &mac, &ciphertext)
            .expect("The backed up key should be decrypted successfully");
    }