From 89a3c81a1946650555d9729bab319753d6afc400 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Sat, 19 Feb 2022 12:09:30 +0100 Subject: [PATCH] OMEMO: don't wait for all device lists... to be fetched from the server before triggering OMEMOInitialized. For some contacts, the IQ to fetch the device list never receives a response. IQ stanzas take 20 seconds to timeout, which means that all OMEMO operations are blocked for 20 seconds (because everything waits for `OMEMOInitialized`). Create a new API method `api.omemo.devicelists.get` and use that to fetch and `await` for any devicelist. That way we lazily wait for devicelists to be fetched from the server and can continue with other OMEMO operations unrelated to users who's clients don't respond to devicelist queries. --- src/headless/plugins/smacks/tests/smacks.js | 17 +++++---- src/plugins/omemo/api.js | 26 +++++++++++++- src/plugins/omemo/devicelist.js | 21 ++++-------- src/plugins/omemo/devicelists.js | 16 +-------- src/plugins/omemo/fingerprints.js | 4 +-- src/plugins/omemo/profile.js | 2 +- src/plugins/omemo/store.js | 6 ++-- src/plugins/omemo/utils.js | 38 +++++++++------------ 8 files changed, 66 insertions(+), 64 deletions(-) diff --git a/src/headless/plugins/smacks/tests/smacks.js b/src/headless/plugins/smacks/tests/smacks.js index b73210f0d..1373b3b08 100644 --- a/src/headless/plugins/smacks/tests/smacks.js +++ b/src/headless/plugins/smacks/tests/smacks.js @@ -30,7 +30,7 @@ describe("XEP-0198 Stream Management", function () { expect(_converse.session.get('smacks_enabled')).toBe(true); let IQ_stanzas = _converse.connection.IQ_stanzas; - await u.waitUntil(() => IQ_stanzas.length === 4); + await u.waitUntil(() => IQ_stanzas.length === 5); let iq = IQ_stanzas[IQ_stanzas.length-1]; expect(Strophe.serialize(iq)).toBe( @@ -57,13 +57,13 @@ describe("XEP-0198 Stream Management", function () { iq = IQ_stanzas.pop(); expect(expected_IQs(disco_iq).includes(Strophe.serialize(disco_iq))).toBe(true); - expect(sent_stanzas.filter(s => (s.nodeName === 'r')).length).toBe(2); - expect(_converse.session.get('unacked_stanzas').length).toBe(5); + expect(sent_stanzas.filter(s => (s.nodeName === 'r')).length).toBe(3); + expect(_converse.session.get('unacked_stanzas').length).toBe(6); // test handling of acks let ack = u.toStanza(``); _converse.connection._dataRecv(mock.createRequest(ack)); - expect(_converse.session.get('unacked_stanzas').length).toBe(3); + expect(_converse.session.get('unacked_stanzas').length).toBe(4); // test handling of ack requests let r = u.toStanza(``); @@ -89,7 +89,7 @@ describe("XEP-0198 Stream Management", function () { ack = u.toStanza(``); _converse.connection._dataRecv(mock.createRequest(ack)); - expect(_converse.session.get('unacked_stanzas').length).toBe(2); + expect(_converse.session.get('unacked_stanzas').length).toBe(3); r = u.toStanza(``); _converse.connection._dataRecv(mock.createRequest(r)); @@ -112,12 +112,17 @@ describe("XEP-0198 Stream Management", function () { expect(_converse.session.get('smacks_enabled')).toBe(true); await new Promise(resolve => _converse.api.listen.once('reconnected', resolve)); - await u.waitUntil(() => IQ_stanzas.length === 1); + await u.waitUntil(() => IQ_stanzas.length === 2); // Test that unacked stanzas get resent out iq = IQ_stanzas.pop(); expect(Strophe.serialize(iq)).toBe(``); + iq = IQ_stanzas.pop(); + expect(Strophe.serialize(iq)).toBe( + ``+ + ``); + expect(IQ_stanzas.filter(iq => sizzle('query[xmlns="jabber:iq:roster"]', iq).pop()).length).toBe(0); })); diff --git a/src/plugins/omemo/api.js b/src/plugins/omemo/api.js index 0fc426417..888173ea7 100644 --- a/src/plugins/omemo/api.js +++ b/src/plugins/omemo/api.js @@ -18,6 +18,29 @@ export default { return _converse.omemo_store.get('device_id'); }, + /** + * The "devicelists" namespace groups methods related to OMEMO device lists + * + * @namespace _converse.api.omemo.devicelists + * @memberOf _converse.api.omemo + */ + 'devicelists': { + /** + * Returns the {@link _converse.DeviceList} for a particular JID. + * The device list will be created if it doesn't exist already. + * @method _converse.api.omemo.devicelists.get + * @param { String } jid - The Jabber ID for which the device list will be returned. + * @param { bool } create=false - Set to `true` if the device list + * should be created if it cannot be found. + */ + async get (jid, create=false) { + const list = _converse.devicelists.get(jid) || + (create ? _converse.devicelists.create({ jid }) : null); + await list.initialized; + return list; + } + }, + /** * The "bundle" namespace groups methods relevant to the user's * OMEMO bundle. @@ -35,7 +58,8 @@ export default { 'generate': async () => { await api.waitUntil('OMEMOInitialized'); // Remove current device - const devicelist = _converse.devicelists.get(_converse.bare_jid); + const devicelist = await api.omemo.devicelists.get(_converse.bare_jid); + const device_id = _converse.omemo_store.get('device_id'); if (device_id) { const device = devicelist.devices.get(device_id); diff --git a/src/plugins/omemo/devicelist.js b/src/plugins/omemo/devicelist.js index d571e1ab1..63529fd2e 100644 --- a/src/plugins/omemo/devicelist.js +++ b/src/plugins/omemo/devicelist.js @@ -43,7 +43,7 @@ const DeviceList = Model.extend({ this.destroy(); } if (this.get('jid') === _converse.bare_jid) { - await this.publishCurrentDevice(ids); + this.publishCurrentDevice(ids); } } }, @@ -95,23 +95,14 @@ const DeviceList = Model.extend({ 'type': 'get', 'from': _converse.bare_jid, 'to': this.get('jid') - }) - .c('pubsub', { 'xmlns': Strophe.NS.PUBSUB }) - .c('items', { 'node': Strophe.NS.OMEMO_DEVICELIST }); + }).c('pubsub', { 'xmlns': Strophe.NS.PUBSUB }) + .c('items', { 'node': Strophe.NS.OMEMO_DEVICELIST }); - let iq; - try { - iq = await api.sendIQ(stanza); - } catch (e) { - log.error(e); - return []; - } + const iq = await api.sendIQ(stanza); const selector = `list[xmlns="${Strophe.NS.OMEMO}"] device`; const device_ids = sizzle(selector, iq).map(d => d.getAttribute('id')); - await Promise.all( - device_ids.map(id => this.devices.create({ id, 'jid': this.get('jid') }, { 'promise': true })) - ); - return device_ids; + const jid = this.get('jid'); + return Promise.all(device_ids.map(id => this.devices.create({ id, jid }, { 'promise': true }))); }, /** diff --git a/src/plugins/omemo/devicelists.js b/src/plugins/omemo/devicelists.js index f516394cd..a42462a76 100644 --- a/src/plugins/omemo/devicelists.js +++ b/src/plugins/omemo/devicelists.js @@ -6,20 +6,6 @@ import { Collection } from '@converse/skeletor/src/collection'; * @namespace _converse.DeviceLists * @memberOf _converse */ -const DeviceLists = Collection.extend({ - model: DeviceList, - - /** - * Returns the {@link _converse.DeviceList} for a particular JID. - * The device list will be created if it doesn't exist already. - * @method _converse.DeviceLists#getDeviceList - * @param { String } jid - The Jabber ID for which the device list will be returned. - */ - async getDeviceList (jid) { - const list = this.get(jid) || this.create({ 'jid': jid }); - await list.initialized; - return list; - } -}); +const DeviceLists = Collection.extend({ model: DeviceList }); export default DeviceLists; diff --git a/src/plugins/omemo/fingerprints.js b/src/plugins/omemo/fingerprints.js index fdf0bd122..411feca22 100644 --- a/src/plugins/omemo/fingerprints.js +++ b/src/plugins/omemo/fingerprints.js @@ -1,6 +1,6 @@ import tpl_fingerprints from './templates/fingerprints.js'; import { CustomElement } from 'shared/components/element.js'; -import { _converse, api } from "@converse/headless/core"; +import { api } from "@converse/headless/core"; export class Fingerprints extends CustomElement { @@ -11,7 +11,7 @@ export class Fingerprints extends CustomElement { } async initialize () { - this.devicelist = await _converse.devicelists.getDeviceList(this.jid); + this.devicelist = await api.omemo.devicelists.get(this.jid, true); this.listenTo(this.devicelist.devices, 'change:bundle', this.requestUpdate); this.listenTo(this.devicelist.devices, 'change:trusted', this.requestUpdate); this.listenTo(this.devicelist.devices, 'remove', this.requestUpdate); diff --git a/src/plugins/omemo/profile.js b/src/plugins/omemo/profile.js index 1fbcbc913..93b826946 100644 --- a/src/plugins/omemo/profile.js +++ b/src/plugins/omemo/profile.js @@ -10,7 +10,7 @@ const { Strophe, sizzle, u } = converse.env; export class Profile extends CustomElement { async initialize () { - this.devicelist = await _converse.devicelists.getDeviceList(_converse.bare_jid); + this.devicelist = await api.omemo.devicelists.get(_converse.bare_jid, true); await this.setAttributes(); this.listenTo(this.devicelist.devices, 'change:bundle', () => this.requestUpdate()); this.listenTo(this.devicelist.devices, 'reset', () => this.requestUpdate()); diff --git a/src/plugins/omemo/store.js b/src/plugins/omemo/store.js index 05c18a80a..f2a5b3d66 100644 --- a/src/plugins/omemo/store.js +++ b/src/plugins/omemo/store.js @@ -199,7 +199,7 @@ const OMEMOStore = Model.extend({ 'id': k.keyId, 'key': u.arrayBufferToBase64(k.pubKey) })); - const devicelist = _converse.devicelists.get(_converse.bare_jid); + const devicelist = await api.omemo.devicelists.get(_converse.bare_jid); const device = devicelist.devices.get(this.get('device_id')); const bundle = await device.getBundle(); device.save('bundle', Object.assign(bundle, { 'prekeys': marshalled_keys })); @@ -218,7 +218,7 @@ const OMEMOStore = Model.extend({ const identity_keypair = await libsignal.KeyHelper.generateIdentityKeyPair(); const bundle = {}; const identity_key = u.arrayBufferToBase64(identity_keypair.pubKey); - const device_id = generateDeviceID(); + const device_id = await generateDeviceID(); bundle['identity_key'] = identity_key; bundle['device_id'] = device_id; @@ -242,7 +242,7 @@ const OMEMOStore = Model.extend({ range(0, _converse.NUM_PREKEYS).map(id => libsignal.KeyHelper.generatePreKey(id)) ); keys.forEach(k => this.storePreKey(k.keyId, k.keyPair)); - const devicelist = _converse.devicelists.get(_converse.bare_jid); + const devicelist = await api.omemo.devicelists.get(_converse.bare_jid); const device = await devicelist.devices.create( { 'id': bundle.device_id, 'jid': _converse.bare_jid }, { 'promise': true } diff --git a/src/plugins/omemo/utils.js b/src/plugins/omemo/utils.js index ad462e592..fd792477e 100644 --- a/src/plugins/omemo/utils.js +++ b/src/plugins/omemo/utils.js @@ -302,7 +302,7 @@ function getJIDForDecryption (attrs) { async function handleDecryptedWhisperMessage (attrs, key_and_tag) { const from_jid = getJIDForDecryption(attrs); - const devicelist = await _converse.devicelists.getDeviceList(from_jid); + const devicelist = await api.omemo.devicelists.get(from_jid, true); const encrypted = attrs.encrypted; let device = devicelist.devices.get(encrypted.device_id); if (!device) { @@ -446,14 +446,15 @@ export async function generateFingerprint (device) { export async function getDevicesForContact (jid) { await api.waitUntil('OMEMOInitialized'); - const devicelist = _converse.devicelists.get(jid) || _converse.devicelists.create({ 'jid': jid }); + const devicelist = await api.omemo.devicelists.get(jid, true); await devicelist.fetchDevices(); return devicelist.devices; } -export function generateDeviceID () { +export async function generateDeviceID () { /* Generates a device ID, making sure that it's unique */ - const existing_ids = _converse.devicelists.get(_converse.bare_jid).devices.pluck('id'); + const devicelist = await api.omemo.devicelists.get(_converse.bare_jid); + const existing_ids = devicelist.devices.pluck('id'); let device_id = libsignal.KeyHelper.generateRegistrationId(); // Before publishing a freshly generated device id for the first time, @@ -519,7 +520,7 @@ async function updateBundleFromStanza (stanza) { const device_id = items_el.getAttribute('node').split(':')[1]; const jid = stanza.getAttribute('from'); const bundle_el = sizzle(`item > bundle`, items_el).pop(); - const devicelist = await _converse.devicelists.getDeviceList(jid); + const devicelist = await api.omemo.devicelists.get(jid, true); const device = devicelist.devices.get(device_id) || devicelist.devices.create({ 'id': device_id, jid }); device.save({ 'bundle': parseBundle(bundle_el) }); } @@ -532,7 +533,7 @@ async function updateDevicesFromStanza (stanza) { const device_selector = `item list[xmlns="${Strophe.NS.OMEMO}"] device`; const device_ids = sizzle(device_selector, items_el).map(d => d.getAttribute('id')); const jid = stanza.getAttribute('from'); - const devicelist = await _converse.devicelists.getDeviceList(jid); + const devicelist = await api.omemo.devicelists.get(jid, true); const devices = devicelist.devices; const removed_ids = difference(devices.pluck('id'), device_ids); @@ -578,35 +579,29 @@ export function registerPEPPushHandler () { ); } -export function restoreOMEMOSession () { +export async function restoreOMEMOSession () { if (_converse.omemo_store === undefined) { const id = `converse.omemosession-${_converse.bare_jid}`; _converse.omemo_store = new _converse.OMEMOStore({ id }); initStorage(_converse.omemo_store, id); } - return _converse.omemo_store.fetchSession(); + await _converse.omemo_store.fetchSession(); } async function fetchDeviceLists () { - _converse.devicelists = new _converse.DeviceLists(); const id = `converse.devicelists-${_converse.bare_jid}`; + _converse.devicelists = new _converse.DeviceLists({ id }); initStorage(_converse.devicelists, id); await new Promise(resolve => { _converse.devicelists.fetch({ 'success': resolve, - 'error': (m, e) => { - log.error(e); - resolve(); - } + 'error': (m, e) => { log.error(e); resolve(); } }) }); - const promises = _converse.devicelists.map(l => l.initialized); - if (!_converse.devicelists.get(_converse.bare_jid)) { - // Create own device list if we none was restored - const own_list = await _converse.devicelists.create({ 'jid': _converse.bare_jid }, { 'promise': true }); - return Promise.all([...promises, own_list.initialized]); - } - return Promise.all(promises); + // Call API method to wait for our own device list to be fetched from the + // server or to be created. If we have no pre-existing OMEMO session, this + // will cause a new device and bundle to be generated and published. + await api.omemo.devicelists.get(_converse.bare_jid, true); } export async function initOMEMO (reconnecting) { @@ -748,7 +743,8 @@ export async function getBundlesAndBuildSessions (chatbox) { err.user_facing = true; throw err; } - const own_devices = _converse.devicelists.get(_converse.bare_jid).devices; + const own_list = await api.omemo.devicelists.get(_converse.bare_jid) + const own_devices = own_list.devices; devices = [...own_devices.models, ...their_devices.models]; } // Filter out our own device