We don't appear to be retrying /sendToDevice requests

[dkasak]

I don't think we're retrying failed /sendToDevice requests. This comment seems to agree.

This is especially bad when e.g. sharing room keys in encryptAndSendKeysToDevices since we might end up marking a key as sent to a device when in reality the request failed. Though I'm not sure whether the then(...) is executed if the request fails or not.

All call sites:

• matrix-js-sdk/src/crypto/algorithms/megolm.ts

  Line 634 in 96e8f65

  return this.baseApis.sendToDevice("m.room.encrypted", contentMap).then(() => {

• matrix-js-sdk/src/crypto/algorithms/megolm.ts

  Line 686 in 96e8f65

  await this.baseApis.sendToDevice("org.matrix.room_key.withheld", contentMap);

• matrix-js-sdk/src/crypto/algorithms/megolm.ts

  Line 780 in 96e8f65

  await this.baseApis.sendToDevice("m.room.encrypted", {

• matrix-js-sdk/src/crypto/algorithms/megolm.ts

  Line 1552 in 96e8f65

  await this.baseApis.sendToDevice("m.room.encrypted", {

• matrix-js-sdk/src/crypto/algorithms/megolm.ts

  Line 1635 in 96e8f65

  return this.baseApis.sendToDevice("m.room.encrypted", contentMap);

• matrix-js-sdk/src/crypto/algorithms/megolm.ts

  Line 1823 in 96e8f65

  await this.baseApis.sendToDevice("m.room.encrypted", contentMap);

• matrix-js-sdk/src/crypto/index.ts

  Line 3313 in 96e8f65

  await this.baseApis.sendToDevice("m.room.encrypted", {

• matrix-js-sdk/src/crypto/OutgoingRoomKeyRequestManager.ts

  Line 481 in 96e8f65

  return this.baseApis.sendToDevice(EventType.RoomKeyRequest, contentMap, txnId);

• matrix-js-sdk/src/crypto/SecretStorage.ts

  Line 402 in 96e8f65

  this.baseApis.sendToDevice("m.secret.request", {

• matrix-js-sdk/src/crypto/SecretStorage.ts

  Line 423 in 96e8f65

  this.baseApis.sendToDevice("m.secret.request", {

• matrix-js-sdk/src/crypto/SecretStorage.ts

  Line 525 in 96e8f65

  this.baseApis.sendToDevice("m.room.encrypted", contentMap);

[novocaine]

what are the consequences of this for the user?

[ara4n]

to-device messages are used for setting up 1:1 olm channels; sharing new megolm sessions; requesting keyshares; and in future VoIP signalling. so if one goes missing, it could wedge the ability to send to a single device in a room (until that device spots and renegotiates, if correctly implemented), or stop the megolm session being shared with all devices in the room, killing the next ~100 msgs sent by that device (until the recipients request the sender to resend, if the sender is still online), or cause a subsequent keyshare req/response to fail.

In other words, it causes UISIs to one or more devices, which should in theory be eventually be recoverable, but may hang around for ages if the receiver tries to read the msgs after the sender has gone offline.

[novocaine]

so the user impact of this is something like: a temporary network failure when sharing E2EE keys may stop them from ever being shared

[novocaine]

hrm, sendToDevice returns the promise from this.http.authRequest (

matrix-js-sdk/src/client.ts

Line 7867 in 96e8f65

public sendToDevice(

) - I think .then(...) for that object would only execute if that network request was successful..

So .. I guess the consequence might not that be that we mark the keys as sent when they weren't, but we might never run that code again in the current session?

[MadLittleMods]

This looks related to element-hq/element-web#18666

[dkasak]

This looks related to vector-im/element-web#18666

Is it related? That issue doesn't read to me like a bug report about not retrying failed requests, but more like a feature request to share keys with new verified devices even though keys were never shared with them in the first place.

[dkasak]

But it is related to element-hq/element-web#12851.

[dkasak]

This is now partially fixed by #2549 for room key sending. Other to-device messages are not retried yet.