matrix/matrix-js-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-js-sdk.git synced 2025-12-17 21:42:17 +03:00
Code Activity

Refactor decryption

Browse Source 
Create the MatrixEvent wrapper before decryption, and then pass that into the
decryptors, which should update it.

Also remove the workaround that sends m.new_device messages when we get an
unknown session; it's just a bandaid which is obscuring more meaningful
problems.
This commit is contained in:
Richard van der Hoff
2016-11-11 16:56:22 +00:00
parent e623b539c4
commit 1a03e534bd
5 changed files with 65 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
lib/client.js
View File

@@ -463,38 +463,31 @@ MatrixClient.prototype.isRoomEncrypted = function(roomId) {
* Decrypt a received event according to the algorithm specified in the event.
*
* @param {MatrixClient} client
* @param {object} raw event
*
* @return {MatrixEvent}
* @param {MatrixEvent} event
*/
function _decryptEvent(client, event) {
if (!client._crypto) {
return _badEncryptedMessage(event, "**Encryption not enabled**");
_badEncryptedMessage(event, "**Encryption not enabled**");
return;
}
var decryptionResult;
try {
decryptionResult = client._crypto.decryptEvent(event);
client._crypto.decryptEvent(event);
} catch (e) {
if (!(e instanceof Crypto.DecryptionError)) {
throw e;
}
return _badEncryptedMessage(event, "**" + e.message + "**");
_badEncryptedMessage(event, "**" + e.message + "**");
return;
}
return new MatrixEvent(
event, decryptionResult.payload,
decryptionResult.keysProved,
decryptionResult.keysClaimed
);
}
function _badEncryptedMessage(event, reason) {
return new MatrixEvent(event, {
event.setClearData({
type: "m.room.message",
content: {
msgtype: "m.bad.encrypted",
body: reason,
content: event.content,
},
});
}
@@ -2829,10 +2822,11 @@ function _resolve(callback, defer, res) {
function _PojoToMatrixEventMapper(client) {
function mapper(plainOldJsObject) {
if (plainOldJsObject.type === "m.room.encrypted") {
return _decryptEvent(client, plainOldJsObject);
var event = new MatrixEvent(plainOldJsObject);
if (event.isEncrypted()) {
_decryptEvent(client, event);
}
return new MatrixEvent(plainOldJsObject);
return event;
}
return mapper;
}

17
lib/crypto/algorithms/megolm.js
View File

@@ -451,7 +451,7 @@ utils.inherits(MegolmDecryption, base.DecryptionAlgorithm);
/**
* @inheritdoc
*
* @param {object} event raw event
* @param {MatrixEvent} event
*
* @return {null} The event referred to an unknown megolm session
* @return {module:crypto.DecryptionResult} decryption result
@@ -460,7 +460,7 @@ utils.inherits(MegolmDecryption, base.DecryptionAlgorithm);
* problem decrypting the event
*/
MegolmDecryption.prototype.decryptEvent = function(event) {
var content = event.content;
var content = event.getWireContent();
if (!content.sender_key || !content.session_id ||
!content.ciphertext
@@ -471,14 +471,15 @@ MegolmDecryption.prototype.decryptEvent = function(event) {
var res;
try {
res = this._olmDevice.decryptGroupMessage(
event.room_id, content.sender_key, content.session_id, content.ciphertext
event.getRoomId(), content.sender_key, content.session_id, content.ciphertext
);
} catch (e) {
throw new base.DecryptionError(e);
}
if (res === null) {
return null;
// We've got a message for a session we don't have.
throw new base.DecryptionError("Unknown inbound session id");
}
var payload = JSON.parse(res.result);
@@ -486,17 +487,13 @@ MegolmDecryption.prototype.decryptEvent = function(event) {
// belt-and-braces check that the room id matches that indicated by the HS
// (this is somewhat redundant, since the megolm session is scoped to the
// room, so neither the sender nor a MITM can lie about the room_id).
if (payload.room_id !== event.room_id) {
if (payload.room_id !== event.getRoomId()) {
throw new base.DecryptionError(
"Message intended for room " + payload.room_id
);
}
return {
payload: payload,
keysClaimed: res.keysClaimed,
keysProved: res.keysProved,
};
event.setClearData(payload, res.keysClaimed, res.keysProved);
};
/**

35
lib/crypto/algorithms/olm.js
View File

@@ -151,15 +151,13 @@ utils.inherits(OlmDecryption, base.DecryptionAlgorithm);
/**
* @inheritdoc
*
* @param {object} event raw event
*
* @return {module:crypto.DecryptionResult} decryption result
* @param {MatrixEvent} event
*
* @throws {module:crypto/algorithms/base.DecryptionError} if there is a
* problem decrypting the event
*/
OlmDecryption.prototype.decryptEvent = function(event) {
var content = event.content;
var content = event.getWireContent();
var deviceKey = content.sender_key;
var ciphertext = content.ciphertext;
@@ -178,7 +176,7 @@ OlmDecryption.prototype.decryptEvent = function(event) {
} catch (e) {
console.warn(
"Failed to decrypt Olm event (id=" +
event.event_id + ") from " + deviceKey +
event.getId() + ") from " + deviceKey +
": " + e.message
);
throw new base.DecryptionError("Bad Encrypted Message");
@@ -192,11 +190,11 @@ OlmDecryption.prototype.decryptEvent = function(event) {
// older versions of riot did not set this field, so we cannot make
// this check. TODO: kill this off once our users have updated
console.warn(
"Olm event (id=" + event.event_id + ") contains no 'recipient' " +
"Olm event (id=" + event.getId() + ") contains no 'recipient' " +
"property; cannot prevent unknown-key attack");
} else if (payload.recipient != this._userId) {
console.warn(
"Event " + event.event_id + ": Intended recipient " +
"Event " + event.getId() + ": Intended recipient " +
payload.recipient + " does not match our id " + this._userId
);
throw new base.DecryptionError(
@@ -207,12 +205,12 @@ OlmDecryption.prototype.decryptEvent = function(event) {
if (payload.recipient_keys === undefined) {
// ditto
console.warn(
"Olm event (id=" + event.event_id + ") contains no " +
"Olm event (id=" + event.getId() + ") contains no " +
"'recipient_keys' property; cannot prevent unknown-key attack");
} else if (payload.recipient_keys.ed25519 !=
this._olmDevice.deviceEd25519Key) {
console.warn(
"Event " + event.event_id + ": Intended recipient ed25519 key " +
"Event " + event.getId() + ": Intended recipient ed25519 key " +
payload.recipient_keys.ed25519 + " did not match ours"
);
throw new base.DecryptionError("Message not intended for this device");
@@ -225,12 +223,12 @@ OlmDecryption.prototype.decryptEvent = function(event) {
if (payload.sender === undefined) {
// ditto
console.warn(
"Olm event (id=" + event.event_id + ") contains no " +
"Olm event (id=" + event.getId() + ") contains no " +
"'sender' property; cannot prevent unknown-key attack");
} else if (payload.sender != event.sender) {
} else if (payload.sender != event.getSender()) {
console.warn(
"Event " + event.event_id + ": original sender " + payload.sender +
" does not match reported sender " + event.sender
"Event " + event.getId() + ": original sender " + payload.sender +
" does not match reported sender " + event.getSender()
);
throw new base.DecryptionError(
"Message forwarded from " + payload.sender
@@ -238,9 +236,9 @@ OlmDecryption.prototype.decryptEvent = function(event) {
}
// Olm events intended for a room have a room_id.
if (payload.room_id !== event.room_id) {
if (payload.room_id !== event.getRoomId()) {
console.warn(
"Event " + event.event_id + ": original room " + payload.room_id +
"Event " + event.getId() + ": original room " + payload.room_id +
" does not match reported room " + event.room_id
);
throw new base.DecryptionError(
@@ -248,12 +246,7 @@ OlmDecryption.prototype.decryptEvent = function(event) {
);
}
return {
payload: payload,
sessionExists: true,
keysProved: {curve25519: deviceKey},
keysClaimed: payload.keys || {}
};
event.setClearData(payload, {curve25519: deviceKey}, payload.keys || {});
};

54
lib/crypto/index.js
View File

@@ -900,63 +900,17 @@ Crypto.prototype.encryptEventIfNeeded = function(event, room) {
});
};
/**
* @typedef {Object} module:crypto.DecryptionResult
*
* @property {Object} payload decrypted payload (with properties 'type',
* 'content').
*
* @property {Object<string, string>} keysClaimed keys that the sender of the
* event claims ownership of: map from key type to base64-encoded key
*
* @property {Object<string, string>} keysProved keys that the sender of the
* event is known to have ownership of: map from key type to base64-encoded
* key
*/
/**
* Decrypt a received event
*
* @param {object} event raw event
*
* @return {module:crypto.DecryptionResult} decryption result
* @param {MatrixEvent} event
*
* @raises {algorithms.DecryptionError} if there is a problem decrypting the event
*/
Crypto.prototype.decryptEvent = function(event) {
var content = event.content;
var alg = this._getRoomDecryptor(event.room_id, content.algorithm);
var r = alg.decryptEvent(event);
if (r !== null) {
return r;
} else {
// We've got a message for a session we don't have. Maybe the sender
// forgot to tell us about the session. Remind the sender that we
// exist so that they might tell us about the session on their next
// send.
//
// (Alternatively, it might be that we are just looking at
// scrollback... at least we rate-limit the m.new_device events :/)
//
// XXX: this is a band-aid which masks symptoms of other bugs. It would
// be nice to get rid of it.
if (event.room_id !== undefined && event.sender !== undefined) {
var device_id = event.content.device_id;
if (device_id === undefined) {
// if the sending device didn't tell us its device_id, fall
// back to all devices.
device_id = null;
}
console.log(
"Unknown session decrypting event id " + event.event_id +
": sending m.new_device event"
);
this._sendPingToDevice(event.sender, device_id, event.room_id);
}
throw new algorithms.DecryptionError("Unknown inbound session id");
}
var content = event.getWireContent();
var alg = this._getRoomDecryptor(event.getRoomId(), content.algorithm);
alg.decryptEvent(event);
};
/**

44
lib/models/event.js
View File

@@ -51,15 +51,6 @@ module.exports.EventStatus = {
*
* @param {Object} event The raw event to be wrapped in this DAO
*
* @param {Object=} clearEvent For encrypted events, the plaintext payload for
* the event (typically containing <tt>type</tt> and <tt>content</tt> fields).
*
* @param {Object=} keysProved Keys owned by the sender of this event.
* See {@link module:models/event.MatrixEvent#getKeysProved}.
*
* @param {Object=} keysClaimed Keys the sender of this event claims.
* See {@link module:models/event.MatrixEvent#getKeysClaimed}.
*
* @prop {Object} event The raw (possibly encrypted) event. <b>Do not access
* this property</b> directly unless you absolutely have to. Prefer the getter
* methods defined on this class. Using the getter methods shields your app
@@ -75,19 +66,18 @@ module.exports.EventStatus = {
* Default: true. <strong>This property is experimental and may change.</strong>
*/
module.exports.MatrixEvent = function MatrixEvent(
event, clearEvent, keysProved, keysClaimed
event
) {
this.event = event || {};
this.sender = null;
this.target = null;
this.status = null;
this.forwardLooking = true;
this._clearEvent = clearEvent || {};
this._pushActions = null;
this._keysProved = keysProved || {};
this._keysClaimed = keysClaimed || {};
this._clearEvent = {};
this._keysProved = {};
this._keysClaimed = {};
};
module.exports.MatrixEvent.prototype = {
@@ -239,12 +229,36 @@ module.exports.MatrixEvent.prototype = {
this._keysClaimed = keys;
},
/**
* Update the cleartext data on this event.
*
* (This is used after decrypting an event; it should not be used by applications).
*
* @internal
*
* @fires module:models/event.MatrixEvent#"Event.decrypted"
*
* @param {Object} clearEvent The plaintext payload for the event
* (typically containing <tt>type</tt> and <tt>content</tt> fields).
*
* @param {Object=} keysProved Keys owned by the sender of this event.
* See {@link module:models/event.MatrixEvent#getKeysProved}.
*
* @param {Object=} keysClaimed Keys the sender of this event claims.
* See {@link module:models/event.MatrixEvent#getKeysClaimed}.
*/
setClearData: function(clearEvent, keysProved, keysClaimed) {
this._clearEvent = clearEvent;
this._keysProved = keysProved || {};
this._keysClaimed = keysClaimed || {};
},
/**
* Check if the event is encrypted.
* @return {boolean} True if this event is encrypted.
*/
isEncrypted: function() {
return Boolean(this._clearEvent.type);
return this.event.type === "m.room.encrypted";
},
/**