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

Catch exceptions when encrypting events #137

Merged
merged 2 commits into from
Jun 9, 2016
Merged

Catch exceptions when encrypting events #137

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
4bf24f0
Client: add some logging to help understand what is going on
richvdh Jun 8, 2016
ce59b72
Catch exceptions when encrypting events
richvdh Jun 3, 2016
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
141 changes: 100 additions & 41 deletions lib/client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,9 +692,11 @@ MatrixClient.prototype.setRoomEncryption = function(roomId, config) {
}
}
if (oneTimeKey) {
self._olmDevice.createOutboundSession(
var sid = self._olmDevice.createOutboundSession(
device[2], oneTimeKey
);
console.log("Started new sessionid " + sid +
" for device " + device[2]);
} else {
missing[device[0]] = missing[device[0]] || [];
missing[device[0]].push([device[1]]);
Expand Down Expand Up @@ -1115,26 +1117,56 @@ MatrixClient.prototype.sendEvent = function(roomId, eventType, content, txnId,
room.addPendingEvent(localEvent, txnId);
}

if (eventType === "m.room.message" && this.sessionStore && CRYPTO_ENABLED) {
var e2eRoomInfo = this.sessionStore.getEndToEndRoom(roomId);
if (e2eRoomInfo) {
var encryptedContent = _encryptMessage(
this, roomId, e2eRoomInfo, eventType, content, txnId, callback
);
localEvent.encryptedType = "m.room.encrypted";
localEvent.encryptedContent = encryptedContent;
return _sendEvent(this, room, localEvent, callback);
};

// TODO: Specify this in the event constructor rather than fiddling
// with the event object internals.
localEvent.encrypted = true;
}

/**
* Encrypt an event according to the configuration of the room, if necessary.
*
* @param {MatrixClient} client
* @param {module:models/event.MatrixEvent} event event to be sent
*
* @private
*/
function _encryptEventIfNeeded(client, event) {
if (event.isEncrypted()) {
// this event has already been encrypted; this happens if the
// encryption step succeeded, but the send step failed on the first
// attempt.
return;
}

return _sendEvent(this, room, localEvent, callback);
};
if (event.getType() !== "m.room.message") {
// we only encrypt m.room.message
return;
}

if (!client.sessionStore) {
// End to end encryption isn't enabled if we don't have a session
// store.
return;
}

var roomId = event.getRoomId();

var e2eRoomInfo = client.sessionStore.getEndToEndRoom(roomId);
if (!e2eRoomInfo || !e2eRoomInfo.algorithm) {
// not encrypting messages in this room
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

Could be wise to wrap up these two 'are we encrypting this room' checks into a single function that will be the same one you can to determine whether there's a padlock or whatever displayed in the room. These two checks make me wonder how easy it would be to show a room as being encrypted and then accidentally end up without a sessionStore which here would cause the message to be sent in the clear.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. There are some more changes afoot, which move the is-the-room-being-encrypted configuration out of the sessionstore and into the room state, which will make this look a bit different anyway. I think I'll leave this for now.


function _encryptMessage(client, roomId, e2eRoomInfo, eventType, content,
txnId, callback) {
var encryptedContent = _encryptMessage(
client, roomId, e2eRoomInfo, event.getType(), event.getContent()
);
event.encryptedType = "m.room.encrypted";
event.encryptedContent = encryptedContent;
// TODO: Specify this in the event constructor rather than fiddling
// with the event object internals.
event.encrypted = true;
}

function _encryptMessage(client, roomId, e2eRoomInfo, eventType, content) {
if (!client.sessionStore) {
throw new Error(
"Client must have an end-to-end session store to encrypt messages"
Expand Down Expand Up @@ -1182,6 +1214,7 @@ function _encryptMessage(client, roomId, e2eRoomInfo, eventType, content,
continue;
}
var sessionId = sessionIds[0];
console.log("Using sessionid " + sessionId + " for device " + deviceKey);
ciphertext[deviceKey] = client._olmDevice.encryptMessage(
deviceKey, sessionId, payloadString
);
Expand All @@ -1197,6 +1230,16 @@ function _encryptMessage(client, roomId, e2eRoomInfo, eventType, content,
}
}


/**
* Decrypt a received event according to the algorithm specified in the event.
*
* @param {MatrixClient} client
* @param {MatrixEvent} event
*
* @return {MatrixEvent} a new MatrixEvent
* @private
*/
function _decryptMessage(client, event) {
if (client.sessionStore === null || !CRYPTO_ENABLED) {
// End to end encryption isn't enabled if we don't have a session
Expand Down Expand Up @@ -1226,6 +1269,7 @@ function _decryptMessage(client, event) {
);
payloadString = res.payload;
if (payloadString) {
console.log("decrypted with sessionId " + sessionId);
break;
}

Expand All @@ -1242,6 +1286,7 @@ function _decryptMessage(client, event) {
payloadString = client._olmDevice.createInboundSession(
deviceKey, message.type, message.body
);
console.log("created new inbound sesion");
} catch (e) {
// Failed to decrypt with a new session.
}
Expand Down Expand Up @@ -1284,41 +1329,55 @@ function _badEncryptedMessage(event, reason) {
}, event);
}

// encrypts the event if necessary
// adds the event to the queue, or sends it
// marks the event as sent/unsent
// returns a promise which resolves with the result of the send request
function _sendEvent(client, room, event, callback) {
var defer = q.defer();
var promise;
// this event may be queued
if (client.scheduler) {
// if this returns a promsie then the scheduler has control now and will
// resolve/reject when it is done. Internally, the scheduler will invoke
// processFn which is set to this._sendEventHttpRequest so the same code
// path is executed regardless.
promise = client.scheduler.queueEvent(event);
if (promise && client.scheduler.getQueueForEvent(event).length > 1) {
// event is processed FIFO so if the length is 2 or more we know
// this event is stuck behind an earlier event.
_updatePendingEventStatus(room, event, EventStatus.QUEUED);
// Add an extra q() to turn synchronous exceptions into promise rejections,
// so that we can handle synchronous and asynchronous exceptions with the
// same code path.
return q().then(function() {
_encryptEventIfNeeded(client, event);

var promise;
// this event may be queued
if (client.scheduler) {
// if this returns a promsie then the scheduler has control now and will
// resolve/reject when it is done. Internally, the scheduler will invoke
// processFn which is set to this._sendEventHttpRequest so the same code
// path is executed regardless.
promise = client.scheduler.queueEvent(event);
if (promise && client.scheduler.getQueueForEvent(event).length > 1) {
// event is processed FIFO so if the length is 2 or more we know
// this event is stuck behind an earlier event.
_updatePendingEventStatus(room, event, EventStatus.QUEUED);
}
}
}

if (!promise) {
promise = _sendEventHttpRequest(client, event);
}

promise.done(function(res) { // the request was sent OK
if (!promise) {
promise = _sendEventHttpRequest(client, event);
}
return promise;
}).then(function(res) { // the request was sent OK
if (room) {
room.updatePendingEvent(event, EventStatus.SENT, res.event_id);
}

_resolve(callback, defer, res);
if (callback) {
callback(null, res);
}
return res;
}, function(err) {
// the request failed to send.
console.error("Error sending event", err.stack || err);

_updatePendingEventStatus(room, event, EventStatus.NOT_SENT);

_reject(callback, defer, err);
if (callback) {
callback(err);
}
throw err;
});

return defer.promise;
}

function _updatePendingEventStatus(room, event, newStatus) {
Expand Down
31 changes: 19 additions & 12 deletions spec/integ/matrix-client-retrying.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"use strict";
var q = require("q");
var sdk = require("../..");
var HttpBackend = require("../mock-request");
var utils = require("../test-utils");
Expand Down Expand Up @@ -66,21 +67,27 @@ describe("MatrixClient retrying", function() {
ev2 = tl[1];

expect(ev1.status).toEqual(EventStatus.SENDING);
expect(ev2.status).toEqual(EventStatus.QUEUED);
expect(ev2.status).toEqual(EventStatus.SENDING);

// now we can cancel the second and check everything looks sane
client.cancelPendingEvent(ev2);
expect(ev2.status).toEqual(EventStatus.CANCELLED);
expect(tl.length).toEqual(1);
// give the reactor a chance to run, so that ev2 gets queued
q().then(function() {
// ev2 should now have been queued
expect(ev2.status).toEqual(EventStatus.QUEUED);

// shouldn't be able to cancel the first message yet
expect(function() { client.cancelPendingEvent(ev1); })
.toThrow();
// now we can cancel the second and check everything looks sane
client.cancelPendingEvent(ev2);
expect(ev2.status).toEqual(EventStatus.CANCELLED);
expect(tl.length).toEqual(1);

// shouldn't be able to cancel the first message yet
expect(function() { client.cancelPendingEvent(ev1); })
.toThrow();

// fail the first send
httpBackend.when("PUT", "/send/m.room.message/")
.respond(400);
httpBackend.flush().then(function() {
// fail the first send
httpBackend.when("PUT", "/send/m.room.message/")
.respond(400);
return httpBackend.flush();
}).then(function() {
expect(ev1.status).toEqual(EventStatus.NOT_SENT);
expect(tl.length).toEqual(1);

Expand Down