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

Fix spurious "Decryption key withheld" messages #3061

Merged
merged 2 commits into from
Jan 13, 2023
Merged

Fix spurious "Decryption key withheld" messages #3061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
53de9f5
Retry decryption after room_key_withheld events
richvdh Jan 12, 2023
22a0997
Fix spurious "Decryption key withheld" messages
richvdh Jan 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions spec/integ/megolm-integ.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1678,4 +1678,79 @@ describe("megolm", () => {
await Promise.all([sendPromise, megolmMessagePromise, aliceTestClient.httpBackend.flush("/keys/query", 1)]);
});
});

describe("m.room_key.withheld handling", () => {
// TODO: there are a bunch more tests for this sort of thing in spec/unit/crypto/algorithms/megolm.spec.ts.
// They should be converted to integ tests and moved.

it("does not block decryption on an 'm.unavailable' report", async function () {
await aliceTestClient.start();

// there may be a key downloads for alice
aliceTestClient.httpBackend.when("POST", "/keys/query").respond(200, {});
aliceTestClient.httpBackend.flush("/keys/query", 1, 5000);

// encrypt a message with a group session.
const groupSession = new Olm.OutboundGroupSession();
groupSession.create();
const messageEncryptedEvent = encryptMegolmEvent({
senderKey: testSenderKey,
groupSession: groupSession,
room_id: ROOM_ID,
});

// Alice gets the room message, but not the key
aliceTestClient.httpBackend.when("GET", "/sync").respond(200, {
next_batch: 1,
rooms: {
join: { [ROOM_ID]: { timeline: { events: [messageEncryptedEvent] } } },
},
});
await aliceTestClient.flushSync();

// alice will (eventually) send a room-key request
aliceTestClient.httpBackend.when("PUT", "/sendToDevice/m.room_key_request/").respond(200, {});
await aliceTestClient.httpBackend.flush("/sendToDevice/m.room_key_request/", 1, 1000);

// at this point, the message should be a decryption failure
const room = aliceTestClient.client.getRoom(ROOM_ID)!;
const event = room.getLiveTimeline().getEvents()[0];
expect(event.isDecryptionFailure()).toBeTruthy();

// we want to wait for the message to be updated, so create a promise for it
const retryPromise = new Promise((resolve) => {
event.once(MatrixEventEvent.Decrypted, (ev) => {
resolve(ev);
});
});

// alice gets back a room-key-withheld notification
aliceTestClient.httpBackend.when("GET", "/sync").respond(200, {
next_batch: 2,
to_device: {
events: [
{
type: "m.room_key.withheld",
sender: "@bob:example.com",
content: {
algorithm: "m.megolm.v1.aes-sha2",
room_id: ROOM_ID,
session_id: groupSession.session_id(),
sender_key: testSenderKey,
code: "m.unavailable",
reason: "",
},
},
],
},
});
await aliceTestClient.flushSync();

// the withheld notification should trigger a retry; wait for it
await retryPromise;

// finally: the message should still be a regular decryption failure, not a withheld notification.
expect(event.getContent().body).not.toContain("withheld");
});
});
});
135 changes: 75 additions & 60 deletions src/crypto/algorithms/megolm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1613,76 +1613,91 @@ export class MegolmDecryption extends DecryptionAlgorithm {
const senderKey = content.sender_key;

if (content.code === "m.no_olm") {
const sender = event.getSender()!;
this.prefixedLogger.warn(`${sender}:${senderKey} was unable to establish an olm session with us`);
// if the sender says that they haven't been able to establish an olm
// session, let's proactively establish one
await this.onNoOlmWithheldEvent(event);
} else if (content.code === "m.unavailable") {
// this simply means that the other device didn't have the key, which isn't very useful information. Don't
// record it in the storage
} else {
await this.olmDevice.addInboundGroupSessionWithheld(
content.room_id,
senderKey,
content.session_id,
content.code,
content.reason,
);
}

// Note: after we record that the olm session has had a problem, we
// trigger retrying decryption for all the messages from the sender's
// Having recorded the problem, retry decryption on any affected messages.
// It's unlikely we'll be able to decrypt sucessfully now, but this will
// update the error message.
//
if (content.session_id) {
await this.retryDecryption(senderKey, content.session_id);
} else {
// no_olm messages aren't specific to a given megolm session, so
// we trigger retrying decryption for all the messages from the sender's
// key, so that we can update the error message to indicate the olm
// session problem.
await this.retryDecryptionFromSender(senderKey);
}
}

if (await this.olmDevice.getSessionIdForDevice(senderKey)) {
// a session has already been established, so we don't need to
// create a new one.
this.prefixedLogger.debug("New session already created. Not creating a new one.");
await this.olmDevice.recordSessionProblem(senderKey, "no_olm", true);
this.retryDecryptionFromSender(senderKey);
return;
}
let device = this.crypto.deviceList.getDeviceByIdentityKey(content.algorithm, senderKey);
private async onNoOlmWithheldEvent(event: MatrixEvent): Promise<void> {
const content = event.getContent();
const senderKey = content.sender_key;
const sender = event.getSender()!;
this.prefixedLogger.warn(`${sender}:${senderKey} was unable to establish an olm session with us`);
// if the sender says that they haven't been able to establish an olm
// session, let's proactively establish one

if (await this.olmDevice.getSessionIdForDevice(senderKey)) {
// a session has already been established, so we don't need to
// create a new one.
this.prefixedLogger.debug("New session already created. Not creating a new one.");
await this.olmDevice.recordSessionProblem(senderKey, "no_olm", true);
return;
}
let device = this.crypto.deviceList.getDeviceByIdentityKey(content.algorithm, senderKey);
if (!device) {
// if we don't know about the device, fetch the user's devices again
// and retry before giving up
await this.crypto.downloadKeys([sender], false);
device = this.crypto.deviceList.getDeviceByIdentityKey(content.algorithm, senderKey);
if (!device) {
// if we don't know about the device, fetch the user's devices again
// and retry before giving up
await this.crypto.downloadKeys([sender], false);
device = this.crypto.deviceList.getDeviceByIdentityKey(content.algorithm, senderKey);
if (!device) {
this.prefixedLogger.info(
"Couldn't find device for identity key " + senderKey + ": not establishing session",
);
await this.olmDevice.recordSessionProblem(senderKey, "no_olm", false);
this.retryDecryptionFromSender(senderKey);
return;
}
this.prefixedLogger.info(
"Couldn't find device for identity key " + senderKey + ": not establishing session",
);
await this.olmDevice.recordSessionProblem(senderKey, "no_olm", false);
return;
}
}

// XXX: switch this to use encryptAndSendToDevices() rather than duplicating it?
// XXX: switch this to use encryptAndSendToDevices() rather than duplicating it?

await olmlib.ensureOlmSessionsForDevices(this.olmDevice, this.baseApis, { [sender]: [device] }, false);
const encryptedContent: IEncryptedContent = {
algorithm: olmlib.OLM_ALGORITHM,
sender_key: this.olmDevice.deviceCurve25519Key!,
ciphertext: {},
[ToDeviceMessageId]: uuidv4(),
};
await olmlib.encryptMessageForDevice(
encryptedContent.ciphertext,
this.userId,
undefined,
this.olmDevice,
sender,
device,
{ type: "m.dummy" },
);
await olmlib.ensureOlmSessionsForDevices(this.olmDevice, this.baseApis, { [sender]: [device] }, false);
const encryptedContent: IEncryptedContent = {
algorithm: olmlib.OLM_ALGORITHM,
sender_key: this.olmDevice.deviceCurve25519Key!,
ciphertext: {},
[ToDeviceMessageId]: uuidv4(),
};
await olmlib.encryptMessageForDevice(
encryptedContent.ciphertext,
this.userId,
undefined,
this.olmDevice,
sender,
device,
{ type: "m.dummy" },
);

await this.olmDevice.recordSessionProblem(senderKey, "no_olm", true);
this.retryDecryptionFromSender(senderKey);
await this.olmDevice.recordSessionProblem(senderKey, "no_olm", true);

await this.baseApis.sendToDevice("m.room.encrypted", {
[sender]: {
[device.deviceId]: encryptedContent,
},
});
} else {
await this.olmDevice.addInboundGroupSessionWithheld(
content.room_id,
senderKey,
content.session_id,
content.code,
content.reason,
);
}
await this.baseApis.sendToDevice("m.room.encrypted", {
[sender]: {
[device.deviceId]: encryptedContent,
},
});
}

public hasKeysForKeyRequest(keyRequest: IncomingRoomKeyRequest): Promise<boolean> {
Expand Down