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.
This commit is contained in:
JC Brand 2022-02-19 12:09:30 +01:00
parent 2133241998
commit 89a3c81a19
8 changed files with 66 additions and 64 deletions

View File

@ -30,7 +30,7 @@ describe("XEP-0198 Stream Management", function () {
expect(_converse.session.get('smacks_enabled')).toBe(true); expect(_converse.session.get('smacks_enabled')).toBe(true);
let IQ_stanzas = _converse.connection.IQ_stanzas; 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]; let iq = IQ_stanzas[IQ_stanzas.length-1];
expect(Strophe.serialize(iq)).toBe( expect(Strophe.serialize(iq)).toBe(
@ -57,13 +57,13 @@ describe("XEP-0198 Stream Management", function () {
iq = IQ_stanzas.pop(); iq = IQ_stanzas.pop();
expect(expected_IQs(disco_iq).includes(Strophe.serialize(disco_iq))).toBe(true); expect(expected_IQs(disco_iq).includes(Strophe.serialize(disco_iq))).toBe(true);
expect(sent_stanzas.filter(s => (s.nodeName === 'r')).length).toBe(2); expect(sent_stanzas.filter(s => (s.nodeName === 'r')).length).toBe(3);
expect(_converse.session.get('unacked_stanzas').length).toBe(5); expect(_converse.session.get('unacked_stanzas').length).toBe(6);
// test handling of acks // test handling of acks
let ack = u.toStanza(`<a xmlns="urn:xmpp:sm:3" h="2"/>`); let ack = u.toStanza(`<a xmlns="urn:xmpp:sm:3" h="2"/>`);
_converse.connection._dataRecv(mock.createRequest(ack)); _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 // test handling of ack requests
let r = u.toStanza(`<r xmlns="urn:xmpp:sm:3"/>`); let r = u.toStanza(`<r xmlns="urn:xmpp:sm:3"/>`);
@ -89,7 +89,7 @@ describe("XEP-0198 Stream Management", function () {
ack = u.toStanza(`<a xmlns="urn:xmpp:sm:3" h="3"/>`); ack = u.toStanza(`<a xmlns="urn:xmpp:sm:3" h="3"/>`);
_converse.connection._dataRecv(mock.createRequest(ack)); _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(`<r xmlns="urn:xmpp:sm:3"/>`); r = u.toStanza(`<r xmlns="urn:xmpp:sm:3"/>`);
_converse.connection._dataRecv(mock.createRequest(r)); _converse.connection._dataRecv(mock.createRequest(r));
@ -112,12 +112,17 @@ describe("XEP-0198 Stream Management", function () {
expect(_converse.session.get('smacks_enabled')).toBe(true); expect(_converse.session.get('smacks_enabled')).toBe(true);
await new Promise(resolve => _converse.api.listen.once('reconnected', resolve)); 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 // Test that unacked stanzas get resent out
iq = IQ_stanzas.pop(); iq = IQ_stanzas.pop();
expect(Strophe.serialize(iq)).toBe(`<iq id="${iq.getAttribute('id')}" type="get" xmlns="jabber:client"><query xmlns="jabber:iq:roster"/></iq>`); expect(Strophe.serialize(iq)).toBe(`<iq id="${iq.getAttribute('id')}" type="get" xmlns="jabber:client"><query xmlns="jabber:iq:roster"/></iq>`);
iq = IQ_stanzas.pop();
expect(Strophe.serialize(iq)).toBe(
`<iq from="romeo@montague.lit" id="${iq.getAttribute('id')}" to="romeo@montague.lit" type="get" xmlns="jabber:client">`+
`<pubsub xmlns="http://jabber.org/protocol/pubsub"><items node="eu.siacs.conversations.axolotl.devicelist"/></pubsub></iq>`);
expect(IQ_stanzas.filter(iq => sizzle('query[xmlns="jabber:iq:roster"]', iq).pop()).length).toBe(0); expect(IQ_stanzas.filter(iq => sizzle('query[xmlns="jabber:iq:roster"]', iq).pop()).length).toBe(0);
})); }));

View File

@ -18,6 +18,29 @@ export default {
return _converse.omemo_store.get('device_id'); 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 * The "bundle" namespace groups methods relevant to the user's
* OMEMO bundle. * OMEMO bundle.
@ -35,7 +58,8 @@ export default {
'generate': async () => { 'generate': async () => {
await api.waitUntil('OMEMOInitialized'); await api.waitUntil('OMEMOInitialized');
// Remove current device // 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'); const device_id = _converse.omemo_store.get('device_id');
if (device_id) { if (device_id) {
const device = devicelist.devices.get(device_id); const device = devicelist.devices.get(device_id);

View File

@ -43,7 +43,7 @@ const DeviceList = Model.extend({
this.destroy(); this.destroy();
} }
if (this.get('jid') === _converse.bare_jid) { if (this.get('jid') === _converse.bare_jid) {
await this.publishCurrentDevice(ids); this.publishCurrentDevice(ids);
} }
} }
}, },
@ -95,23 +95,14 @@ const DeviceList = Model.extend({
'type': 'get', 'type': 'get',
'from': _converse.bare_jid, 'from': _converse.bare_jid,
'to': this.get('jid') 'to': this.get('jid')
}) }).c('pubsub', { 'xmlns': Strophe.NS.PUBSUB })
.c('pubsub', { 'xmlns': Strophe.NS.PUBSUB }) .c('items', { 'node': Strophe.NS.OMEMO_DEVICELIST });
.c('items', { 'node': Strophe.NS.OMEMO_DEVICELIST });
let iq; const iq = await api.sendIQ(stanza);
try {
iq = await api.sendIQ(stanza);
} catch (e) {
log.error(e);
return [];
}
const selector = `list[xmlns="${Strophe.NS.OMEMO}"] device`; const selector = `list[xmlns="${Strophe.NS.OMEMO}"] device`;
const device_ids = sizzle(selector, iq).map(d => d.getAttribute('id')); const device_ids = sizzle(selector, iq).map(d => d.getAttribute('id'));
await Promise.all( const jid = this.get('jid');
device_ids.map(id => this.devices.create({ id, 'jid': this.get('jid') }, { 'promise': true })) return Promise.all(device_ids.map(id => this.devices.create({ id, jid }, { 'promise': true })));
);
return device_ids;
}, },
/** /**

View File

@ -6,20 +6,6 @@ import { Collection } from '@converse/skeletor/src/collection';
* @namespace _converse.DeviceLists * @namespace _converse.DeviceLists
* @memberOf _converse * @memberOf _converse
*/ */
const DeviceLists = Collection.extend({ const DeviceLists = Collection.extend({ model: DeviceList });
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;
}
});
export default DeviceLists; export default DeviceLists;

View File

@ -1,6 +1,6 @@
import tpl_fingerprints from './templates/fingerprints.js'; import tpl_fingerprints from './templates/fingerprints.js';
import { CustomElement } from 'shared/components/element.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 { export class Fingerprints extends CustomElement {
@ -11,7 +11,7 @@ export class Fingerprints extends CustomElement {
} }
async initialize () { 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:bundle', this.requestUpdate);
this.listenTo(this.devicelist.devices, 'change:trusted', this.requestUpdate); this.listenTo(this.devicelist.devices, 'change:trusted', this.requestUpdate);
this.listenTo(this.devicelist.devices, 'remove', this.requestUpdate); this.listenTo(this.devicelist.devices, 'remove', this.requestUpdate);

View File

@ -10,7 +10,7 @@ const { Strophe, sizzle, u } = converse.env;
export class Profile extends CustomElement { export class Profile extends CustomElement {
async initialize () { 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(); await this.setAttributes();
this.listenTo(this.devicelist.devices, 'change:bundle', () => this.requestUpdate()); this.listenTo(this.devicelist.devices, 'change:bundle', () => this.requestUpdate());
this.listenTo(this.devicelist.devices, 'reset', () => this.requestUpdate()); this.listenTo(this.devicelist.devices, 'reset', () => this.requestUpdate());

View File

@ -199,7 +199,7 @@ const OMEMOStore = Model.extend({
'id': k.keyId, 'id': k.keyId,
'key': u.arrayBufferToBase64(k.pubKey) '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 device = devicelist.devices.get(this.get('device_id'));
const bundle = await device.getBundle(); const bundle = await device.getBundle();
device.save('bundle', Object.assign(bundle, { 'prekeys': marshalled_keys })); 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 identity_keypair = await libsignal.KeyHelper.generateIdentityKeyPair();
const bundle = {}; const bundle = {};
const identity_key = u.arrayBufferToBase64(identity_keypair.pubKey); const identity_key = u.arrayBufferToBase64(identity_keypair.pubKey);
const device_id = generateDeviceID(); const device_id = await generateDeviceID();
bundle['identity_key'] = identity_key; bundle['identity_key'] = identity_key;
bundle['device_id'] = device_id; bundle['device_id'] = device_id;
@ -242,7 +242,7 @@ const OMEMOStore = Model.extend({
range(0, _converse.NUM_PREKEYS).map(id => libsignal.KeyHelper.generatePreKey(id)) range(0, _converse.NUM_PREKEYS).map(id => libsignal.KeyHelper.generatePreKey(id))
); );
keys.forEach(k => this.storePreKey(k.keyId, k.keyPair)); 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( const device = await devicelist.devices.create(
{ 'id': bundle.device_id, 'jid': _converse.bare_jid }, { 'id': bundle.device_id, 'jid': _converse.bare_jid },
{ 'promise': true } { 'promise': true }

View File

@ -302,7 +302,7 @@ function getJIDForDecryption (attrs) {
async function handleDecryptedWhisperMessage (attrs, key_and_tag) { async function handleDecryptedWhisperMessage (attrs, key_and_tag) {
const from_jid = getJIDForDecryption(attrs); 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; const encrypted = attrs.encrypted;
let device = devicelist.devices.get(encrypted.device_id); let device = devicelist.devices.get(encrypted.device_id);
if (!device) { if (!device) {
@ -446,14 +446,15 @@ export async function generateFingerprint (device) {
export async function getDevicesForContact (jid) { export async function getDevicesForContact (jid) {
await api.waitUntil('OMEMOInitialized'); 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(); await devicelist.fetchDevices();
return devicelist.devices; return devicelist.devices;
} }
export function generateDeviceID () { export async function generateDeviceID () {
/* Generates a device ID, making sure that it's unique */ /* 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(); let device_id = libsignal.KeyHelper.generateRegistrationId();
// Before publishing a freshly generated device id for the first time, // 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 device_id = items_el.getAttribute('node').split(':')[1];
const jid = stanza.getAttribute('from'); const jid = stanza.getAttribute('from');
const bundle_el = sizzle(`item > bundle`, items_el).pop(); 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 }); const device = devicelist.devices.get(device_id) || devicelist.devices.create({ 'id': device_id, jid });
device.save({ 'bundle': parseBundle(bundle_el) }); 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_selector = `item list[xmlns="${Strophe.NS.OMEMO}"] device`;
const device_ids = sizzle(device_selector, items_el).map(d => d.getAttribute('id')); const device_ids = sizzle(device_selector, items_el).map(d => d.getAttribute('id'));
const jid = stanza.getAttribute('from'); 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 devices = devicelist.devices;
const removed_ids = difference(devices.pluck('id'), device_ids); 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) { if (_converse.omemo_store === undefined) {
const id = `converse.omemosession-${_converse.bare_jid}`; const id = `converse.omemosession-${_converse.bare_jid}`;
_converse.omemo_store = new _converse.OMEMOStore({ id }); _converse.omemo_store = new _converse.OMEMOStore({ id });
initStorage(_converse.omemo_store, id); initStorage(_converse.omemo_store, id);
} }
return _converse.omemo_store.fetchSession(); await _converse.omemo_store.fetchSession();
} }
async function fetchDeviceLists () { async function fetchDeviceLists () {
_converse.devicelists = new _converse.DeviceLists();
const id = `converse.devicelists-${_converse.bare_jid}`; const id = `converse.devicelists-${_converse.bare_jid}`;
_converse.devicelists = new _converse.DeviceLists({ id });
initStorage(_converse.devicelists, id); initStorage(_converse.devicelists, id);
await new Promise(resolve => { await new Promise(resolve => {
_converse.devicelists.fetch({ _converse.devicelists.fetch({
'success': resolve, 'success': resolve,
'error': (m, e) => { 'error': (m, e) => { log.error(e); resolve(); }
log.error(e);
resolve();
}
}) })
}); });
const promises = _converse.devicelists.map(l => l.initialized); // Call API method to wait for our own device list to be fetched from the
if (!_converse.devicelists.get(_converse.bare_jid)) { // server or to be created. If we have no pre-existing OMEMO session, this
// Create own device list if we none was restored // will cause a new device and bundle to be generated and published.
const own_list = await _converse.devicelists.create({ 'jid': _converse.bare_jid }, { 'promise': true }); await api.omemo.devicelists.get(_converse.bare_jid, true);
return Promise.all([...promises, own_list.initialized]);
}
return Promise.all(promises);
} }
export async function initOMEMO (reconnecting) { export async function initOMEMO (reconnecting) {
@ -748,7 +743,8 @@ export async function getBundlesAndBuildSessions (chatbox) {
err.user_facing = true; err.user_facing = true;
throw err; 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]; devices = [...own_devices.models, ...their_devices.models];
} }
// Filter out our own device // Filter out our own device