diff --git a/CHANGELOG.md b/CHANGELOG.md index b88da3c3d..bdfc2363a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,58 @@ +Changes in [9.9.0](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v9.9.0) (2021-03-15) +================================================================================================ +[Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v9.9.0-rc.1...v9.9.0) + + * No changes since rc.1 + +Changes in [9.9.0-rc.1](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v9.9.0-rc.1) (2021-03-10) +========================================================================================================== +[Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v9.8.0...v9.9.0-rc.1) + + * Remove detailed Olm session logging + [\#1638](https://github.com/matrix-org/matrix-js-sdk/pull/1638) + * Add space summary suggested only param + [\#1637](https://github.com/matrix-org/matrix-js-sdk/pull/1637) + * Check TURN servers periodically, and at start of calls + [\#1634](https://github.com/matrix-org/matrix-js-sdk/pull/1634) + * Support sending invite reasons + [\#1624](https://github.com/matrix-org/matrix-js-sdk/pull/1624) + * Bump elliptic from 6.5.3 to 6.5.4 + [\#1636](https://github.com/matrix-org/matrix-js-sdk/pull/1636) + * Add a function to get a room's MXC URI + [\#1635](https://github.com/matrix-org/matrix-js-sdk/pull/1635) + * Stop streams if the call has ended + [\#1633](https://github.com/matrix-org/matrix-js-sdk/pull/1633) + * Remove export keyword from global.d.ts + [\#1631](https://github.com/matrix-org/matrix-js-sdk/pull/1631) + * Fix IndexedDB store creation example + [\#1445](https://github.com/matrix-org/matrix-js-sdk/pull/1445) + * An attempt to cleanup how constraints are handled in calls + [\#1613](https://github.com/matrix-org/matrix-js-sdk/pull/1613) + * Extract display name patterns to constants + [\#1628](https://github.com/matrix-org/matrix-js-sdk/pull/1628) + * Bump pug-code-gen from 2.0.2 to 2.0.3 + [\#1630](https://github.com/matrix-org/matrix-js-sdk/pull/1630) + * Avoid deadlocks when ensuring Olm sessions for devices + [\#1627](https://github.com/matrix-org/matrix-js-sdk/pull/1627) + * Filter out edits from other senders in history + [\#1626](https://github.com/matrix-org/matrix-js-sdk/pull/1626) + * Fix ContentHelpers export + [\#1618](https://github.com/matrix-org/matrix-js-sdk/pull/1618) + * Add logging to in progress Olm sessions + [\#1621](https://github.com/matrix-org/matrix-js-sdk/pull/1621) + * Don't ignore ICE candidates received before offer/answer + [\#1623](https://github.com/matrix-org/matrix-js-sdk/pull/1623) + * Better handling of send failures on VoIP events + [\#1622](https://github.com/matrix-org/matrix-js-sdk/pull/1622) + * Log when turn creds expire + [\#1620](https://github.com/matrix-org/matrix-js-sdk/pull/1620) + * Initial Spaces [MSC1772] support + [\#1563](https://github.com/matrix-org/matrix-js-sdk/pull/1563) + * Add logging to crypto store transactions + [\#1617](https://github.com/matrix-org/matrix-js-sdk/pull/1617) + * Room helper for getting type and checking if it is a space room + [\#1610](https://github.com/matrix-org/matrix-js-sdk/pull/1610) + Changes in [9.8.0](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v9.8.0) (2021-03-01) ================================================================================================ [Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v9.8.0-rc.1...v9.8.0) diff --git a/package.json b/package.json index b7d1a29d9..c4b9e4047 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "matrix-js-sdk", - "version": "9.8.0", + "version": "9.9.0", "description": "Matrix Client-Server SDK for Javascript", "scripts": { "prepublishOnly": "yarn build", @@ -15,7 +15,7 @@ "build:minify-browser": "terser dist/browser-matrix.js --compress --mangle --source-map --output dist/browser-matrix.min.js", "gendoc": "jsdoc -c jsdoc.json -P package.json", "lint": "yarn lint:types && yarn lint:js", - "lint:js": "eslint --max-warnings 73 src spec", + "lint:js": "eslint --max-warnings 72 src spec", "lint:types": "tsc --noEmit", "test": "jest spec/ --coverage --testEnvironment node", "test:watch": "jest spec/ --coverage --testEnvironment node --watch" diff --git a/spec/unit/crypto/DeviceList.spec.js b/spec/unit/crypto/DeviceList.spec.js index 3ba25ae1f..a9ff83054 100644 --- a/spec/unit/crypto/DeviceList.spec.js +++ b/spec/unit/crypto/DeviceList.spec.js @@ -51,6 +51,36 @@ const signedDeviceList = { }, }; +const signedDeviceList2 = { + "failures": {}, + "device_keys": { + "@test2:sw1v.org": { + "QJVRHWAKGH": { + "signatures": { + "@test2:sw1v.org": { + "ed25519:QJVRHWAKGH": + "w1xxdLe1iIqzEFHLRVYQeuiM6t2N2ZRiI8s5nDKxf054BP8" + + "1CPEX/AQXh5BhkKAVMlKnwg4T9zU1/wBALeajk3", + }, + }, + "user_id": "@test2:sw1v.org", + "keys": { + "ed25519:QJVRHWAKGH": + "Ig0/C6T+bBII1l2By2Wnnvtjp1nm/iXBlLU5/QESFXL", + "curve25519:QJVRHWAKGH": + "YR3eQnUvTQzGlWih4rsmJkKxpDxzgkgIgsBd1DEZIbm", + }, + "algorithms": [ + "m.olm.v1.curve25519-aes-sha2", + "m.megolm.v1.aes-sha2", + ], + "device_id": "QJVRHWAKGH", + "unsigned": {}, + }, + }, + }, +}; + describe('DeviceList', function() { let downloadSpy; let cryptoStore; @@ -69,7 +99,7 @@ describe('DeviceList', function() { } }); - function createTestDeviceList() { + function createTestDeviceList(keyDownloadChunkSize = 250) { const baseApis = { downloadKeysForUsers: downloadSpy, getUserId: () => '@test1:sw1v.org', @@ -78,7 +108,7 @@ describe('DeviceList', function() { const mockOlm = { verifySignature: function(key, message, signature) {}, }; - const dl = new DeviceList(baseApis, cryptoStore, mockOlm); + const dl = new DeviceList(baseApis, cryptoStore, mockOlm, keyDownloadChunkSize); deviceLists.push(dl); return dl; } @@ -150,4 +180,30 @@ describe('DeviceList', function() { expect(Object.keys(storedKeys)).toEqual(['HGKAWHRVJQ']); }); }); + + it("should download device keys in batches", function() { + const dl = createTestDeviceList(1); + + dl.startTrackingDeviceList('@test1:sw1v.org'); + dl.startTrackingDeviceList('@test2:sw1v.org'); + + const queryDefer1 = utils.defer(); + downloadSpy.mockReturnValueOnce(queryDefer1.promise); + const queryDefer2 = utils.defer(); + downloadSpy.mockReturnValueOnce(queryDefer2.promise); + + const prom1 = dl.refreshOutdatedDeviceLists(); + expect(downloadSpy).toBeCalledTimes(2); + expect(downloadSpy).toHaveBeenNthCalledWith(1, ['@test1:sw1v.org'], {}); + expect(downloadSpy).toHaveBeenNthCalledWith(2, ['@test2:sw1v.org'], {}); + queryDefer1.resolve(utils.deepCopy(signedDeviceList)); + queryDefer2.resolve(utils.deepCopy(signedDeviceList2)); + + return prom1.then(() => { + const storedKeys1 = dl.getRawStoredDevicesForUser('@test1:sw1v.org'); + expect(Object.keys(storedKeys1)).toEqual(['HGKAWHRVJQ']); + const storedKeys2 = dl.getRawStoredDevicesForUser('@test2:sw1v.org'); + expect(Object.keys(storedKeys2)).toEqual(['QJVRHWAKGH']); + }); + }); }); diff --git a/spec/unit/crypto/algorithms/olm.spec.js b/spec/unit/crypto/algorithms/olm.spec.js index 0efe74e77..c548355b5 100644 --- a/spec/unit/crypto/algorithms/olm.spec.js +++ b/spec/unit/crypto/algorithms/olm.spec.js @@ -190,5 +190,91 @@ describe("OlmDevice", function() { // new session and will have called claimOneTimeKeys expect(count).toBe(2); }); + + it("avoids deadlocks when two tasks are ensuring the same devices", async function() { + // This test checks whether `ensureOlmSessionsForDevices` properly + // handles multiple tasks in flight ensuring some set of devices in + // common without deadlocks. + + let claimRequestCount = 0; + const baseApis = { + claimOneTimeKeys: () => { + // simulate a very slow server (.5 seconds to respond) + claimRequestCount++; + return new Promise((resolve, reject) => { + setTimeout(reject, 500); + }); + }, + }; + + const deviceBobA = DeviceInfo.fromStorage({ + keys: { + "curve25519:BOB-A": "akey", + }, + }, "BOB-A"); + const deviceBobB = DeviceInfo.fromStorage({ + keys: { + "curve25519:BOB-B": "bkey", + }, + }, "BOB-B"); + + // There's no required ordering of devices per user, so here we + // create two different orderings so that each task reserves a + // device the other task needs before continuing. + const devicesByUserAB = { + "@bob:example.com": [ + deviceBobA, + deviceBobB, + ], + }; + const devicesByUserBA = { + "@bob:example.com": [ + deviceBobB, + deviceBobA, + ], + }; + + function alwaysSucceed(promise) { + // swallow any exception thrown by a promise, so that + // Promise.all doesn't abort + return promise.catch(() => {}); + } + + const task1 = alwaysSucceed(olmlib.ensureOlmSessionsForDevices( + aliceOlmDevice, baseApis, devicesByUserAB, + )); + + // After a single tick through the first task, it should have + // claimed ownership of all devices to avoid deadlocking others. + expect(Object.keys(aliceOlmDevice._sessionsInProgress).length).toBe(2); + + const task2 = alwaysSucceed(olmlib.ensureOlmSessionsForDevices( + aliceOlmDevice, baseApis, devicesByUserBA, + )); + + // The second task should not have changed the ownership count, as + // it's waiting on the first task. + expect(Object.keys(aliceOlmDevice._sessionsInProgress).length).toBe(2); + + // Track the tasks, but don't await them yet. + const promises = Promise.all([ + task1, + task2, + ]); + + await new Promise((resolve) => { + setTimeout(resolve, 200); + }); + + // After .2s, the first task should have made an initial claim request. + expect(claimRequestCount).toBe(1); + + await promises; + + // After waiting for both tasks to complete, the first task should + // have failed, so the second task should have tried to create a + // new session and will have called claimOneTimeKeys + expect(claimRequestCount).toBe(2); + }); }); }); diff --git a/spec/unit/crypto/cross-signing.spec.js b/spec/unit/crypto/cross-signing.spec.js index 056344e5d..1195db2db 100644 --- a/spec/unit/crypto/cross-signing.spec.js +++ b/spec/unit/crypto/cross-signing.spec.js @@ -193,7 +193,9 @@ describe("Cross Signing", function() { const keyChangePromise = new Promise((resolve, reject) => { alice.once("crossSigning.keysChanged", async (e) => { resolve(e); - await alice.checkOwnCrossSigningTrust(); + await alice.checkOwnCrossSigningTrust({ + allowPrivateKeyRequests: true, + }); }); }); diff --git a/spec/unit/utils.spec.js b/spec/unit/utils.spec.js index 0b9f8ab93..686912913 100644 --- a/spec/unit/utils.spec.js +++ b/spec/unit/utils.spec.js @@ -282,4 +282,30 @@ describe("utils", function() { expect(target.nonenumerableProp).toBe(undefined); }); }); + + describe("chunkPromises", function() { + it("should execute promises in chunks", async function() { + let promiseCount = 0; + + function fn1() { + return new Promise(async function(resolve, reject) { + await utils.sleep(1); + expect(promiseCount).toEqual(0); + ++promiseCount; + resolve(); + }); + } + + function fn2() { + return new Promise(function(resolve, reject) { + expect(promiseCount).toEqual(1); + ++promiseCount; + resolve(); + }); + } + + await utils.chunkPromises([fn1, fn2], 1); + expect(promiseCount).toEqual(2); + }); + }); }); diff --git a/spec/unit/webrtc/call.spec.ts b/spec/unit/webrtc/call.spec.ts index 7336a7371..0e7b67547 100644 --- a/spec/unit/webrtc/call.spec.ts +++ b/spec/unit/webrtc/call.spec.ts @@ -79,6 +79,7 @@ class MockRTCPeerConnection { return Promise.resolve(); } close() {} + getStats() { return []; } } describe('Call', function() { @@ -122,6 +123,7 @@ describe('Call', function() { // We just stub out sendEvent: we're not interested in testing the client's // event sending code here client.client.sendEvent = () => {}; + client.httpBackend.when("GET", "/voip/turnServer").respond(200, {}); call = new MatrixCall({ client: client.client, roomId: '!foo:bar', @@ -138,7 +140,9 @@ describe('Call', function() { }); it('should ignore candidate events from non-matching party ID', async function() { - await call.placeVoiceCall(); + const callPromise = call.placeVoiceCall(); + await client.httpBackend.flush(); + await callPromise; await call.onAnswerReceived({ getContent: () => { return { @@ -192,7 +196,9 @@ describe('Call', function() { }); it('should add candidates received before answer if party ID is correct', async function() { - await call.placeVoiceCall(); + const callPromise = call.placeVoiceCall(); + await client.httpBackend.flush(); + await callPromise; call.peerConn.addIceCandidate = jest.fn(); call.onRemoteIceCandidatesReceived({ diff --git a/src/@types/global.d.ts b/src/@types/global.d.ts index 5fdfda5fc..dd437247a 100644 --- a/src/@types/global.d.ts +++ b/src/@types/global.d.ts @@ -41,7 +41,7 @@ declare global { getUserMedia(constraints: MediaStreamConstraints | DesktopCapturerConstraints): Promise; } - export interface DesktopCapturerConstraints { + interface DesktopCapturerConstraints { audio: boolean | { mandatory: { chromeMediaSource: string; @@ -56,7 +56,7 @@ declare global { }; } - export interface DesktopCapturerSource { + interface DesktopCapturerSource { id: string; name: string; thumbnailURL: string; diff --git a/src/base-apis.js b/src/base-apis.js index 37c3d76d6..1bdfb1f7e 100644 --- a/src/base-apis.js +++ b/src/base-apis.js @@ -246,10 +246,25 @@ MatrixBaseApis.prototype.register = function( /** * Register a guest account. + * This method returns the auth info needed to create a new authenticated client, + * Remember to call `setGuest(true)` on the (guest-)authenticated client, e.g: + * ```javascript + * const tmpClient = await sdk.createClient(MATRIX_INSTANCE); + * const { user_id, device_id, access_token } = tmpClient.registerGuest(); + * const client = createClient({ + * baseUrl: MATRIX_INSTANCE, + * accessToken: access_token, + * userId: user_id, + * deviceId: device_id, + * }) + * client.setGuest(true); + * ``` + * * @param {Object=} opts Registration options * @param {Object} opts.body JSON HTTP body to provide. * @param {module:client.callback} callback Optional. - * @return {Promise} Resolves: TODO + * @return {Promise} Resolves: JSON object that contains: + * { user_id, device_id, access_token, home_server } * @return {module:http-api.MatrixError} Rejects: with an error response. */ MatrixBaseApis.prototype.registerGuest = function(opts, callback) { @@ -1518,6 +1533,21 @@ MatrixBaseApis.prototype.getDevices = function() { ); }; +/** + * Gets specific device details for the logged-in user + * @param {string} device_id device to query + * @return {Promise} Resolves: result object + * @return {module:http-api.MatrixError} Rejects: with an error response. + */ +MatrixBaseApis.prototype.getDevice = function(device_id) { + const path = utils.encodeUri("/devices/$device_id", { + $device_id: device_id, + }); + return this._http.authedRequest( + undefined, 'GET', path, undefined, undefined, + ); +}; + /** * Update the given device * @@ -2378,18 +2408,27 @@ MatrixBaseApis.prototype.reportEvent = function(roomId, eventId, score, reason) * Fetches or paginates a summary of a space as defined by MSC2946 * @param {string} roomId The ID of the space-room to use as the root of the summary. * @param {number?} maxRoomsPerSpace The maximum number of rooms to return per subspace. + * @param {boolean?} suggestedOnly Whether to only return rooms with suggested=true. * @param {boolean?} autoJoinOnly Whether to only return rooms with auto_join=true. * @param {number?} limit The maximum number of rooms to return in total. * @param {string?} batch The opaque token to paginate a previous summary request. * @returns {Promise} the response, with next_batch, rooms, events fields. */ -MatrixBaseApis.prototype.getSpaceSummary = function(roomId, maxRoomsPerSpace, autoJoinOnly, limit, batch) { +MatrixBaseApis.prototype.getSpaceSummary = function( + roomId, + maxRoomsPerSpace, + suggestedOnly, + autoJoinOnly, + limit, + batch, +) { const path = utils.encodeUri("/rooms/$roomId/spaces", { $roomId: roomId, }); return this._http.authedRequest(undefined, "POST", path, null, { max_rooms_per_space: maxRoomsPerSpace, + suggested_only: suggestedOnly, auto_join_only: autoJoinOnly, limit, batch, diff --git a/src/client.js b/src/client.js index 65ca597eb..f845a5a08 100644 --- a/src/client.js +++ b/src/client.js @@ -61,6 +61,7 @@ import {DEHYDRATION_ALGORITHM} from "./crypto/dehydration"; const SCROLLBACK_DELAY_MS = 3000; export const CRYPTO_ENABLED = isCryptoAvailable(); const CAPABILITIES_CACHE_MS = 21600000; // 6 hours - an arbitrary value +const TURN_CHECK_INTERVAL = 10 * 60 * 1000; // poll for turn credentials every 10 minutes function keysFromRecoverySession(sessions, decryptionKey, roomId) { const keys = []; @@ -394,7 +395,8 @@ export function MatrixClient(opts) { this._clientWellKnownPromise = undefined; this._turnServers = []; - this._turnServersExpiry = null; + this._turnServersExpiry = 0; + this._checkTurnServersIntervalID = null; // The SDK doesn't really provide a clean way for events to recalculate the push // actions for themselves, so we have to kinda help them out when they are encrypted. @@ -498,19 +500,8 @@ MatrixClient.prototype.rehydrateDevice = async function() { return; } - let getDeviceResult; - try { - getDeviceResult = await this._http.authedRequest( - undefined, - "GET", - "/dehydrated_device", - undefined, undefined, - { - prefix: "/_matrix/client/unstable/org.matrix.msc2697.v2", - }, - ); - } catch (e) { - logger.info("could not get dehydrated device", e.toString()); + const getDeviceResult = this.getDehydratedDevice(); + if (!getDeviceResult) { return; } @@ -572,6 +563,27 @@ MatrixClient.prototype.rehydrateDevice = async function() { } }; +/** + * Get the current dehydrated device, if any + * @return {Promise} A promise of an object containing the dehydrated device + */ +MatrixClient.prototype.getDehydratedDevice = async function() { + try { + return await this._http.authedRequest( + undefined, + "GET", + "/dehydrated_device", + undefined, undefined, + { + prefix: "/_matrix/client/unstable/org.matrix.msc2697.v2", + }, + ); + } catch (e) { + logger.info("could not get dehydrated device", e.toString()); + return; + } +}; + /** * Set the dehydration key. This will also periodically dehydrate devices to * the server. @@ -3483,11 +3495,12 @@ MatrixClient.prototype.getRoomUpgradeHistory = function(roomId, verifyLinks=fals * @param {string} roomId * @param {string} userId * @param {module:client.callback} callback Optional. + * @param {string} reason Optional. * @return {Promise} Resolves: TODO * @return {module:http-api.MatrixError} Rejects: with an error response. */ -MatrixClient.prototype.invite = function(roomId, userId, callback) { - return _membershipChange(this, roomId, userId, "invite", undefined, +MatrixClient.prototype.invite = function(roomId, userId, callback, reason) { + return _membershipChange(this, roomId, userId, "invite", reason, callback); }; @@ -4987,6 +5000,48 @@ MatrixClient.prototype.getTurnServersExpiry = function() { return this._turnServersExpiry; }; +MatrixClient.prototype._checkTurnServers = async function() { + if (!this._supportsVoip) { + return; + } + + let credentialsGood = false; + const remainingTime = this._turnServersExpiry - Date.now(); + if (remainingTime > TURN_CHECK_INTERVAL) { + logger.debug("TURN creds are valid for another " + remainingTime + " ms: not fetching new ones."); + credentialsGood = true; + } else { + logger.debug("Fetching new TURN credentials"); + try { + const res = await this.turnServer(); + if (res.uris) { + logger.log("Got TURN URIs: " + res.uris + " refresh in " + res.ttl + " secs"); + // map the response to a format that can be fed to RTCPeerConnection + const servers = { + urls: res.uris, + username: res.username, + credential: res.password, + }; + this._turnServers = [servers]; + // The TTL is in seconds but we work in ms + this._turnServersExpiry = Date.now() + (res.ttl * 1000); + credentialsGood = true; + } + } catch (err) { + logger.error("Failed to get TURN URIs", err); + // If we get a 403, there's no point in looping forever. + if (err.httpStatus === 403) { + logger.info("TURN access unavailable for this account: stopping credentials checks"); + if (this._checkTurnServersIntervalID !== null) global.clearInterval(this._checkTurnServersIntervalID); + this._checkTurnServersIntervalID = null; + } + } + // otherwise, if we failed for whatever reason, try again the next time we're called. + } + + return credentialsGood; +}; + /** * Set whether to allow a fallback ICE server should be used for negotiating a * WebRTC connection if the homeserver doesn't provide any servers. Defaults to @@ -5139,7 +5194,12 @@ MatrixClient.prototype.startClient = async function(opts) { } // periodically poll for turn servers if we support voip - checkTurnServers(this); + if (this._supportsVoip) { + this._checkTurnServersIntervalID = setInterval(() => { + this._checkTurnServers(); + }, TURN_CHECK_INTERVAL); + this._checkTurnServers(); + } if (this._syncApi) { // This shouldn't happen since we thought the client was not running @@ -5251,7 +5311,7 @@ MatrixClient.prototype.stopClient = function() { this._callEventHandler = null; } - global.clearTimeout(this._checkTurnServersTimeoutID); + global.clearInterval(this._checkTurnServersIntervalID); if (this._clientWellKnownIntervalID !== undefined) { global.clearInterval(this._clientWellKnownIntervalID); } @@ -5458,6 +5518,9 @@ async function(roomId, eventId, relationType, eventType, opts = {}) { })); events = events.filter(e => e.getType() === eventType); } + if (originalEvent && relationType === "m.replace") { + events = events.filter(e => e.getSender() === originalEvent.getSender()); + } return { originalEvent, events, @@ -5465,42 +5528,6 @@ async function(roomId, eventId, relationType, eventType, opts = {}) { }; }; -function checkTurnServers(client) { - if (!client._supportsVoip) { - return; - } - - client.turnServer().then(function(res) { - if (res.uris) { - logger.log("Got TURN URIs: " + res.uris + " refresh in " + - res.ttl + " secs"); - // map the response to a format that can be fed to - // RTCPeerConnection - const servers = { - urls: res.uris, - username: res.username, - credential: res.password, - }; - client._turnServers = [servers]; - client._turnServersExpiry = Date.now() + res.ttl; - // re-fetch when we're about to reach the TTL - client._checkTurnServersTimeoutID = setTimeout(() => { - checkTurnServers(client); - }, (res.ttl || (60 * 60)) * 1000 * 0.9); - } - }, function(err) { - logger.error("Failed to get TURN URIs"); - // If we get a 403, there's no point in looping forever. - if (err.httpStatus === 403) { - logger.info("TURN access unavailable for this account"); - return; - } - client._checkTurnServersTimeoutID = setTimeout(function() { - checkTurnServers(client); - }, 60000); - }); -} - function _reject(callback, reject, err) { if (callback) { callback(err); diff --git a/src/crypto/DeviceList.js b/src/crypto/DeviceList.js index 797f4b860..97836b5a4 100644 --- a/src/crypto/DeviceList.js +++ b/src/crypto/DeviceList.js @@ -28,7 +28,7 @@ import {DeviceInfo} from './deviceinfo'; import {CrossSigningInfo} from './CrossSigning'; import * as olmlib from './olmlib'; import {IndexedDBCryptoStore} from './store/indexeddb-crypto-store'; -import {defer, sleep} from '../utils'; +import {chunkPromises, defer, sleep} from '../utils'; /* State transition diagram for DeviceList._deviceTrackingStatus @@ -62,7 +62,7 @@ const TRACKING_STATUS_UP_TO_DATE = 3; * @alias module:crypto/DeviceList */ export class DeviceList extends EventEmitter { - constructor(baseApis, cryptoStore, olmDevice) { + constructor(baseApis, cryptoStore, olmDevice, keyDownloadChunkSize = 250) { super(); this._cryptoStore = cryptoStore; @@ -98,6 +98,9 @@ export class DeviceList extends EventEmitter { // userId -> promise this._keyDownloadsInProgressByUser = {}; + // Maximum number of user IDs per request to prevent server overload (#1619) + this._keyDownloadChunkSize = keyDownloadChunkSize; + // Set whenever changes are made other than setting the sync token this._dirty = false; @@ -780,13 +783,17 @@ class DeviceListUpdateSerialiser { opts.token = this._syncToken; } - this._baseApis.downloadKeysForUsers( - downloadUsers, opts, - ).then(async (res) => { - const dk = res.device_keys || {}; - const masterKeys = res.master_keys || {}; - const ssks = res.self_signing_keys || {}; - const usks = res.user_signing_keys || {}; + const factories = []; + for (let i = 0; i < downloadUsers.length; i += this._deviceList._keyDownloadChunkSize) { + const userSlice = downloadUsers.slice(i, i + this._deviceList._keyDownloadChunkSize); + factories.push(() => this._baseApis.downloadKeysForUsers(userSlice, opts)); + } + + chunkPromises(factories, 3).then(async (responses) => { + const dk = Object.assign({}, ...(responses.map(res => res.device_keys || {}))); + const masterKeys = Object.assign({}, ...(responses.map(res => res.master_keys || {}))); + const ssks = Object.assign({}, ...(responses.map(res => res.self_signing_keys || {}))); + const usks = Object.assign({}, ...(responses.map(res => res.user_signing_keys || {}))); // yield to other things that want to execute in between users, to // avoid wedging the CPU diff --git a/src/crypto/index.js b/src/crypto/index.js index 5c12076c5..b74d60dc7 100644 --- a/src/crypto/index.js +++ b/src/crypto/index.js @@ -568,7 +568,9 @@ Crypto.prototype.bootstrapCrossSigning = async function({ "Cross-signing private keys not found locally, but they are available " + "in secret storage, reading storage and caching locally", ); - await this.checkOwnCrossSigningTrust(); + await this.checkOwnCrossSigningTrust({ + allowPrivateKeyRequests: true, + }); } // Assuming no app-supplied callback, default to storing new private keys in @@ -1300,13 +1302,19 @@ Crypto.prototype._onDeviceListUserCrossSigningUpdated = async function(userId) { * Check the copy of our cross-signing key that we have in the device list and * see if we can get the private key. If so, mark it as trusted. */ -Crypto.prototype.checkOwnCrossSigningTrust = async function() { +Crypto.prototype.checkOwnCrossSigningTrust = async function({ + allowPrivateKeyRequests = false, +} = {}) { const userId = this._userId; // Before proceeding, ensure our cross-signing public keys have been // downloaded via the device list. await this.downloadKeys([this._userId]); + // Also check which private keys are locally cached. + const crossSigningPrivateKeys = + await this._crossSigningInfo.getCrossSigningKeysFromCache(); + // If we see an update to our own master key, check it against the master // key we have and, if it matches, mark it as verified @@ -1324,6 +1332,11 @@ Crypto.prototype.checkOwnCrossSigningTrust = async function() { const masterChanged = this._crossSigningInfo.getId() !== seenPubkey; if (masterChanged) { logger.info("Got new master public key", seenPubkey); + } + if ( + allowPrivateKeyRequests && + (masterChanged || !crossSigningPrivateKeys.has("master")) + ) { logger.info("Attempting to retrieve cross-signing master private key"); let signing = null; try { @@ -1352,6 +1365,11 @@ Crypto.prototype.checkOwnCrossSigningTrust = async function() { if (selfSigningChanged) { logger.info("Got new self-signing key", newCrossSigning.getId("self_signing")); + } + if ( + allowPrivateKeyRequests && + (selfSigningChanged || !crossSigningPrivateKeys.has("self_signing")) + ) { logger.info("Attempting to retrieve cross-signing self-signing private key"); let signing = null; try { @@ -1374,6 +1392,11 @@ Crypto.prototype.checkOwnCrossSigningTrust = async function() { } if (userSigningChanged) { logger.info("Got new user-signing key", newCrossSigning.getId("user_signing")); + } + if ( + allowPrivateKeyRequests && + (userSigningChanged || !crossSigningPrivateKeys.has("user_signing")) + ) { logger.info("Attempting to retrieve cross-signing user-signing private key"); let signing = null; try { diff --git a/src/crypto/olmlib.js b/src/crypto/olmlib.js index c4e3278cf..f1e487e97 100644 --- a/src/crypto/olmlib.js +++ b/src/crypto/olmlib.js @@ -208,6 +208,35 @@ export async function ensureOlmSessionsForDevices( const result = {}; const resolveSession = {}; + // Mark all sessions this task intends to update as in progress. It is + // important to do this for all devices this task cares about in a single + // synchronous operation, as otherwise it is possible to have deadlocks + // where multiple tasks wait indefinitely on another task to update some set + // of common devices. + for (const [, devices] of Object.entries(devicesByUser)) { + for (const deviceInfo of devices) { + const key = deviceInfo.getIdentityKey(); + + if (key === olmDevice.deviceCurve25519Key) { + // We don't start sessions with ourself, so there's no need to + // mark it in progress. + continue; + } + + if (!olmDevice._sessionsInProgress[key]) { + // pre-emptively mark the session as in-progress to avoid race + // conditions. If we find that we already have a session, then + // we'll resolve + olmDevice._sessionsInProgress[key] = new Promise(resolve => { + resolveSession[key] = (...args) => { + delete olmDevice._sessionsInProgress[key]; + resolve(...args); + }; + }); + } + } + } + for (const [userId, devices] of Object.entries(devicesByUser)) { result[userId] = {}; for (const deviceInfo of devices) { @@ -233,40 +262,14 @@ export async function ensureOlmSessionsForDevices( } const forWhom = `for ${key} (${userId}:${deviceId})`; - log.debug(`Ensuring Olm session ${forWhom}`); - if (!olmDevice._sessionsInProgress[key]) { - // pre-emptively mark the session as in-progress to avoid race - // conditions. If we find that we already have a session, then - // we'll resolve - log.debug(`Marking Olm session in progress ${forWhom}`); - olmDevice._sessionsInProgress[key] = new Promise( - (resolve, reject) => { - resolveSession[key] = { - resolve: (...args) => { - log.debug(`Resolved Olm session in progress ${forWhom}`); - delete olmDevice._sessionsInProgress[key]; - resolve(...args); - }, - reject: (...args) => { - log.debug(`Rejected Olm session in progress ${forWhom}`); - delete olmDevice._sessionsInProgress[key]; - reject(...args); - }, - }; - }, - ); - } const sessionId = await olmDevice.getSessionIdForDevice( key, resolveSession[key], log, ); - log.debug(`Got Olm session ${sessionId} ${forWhom}`); if (sessionId !== null && resolveSession[key]) { // we found a session, but we had marked the session as - // in-progress, so unmark it and unblock anything that was - // waiting - delete olmDevice._sessionsInProgress[key]; - resolveSession[key].resolve(); - delete resolveSession[key]; + // in-progress, so resolve it now, which will unmark it and + // unblock anything that was waiting + resolveSession[key](); } if (sessionId === null || force) { if (force) { @@ -290,14 +293,6 @@ export async function ensureOlmSessionsForDevices( const oneTimeKeyAlgorithm = "signed_curve25519"; let res; let taskDetail = `one-time keys for ${devicesWithoutSession.length} devices`; - // If your homeserver takes a nap here and never replies, this process - // would hang indefinitely. While that's easily fixed by setting a - // timeout on this request, let's first log whether that's the root - // cause we're seeing in practice. - // See also https://github.com/vector-im/element-web/issues/16194 - const otkTimeoutLogger = setTimeout(() => { - log.error(`Homeserver never replied while claiming ${taskDetail}`); - }, otkTimeout); try { log.debug(`Claiming ${taskDetail}`); res = await baseApis.claimOneTimeKeys( @@ -306,22 +301,20 @@ export async function ensureOlmSessionsForDevices( log.debug(`Claimed ${taskDetail}`); } catch (e) { for (const resolver of Object.values(resolveSession)) { - resolver.resolve(); + resolver(); } log.log(`Failed to claim ${taskDetail}`, e, devicesWithoutSession); throw e; - } finally { - clearTimeout(otkTimeoutLogger); } if (failedServers && "failures" in res) { failedServers.push(...Object.keys(res.failures)); } - const otk_res = res.one_time_keys || {}; + const otkResult = res.one_time_keys || {}; const promises = []; for (const [userId, devices] of Object.entries(devicesByUser)) { - const userRes = otk_res[userId] || {}; + const userRes = otkResult[userId] || {}; for (let j = 0; j < devices.length; j++) { const deviceInfo = devices[j]; const deviceId = deviceInfo.deviceId; @@ -353,7 +346,7 @@ export async function ensureOlmSessionsForDevices( `for device ${userId}:${deviceId}`, ); if (resolveSession[key]) { - resolveSession[key].resolve(); + resolveSession[key](); } continue; } @@ -363,12 +356,12 @@ export async function ensureOlmSessionsForDevices( olmDevice, oneTimeKey, userId, deviceInfo, ).then((sid) => { if (resolveSession[key]) { - resolveSession[key].resolve(sid); + resolveSession[key](sid); } result[userId][deviceId].sessionId = sid; }, (e) => { if (resolveSession[key]) { - resolveSession[key].resolve(); + resolveSession[key](); } throw e; }), diff --git a/src/crypto/store/indexeddb-crypto-store-backend.js b/src/crypto/store/indexeddb-crypto-store-backend.js index a3473f7c1..58bfee576 100644 --- a/src/crypto/store/indexeddb-crypto-store-backend.js +++ b/src/crypto/store/indexeddb-crypto-store-backend.js @@ -20,6 +20,7 @@ import {logger} from '../../logger'; import * as utils from "../../utils"; export const VERSION = 10; +const PROFILE_TRANSACTIONS = false; /** * Implementation of a CryptoStore which is backed by an existing @@ -791,20 +792,26 @@ export class Backend { } doTxn(mode, stores, func, log = logger) { - const txnId = this._nextTxnId++; - const startTime = Date.now(); - const description = `${mode} crypto store transaction ${txnId} in ${stores}`; - log.debug(`Starting ${description}`); + let startTime; + let description; + if (PROFILE_TRANSACTIONS) { + const txnId = this._nextTxnId++; + startTime = Date.now(); + description = `${mode} crypto store transaction ${txnId} in ${stores}`; + log.debug(`Starting ${description}`); + } const txn = this._db.transaction(stores, mode); const promise = promiseifyTxn(txn); const result = func(txn); - promise.then(() => { - const elapsedTime = Date.now() - startTime; - log.debug(`Finished ${description}, took ${elapsedTime} ms`); - }, () => { - const elapsedTime = Date.now() - startTime; - log.error(`Failed ${description}, took ${elapsedTime} ms`); - }); + if (PROFILE_TRANSACTIONS) { + promise.then(() => { + const elapsedTime = Date.now() - startTime; + log.debug(`Finished ${description}, took ${elapsedTime} ms`); + }, () => { + const elapsedTime = Date.now() - startTime; + log.error(`Failed ${description}, took ${elapsedTime} ms`); + }); + } return promise.then(() => { return result; }); diff --git a/src/crypto/verification/request/ToDeviceChannel.js b/src/crypto/verification/request/ToDeviceChannel.js index d04a75853..9f3659389 100644 --- a/src/crypto/verification/request/ToDeviceChannel.js +++ b/src/crypto/verification/request/ToDeviceChannel.js @@ -179,7 +179,9 @@ export class ToDeviceChannel { const isAcceptingEvent = type === START_TYPE || type === READY_TYPE; // the request has picked a ready or start event, tell the other devices about it if (isAcceptingEvent && !wasStarted && isStarted && this._deviceId) { - const nonChosenDevices = this._devices.filter(d => d !== this._deviceId); + const nonChosenDevices = this._devices.filter( + d => d !== this._deviceId && d !== this._client.getDeviceId(), + ); if (nonChosenDevices.length) { const message = this.completeContent({ code: "m.accepted", diff --git a/src/http-api.js b/src/http-api.js index cc86506ad..26b3d5f5b 100644 --- a/src/http-api.js +++ b/src/http-api.js @@ -271,10 +271,10 @@ MatrixHttpApi.prototype = { xhr.timeout_timer = callbacks.setTimeout(timeout_fn, 30000); xhr.onreadystatechange = function() { + let resp; switch (xhr.readyState) { case global.XMLHttpRequest.DONE: callbacks.clearTimeout(xhr.timeout_timer); - var resp; try { if (xhr.status === 0) { throw new AbortError(); diff --git a/src/models/room-member.js b/src/models/room-member.js index 0ba6edd60..444c45609 100644 --- a/src/models/room-member.js +++ b/src/models/room-member.js @@ -290,6 +290,9 @@ RoomMember.prototype.getMxcAvatarUrl = function() { return null; }; +const MXID_PATTERN = /@.+:.+/; +const LTR_RTL_PATTERN = /[\u200E\u200F\u202A-\u202F]/; + function calculateDisplayName(selfUserId, displayName, roomState) { if (!displayName || displayName === selfUserId) { return selfUserId; @@ -308,13 +311,13 @@ function calculateDisplayName(selfUserId, displayName, roomState) { // Next check if the name contains something that look like a mxid // If it does, it may be someone trying to impersonate someone else // Show full mxid in this case - let disambiguate = /@.+:.+/.test(displayName); + let disambiguate = MXID_PATTERN.test(displayName); if (!disambiguate) { // Also show mxid if the display name contains any LTR/RTL characters as these // make it very difficult for us to find similar *looking* display names // E.g "Mark" could be cloned by writing "kraM" but in RTL. - disambiguate = /[\u200E\u200F\u202A-\u202F]/.test(displayName); + disambiguate = LTR_RTL_PATTERN.test(displayName); } if (!disambiguate) { diff --git a/src/models/room.js b/src/models/room.js index 9c321e1bb..1f19cf198 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -818,7 +818,7 @@ Room.prototype.getBlacklistUnverifiedDevices = function() { */ Room.prototype.getAvatarUrl = function(baseUrl, width, height, resizeMethod, allowDefault) { - const roomAvatarEvent = this.currentState.getStateEvents("m.room.avatar", ""); + const roomAvatarEvent = this.currentState.getStateEvents(EventType.RoomAvatar, ""); if (allowDefault === undefined) { allowDefault = true; } @@ -836,6 +836,15 @@ Room.prototype.getAvatarUrl = function(baseUrl, width, height, resizeMethod, return null; }; +/** + * Get the mxc avatar url for the room, if one was set. + * @return {string} the mxc avatar url or falsy + */ +Room.prototype.getMxcAvatarUrl = function() { + const roomAvatarEvent = this.currentState.getStateEvents(EventType.RoomAvatar, ""); + return roomAvatarEvent ? roomAvatarEvent.getContent().url : null; +}; + /** * Get the aliases this room has according to the room's state * The aliases returned by this function may not necessarily diff --git a/src/store/indexeddb-local-backend.js b/src/store/indexeddb-local-backend.js index d538e88f0..3a0af28b1 100644 --- a/src/store/indexeddb-local-backend.js +++ b/src/store/indexeddb-local-backend.js @@ -435,7 +435,7 @@ LocalIndexedDBStoreBackend.prototype = { * @return {Promise} Resolves if the data was persisted. */ _persistSyncData: function(nextBatch, roomsData, groupsData) { - logger.log("Persisting sync data up to ", nextBatch); + logger.log("Persisting sync data up to", nextBatch); return utils.promiseTry(() => { const txn = this.db.transaction(["sync"], "readwrite"); const store = txn.objectStore("sync"); diff --git a/src/store/indexeddb.js b/src/store/indexeddb.js index 9cde4467d..50049bf9b 100644 --- a/src/store/indexeddb.js +++ b/src/store/indexeddb.js @@ -51,8 +51,8 @@ const WRITE_DELAY_MS = 1000 * 60 * 5; // once every 5 minutes * sync from the server is not required. This does not reduce memory usage as all * the data is eagerly fetched when startup() is called. * 
- * let opts = { localStorage: window.localStorage };
- * let store = new IndexedDBStore();
+ * let opts = { indexedDB: window.indexedDB, localStorage: window.localStorage };
+ * let store = new IndexedDBStore(opts);
  * await store.startup(); // load from indexed db
  * let client = sdk.createClient({
  *     store: store,
diff --git a/src/utils.ts b/src/utils.ts
index 95b3c11c5..17b3924cd 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -745,6 +745,15 @@ export function promiseTry(fn: () => T): Promise {
     return new Promise((resolve) => resolve(fn()));
 }
 
+// Creates and awaits all promises, running no more than `chunkSize` at the same time
+export async function chunkPromises(fns: (() => Promise)[], chunkSize: number): Promise {
+    const results: T[] = [];
+    for (let i = 0; i < fns.length; i += chunkSize) {
+        results.push(...(await Promise.all(fns.slice(i, i + chunkSize).map(fn => fn()))));
+    }
+    return results;
+}
+
 // We need to be able to access the Node.js crypto library from within the
 // Matrix SDK without needing to `require("crypto")`, which will fail in
 // browsers.  So `index.ts` will call `setCrypto` to store it, and when we need
diff --git a/src/webrtc/call.ts b/src/webrtc/call.ts
index b2ecdddd1..eb567ce70 100644
--- a/src/webrtc/call.ts
+++ b/src/webrtc/call.ts
@@ -174,6 +174,11 @@ export enum CallErrorCode {
     SignallingFailed = 'signalling_timeout',
 }
 
+enum ConstraintsType {
+    Audio = "audio",
+    Video = "video",
+}
+
 /**
  * The version field that we set in m.call.* events
  */
@@ -328,10 +333,11 @@ export class MatrixCall extends EventEmitter {
      * Place a voice call to this room.
      * @throws If you have not specified a listener for 'error' events.
      */
-    placeVoiceCall() {
+    async placeVoiceCall() {
         logger.debug("placeVoiceCall");
         this.checkForErrorListener();
-        this.placeCallWithConstraints(getUserMediaVideoContraints(CallType.Voice));
+        const constraints = getUserMediaContraints(ConstraintsType.Audio);
+        await this.placeCallWithConstraints(constraints);
         this.type = CallType.Voice;
     }
 
@@ -343,12 +349,13 @@ export class MatrixCall extends EventEmitter {
      * to render the local camera preview.
      * @throws If you have not specified a listener for 'error' events.
      */
-    placeVideoCall(remoteVideoElement: HTMLVideoElement, localVideoElement: HTMLVideoElement) {
+    async placeVideoCall(remoteVideoElement: HTMLVideoElement, localVideoElement: HTMLVideoElement) {
         logger.debug("placeVideoCall");
         this.checkForErrorListener();
         this.localVideoElement = localVideoElement;
         this.remoteVideoElement = remoteVideoElement;
-        this.placeCallWithConstraints(getUserMediaVideoContraints(CallType.Video));
+        const constraints = getUserMediaContraints(ConstraintsType.Video);
+        await this.placeCallWithConstraints(constraints);
         this.type = CallType.Video;
     }
 
@@ -365,61 +372,37 @@ export class MatrixCall extends EventEmitter {
     async placeScreenSharingCall(
         remoteVideoElement: HTMLVideoElement,
         localVideoElement: HTMLVideoElement,
-        selectDesktopCapturerSource: () => Promise,
+        selectDesktopCapturerSource?: () => Promise,
     ) {
         logger.debug("placeScreenSharingCall");
         this.checkForErrorListener();
         this.localVideoElement = localVideoElement;
         this.remoteVideoElement = remoteVideoElement;
 
-        if (window.electron?.getDesktopCapturerSources) {
-            // We have access to getDesktopCapturerSources()
-            logger.debug("Electron getDesktopCapturerSources() is available...");
-            try {
-                const selectedSource = await selectDesktopCapturerSource();
-                // If no source was selected cancel call
-                if (!selectedSource) return;
-                const getUserMediaOptions: MediaStreamConstraints | DesktopCapturerConstraints = {
-                    audio: false,
-                    video: {
-                        mandatory: {
-                            chromeMediaSource: "desktop",
-                            chromeMediaSourceId: selectedSource.id,
-                        },
-                    },
-                }
-                this.screenSharingStream = await window.navigator.mediaDevices.getUserMedia(getUserMediaOptions);
+        try {
+            const screenshareConstraints = await getScreenshareContraints(selectDesktopCapturerSource);
+            if (!screenshareConstraints) return;
+            if (window.electron?.getDesktopCapturerSources) {
+                // We are using Electron
+                logger.debug("Getting screen stream using getUserMedia()...");
+                this.screenSharingStream = await navigator.mediaDevices.getUserMedia(screenshareConstraints);
+            } else {
+                // We are not using Electron
+                logger.debug("Getting screen stream using getDisplayMedia()...");
+                this.screenSharingStream = await navigator.mediaDevices.getDisplayMedia(screenshareConstraints);
+            }
 
-                logger.debug("Got screen stream, requesting audio stream...");
-                const audioConstraints = getUserMediaVideoContraints(CallType.Voice);
-                this.placeCallWithConstraints(audioConstraints);
-            } catch (err) {
-                this.emit(CallEvent.Error,
-                    new CallError(
-                        CallErrorCode.NoUserMedia,
-                        "Failed to get screen-sharing stream: ", err,
-                    ),
-                );
-            }
-        } else {
-            /* We do not have access to the Electron desktop capturer,
-             * therefore we can assume we are on the web */
-            logger.debug("Electron desktopCapturer is not available...");
-            try {
-                this.screenSharingStream = await navigator.mediaDevices.getDisplayMedia({'audio': false});
-                logger.debug("Got screen stream, requesting audio stream...");
-                const audioConstraints = getUserMediaVideoContraints(CallType.Voice);
-                this.placeCallWithConstraints(audioConstraints);
-            } catch (err) {
-                this.emit(CallEvent.Error,
-                    new CallError(
-                        CallErrorCode.NoUserMedia,
-                        "Failed to get screen-sharing stream: ", err,
-                    ),
-                );
-            }
+            logger.debug("Got screen stream, requesting audio stream...");
+            const audioConstraints = getUserMediaContraints(ConstraintsType.Audio);
+            this.placeCallWithConstraints(audioConstraints);
+        } catch (err) {
+            this.emit(CallEvent.Error,
+                new CallError(
+                    CallErrorCode.NoUserMedia,
+                    "Failed to get screen-sharing stream: ", err,
+                ),
+            );
         }
-
         this.type = CallType.Video;
     }
 
@@ -544,6 +527,13 @@ export class MatrixCall extends EventEmitter {
         const invite = event.getContent();
         this.direction = CallDirection.Inbound;
 
+        // make sure we have valid turn creds. Unless something's gone wrong, it should
+        // poll and keep the credentials valid so this should be instant.
+        const haveTurnCreds = await this.client._checkTurnServers();
+        if (!haveTurnCreds) {
+            logger.warn("Failed to get TURN credentials! Proceeding with call anyway...");
+        }
+
         this.peerConn = this.createPeerConnection();
         // we must set the party ID before await-ing on anything: the call event
         // handler will start giving us more call events (eg. candidates) so if
@@ -551,6 +541,7 @@ export class MatrixCall extends EventEmitter {
         this.chooseOpponent(event);
         try {
             await this.peerConn.setRemoteDescription(invite.offer);
+            await this.addBufferedIceCandidates();
         } catch (e) {
             logger.debug("Failed to set remote description", e);
             this.terminate(CallParty.Local, CallErrorCode.SetRemoteDescription, false);
@@ -608,7 +599,11 @@ export class MatrixCall extends EventEmitter {
         logger.debug(`Answering call ${this.callId} of type ${this.type}`);
 
         if (!this.localAVStream && !this.waitForLocalAVStream) {
-            const constraints = getUserMediaVideoContraints(this.type);
+            const constraints = getUserMediaContraints(
+                this.type == CallType.Video ?
+                    ConstraintsType.Video:
+                    ConstraintsType.Audio,
+            );
             logger.log("Getting user media with constraints", constraints);
             this.setState(CallState.WaitLocalMedia);
             this.waitForLocalAVStream = true;
@@ -665,6 +660,8 @@ export class MatrixCall extends EventEmitter {
 
         logger.debug("Ending call " + this.callId);
         this.terminate(CallParty.Local, reason, !suppressEvent);
+        // We don't want to send hangup here if we didn't even get to sending an invite
+        if (this.state === CallState.WaitLocalMedia) return;
         const content = {};
         // Continue to send no reason for user hangups temporarily, until
         // clients understand the user_hangup reason (voip v1)
@@ -838,8 +835,11 @@ export class MatrixCall extends EventEmitter {
             return;
         }
         if (this.callHasEnded()) {
+            this.stopAllMedia();
             return;
         }
+        this.localAVStream = stream;
+        logger.info("Got local AV stream with id " + this.localAVStream.id);
 
         this.setState(CallState.CreateOffer);
 
@@ -865,11 +865,8 @@ export class MatrixCall extends EventEmitter {
             }
         }
 
-        this.localAVStream = stream;
-        logger.info("Got local AV stream with id " + this.localAVStream.id);
         // why do we enable audio (and only audio) tracks here? -- matthew
         setTracksEnabled(stream.getAudioTracks(), true);
-        this.peerConn = this.createPeerConnection();
 
         for (const audioTrack of stream.getAudioTracks()) {
             logger.info("Adding audio track with id " + audioTrack.id);
@@ -991,7 +988,7 @@ export class MatrixCall extends EventEmitter {
     private gotLocalIceCandidate = (event: RTCPeerConnectionIceEvent) => {
         if (event.candidate) {
             logger.debug(
-                "Got local ICE " + event.candidate.sdpMid + " candidate: " +
+                "Call " + this.callId + " got local ICE " + event.candidate.sdpMid + " candidate: " +
                 event.candidate.candidate,
             );
 
@@ -1025,7 +1022,7 @@ export class MatrixCall extends EventEmitter {
         }
     };
 
-    onRemoteIceCandidatesReceived(ev: MatrixEvent) {
+    async onRemoteIceCandidatesReceived(ev: MatrixEvent) {
         if (this.callHasEnded()) {
             //debuglog("Ignoring remote ICE candidate because call has ended");
             return;
@@ -1057,7 +1054,7 @@ export class MatrixCall extends EventEmitter {
             return;
         }
 
-        this.addIceCandidates(cands);
+        await this.addIceCandidates(cands);
     }
 
     /**
@@ -1065,7 +1062,10 @@ export class MatrixCall extends EventEmitter {
      * @param {Object} msg
      */
     async onAnswerReceived(event: MatrixEvent) {
+        logger.debug(`Got answer for call ID ${this.callId} from party ID ${event.getContent().party_id}`);
+
         if (this.callHasEnded()) {
+            logger.debug(`Ignoring answer because call ID ${this.callId} has ended`);
             return;
         }
 
@@ -1078,6 +1078,7 @@ export class MatrixCall extends EventEmitter {
         }
 
         this.chooseOpponent(event);
+        await this.addBufferedIceCandidates();
 
         this.setState(CallState.Connecting);
 
@@ -1674,11 +1675,18 @@ export class MatrixCall extends EventEmitter {
         this.setState(CallState.WaitLocalMedia);
         this.direction = CallDirection.Outbound;
         this.config = constraints;
-        // It would be really nice if we could start gathering candidates at this point
-        // so the ICE agent could be gathering while we open our media devices: we already
-        // know the type of the call and therefore what tracks we want to send.
-        // Perhaps we could do this by making fake tracks now and then using replaceTrack()
-        // once we have the actual tracks? (Can we make fake tracks?)
+
+        // make sure we have valid turn creds. Unless something's gone wrong, it should
+        // poll and keep the credentials valid so this should be instant.
+        const haveTurnCreds = await this.client._checkTurnServers();
+        if (!haveTurnCreds) {
+            logger.warn("Failed to get TURN credentials! Proceeding with call anyway...");
+        }
+
+        // create the peer connection now so it can be gathering candidates while we get user
+        // media (assuming a candidate pool size is configured)
+        this.peerConn = this.createPeerConnection();
+
         try {
             const mediaStream = await navigator.mediaDevices.getUserMedia(constraints);
             this.gotUserMediaForInvite(mediaStream);
@@ -1721,6 +1729,8 @@ export class MatrixCall extends EventEmitter {
         // I choo-choo-choose you
         const msg = ev.getContent();
 
+        logger.debug(`Choosing party ID ${msg.party_id} for call ID ${this.callId}`);
+
         this.opponentVersion = msg.version;
         if (this.opponentVersion === 0) {
             // set to null to indicate that we've chosen an opponent, but because
@@ -1734,30 +1744,32 @@ export class MatrixCall extends EventEmitter {
         }
         this.opponentCaps = msg.capabilities || {};
         this.opponentMember = ev.sender;
+    }
 
+    private async addBufferedIceCandidates() {
         const bufferedCands = this.remoteCandidateBuffer.get(this.opponentPartyId);
         if (bufferedCands) {
             logger.info(`Adding ${bufferedCands.length} buffered candidates for opponent ${this.opponentPartyId}`);
-            this.addIceCandidates(bufferedCands);
+            await this.addIceCandidates(bufferedCands);
         }
         this.remoteCandidateBuffer = null;
     }
 
-    private addIceCandidates(cands: RTCIceCandidate[]) {
+    private async addIceCandidates(cands: RTCIceCandidate[]) {
         for (const cand of cands) {
             if (
                 (cand.sdpMid === null || cand.sdpMid === undefined) &&
                 (cand.sdpMLineIndex === null || cand.sdpMLineIndex === undefined)
             ) {
                 logger.debug("Ignoring remote ICE candidate with no sdpMid or sdpMLineIndex");
-                return;
+                continue;
             }
-            logger.debug("Got remote ICE " + cand.sdpMid + " candidate: " + cand.candidate);
+            logger.debug("Call " + this.callId + " got remote ICE " + cand.sdpMid + " candidate: " + cand.candidate);
             try {
-                this.peerConn.addIceCandidate(cand);
+                await this.peerConn.addIceCandidate(cand);
             } catch (err) {
                 if (!this.ignoreOffer) {
-                    logger.info("Failed to add remore ICE candidate", err);
+                    logger.info("Failed to add remote ICE candidate", err);
                 }
             }
         }
@@ -1770,17 +1782,19 @@ function setTracksEnabled(tracks: Array, enabled: boolean) {
     }
 }
 
-function getUserMediaVideoContraints(callType: CallType) {
+function getUserMediaContraints(type: ConstraintsType) {
     const isWebkit = !!navigator.webkitGetUserMedia;
 
-    switch (callType) {
-        case CallType.Voice:
+    switch (type) {
+        case ConstraintsType.Audio: {
             return {
                 audio: {
                     deviceId: audioInput ? {ideal: audioInput} : undefined,
-                }, video: false,
+                },
+                video: false,
             };
-        case CallType.Video:
+        }
+        case ConstraintsType.Video: {
             return {
                 audio: {
                     deviceId: audioInput ? {ideal: audioInput} : undefined,
@@ -1795,6 +1809,33 @@ function getUserMediaVideoContraints(callType: CallType) {
                     height: isWebkit ? { exact: 360 } : { ideal: 360 },
                 },
             };
+        }
+    }
+}
+
+async function getScreenshareContraints(selectDesktopCapturerSource?: () => Promise) {
+    if (window.electron?.getDesktopCapturerSources && selectDesktopCapturerSource) {
+        // We have access to getDesktopCapturerSources()
+        logger.debug("Electron getDesktopCapturerSources() is available...");
+        const selectedSource = await selectDesktopCapturerSource();
+        if (!selectedSource) return null;
+        return {
+            audio: false,
+            video: {
+                mandatory: {
+                    chromeMediaSource: "desktop",
+                    chromeMediaSourceId: selectedSource.id,
+                },
+            },
+        };
+    } else {
+        // We do not have access to the Electron desktop capturer,
+        // therefore we can assume we are on the web
+        logger.debug("Electron desktopCapturer is not available...");
+        return {
+            audio: false,
+            video: true,
+        };
     }
 }
 
diff --git a/src/webrtc/callEventHandler.ts b/src/webrtc/callEventHandler.ts
index 304eacfdd..c45bf62b3 100644
--- a/src/webrtc/callEventHandler.ts
+++ b/src/webrtc/callEventHandler.ts
@@ -139,7 +139,7 @@ export class CallEventHandler {
             }
 
             const timeUntilTurnCresExpire = this.client.getTurnServersExpiry() - Date.now();
-            logger.info("Current turn creds expire in " + timeUntilTurnCresExpire + " seconds");
+            logger.info("Current turn creds expire in " + timeUntilTurnCresExpire + " ms");
             call = createNewMatrixCall(this.client, event.getRoomId(), {
                 forceTURN: this.client._forceTURN,
             });
diff --git a/yarn.lock b/yarn.lock
index a47f6eb4e..e952d1aea 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -1876,10 +1876,10 @@ bluebird@^3.5.0, bluebird@^3.7.2:
   resolved "https://registry.yarnpkg.com/bluebird/-/bluebird-3.7.2.tgz#9f229c15be272454ffa973ace0dbee79a1b0c36f"
   integrity sha512-XpNj6GDQzdfW+r2Wnn7xiSAd7TM3jzkxGXBGTtWKuSXv1xUV+azxAm8jdWZN06QTQk+2N2XB9jRDkvbmQmcRtg==
 
-bn.js@^4.0.0, bn.js@^4.1.0, bn.js@^4.4.0:
-  version "4.11.9"
-  resolved "https://registry.yarnpkg.com/bn.js/-/bn.js-4.11.9.tgz#26d556829458f9d1e81fc48952493d0ba3507828"
-  integrity sha512-E6QoYqCKZfgatHTdHzs1RRKP7ip4vvm+EyRUeE2RF0NblwVvb0p6jSVeNTOFxPn26QXN2o6SMfNxKp6kU8zQaw==
+bn.js@^4.0.0, bn.js@^4.1.0, bn.js@^4.11.9:
+  version "4.12.0"
+  resolved "https://registry.yarnpkg.com/bn.js/-/bn.js-4.12.0.tgz#775b3f278efbb9718eec7361f483fb36fbbfea88"
+  integrity sha512-c98Bf3tPniI+scsdk237ku1Dc3ujXQTSgyiPUDEOe7tRkhrqridvh8klBv0HCEso1OLOYcHuCv/cS6DNxKH+ZA==
 
 bn.js@^5.0.0, bn.js@^5.1.1:
   version "5.1.3"
@@ -1922,7 +1922,7 @@ braces@^3.0.1, braces@~3.0.2:
   dependencies:
     fill-range "^7.0.1"
 
-brorand@^1.0.1:
+brorand@^1.0.1, brorand@^1.1.0:
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/brorand/-/brorand-1.1.0.tgz#12c25efe40a45e3c323eb8675a0a0ce57b22371f"
   integrity sha1-EsJe/kCkXjwyPrhnWgoM5XsiNx8=
@@ -2796,17 +2796,17 @@ electron-to-chromium@^1.3.634:
   integrity sha512-cev+jOrz/Zm1i+Yh334Hed6lQVOkkemk2wRozfMF4MtTR7pxf3r3L5Rbd7uX1zMcEqVJ7alJBnJL7+JffkC6FQ==
 
 elliptic@^6.5.3:
-  version "6.5.3"
-  resolved "https://registry.yarnpkg.com/elliptic/-/elliptic-6.5.3.tgz#cb59eb2efdaf73a0bd78ccd7015a62ad6e0f93d6"
-  integrity sha512-IMqzv5wNQf+E6aHeIqATs0tOLeOTwj1QKbRcS3jBbYkl5oLAserA8yJTT7/VyHUYG91PRmPyeQDObKLPpeS4dw==
+  version "6.5.4"
+  resolved "https://registry.yarnpkg.com/elliptic/-/elliptic-6.5.4.tgz#da37cebd31e79a1367e941b592ed1fbebd58abbb"
+  integrity sha512-iLhC6ULemrljPZb+QutR5TQGB+pdW6KGD5RSegS+8sorOZT+rdQFbsQFJgvN3eRqNALqJer4oQ16YvJHlU8hzQ==
   dependencies:
-    bn.js "^4.4.0"
-    brorand "^1.0.1"
+    bn.js "^4.11.9"
+    brorand "^1.1.0"
     hash.js "^1.0.0"
-    hmac-drbg "^1.0.0"
-    inherits "^2.0.1"
-    minimalistic-assert "^1.0.0"
-    minimalistic-crypto-utils "^1.0.0"
+    hmac-drbg "^1.0.1"
+    inherits "^2.0.4"
+    minimalistic-assert "^1.0.1"
+    minimalistic-crypto-utils "^1.0.1"
 
 emittery@^0.7.1:
   version "0.7.2"
@@ -3820,7 +3820,7 @@ he@^1.1.0:
   resolved "https://registry.yarnpkg.com/he/-/he-1.2.0.tgz#84ae65fa7eafb165fddb61566ae14baf05664f0f"
   integrity sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw==
 
-hmac-drbg@^1.0.0:
+hmac-drbg@^1.0.1:
   version "1.0.1"
   resolved "https://registry.yarnpkg.com/hmac-drbg/-/hmac-drbg-1.0.1.tgz#d2745701025a6c775a6c545793ed502fc0c649a1"
   integrity sha1-0nRXAQJabHdabFRXk+1QL8DGSaE=
@@ -5069,11 +5069,16 @@ lodash.sortby@^4.7.0:
   resolved "https://registry.yarnpkg.com/lodash.sortby/-/lodash.sortby-4.7.0.tgz#edd14c824e2cc9c1e0b0a1b42bb5210516a42438"
   integrity sha1-7dFMgk4sycHgsKG0K7UhBRakJDg=
 
-lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.20, lodash@^4.17.4:
+lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.20:
   version "4.17.20"
   resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.20.tgz#b44a9b6297bcb698f1c51a3545a2b3b368d59c52"
   integrity sha512-PlhdFcillOINfeV7Ni6oF1TAEayyZBoZ8bcshTHqOYJYlrqzRK5hagpagky5o4HfCzzd1TRkXPMFq6cKk9rGmA==
 
+lodash@^4.17.4:
+  version "4.17.21"
+  resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c"
+  integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==
+
 loglevel@^1.7.1:
   version "1.7.1"
   resolved "https://registry.yarnpkg.com/loglevel/-/loglevel-1.7.1.tgz#005fde2f5e6e47068f935ff28573e125ef72f197"
@@ -5255,7 +5260,7 @@ minimalistic-assert@^1.0.0, minimalistic-assert@^1.0.1:
   resolved "https://registry.yarnpkg.com/minimalistic-assert/-/minimalistic-assert-1.0.1.tgz#2e194de044626d4a10e7f7fbc00ce73e83e4d5c7"
   integrity sha512-UtJcAD4yEaGtjPezWuO9wC4nwUnVH/8/Im3yEHQP4b67cXlD/Qr9hdITCU1xDbSEXg2XKNaP8jsReV7vQd00/A==
 
-minimalistic-crypto-utils@^1.0.0, minimalistic-crypto-utils@^1.0.1:
+minimalistic-crypto-utils@^1.0.1:
   version "1.0.1"
   resolved "https://registry.yarnpkg.com/minimalistic-crypto-utils/-/minimalistic-crypto-utils-1.0.1.tgz#f6c00c1c0b082246e5c4d99dfb8c7c083b2b582a"
   integrity sha1-9sAMHAsIIkblxNmd+4x8CDsrWCo=
@@ -5917,9 +5922,9 @@ pug-attrs@^2.0.4:
     pug-runtime "^2.0.5"
 
 pug-code-gen@^2.0.2:
-  version "2.0.2"
-  resolved "https://registry.yarnpkg.com/pug-code-gen/-/pug-code-gen-2.0.2.tgz#ad0967162aea077dcf787838d94ed14acb0217c2"
-  integrity sha512-kROFWv/AHx/9CRgoGJeRSm+4mLWchbgpRzTEn8XCiwwOy6Vh0gAClS8Vh5TEJ9DBjaP8wCjS3J6HKsEsYdvaCw==
+  version "2.0.3"
+  resolved "https://registry.yarnpkg.com/pug-code-gen/-/pug-code-gen-2.0.3.tgz#122eb9ada9b5bf601705fe15aaa0a7d26bc134ab"
+  integrity sha512-r9sezXdDuZJfW9J91TN/2LFbiqDhmltTFmGpHTsGdrNGp3p4SxAjjXEfnuK2e4ywYsRIVP0NeLbSAMHUcaX1EA==
   dependencies:
     constantinople "^3.1.2"
     doctypes "^1.1.0"