incoming encrypted to-device messages can be lost when the application is restarted

[BillCarsonFr]

If you restart /upgrade (web) or the app is killed in background while processing to_devices coming down the sync, the non decrypted to_devices will be dropped and lost producing UISIs.

This is problem with atomicity of /sync. The sync token is persisted before all to_devices have been processed, so the next time the app launches it will use the saved token, thus ack'ing the previous to_device chunk that was received. In such case the app should start syncing with the previous token to force a redelivery of the initial to_device chunk (or make sure that it permanently persists the to-device messages before storing the /sync token). Olm session internal state should also be reverted if the sync does not complete.

• If you restart (e.g. upgrade) Element while it's waiting to process a m.room_key, it'll drop it and you'll get UISIs element-web#23113
• Similar issue on android
• Similar on iOS?

[richvdh]

@BillCarsonFr:

Olm session internal state should also be reverted if the sync does not complete.

Why is this required?

[pmaier1]

Will be fixed with the introduction of ER.

[BillCarsonFr]

Will be fixed with the introduction of ER.

Looks like not. The sync token is save way to early (on purpose?), before the to_devices have been processed by the rust sdk.
So in case of restart, the next token will be used and thus the to_devices not processed are lost.

https://github.com/matrix-org/matrix-js-sdk/blob/748d03ba1129dd0ab6ce6e44f6202b81e604a4b3/src/sync.ts#L895

            // set the sync token NOW *before* processing the events. We do this so
            // if something barfs on an event we can skip it rather than constantly
            // polling with the same token.
            this.client.store.setSyncToken(data.next_batch);

[richvdh]

Similar issue on android
Similar on iOS?

Do we actually have any evidence that EAR and EIR are similarly affected? The Rust SDK makes this much harder to mess up than before, so I don't see it as a given that the other platforms have the same problem

[richvdh]

Given that element-hq/element-web#23113 is now closed, and we have no reports of it happening on EAR and EIR, can we close this?

[BillCarsonFr]

Given that element-hq/element-web#23113 is now closed, and we have no reports of it happening on EAR and EIR, can we close this?

Confirm that android only saves the token after all the to_devices have been processed.

For iOS I am not sure for the background sync (NSE). Looks like the crypto sync is handled after the call to syncResponseStoreManager.updateStore

So possible that the background sync of IOS is affected?

[BillCarsonFr]

Ok so it's also fine for IOS.
The NSE background sync will save the sync response anyhow, and the next time the app starts it will then start from the token persisted by the app side, and will process the cached syncs (to avoid another server call). So IOS is actually in some condition replaying the to_device (can be seen in various RS An Olm message got replayed) and not dropping them.

So closing.