feat(crypto): Support EncryptionInfo for olm encrypted messages when using `add_event_handler` by BillCarsonFr · Pull Request #5099 · matrix-org/matrix-rust-sdk

BillCarsonFr · 2025-05-26T12:44:50Z

Main change

In event_handler/mod.rs there is a new method handle_sync_to_device_events(..) that will dispatch the to-device messages with the olm variant of EncryptionInfo.

The sync response struct SyncResponse now have pub to_device: Vec<ProcessedToDeviceEvent> instead of the old pub to_device: Vec<Raw<AnyToDeviceEvent>>

Context

It is now possible to get an Olm EncryptionInfo when register an event handler for to-device messages (previously the encryption_info was always None for to-device`.

        client.add_event_handler({
            move |ev: AnyToDeviceEvent, encryption_info: Option<EncryptionInfo>| {
                // If `encryption_info` is some, it was a successfully decrypted to-device message
                future::ready(())
            }
        });

Make it possible to make a difference between a plaintext and a successfully decrypted to-device message.
There is now a new variant of EncryptionInfo that will be passed to the to-device event handlers.

There might be a change that would be better to review alone 125aec6

=> I needed to move ProcessedToDeviceEvent from the crypto crate to the common crate. If not it was making it complex to compile with the e2e feature not enabled. This is the same reason why SyncTimelineEvent/UnableToDecryptReason/Encryption info are already in the common crate.

Based on #5074

Public API changes documented in changelogs (optional)

Signed-off-by:

codecov · 2025-06-06T07:46:57Z

Codecov Report

Attention: Patch coverage is 68.29268% with 13 lines in your changes missing coverage. Please review.

Project coverage is 85.13%. Comparing base (6ab11a0) to head (d92eceb).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...sdk-base/src/response_processors/e2ee/to_device.rs	12.50%	7 Missing ⚠️
...es/matrix-sdk-common/src/deserialized_responses.rs	75.00%	3 Missing ⚠️
...ates/matrix-sdk/src/test_utils/mocks/encryption.rs	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5099      +/-   ##
==========================================
- Coverage   85.14%   85.13%   -0.01%     
==========================================
  Files         329      329              
  Lines       36912    36939      +27     
==========================================
+ Hits        31429    31449      +20     
- Misses       5483     5490       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

BillCarsonFr · 2025-06-06T10:08:58Z

+                    false // Exclude events with no type or encrypted
+                }
+            })
+            .map(matrix_sdk_common::deserialized_responses::ProcessedToDeviceEvent::PlainText)


When there is no encryption, to-device are not processed by the olm machine. There is no UnProcessed variant of ProcessedToDeviceEvent (not sure it would make sense :/).
So anyhow, we can just ignore the encrypted to-device as they are useless. So we can just map to PlainText variant?
I wonder if we should at some point remove the optional e2e-encryption feature flag

Huh, but how are events we fail to decrypt treated? Why not treat them just like that?

andybalaam

Looks sensible to me. A couple of optional minor comments.

andybalaam · 2025-06-06T10:11:07Z

+        let captured_info: Arc<Mutex<Option<EncryptionInfo>>> = Arc::new(Mutex::new(None));
+
+        client.add_event_handler({
+            let captured = captured_event.clone();


I'd prefer this to be called captured_event too.

Thx @andybalam I updated the test 041e50d

andybalaam · 2025-06-06T10:12:01Z

+            let captured_info = captured_info.clone();
+            move |ev: AnyToDeviceEvent, encryption_info: Option<EncryptionInfo>| {
+                let mut captured_lock = captured.lock();
+                *captured_lock = Some(ev);


Can we lock and update in the same line of code? Might be nicer?

poljar

Alright, I think most of this looks good and makes sense.

I don't think we should completely filter out UTDs in case E2EE is disabled at compile time, the rest are small nits.

poljar · 2025-06-11T11:10:44Z

+                    false // Exclude events with no type or encrypted
+                }
+            })
+            .map(matrix_sdk_common::deserialized_responses::ProcessedToDeviceEvent::PlainText)


Huh, but how are events we fail to decrypt treated? Why not treat them just like that?

poljar · 2025-06-11T11:13:41Z

+            .field(
+                "to_device",
+                &DebugListOfRawEventsNoId(
+                    self.to_device.iter().map(|p| p.to_raw()).collect::<Vec<_>>().as_slice(),


Feels a bit expensive to clone the whole thing for a debug output, can't we have an as_raw() that will borrow the thing instead?

I did a similar thing to DebugListOfRawEventsNoId for list of processed to device 3eaaa30

poljar · 2025-06-11T11:33:23Z

        }

+        // let bob be aware of Alice keys in order to be able to decrypt custom
+        // to-device (the device keys check are not delayed for non-crypto to-device


I think they are in fact only delayed or rather deferred for m.room.key events since dropping those would lead to a lot of UTDs.

I clarified (I am not sure what is exactly done for key bundles, but no need to go in details here)

poljar

I think this is now good to go, we just need to fix the changelog entry.

To double check, this isn't a breaking change, right? Existing event handlers will continue on working, they'll just ignore the new EncryptionInfo field.

poljar · 2025-06-13T09:23:58Z

+- `Client::add_event_handler`: Set `Option<EncryptionInfo>` in `EventHandlerData` for to-device messages.
+  If the to-device message was encrypted, the `EncryptionInfo` will be set. If it is `None` the message was sent in clear.
+  ([#5099](https://github.com/matrix-org/matrix-rust-sdk/pull/5099))


This has been put in the wrong place, it's not part of the 0.12.0 release.

oh right, got hit by the merge. d92eceb

To double check, this isn't a breaking change, right? Existing event handlers will continue on working

Yes no change for existing event handlers.
Existing event handlers for to_device that requested the EncryptionInfo were always getting None, now they will get something if the event was encrypted.

poljar

Thanks, looks good now.

Should I just squash merge?

... which includes matrix-org/matrix-rust-sdk#5099, which moves `ProcessedToDeviceEvent` to matrix-sdk-common.

BillCarsonFr added 3 commits May 22, 2025 15:15

feat(crypto): Add EncryptionInfo to Decrypted to-device variant

f14ecc9

fixup: coverage, test deserialize new format

0c4a895

feat(crypto): Support EncryptionInfo for olm encrypted via event handler

c4cbcf9

This was referenced May 26, 2025

[ER-59] E2EE: Send media keys via to_device message element-hq/element-call#1281

Closed

feat(widget): Receive custom to-device messages in widgets in e2ee rooms #5116

Merged

Base automatically changed from valere/crypto/olm_encryption_info to main June 4, 2025 09:54

BillCarsonFr added 8 commits June 5, 2025 10:48

Merge branch 'main' into valere/crypto/receive_encrypted_to_device

fc4a97e

refactor: Move ProcessedToDeviceEvent to common crate

125aec6

fixup: when not(e2e-encryption) ignore encrypted to-device processing

d9137e9

fixup: Test wrongly expecting encrypted to-device

53aeeb7

fixup: flag dependent imports

290f963

Merge branch 'main' into valere/crypto/receive_encrypted_to_device

10a9688

fixup: merge error in crypto changelog added a double entry

7dd01ee

Merge branch 'main' into valere/crypto/receive_encrypted_to_device

566d935

BillCarsonFr marked this pull request as ready for review June 6, 2025 08:30

BillCarsonFr requested review from a team as code owners June 6, 2025 08:30

BillCarsonFr requested review from Hywan and andybalaam and removed request for a team June 6, 2025 08:30

BillCarsonFr commented Jun 6, 2025

View reviewed changes

andybalaam approved these changes Jun 6, 2025

View reviewed changes

review: Improve test, better var names

041e50d

Hywan removed their request for review June 6, 2025 13:02

BillCarsonFr added 2 commits June 6, 2025 15:07

update changelog

27bc725

Merge branch 'main' into valere/crypto/receive_encrypted_to_device

d705261

poljar self-requested a review June 10, 2025 13:13

poljar requested changes Jun 11, 2025

View reviewed changes

BillCarsonFr added 5 commits June 12, 2025 15:22

review: Better mapping to ProcessedToDeviceEvent when no e2e

7639807

review: avoid cloning for dbg format of ProcessedToDeviceEvent list

3eaaa30

review: fix olm -> Olm typo

f8a157f

review: clarify doc

2e2970c

Merge branch 'main' into valere/crypto/receive_encrypted_to_device

0ff9c99

BillCarsonFr requested a review from poljar June 12, 2025 14:42

poljar requested changes Jun 13, 2025

View reviewed changes

fixup! changelog merge problem

d92eceb

poljar approved these changes Jun 13, 2025

View reviewed changes

poljar merged commit 7126fc8 into main Jun 13, 2025
41 of 42 checks passed

poljar deleted the valere/crypto/receive_encrypted_to_device branch June 13, 2025 12:31

richvdh added a commit to matrix-org/matrix-sdk-crypto-wasm that referenced this pull request Jun 30, 2025

Bump sdk to 7126fc8a2

5a8aef8

... which includes matrix-org/matrix-rust-sdk#5099, which moves `ProcessedToDeviceEvent` to matrix-sdk-common.

Conversation

BillCarsonFr commented May 26, 2025 • edited by poljar Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Main change

Context

Uh oh!

codecov Bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andybalaam left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poljar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poljar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poljar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

BillCarsonFr commented May 26, 2025 •

edited by poljar

Loading

codecov Bot commented Jun 6, 2025 •

edited

Loading