Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto: Log content of received m.room_key_withheld messages #3591

Merged
merged 7 commits into from
Jun 24, 2024

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Jun 20, 2024

Log the content of received m.room_key_withheld to-device messages, to fix #3524.

Resulting logging looks like:

12:41:27.593 DEBUG matrix_sdk_crypto::machine: Received a to-device event
    at /home/rav/work/matrix-rust-sdk/crates/matrix-sdk-crypto/src/machine.rs:1194
    in matrix_sdk_crypto::machine::receive_to_device_event with sender="@t:xps9320.sw1v.org" event_type="m.room_key.withheld" 
    in matrix_sdk_crypto::machine::receive_sync_changes
12:41:27.594 DEBUG matrix_sdk_crypto::machine: Processing `m.room_key.withheld` event
    event.content=MegolmV1AesSha2(Unverified(AnyWithheldContent { room_id: "!vFUWYRWcpYVFEHwNMQ:xps9320.sw1v.org", session_id: "yVEWjMz5X2dEsPYmZq1MGM3Fck6fuw4eglv/ARSsbL0", sender_key: "curve25519:O3DORf8A5YJCgxYVwpkKRu3OWd99t9Pn4+7cQjz2CXQ", from_device: Some("KSNXCKMOPV"), .. }))
    at /home/rav/work/matrix-rust-sdk/crates/matrix-sdk-crypto/src/machine.rs:870
    in matrix_sdk_crypto::machine::receive_to_device_event with sender="@t:xps9320.sw1v.org" event_type="m.room_key.withheld" 
    in matrix_sdk_crypto::machine::receive_sync_changes

Original description, now outdated:

This is a first attempt at fixing #3524. Obviously, it solves the reported problem, however there is a concern it might be too spammy.

Otherwise, we might have to be more selective here... maybe only log withheld events?

It shouldn't lead to logging any extra sensitive data - this is all stuff that is shared with the homeserver anyway.

@richvdh richvdh requested a review from a team as a code owner June 20, 2024 16:49
@richvdh richvdh requested review from Hywan and removed request for a team June 20, 2024 16:49
@richvdh richvdh linked an issue Jun 20, 2024 that may be closed by this pull request
@richvdh
Copy link
Member Author

richvdh commented Jun 20, 2024

Resulting logging looks like:

 DEBUG matrix_sdk_crypto::machine: Received a to-device event
    event=RoomKeyWithheld(ToDeviceEvent { sender: "@t:xps9320.sw1v.org", content: MegolmV1AesSha2(Unverified(AnyWithheldContent { room_id: "!vFUWYRWcpYVFEHwNMQ:xps9320.sw1v.org", session_id: "Z77mee5wEtJFA4FBuMZ5s0OSUFmYQhJ1F5vdEVAfSEI", sender_key: "curve25519:O3DORf8A5YJCgxYVwpkKRu3OWd99t9Pn4+7cQjz2CXQ", from_device: Some("KSNXCKMOPV"), .. })), other: {} })
    at /home/rav/work/matrix-rust-sdk/crates/matrix-sdk-crypto/src/machine.rs:1193
    in matrix_sdk_crypto::machine::receive_to_device_event with sender="@t:xps9320.sw1v.org" event_type="m.room_key.withheld" 
    in matrix_sdk_crypto::machine::receive_sync_changes

Copy link

codecov bot commented Jun 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.83%. Comparing base (e89659b) to head (5433a94).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3591   +/-   ##
=======================================
  Coverage   83.83%   83.83%           
=======================================
  Files         254      254           
  Lines       26167    26168    +1     
=======================================
+ Hits        21936    21937    +1     
  Misses       4231     4231           

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

Copy link
Contributor

@poljar poljar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not log contents of every event we receive.

crates/matrix-sdk-crypto/src/machine.rs Outdated Show resolved Hide resolved
@richvdh richvdh changed the title crypto: Log content of received to-device messages crypto: Log content of received m.room_key_withheld messages Jun 21, 2024
@richvdh richvdh requested a review from poljar June 21, 2024 11:46
Co-authored-by: Damir Jelić <[email protected]>
Signed-off-by: Richard van der Hoff <[email protected]>
@richvdh richvdh enabled auto-merge (squash) June 21, 2024 12:14
@richvdh richvdh disabled auto-merge June 21, 2024 16:24
@richvdh richvdh merged commit 1b48bf7 into main Jun 24, 2024
38 checks passed
@richvdh richvdh deleted the rav/log_todevice_messages branch June 24, 2024 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Log the session ID in m.room_key.withheld
2 participants