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.

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

Support for warning users when unknown devices show up #335

Merged
merged 11 commits into from
Feb 2, 2017
17 changes: 17 additions & 0 deletions src/crypto/algorithms/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,23 @@ module.exports.DecryptionError = function(msg) {
};
utils.inherits(module.exports.DecryptionError, Error);

/**
* Exception thrown specifically when we want to warn the user to consider
* the security of their conversation before continuing
*
* @constructor
* @param {string} msg message describing the problem
* @param {Object} devices userId -> {deviceId -> object}
* set of unknown devices per user we're warning about
* @extends Error
*/
module.exports.UnknownDeviceError = function(msg, devices) {
this.name = "UnknownDeviceError";
this.message = msg;
this.devices = devices;
};
utils.inherits(module.exports.UnknownDeviceError, Error);

/**
* Registers an encryption/decryption class for a particular algorithm
*
Expand Down
42 changes: 42 additions & 0 deletions src/crypto/algorithms/megolm.js
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,10 @@ MegolmEncryption.prototype._shareKeyWithDevices = function(session, devicesByUse
MegolmEncryption.prototype.encryptMessage = function(room, eventType, content) {
const self = this;
return this._getDevicesInRoom(room).then(function(devicesInRoom) {
// check if any of these devices are not yet known to the user.
// if so, warn the user so they can verify or ignore.
self._checkForUnknownDevices(devicesInRoom);

return self._ensureOutboundSession(devicesInRoom);
}).then(function(session) {
const payloadJson = {
Expand Down Expand Up @@ -415,6 +419,41 @@ MegolmEncryption.prototype.encryptMessage = function(room, eventType, content) {
});
};

/**
* Checks the devices we're about to send to and see if any are entirely
* unknown to the user. If so, warn the user, and mark them as known to
* give the user a chance to go verify them before re-sending this message.
*
* @param {Object} devicesInRoom userId -> {deviceId -> object}
* devices we should shared the session with.
*/
MegolmEncryption.prototype._checkForUnknownDevices = function(devicesInRoom) {
const unknownDevices = {};

Object.keys(devicesInRoom).forEach((userId)=>{
Object.keys(devicesInRoom[userId]).forEach((deviceId)=>{
const device = devicesInRoom[userId][deviceId];
if (device.isUnverified() && !device.isKnown()) {
// mark the devices as known to the user, given we're about to
// yell at them.
this._crypto.setDeviceVerification(userId, device.deviceId,
undefined, undefined, true);
if (!unknownDevices[userId]) {
unknownDevices[userId] = {};
}
unknownDevices[userId][deviceId] = device;
}
});
});

if (Object.keys(unknownDevices).length) {
// it'd be kind to pass unknownDevices up to the user in this error
throw new base.UnknownDeviceError(
"This room contains unknown devices which have not been verified. " +
"We strongly recommend you verify them before continuing.", unknownDevices);
}
};

/**
* Get the list of unblocked devices for all users in the room
*
Expand All @@ -433,6 +472,9 @@ MegolmEncryption.prototype._getDevicesInRoom = function(room) {
// have a list of the user's devices, then we already share an e2e room
// with them, which means that they will have announced any new devices via
// an m.new_device.
//
// XXX: what if the cache is stale, and the user left the room we had in common
// and then added new devices before joining this one? --Matthew
Copy link
Member

Choose a reason for hiding this comment

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

yup, this is exactly what element-hq/element-web#2305 (comment) is on about

return this._crypto.downloadKeys(roomMembers, false).then(function(devices) {
// remove any blocked devices
for (const userId in devices) {
Expand Down
25 changes: 24 additions & 1 deletion src/crypto/deviceinfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ limitations under the License.
* <key type>:<id> -> <base64-encoded key>>
*
* @property {module:crypto/deviceinfo.DeviceVerification} verified
* whether the device has been verified by the user
* whether the device has been verified/blocked by the user
*
* @property {boolean} known
* whether the user knows of this device's existence (useful when warning
* the user that a user has added new devices)
*
* @property {Object} unsigned additional data from the homeserver
*
Expand All @@ -50,6 +54,7 @@ function DeviceInfo(deviceId) {
this.algorithms = [];
this.keys = {};
this.verified = DeviceVerification.UNVERIFIED;
this.known = false;
this.unsigned = {};
}

Expand Down Expand Up @@ -130,6 +135,24 @@ DeviceInfo.prototype.isVerified = function() {
return this.verified == DeviceVerification.VERIFIED;
};

/**
* Returns true if this device is unverified
*
* @return {Boolean} true if unverified
*/
DeviceInfo.prototype.isUnverified = function() {
return this.verified == DeviceVerification.UNVERIFIED;
};

/**
* Returns true if the user knows about this device's existence
*
* @return {Boolean} true if known
*/
DeviceInfo.prototype.isKnown = function() {
return this.known == true;
};

/**
* @enum
*/
Expand Down
14 changes: 12 additions & 2 deletions src/crypto/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -684,8 +684,12 @@ Crypto.prototype.getDeviceByIdentityKey = function(userId, algorithm, sender_key
*
* @param {?boolean} blocked whether to mark the device as blocked. Null to
* leave unchanged.
*
* @param {?boolean} known whether to mark that the user has been made aware of
* the existence of this device. Null to leave unchanged
*/
Crypto.prototype.setDeviceVerification = function(userId, deviceId, verified, blocked) {
Crypto.prototype.setDeviceVerification = function(userId, deviceId, verified,
blocked, known) {
const devices = this._sessionStore.getEndToEndDevicesForUser(userId);
if (!devices || !devices[deviceId]) {
throw new Error("Unknown device " + userId + ":" + deviceId);
Expand All @@ -706,10 +710,16 @@ Crypto.prototype.setDeviceVerification = function(userId, deviceId, verified, bl
verificationStatus = DeviceVerification.UNVERIFIED;
}

if (dev.verified === verificationStatus) {
let knownStatus = dev.known;
if (known !== null) {
Copy link
Member

Choose a reason for hiding this comment

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

should probably also check for undefined here

knownStatus = known;
}

if (dev.verified === verificationStatus && dev.known === knownStatus) {
return;
}
dev.verified = verificationStatus;
dev.known = knownStatus;
this._sessionStore.storeEndToEndDevicesForUser(userId, devices);
};

Expand Down