Skip to content

Commit

Permalink
Verify e2e keys on download
Browse files Browse the repository at this point in the history 
Check the signature on downloaded e2e keys, and ignore those that don't match.
  • Loading branch information
@richvdh
richvdh committed Aug 4, 2016
1 parent 6001077 commit 75a505c
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 48 deletions.
38 changes: 38 additions & 0 deletions lib/OlmDevice.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,23 @@ OlmDevice.prototype._saveSession = function(deviceKey, session) {
};


/**
* get an OlmUtility and call the given function
*
* @param {function} func
* @return {object} result of func
* @private
*/
OlmDevice.prototype._getUtility = function(func) {
var utility = new Olm.Utility();
try {
return func(utility);
} finally {
utility.free();
}
};


/**
* Signs a message with the ed25519 key for this account.
*
Expand Down Expand Up @@ -332,5 +349,26 @@ OlmDevice.prototype.decryptMessage = function(
});
};


/**
* Verify an ed25519 signature.
*
* @param {string} key ed25519 key
* @param {string} message message which was signed
* @param {string} signature base64-encoded signature to be checked
*
* @raises {Error} if there is a problem with the verification. If the key was
* too small then the message will be "OLM.INVALID_BASE64". If the signature
* was invalid then the message will be "OLM.BAD_MESSAGE_MAC".
*/
OlmDevice.prototype.verifySignature = function(
key, message, signature
) {

this._getUtility(function(util) {
util.ed25519_verify(key, message, signature);
});
};

/** */
module.exports = OlmDevice;
113 changes: 74 additions & 39 deletions lib/crypto.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ Crypto.prototype.downloadKeys = function(userIds, forceDownload) {

var userStore = stored[userId];
var updated = _updateStoredDeviceKeysForUser(
userId, userStore, res.device_keys[userId]
self._olmDevice, userId, userStore, res.device_keys[userId]
);

if (updated) {
Expand All @@ -233,7 +233,8 @@ Crypto.prototype.downloadKeys = function(userIds, forceDownload) {
});
};

function _updateStoredDeviceKeysForUser(userId, userStore, userResult) {
function _updateStoredDeviceKeysForUser(_olmDevice, userId, userStore,
userResult) {
var updated = false;

// remove any devices in the store which aren't in the response
Expand All @@ -255,50 +256,84 @@ function _updateStoredDeviceKeysForUser(userId, userStore, userResult) {
continue;
}

var deviceRes = userResult[deviceId];
var deviceStore;

if (!deviceRes.keys) {
// no keys?
continue;
if (_storeDeviceKeys(
_olmDevice, userId, deviceId, userStore, userResult[deviceId]
)) {
updated = true;
}
}

var signKey = deviceRes.keys["ed25519:" + deviceId];
if (!signKey) {
console.log("Device " + userId + ": " +
deviceId + " has no ed25519 key");
continue;
}
return updated;
}

if (deviceId in userStore) {
// already have this device.
deviceStore = userStore[deviceId];

if (deviceStore.keys["ed25519:" + deviceId] != signKey) {
// this should only happen if the list has been MITMed; we are
// best off sticking with the original keys.
//
// Should we warn the user about it somehow?
console.warn("Ed25519 key for device" + userId + ": " +
deviceId + " has changed");
continue;
}
} else {
userStore[deviceId] = deviceStore = {
verified: DeviceVerification.UNVERIFIED
};
}
/*
* Process a device in a /query response, and add it to the userStore
*
* returns true if a change was made, else false
*/
function _storeDeviceKeys(_olmDevice, userId, deviceId, userStore, deviceResult) {
if (!deviceResult.keys) {
// no keys?
return false;
}

// TODO: check signature. Remember that we need to check for
// _olmDevice.
var signKeyId = "ed25519:" + deviceId;
var signKey = deviceResult.keys[signKeyId];
if (!signKey) {
console.log("Device " + userId + ":" + deviceId +
" has no ed25519 key");
return false;
}

deviceStore.keys = deviceRes.keys;
deviceStore.algorithms = deviceRes.algorithms;
deviceStore.unsigned = deviceRes.unsigned;
updated = true;
var unsigned = deviceResult.unsigned;
var signatures = deviceResult.signatures || {};
var userSigs = signatures[userId] || {};
var signature = userSigs[signKeyId];
if (!signature) {
console.log("Device " + userId + ":" + deviceId +
" is not signed");
return false;
}

return updated;
// prepare the canonical json: remove 'unsigned' and sigxsnatures, and
// stringify with anotherjson
delete deviceResult.unsigned;
delete deviceResult.signatures;
var json = anotherjson.stringify(deviceResult);

console.log("msg:", json);
try {
_olmDevice.verifySignature(signKey, json, signature);
} catch (e) {
console.log("Unable to verify signature on device " +
userId + ":" + deviceId + ":", e);
return false;
}

var deviceStore;
if (deviceId in userStore) {
// already have this device.
deviceStore = userStore[deviceId];

if (deviceStore.keys["ed25519:" + deviceId] != signKey) {
// this should only happen if the list has been MITMed; we are
// best off sticking with the original keys.
//
// Should we warn the user about it somehow?
console.warn("Ed25519 key for device" + userId + ": " +
deviceId + " has changed");
return false;
}
} else {
userStore[deviceId] = deviceStore = {
verified: DeviceVerification.UNVERIFIED
};
}

deviceStore.keys = deviceResult.keys;
deviceStore.algorithms = deviceResult.algorithms;
deviceStore.unsigned = unsigned;
return true;
}


Expand Down
16 changes: 13 additions & 3 deletions spec/integ/matrix-client-crypto.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -443,11 +443,21 @@ describe("MatrixClient crypto", function() {
.catch(utils.failTest).done(done);
});

it("Ali enables encryption", function(done) {
it("Ali gets keys with an invalid signature", function(done) {
q()
.then(bobUploadsKeys)
.then(aliStartClient)
.then(aliEnablesEncryption)
.then(function() {
// tamper bob's keys!
expect(bobDeviceKeys.keys["curve25519:" + bobDeviceId]).toBeDefined();
bobDeviceKeys.keys["curve25519:" + bobDeviceId] += "abc";

return q.all(aliClient.downloadKeys([bobUserId]),
aliQueryKeys());
})
.then(function() {
// should get an empty list
expect(aliClient.listDeviceKeys(bobUserId)).toEqual([]);
})
.catch(utils.failTest).done(done);
});

Expand Down
56 changes: 50 additions & 6 deletions spec/integ/matrix-client-methods.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,34 +184,78 @@ describe("MatrixClient", function() {

describe("downloadKeys", function() {
it("should do an HTTP request and then store the keys", function(done) {
var borisKeys = {dev1: {algorithms: ["1"], keys: { "ed25519:dev1": "k1" }}};
var chazKeys = {dev2: {algorithms: ["2"], keys: { "ed25519:dev2": "k2" }}};
var ed25519key = "wV5E3EUSHpHuoZLljNzojlabjGdXT3Mz7rugG9zgbkI";
var borisKeys = {
dev1: {
algorithms: ["1"], keys: { "ed25519:dev1": ed25519key },
signatures: {
boris: {
"ed25519:dev1":
"u99n8WZ61G//K6eVgYc+RDLVapmjttxqhjNucIFGEIJ" +
"oA4TUY8FmiGv3zl0EA71zrvPDfnFL5XLNsdc55NGbDg"
}
},
unsigned: { "abc": "def" },
}
};
var chazKeys = {
dev2: {
algorithms: ["2"], keys: { "ed25519:dev2": ed25519key },
signatures: {
chaz: {
"ed25519:dev2":
"8eaeXUWy9AQzjaNVOjVLs4FQk+cgobkNS811EjZBCMA" +
"apd8aPOfE26E13nFFOCLC1V6fOH5wVo61hxGR/j4PBA"
}
},
unsigned: { "ghi": "def" },
}
};
var daveKeys = {
dev3: {
algorithms: ["3"], keys: { "ed25519:dev2": ed25519key },
signatures: {
dave: {
"ed25519:dev2":
"8eaeXUWy9AQzjaNVOjVLs4FQk+cgobkNS811EjZBCMA" +
"apd8aPOfE26E13nFFOCLC1V6fOH5wVo61hxGR/j4PBA"
}
},
unsigned: { "ghi": "def" },
}
};

httpBackend.when("POST", "/keys/query").check(function(req) {
expect(req.data).toEqual({device_keys: {boris: {}, chaz: {}}});
expect(req.data).toEqual({device_keys: {boris: {}, chaz: {}, dave: {}}});
}).respond(200, {
device_keys: {
boris: borisKeys,
chaz: chazKeys,
dave: daveKeys,
},
});

client.downloadKeys(["boris", "chaz"]).then(function(res) {
client.downloadKeys(["boris", "chaz", "dave"]).then(function(res) {
expect(res).toEqual({
boris: {
dev1: {
verified: 0, // DeviceVerification.UNVERIFIED
keys: { "ed25519:dev1": "k1" },
keys: { "ed25519:dev1": ed25519key },
algorithms: ["1"],
unsigned: { "abc": "def" },
},
},
chaz: {
dev2: {
verified: 0, // DeviceVerification.UNVERIFIED
keys: { "ed25519:dev2" : "k2" },
keys: { "ed25519:dev2" : ed25519key },
algorithms: ["2"],
unsigned: { "ghi": "def" },
},
},
dave: {
// dave's key fails validation.
},
});
}).catch(utils.failTest).done(done);

Expand Down

0 comments on commit 75a505c

Please sign in to comment.