diff --git a/CHANGELOG.md b/CHANGELOG.md index 856d6352b..80c281831 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ -Unreleased changes -================== +Changes in [0.8.0](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v0.8.0) (2017-08-15) +================================================================================================ +[Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v0.7.13...v0.8.0) BREAKING CHANGE --------------- @@ -36,6 +37,91 @@ applications which support end-to-end encryption: in a slightly different format. + * Make bits of `olmlib` asynchronous + [\#521](https://github.com/matrix-org/matrix-js-sdk/pull/521) + * Make some of DeviceList asynchronous + [\#520](https://github.com/matrix-org/matrix-js-sdk/pull/520) + * Make methods in crypto/algorithms async + [\#519](https://github.com/matrix-org/matrix-js-sdk/pull/519) + * Avoid sending unencrypted messages in e2e room + [\#518](https://github.com/matrix-org/matrix-js-sdk/pull/518) + * Make tests wait for syncs to happen + [\#517](https://github.com/matrix-org/matrix-js-sdk/pull/517) + * Make a load of methods in the 'Crypto' module asynchronous + [\#510](https://github.com/matrix-org/matrix-js-sdk/pull/510) + * Set `rawDisplayName` to `userId` if membership has `displayname=null` + [\#515](https://github.com/matrix-org/matrix-js-sdk/pull/515) + * Refactor handling of crypto events for async + [\#508](https://github.com/matrix-org/matrix-js-sdk/pull/508) + * Let event decryption be asynchronous + [\#509](https://github.com/matrix-org/matrix-js-sdk/pull/509) + * Transform `async` functions to bluebird promises + [\#511](https://github.com/matrix-org/matrix-js-sdk/pull/511) + * Add more group APIs + [\#512](https://github.com/matrix-org/matrix-js-sdk/pull/512) + * Retrying test: wait for localEchoUpdated event + [\#507](https://github.com/matrix-org/matrix-js-sdk/pull/507) + * Fix member events breaking on timeline reset, 2 + [\#504](https://github.com/matrix-org/matrix-js-sdk/pull/504) + * Make bits of the js-sdk api asynchronous + [\#503](https://github.com/matrix-org/matrix-js-sdk/pull/503) + * Yet more js-sdk test deflakification + [\#499](https://github.com/matrix-org/matrix-js-sdk/pull/499) + * Fix racy 'matrixclient retrying' test + [\#497](https://github.com/matrix-org/matrix-js-sdk/pull/497) + * Fix spamming of key-share-requests + [\#495](https://github.com/matrix-org/matrix-js-sdk/pull/495) + * Add progress handler to `uploadContent` + [\#500](https://github.com/matrix-org/matrix-js-sdk/pull/500) + * Switch matrix-js-sdk to bluebird + [\#490](https://github.com/matrix-org/matrix-js-sdk/pull/490) + * Fix some more flakey tests + [\#492](https://github.com/matrix-org/matrix-js-sdk/pull/492) + * make the npm test script windows-friendly + [\#489](https://github.com/matrix-org/matrix-js-sdk/pull/489) + * Fix a bunch of races in the tests + [\#488](https://github.com/matrix-org/matrix-js-sdk/pull/488) + * Fix early return in MatrixClient.setGuestAccess + [\#487](https://github.com/matrix-org/matrix-js-sdk/pull/487) + * Remove testUtils.failTest + [\#486](https://github.com/matrix-org/matrix-js-sdk/pull/486) + * Add test:watch script + [\#485](https://github.com/matrix-org/matrix-js-sdk/pull/485) + * Make it possible to use async/await + [\#484](https://github.com/matrix-org/matrix-js-sdk/pull/484) + * Remove m.new_device support + [\#483](https://github.com/matrix-org/matrix-js-sdk/pull/483) + * Use access-token in header + [\#478](https://github.com/matrix-org/matrix-js-sdk/pull/478) + * Sanity-check response from /thirdparty/protocols + [\#482](https://github.com/matrix-org/matrix-js-sdk/pull/482) + * Avoid parsing plain-text errors as JSON + [\#479](https://github.com/matrix-org/matrix-js-sdk/pull/479) + * Use external mock-request + [\#481](https://github.com/matrix-org/matrix-js-sdk/pull/481) + * Fix some races in the tests + [\#480](https://github.com/matrix-org/matrix-js-sdk/pull/480) + * Fall back to MemoryCryptoStore if indexeddb fails + [\#475](https://github.com/matrix-org/matrix-js-sdk/pull/475) + * Fix load failure in firefox when indexedDB is disabled + [\#474](https://github.com/matrix-org/matrix-js-sdk/pull/474) + * Fix a race in a test + [\#471](https://github.com/matrix-org/matrix-js-sdk/pull/471) + * Avoid throwing an unhandled error when the indexeddb is deleted + [\#470](https://github.com/matrix-org/matrix-js-sdk/pull/470) + * fix jsdoc + [\#469](https://github.com/matrix-org/matrix-js-sdk/pull/469) + * Handle m.forwarded_room_key events + [\#468](https://github.com/matrix-org/matrix-js-sdk/pull/468) + * Improve error reporting from indexeddbstore.clearDatabase + [\#466](https://github.com/matrix-org/matrix-js-sdk/pull/466) + * Implement sharing of megolm keys + [\#454](https://github.com/matrix-org/matrix-js-sdk/pull/454) + * Process received room key requests + [\#449](https://github.com/matrix-org/matrix-js-sdk/pull/449) + * Send m.room_key_request events when we fail to decrypt an event + [\#448](https://github.com/matrix-org/matrix-js-sdk/pull/448) + Changes in [0.7.13](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v0.7.13) (2017-06-22) ================================================================================================== [Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v0.7.12...v0.7.13) diff --git a/package.json b/package.json index fb2378045..37c96b847 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "matrix-js-sdk", - "version": "0.7.13", + "version": "0.8.0", "description": "Matrix Client-Server SDK for Javascript", "main": "index.js", "scripts": { diff --git a/spec/unit/crypto/algorithms/megolm.spec.js b/spec/unit/crypto/algorithms/megolm.spec.js index 79257e6d7..22c3bc4f6 100644 --- a/spec/unit/crypto/algorithms/megolm.spec.js +++ b/spec/unit/crypto/algorithms/megolm.spec.js @@ -71,9 +71,13 @@ describe("MegolmDecryption", function() { groupSession = new global.Olm.OutboundGroupSession(); groupSession.create(); - const event = new MatrixEvent({}); - event.setClearData( - { + // construct a fake decrypted key event via the use of a mocked + // 'crypto' implementation. + const event = new MatrixEvent({ + type: 'm.room.encrypted', + }); + const decryptedData = { + clearEvent: { type: 'm.room_key', content: { algorithm: 'm.megolm.v1.aes-sha2', @@ -82,11 +86,19 @@ describe("MegolmDecryption", function() { session_key: groupSession.session_key(), }, }, - "SENDER_CURVE25519", - "SENDER_ED25519", - ); + senderCurve25519Key: "SENDER_CURVE25519", + claimedEd25519Key: "SENDER_ED25519", + }; - megolmDecryption.onRoomKeyEvent(event); + const mockCrypto = { + decryptEvent: function() { + return Promise.resolve(decryptedData); + }, + }; + + return event.attemptDecryption(mockCrypto).then(() => { + megolmDecryption.onRoomKeyEvent(event); + }); }); it('can decrypt an event', function() { @@ -104,8 +116,8 @@ describe("MegolmDecryption", function() { }, }); - return megolmDecryption.decryptEvent(event).then(() => { - expect(event.getContent()).toEqual('testytest'); + return megolmDecryption.decryptEvent(event).then((res) => { + expect(res.clearEvent.content).toEqual('testytest'); }); }); diff --git a/spec/unit/event.spec.js b/spec/unit/event.spec.js new file mode 100644 index 000000000..47de6615f --- /dev/null +++ b/spec/unit/event.spec.js @@ -0,0 +1,82 @@ +/* +Copyright 2017 New Vector Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import sdk from '../..'; +const MatrixEvent = sdk.MatrixEvent; + +import testUtils from '../test-utils'; + +import expect from 'expect'; +import Promise from 'bluebird'; + +describe("MatrixEvent", () => { + beforeEach(function() { + testUtils.beforeEach(this); // eslint-disable-line no-invalid-this + }); + + describe(".attemptDecryption", () => { + let encryptedEvent; + + beforeEach(() => { + encryptedEvent = new MatrixEvent({ + id: 'test_encrypted_event', + type: 'm.room.encrypted', + content: { + ciphertext: 'secrets', + }, + }); + }); + + it('should retry decryption if a retry is queued', () => { + let callCount = 0; + + let prom2; + + const crypto = { + decryptEvent: function() { + ++callCount; + console.log(`decrypt: ${callCount}`); + if (callCount == 1) { + // schedule a second decryption attempt while + // the first one is still running. + prom2 = encryptedEvent.attemptDecryption(crypto); + + const error = new Error("nope"); + error.name = 'DecryptionError'; + return Promise.reject(error); + } else { + expect(prom2.isFulfilled()).toBe( + false, 'second attemptDecryption resolved too soon'); + + return Promise.resolve({ + clearEvent: { + type: 'm.room.message', + }, + }); + } + }, + }; + + return encryptedEvent.attemptDecryption(crypto).then(() => { + expect(callCount).toEqual(2); + expect(encryptedEvent.getType()).toEqual('m.room.message'); + + // make sure the second attemptDecryption resolves + return prom2; + }); + }); + }); +}); diff --git a/src/crypto/algorithms/base.js b/src/crypto/algorithms/base.js index 15f8aed25..c74e81f00 100644 --- a/src/crypto/algorithms/base.js +++ b/src/crypto/algorithms/base.js @@ -117,7 +117,8 @@ class DecryptionAlgorithm { * * @param {MatrixEvent} event undecrypted event * - * @return {Promise} resolves once we have finished decrypting. Rejects with an + * @return {Promise} promise which + * resolves once we have finished decrypting. Rejects with an * `algorithms.DecryptionError` if there is a problem decrypting the event. */ diff --git a/src/crypto/algorithms/megolm.js b/src/crypto/algorithms/megolm.js index 7c9a6a1f4..a8dea2ed4 100644 --- a/src/crypto/algorithms/megolm.js +++ b/src/crypto/algorithms/megolm.js @@ -535,18 +535,12 @@ utils.inherits(MegolmDecryption, base.DecryptionAlgorithm); * * @param {MatrixEvent} event * - * @return {Promise} resolves once we have finished decrypting. Rejects with an - * `algorithms.DecryptionError` if there is a problem decrypting the event. + * returns a promise which resolves to a + * {@link module:crypto~EventDecryptionResult} once we have finished + * decrypting, or rejects with an `algorithms.DecryptionError` if there is a + * problem decrypting the event. */ -MegolmDecryption.prototype.decryptEvent = function(event) { - return this._decryptEvent(event, true); -}; - - -// helper for the real decryptEvent and for _retryDecryption. If -// requestKeysOnFail is true, we'll send an m.room_key_request when we fail -// to decrypt the event due to missing megolm keys. -MegolmDecryption.prototype._decryptEvent = async function(event, requestKeysOnFail) { +MegolmDecryption.prototype.decryptEvent = async function(event) { const content = event.getWireContent(); if (!content.sender_key || !content.session_id || @@ -563,9 +557,7 @@ MegolmDecryption.prototype._decryptEvent = async function(event, requestKeysOnFa } catch (e) { if (e.message === 'OLM.UNKNOWN_MESSAGE_INDEX') { this._addEventToPendingList(event); - if (requestKeysOnFail) { - this._requestKeysForEvent(event); - } + this._requestKeysForEvent(event); } throw new base.DecryptionError( e.toString(), { @@ -577,9 +569,7 @@ MegolmDecryption.prototype._decryptEvent = async function(event, requestKeysOnFa if (res === null) { // We've got a message for a session we don't have. this._addEventToPendingList(event); - if (requestKeysOnFail) { - this._requestKeysForEvent(event); - } + this._requestKeysForEvent(event); throw new base.DecryptionError( "The sender's device has not sent us the keys for this message.", { @@ -599,8 +589,12 @@ MegolmDecryption.prototype._decryptEvent = async function(event, requestKeysOnFa ); } - event.setClearData(payload, res.senderKey, res.keysClaimed.ed25519, - res.forwardingCurve25519KeyChain); + return { + clearEvent: payload, + senderCurve25519Key: res.senderKey, + claimedEd25519Key: res.keysClaimed.ed25519, + forwardingCurve25519KeyChain: res.forwardingCurve25519KeyChain, + }; }; MegolmDecryption.prototype._requestKeysForEvent = function(event) { diff --git a/src/crypto/algorithms/olm.js b/src/crypto/algorithms/olm.js index 8842fcbda..51b3d1861 100644 --- a/src/crypto/algorithms/olm.js +++ b/src/crypto/algorithms/olm.js @@ -157,8 +157,10 @@ utils.inherits(OlmDecryption, base.DecryptionAlgorithm); * * @param {MatrixEvent} event * - * returns a promise which resolves once we have finished decrypting. Rejects with an - * `algorithms.DecryptionError` if there is a problem decrypting the event. + * returns a promise which resolves to a + * {@link module:crypto~EventDecryptionResult} once we have finished + * decrypting. Rejects with an `algorithms.DecryptionError` if there is a + * problem decrypting the event. */ OlmDecryption.prototype.decryptEvent = async function(event) { const content = event.getWireContent(); @@ -227,9 +229,13 @@ OlmDecryption.prototype.decryptEvent = async function(event) { } const claimedKeys = payload.keys || {}; - event.setClearData(payload, deviceKey, claimedKeys.ed25519 || null); -}; + return { + clearEvent: payload, + senderCurve25519Key: deviceKey, + claimedEd25519Key: claimedKeys.ed25519 || null, + }; +}; /** * Attempt to decrypt an Olm message diff --git a/src/crypto/index.js b/src/crypto/index.js index 510bd395b..2571330e6 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -772,8 +772,9 @@ Crypto.prototype.encryptEvent = function(event, room) { * * @param {MatrixEvent} event * - * @return {Promise} resolves once we have finished decrypting. Rejects with an - * `algorithms.DecryptionError` if there is a problem decrypting the event. + * @return {Promise} resolves once we have + * finished decrypting. Rejects with an `algorithms.DecryptionError` if there + * is a problem decrypting the event. */ Crypto.prototype.decryptEvent = function(event) { const content = event.getWireContent(); @@ -1295,6 +1296,27 @@ class IncomingRoomKeyRequestCancellation { } } +/** + * The result of a (successful) call to decryptEvent. + * + * @typedef {Object} EventDecryptionResult + * + * @property {Object} clearEvent The plaintext payload for the event + * (typically containing type and content fields). + * + * @property {?string} senderCurve25519Key Key owned by the sender of this + * event. See {@link module:models/event.MatrixEvent#getSenderKey}. + * + * @property {?string} claimedEd25519Key ed25519 key claimed by the sender of + * this event. See + * {@link module:models/event.MatrixEvent#getClaimedEd25519Key}. + * + * @property {?Array} forwardingCurve25519KeyChain list of curve25519 + * keys involved in telling us about the senderCurve25519Key and + * claimedEd25519Key. See + * {@link module:models/event.MatrixEvent#getForwardingCurve25519KeyChain}. + */ + /** * Fires when we receive a room key request * diff --git a/src/models/event.js b/src/models/event.js index 0e2f862ac..89f574851 100644 --- a/src/models/event.js +++ b/src/models/event.js @@ -21,11 +21,9 @@ limitations under the License. * @module models/event */ -const EventEmitter = require("events").EventEmitter; - -const utils = require('../utils.js'); - import Promise from 'bluebird'; +import {EventEmitter} from 'events'; +import utils from '../utils.js'; /** * Enum for event statuses. @@ -131,8 +129,10 @@ module.exports.MatrixEvent = function MatrixEvent( */ this._forwardingCurve25519KeyChain = []; - /* flag to indicate if we have a process decrypting this event */ - this._decrypting = false; + /* if we have a process decrypting this event, a Promise which resolves + * when it is finished. Normally null. + */ + this._decryptionPromise = null; /* flag to indicate if we should retry decrypting this event after the * first attempt (eg, we have received new data which means that a second @@ -315,7 +315,7 @@ utils.extend(module.exports.MatrixEvent.prototype, { * @return {boolean} True if this event is currently being decrypted, else false. */ isBeingDecrypted: function() { - return this._decrypting; + return this._decryptionPromise != null; }, /** @@ -326,13 +326,12 @@ utils.extend(module.exports.MatrixEvent.prototype, { * @internal * * @param {module:crypto} crypto crypto module + * + * @returns {Promise} promise which resolves (to undefined) when the decryption + * attempt is completed. */ - attemptDecryption: function(crypto) { - if (!crypto) { - this._badEncryptedMessage("Encryption not enabled"); - return; - } - + attemptDecryption: async function(crypto) { + // start with a couple of sanity checks. if (!this.isEncrypted()) { throw new Error("Attempt to decrypt event which isn't encrypted"); } @@ -347,60 +346,112 @@ utils.extend(module.exports.MatrixEvent.prototype, { ); } - if (this._decrypting) { + // if we already have a decryption attempt in progress, then it may + // fail because it was using outdated info. We now have reason to + // succeed where it failed before, but we don't want to have multiple + // attempts going at the same time, so just set a flag that says we have + // new info. + // + if (this._decryptionPromise) { console.log( `Event ${this.getId()} already being decrypted; queueing a retry`, ); this._retryDecryption = true; - return; + return this._decryptionPromise; } - this._decrypting = true; - - this._doDecryption(crypto).finally(() => { - this._decrypting = false; - this._retryDecryption = false; - }); + this._decryptionPromise = this._decryptionLoop(crypto); + return this._decryptionPromise; }, - _doDecryption: function(crypto) { - return Promise.try(() => { - return crypto.decryptEvent(this); - }).catch((e) => { - if (e.name !== "DecryptionError") { - // not a decryption error: log the whole exception as an error. - console.error( - `Error decrypting event (id=${this.getId()}): ${e.stack || e}`, - ); - return null; - } else if (this._retryDecryption) { - // decryption error, but we have a retry queued. - console.log( - `Got error decrypting event (id=${this.getId()}), but retrying`, - ); - this._retryDecryption = false; - return this._doDecryption(crypto); - } else { + _decryptionLoop: async function(crypto) { + // make sure that this method never runs completely synchronously. + // (doing so would mean that we would clear _decryptionPromise *before* + // it is set in attemptDecryption - and hence end up with a stuck + // `_decryptionPromise`). + await Promise.resolve(); + + while (true) { + this._retryDecryption = false; + + let res; + try { + if (!crypto) { + res = this._badEncryptedMessage("Encryption not enabled"); + } else { + res = await crypto.decryptEvent(this); + } + } catch (e) { + if (e.name !== "DecryptionError") { + // not a decryption error: log the whole exception as an error + // (and don't bother with a retry) + console.error( + `Error decrypting event (id=${this.getId()}): ${e.stack || e}`, + ); + this._decryptionPromise = null; + this._retryDecryption = false; + return; + } + + // see if we have a retry queued. + // + // NB: make sure to keep this check in the same tick of the + // event loop as `_decryptionPromise = null` below - otherwise we + // risk a race: + // + // * A: we check _retryDecryption here and see that it is + // false + // * B: we get a second call to attemptDecryption, which sees + // that _decryptionPromise is set so sets + // _retryDecryption + // * A: we continue below, clear _decryptionPromise, and + // never do the retry. + // + if (this._retryDecryption) { + // decryption error, but we have a retry queued. + console.log( + `Got error decrypting event (id=${this.getId()}: ` + + `${e.message}), but retrying`, + ); + continue; + } + // decryption error, no retries queued. Warn about the error and // set it to m.bad.encrypted. console.warn( `Error decrypting event (id=${this.getId()}): ${e}`, ); - this._badEncryptedMessage(e.message); - return null; + res = this._badEncryptedMessage(e.message); } - }); + + // at this point, we've either successfully decrypted the event, or have given up + // (and set res to a 'badEncryptedMessage'). Either way, we can now set the + // cleartext of the event and raise Event.decrypted. + // + // make sure we clear '_decryptionPromise' before sending the 'Event.decrypted' event, + // otherwise the app will be confused to see `isBeingDecrypted` still set when + // there isn't an `Event.decrypted` on the way. + // + // see also notes on _retryDecryption above. + // + this._decryptionPromise = null; + this._retryDecryption = false; + this._setClearData(res); + return; + } }, _badEncryptedMessage: function(reason) { - this.setClearData({ - type: "m.room.message", - content: { - msgtype: "m.bad.encrypted", - body: "** Unable to decrypt: " + reason + " **", + return { + clearEvent: { + type: "m.room.message", + content: { + msgtype: "m.bad.encrypted", + body: "** Unable to decrypt: " + reason + " **", + }, }, - }); + }; }, /** @@ -412,29 +463,17 @@ utils.extend(module.exports.MatrixEvent.prototype, { * * @fires module:models/event.MatrixEvent#"Event.decrypted" * - * @param {Object} clearEvent The plaintext payload for the event - * (typically containing type and content fields). - * - * @param {string=} senderCurve25519Key Key owned by the sender of this event. - * See {@link module:models/event.MatrixEvent#getSenderKey}. - * - * @param {string=} claimedEd25519Key ed25519 key claimed by the sender of - * this event. See {@link module:models/event.MatrixEvent#getClaimedEd25519Key}. - * - * @param {Array=} forwardingCurve25519KeyChain list of curve25519 keys - * involved in telling us about the senderCurve25519Key and claimedEd25519Key. - * See {@link module:models/event.MatrixEvent#getForwardingCurve25519KeyChain}. + * @param {module:crypto~EventDecryptionResult} decryptionResult + * the decryption result, including the plaintext and some key info */ - setClearData: function( - clearEvent, - senderCurve25519Key, - claimedEd25519Key, - forwardingCurve25519KeyChain, - ) { - this._clearEvent = clearEvent; - this._senderCurve25519Key = senderCurve25519Key || null; - this._claimedEd25519Key = claimedEd25519Key || null; - this._forwardingCurve25519KeyChain = forwardingCurve25519KeyChain || []; + _setClearData: function(decryptionResult) { + this._clearEvent = decryptionResult.clearEvent; + this._senderCurve25519Key = + decryptionResult.senderCurve25519Key || null; + this._claimedEd25519Key = + decryptionResult.claimedEd25519Key || null; + this._forwardingCurve25519KeyChain = + decryptionResult.forwardingCurve25519KeyChain || []; this.emit("Event.decrypted", this); },