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

Avoid triggering decryption errors when decrypting redacted events #3004

Merged
merged 2 commits into from
Jan 3, 2023

Conversation

uhoreg
Copy link
Member

@uhoreg uhoreg commented Dec 21, 2022

Currently, when processing a redacted event, it tries to decrypt the redacted_because (#1589). However, since this often is not encrypted, the decryption will fail, and this failure will bubble up to the parent event. This patch simply uses the plain redacted_because if it is unable to decrypt it.

fixes element-hq/element-web#24084 and #1648


Here's what your changelog entry will look like:

🐛 Bug Fixes

Copy link
Contributor

@germain-gg germain-gg left a comment

Choose a reason for hiding this comment

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

Otherwise looks good to me

@@ -167,6 +167,35 @@ describe("Crypto", function () {

client.stopClient();
});

it("doesn't throw an error when attempting to decrypt a redacted event", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we also assess that redacted_because is correctly set? That sounds like an important edge to test against

Comment on lines 2877 to 2882
try {
const decryptedEvent = await this.decryptEvent(redactionEvent);
redactedBecause = decryptedEvent.clearEvent as IEvent;
} catch {
redactedBecause = event.getUnsigned().redacted_because!;
}
Copy link
Member

Choose a reason for hiding this comment

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

I feel like we should make this conditional on redactionEvent.isEncrypted(), and log a warning if we find it is encrypted but we can't decrypt.

@uhoreg uhoreg merged commit 6168ced into matrix-org:develop Jan 3, 2023
su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this pull request Jan 19, 2023
* Remove extensible events v1 field population on legacy events ([\matrix-org#3040](matrix-org#3040)).
* Improve hasUserReadEvent and getUserReadUpTo realibility with threads ([\matrix-org#3031](matrix-org#3031)). Fixes element-hq/element-web#24164.
* Remove video track when muting video ([\matrix-org#3028](matrix-org#3028)). Fixes element-hq/element-call#209.
* Make poll start event type available (PSG-962) ([\matrix-org#3034](matrix-org#3034)).
* Add alt event type matching in Relations model ([\matrix-org#3018](matrix-org#3018)).
* Remove usage of v1 Identity Server API ([\matrix-org#3003](matrix-org#3003)).
* Add `device_id` to `/account/whoami` types ([\matrix-org#3005](matrix-org#3005)).
* Implement MSC3912: Relation-based redactions ([\matrix-org#2954](matrix-org#2954)).
* Introduce a mechanism for using the rust-crypto-sdk ([\matrix-org#2969](matrix-org#2969)).
* Support MSC3391: Account data deletion ([\matrix-org#2967](matrix-org#2967)).
* Fix threaded cache receipt when event holds multiple receipts ([\matrix-org#3026](matrix-org#3026)).
* Fix false key requests after verifying new device ([\matrix-org#3029](matrix-org#3029)). Fixes element-hq/element-web#24167 and element-hq/element-web#23333.
* Avoid triggering decryption errors when decrypting redacted events ([\matrix-org#3004](matrix-org#3004)). Fixes element-hq/element-web#24084.
* bugfix: upload OTKs in sliding sync mode ([\matrix-org#3008](matrix-org#3008)).
* Apply edits discovered from sync after thread is initialised ([\matrix-org#3002](matrix-org#3002)). Fixes element-hq/element-web#23921.
* Sliding sync: Fix issue where no unsubs are sent when switching rooms ([\matrix-org#2991](matrix-org#2991)).
* Threads are missing from the timeline ([\matrix-org#2996](matrix-org#2996)). Fixes element-hq/element-web#24036.
* Close all streams when a call ends ([\matrix-org#2992](matrix-org#2992)). Fixes element-hq/element-call#742.
* Resume to-device message queue after resumed sync ([\matrix-org#2920](matrix-org#2920)). Fixes matrix-org/element-web-rageshakes#17170.
* Fix browser entrypoint ([\matrix-org#3051](matrix-org#3051)). Fixes matrix-org#3013.
* Fix failure to start in firefox private browser ([\matrix-org#3058](matrix-org#3058)). Fixes element-hq/element-web#24216.
* Correctly handle limited sync responses by resetting the thread timeline ([\matrix-org#3056](matrix-org#3056)). Fixes element-hq/element-web#23952.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

redacted events are shown as decryption errors
3 participants