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.

Sign up for GitHub

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

Jump to bottom

Failures when sharing outbound session keys are swallowed #23792

Closed
richvdh opened this issue Nov 18, 2022 · 4 comments · Fixed by matrix-org/matrix-js-sdk#2962
Closed

Failures when sharing outbound session keys are swallowed #23792

richvdh opened this issue Nov 18, 2022 · 4 comments · Fixed by matrix-org/matrix-js-sdk#2962
Assignees
@poljar
Labels
A-E2EE O-Occasional Affects or can be seen by some users regularly or most users rarely S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect Team: Crypto Z-UISI Unable to decrypt errors

Comments

@richvdh
Copy link
Member

richvdh commented Nov 18, 2022 •
edited by pmaier1
Loading

Sometimes there is an error queuing the to-device messages to share a megolm session; for example:

2022-11-15T09:10:40.454Z I Created batch of to-device messages with txn id m1668503440454.99 for @Manu:matrix.org:LKJZLJSTCC,@Manu:matrix.org:PSMTLDOWQW,@Manu:matrix.org:QHXUARLSTR,@Manu:matrix.org:HCYZTMSXSO
2022-11-15T09:10:40.454Z E sendToDevice failed Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.
InvalidStateError: Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.
    at Worker.<anonymous> (https://develop.element.io/bundles/a20ea4fbdbb209900525/init.js:88547:25)
2022-11-15T09:10:40.454Z E encryptAndSendToDevices promises failed Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.
InvalidStateError: Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.
    at Worker.<anonymous> (https://develop.element.io/bundles/a20ea4fbdbb209900525/init.js:88547:25)
2022-11-15T09:10:40.454Z E failed to encryptAndSendToDevices Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.
InvalidStateError: Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.
    at Worker.<anonymous> (https://develop.element.io/bundles/a20ea4fbdbb209900525/init.js:88547:25)
2022-11-15T09:10:40.454Z E Failed to share megolm keys for WpOHNMd4I3K9glwFfyxo2nsfr9T1ZUDvsP79v4okTsM in !xlFGXqMroSeHUQLGdq:matrix.org (slice 1/6)
2022-11-15T09:10:40.454Z E Failed to ensure outbound session in !xlFGXqMroSeHUQLGdq:matrix.org Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.
InvalidStateError: Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.
    at Worker.<anonymous> (https://develop.element.io/bundles/a20ea4fbdbb209900525/init.js:88547:25)

However, these errors are just logged, rather than being fed back to the user.

Exactly why the IDBDatabase is complaining like this is something of a mystery to me (see also https://github.com/vector-im/element-web/issues/14174\), but I would argue it is better to show the user even a cryptic error than to let them send a message that nobody will be able to decrypt.
@richvdh richvdh added T-Defect S-Major Severely degrades major functionality or product features, with no satisfactory workaround O-Occasional Affects or can be seen by some users regularly or most users rarely Team: Crypto Z-UISI Unable to decrypt errors labels Nov 18, 2022
@richvdh richvdh mentioned this issue Nov 18, 2022
Open
82 tasks
@richvdh
Copy link
Member Author

richvdh commented Nov 18, 2022

One thing that might cause the IndexedDB database to fail this way is running Element in two tabs at once.

@richvdh richvdh mentioned this issue Nov 18, 2022
Closed
@poljar
Copy link
Contributor

poljar commented Nov 23, 2022

One thing that the logs from the original message don't indicate is that we say that we're ready to encrypt for the room, while this is obviously false:

2022-11-23T08:54:47.951Z E Failed to ensure outbound session in !xlFGXqMroSeHUQLGdq:matrix.org Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.
InvalidStateError: Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.
    at Worker.<anonymous> (https://develop.element.io/bundles/b2ddf1ed560f96309016/init.js:89141:25)
2022-11-23T08:54:47.951Z I Ready to encrypt events for !xlFGXqMroSeHUQLGdq:matrix.org

Propagating the error and not marking things as ready to encrypt seems like the wise choice here, unless we can figure out why the connection is closing and reopen it automagically.

If things don't get marked as ready to encrypt and we let users know that a failure happened, the user might will have the ability to retry the send which might then succeed. This would upgrade the silent UTD to a visible retry.

@poljar
Copy link
Contributor

poljar commented Nov 23, 2022

I think this is the place where the error gets swallowed even though callers of the method catch and log errors as well:

https://github.com/matrix-org/matrix-js-sdk/blob/b318a77ecef179a6fd288cdf32d3ff9c5e8ea989/src/crypto/algorithms/megolm.ts#L288-L292

@richvdh
Copy link
Member Author

richvdh commented Nov 23, 2022 •
edited
Loading

I think this is the place where the error gets swallowed even though callers of the method catch and log errors as well:

https://github.com/matrix-org/matrix-js-sdk/blob/b318a77ecef179a6fd288cdf32d3ff9c5e8ea989/src/crypto/algorithms/megolm.ts#L288-L292

Yes, indeed.

@poljar poljar self-assigned this Nov 23, 2022
@poljar poljar mentioned this issue Nov 24, 2022
Closed
@poljar poljar mentioned this issue Dec 9, 2022
Merged
@richvdh richvdh mentioned this issue Jan 12, 2023
Closed
su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this issue Jan 15, 2023
@su-ex
Merge tag 'v23.0.0' into sc
b9afe23 
* Process `m.room.encryption` events before emitting `RoomMember` events ([\matrix-org#2914](matrix-org#2914)). Fixes element-hq/element-web#23819.
* Don't expose `calls` on `GroupCall` ([\matrix-org#2941](matrix-org#2941)).
* Support MSC3391: Account data deletion ([\matrix-org#2967](matrix-org#2967)).
* Add a message ID on each to-device message ([\matrix-org#2938](matrix-org#2938)).
* Enable multiple users' power levels to be set at once ([\matrix-org#2892](matrix-org#2892)). Contributed by @GoodGuyMarco.
* Include pending events in thread summary and count again ([\matrix-org#2922](matrix-org#2922)). Fixes element-hq/element-web#23642.
* Make GroupCall work better with widgets ([\matrix-org#2935](matrix-org#2935)).
* Add method to get outgoing room key requests for a given event ([\matrix-org#2930](matrix-org#2930)).
* Fix messages loaded during initial fetch ending up out of order ([\matrix-org#2971](matrix-org#2971)). Fixes element-hq/element-web#23972.
* Fix #23919: Root message for new thread loaded from network ([\matrix-org#2965](matrix-org#2965)). Fixes element-hq/element-web#23919.
* Fix #23916: Prevent edits of the last message in a thread getting lost ([\matrix-org#2951](matrix-org#2951)). Fixes element-hq/element-web#23916 and element-hq/element-web#23942.
* Fix infinite loop when restoring cached read receipts ([\matrix-org#2963](matrix-org#2963)). Fixes element-hq/element-web#23951.
* Don't swallow errors coming from the shareSession call ([\matrix-org#2962](matrix-org#2962)). Fixes element-hq/element-web#23792.
* Make sure that MegolmEncryption.setupPromise always resolves  ([\matrix-org#2960](matrix-org#2960)).
* Do not calculate highlight notifs for threads unknown to the room ([\matrix-org#2957](matrix-org#2957)).
* Cache read receipts for unknown threads ([\matrix-org#2953](matrix-org#2953)).
* bugfix: sliding sync initial room timelines shouldn't notify ([\matrix-org#2933](matrix-org#2933)).
* Redo key sharing after own device verification ([\matrix-org#2921](matrix-org#2921)). Fixes element-hq/element-web#23333.
* Move updated threads to the end of the thread list ([\matrix-org#2923](matrix-org#2923)). Fixes element-hq/element-web#23876.
* Fix highlight notifications increasing when total notification is zero ([\matrix-org#2937](matrix-org#2937)). Fixes element-hq/element-web#23885.
* Fix synthesizeReceipt ([\matrix-org#2916](matrix-org#2916)). Fixes element-hq/element-web#23827 element-hq/element-web#23754 and element-hq/element-web#23847.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@poljar poljar

Labels
A-E2EE O-Occasional Affects or can be seen by some users regularly or most users rarely S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect Team: Crypto Z-UISI Unable to decrypt errors
Projects
None yet
Milestone
No milestone
3 participants
@poljar @richvdh @robintown