From 447fe8ba0892319a27cc1cb0480867185a167cd5 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Tue, 27 Dec 2022 21:48:17 +0100 Subject: [PATCH] Refactor the code related to storing SCRAM keys - No need to create a new storage mechanism, just use `persistent`. - Store SCRAM keys per JID - Upon succesfull login, store the current session JID, so that we know who to fetch SCRAM keys for - Only store SCRAM keys when the device is trusted Fixes #3001 --- CHANGES.md | 3 + dev.html | 1 + docs/source/configuration.rst | 63 +++------ package-lock.json | 1 + src/headless/core.js | 52 +------- src/headless/shared/connection/index.js | 6 + src/headless/shared/settings/constants.js | 4 +- src/headless/utils/core.js | 24 +--- src/headless/utils/init.js | 151 ++++++++++++++-------- 9 files changed, 136 insertions(+), 169 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 0875d6eb1..6f3c2db17 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,8 +5,11 @@ - #326: Add the ability to reset your password - #2816: Chat highlight behaves odd - #2925: File upload is not always enabled +- #3001: Add option to save SCRAM details and to use them to stay logged in upon reload - Add a "Add to Contacts" button in MUC occupant modals +- New config option [save_scram_keys](https://conversejs.org/docs/html/configuration.html#save-scram-keys) + ## 10.0.0 (2022-10-30) - Update to Strophe.js 1.6.0 which adds support for SCRAM-SHA-256 and SCRAM-SHA-512 diff --git a/dev.html b/dev.html index d1a312a5f..c7a42bd32 100644 --- a/dev.html +++ b/dev.html @@ -32,6 +32,7 @@ auto_away: 300, enable_smacks: true, loglevel: 'debug', + reuse_scram_keys: true, prune_messages_above: 100, message_archiving: 'always', muc_respect_autojoin: true, diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst index 87ab570f7..4b068704a 100644 --- a/docs/source/configuration.rst +++ b/docs/source/configuration.rst @@ -407,48 +407,6 @@ in to their XMPP account. So currently if EITHER ``keepalive`` or ``auto_login`` is ``true`` and `authentication`_ is set to ``login``, then Converse will try to log the user in. -save_scram_keys ---------------- -* Default: ``false`` - -Most XMPP servers enable the Salted Challenge Response Authentication Mechanism -or SCRAM for short. This allows the user and the server to mutually -authenticate *without* the need to transmit the user's password in plaintext. -Coincidentally, assuming the server does not alter the user's password or the -storage parameters, we can authenticate with the same SCRAM key multiple times. -This opens an opportunity: we can store the user's login credentials in the -browser without the need to store their sensitive plaintext password, or the -need to set up complicated third party backends, like oauth. - -Enabling this option will cause converse to save the SCRAM keys on successful -login into browser storage. This information can be recovered from the public -API method ``converse.savedLoginInfo()``, which returns on success a Promise -which resolves to an object whose ``attributes`` object contains the following -information: - -:: - { 'id': 'converse.savedLoginInfo', - 'users': Usermap Object - } - -Where the ``Usermap`` Object has keys corresponding to users and values -which are valid login credentials (which can be passed in as the -``password`` field on login), like so: - -:: - { 'user1@xmpp.org': Credentials, - 'user2@opkode.com': Credentials, - ... - } - -From here, one may configure their client to simply choose one of the logins, -depending on their needs, and pass the username and credentials into the -settings. -Note well that this method will only work once converse has been loaded. -If you need the utilities provided here before login, call -`window.converse.load()`. - - auto_away --------- @@ -1861,6 +1819,27 @@ Based on the OGP metadata Converse will render a URL preview (also known as an the ``show_images_inline``, ``embed_audio`` and ``embed_videos`` settings. +reuse_scram_keys +---------------- + +* Default: ``false`` + +Most XMPP servers enable the Salted Challenge Response Authentication Mechanism +or SCRAM for short. This allows the user and the server to mutually +authenticate *without* the need to transmit the user's password in plaintext. + +Assuming the server does not alter the user's password or the +storage parameters, we can authenticate with the same SCRAM key multiple times. + +This opens an opportunity: we can store the user's login credentials in the +browser without storing the sensitive plaintext password, or the +need to set up complicated third party backends, like OAuth. + +Enabling this option will let Converse save a user's SCRAM keys upon successful +login, and next time Converse is loaded the user will be automatically logged in +with those SCRAM keys. + + .. _`roomconfig_whitelist`: roomconfig_whitelist diff --git a/package-lock.json b/package-lock.json index f054a95e4..bb97fca22 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18098,6 +18098,7 @@ } }, "src/headless": { + "name": "@converse/headless", "version": "10.0.0", "dev": true, "license": "MPL-2.0", diff --git a/src/headless/core.js b/src/headless/core.js index b8604e158..37ea13751 100644 --- a/src/headless/core.js +++ b/src/headless/core.js @@ -14,7 +14,6 @@ import log from '@converse/headless/log.js'; import pluggable from 'pluggable.js/src/pluggable.js'; import sizzle from 'sizzle'; import u, { setUnloadEvent, replacePromise } from '@converse/headless/utils/core.js'; -import { initStorage } from './utils/storage.js'; import { CHAT_STATES, KEYCODES } from './shared/constants.js'; import { Collection } from "@converse/skeletor/src/collection"; import { Connection, MockConnection } from '@converse/headless/shared/connection/index.js'; @@ -38,7 +37,6 @@ import { initClientConfig, initPlugins, initSessionStorage, - initScramStorage, registerGlobalEventHandlers, setUserJID, } from './utils/init.js'; @@ -473,23 +471,7 @@ export const api = _converse.api = { } api.trigger('send', stanza); return promise; - }, - - /** - * Fetch previously used login information, username and SCRAM keys if available - * @method _converse.api.savedLoginInfo - * @returns {Promise} A promise which resolves (or potentially rejects) once we - * fetch the previously used login keys. - */ - async savedLoginInfo () { - const id = "converse.savedLoginInfo"; - const login_info = new Model({id}); - initStorage(login_info, id, 'scramStorage'); - await new Promise(f => login_info.fetch({'success': f, 'error': f})); - - return login_info; - }, - + } }; @@ -693,38 +675,6 @@ Object.assign(converse, { } }, - /** - * Fetch previously used login information, username and SCRAM keys if available - * @method _converse.api.getSavedLoginInfo - * @returns {Promise} A promise which resolves (or potentially rejects) once - * we fetch the previously used login keys. The object returned on success - * has an attributes object of the following form: - * { 'id': 'converse.savedLoginInfo', - * 'users': Usermap Object - * } - * Where the Usermap Object has keys corresponding to users and values - * which are valid login credentials (which can be passed in as the - * password field on login), like so: - * { 'user1@xmpp.org': Credentials, - * 'user2@opkode.com': Credentials, - * ... - * } - * It should be noted that these Credentials will *NEVER* store the user's - * plaintext password, nor any material from which the user's plaintext - * password could be recovered. It uses SASL SCRAM internally, which - * secures the user's login information and ensures* the authenticating - * server is the server which was supplied the credentials initially. - * - * *With some caveats, we don't yet actively protect against active MITM - * attacks. - */ - savedLoginInfo: async () => { - if (!_converse.storage) { - await initScramStorage(_converse); - } - return _converse.api.savedLoginInfo() - }, - /** * Exposes methods for adding and removing plugins. You'll need to write a plugin * if you want to have access to the private API methods defined further down below. diff --git a/src/headless/shared/connection/index.js b/src/headless/shared/connection/index.js index aef4ecc0a..84107bf33 100644 --- a/src/headless/shared/connection/index.js +++ b/src/headless/shared/connection/index.js @@ -181,6 +181,12 @@ export class Connection extends Strophe.Connection { this.flush(); // Solves problem of returned PubSub BOSH response not received by browser await setUserJID(this.jid); + // Save the current JID in persistent storage so that we can attempt to + // recreate the session from SCRAM keys + if (_converse.config.get('trusted')) { + localStorage.setItem('conversejs-session-jid', _converse.bare_jid); + } + /** * Synchronous event triggered after we've sent an IQ to bind the * user's JID resource for this session. diff --git a/src/headless/shared/settings/constants.js b/src/headless/shared/settings/constants.js index 402b9c67c..ac7ae6c0b 100644 --- a/src/headless/shared/settings/constants.js +++ b/src/headless/shared/settings/constants.js @@ -6,7 +6,7 @@ * @property { String } [assets_path='/dist'] * @property { ('login'|'prebind'|'anonymous'|'external') } [authentication='login'] * @property { Boolean } [auto_login=false] - Currently only used in connection with anonymous login - * @property { Boolean } [save_scram_keys=false] - Save SCRAM keys after login to allow for future auto login + * @property { Boolean } [reuse_scram_keys=false] - Save SCRAM keys after login to allow for future auto login * @property { Boolean } [auto_reconnect=true] * @property { Array} [blacklisted_plugins] * @property { Boolean } [clear_cache_on_logout=false] @@ -38,7 +38,7 @@ export const DEFAULT_SETTINGS = { assets_path: '/dist', authentication: 'login', // Available values are "login", "prebind", "anonymous" and "external". auto_login: false, // Currently only used in connection with anonymous login - save_scram_keys: false, + reuse_scram_keys: false, auto_reconnect: true, blacklisted_plugins: [], clear_cache_on_logout: false, diff --git a/src/headless/utils/core.js b/src/headless/utils/core.js index 6053bdc86..970ad9269 100644 --- a/src/headless/utils/core.js +++ b/src/headless/utils/core.js @@ -14,7 +14,6 @@ import sizzle from "sizzle"; import { Model } from '@converse/skeletor/src/model.js'; import { Strophe } from 'strophe.js/src/strophe.js'; import { getOpenPromise } from '@converse/openpromise'; -import { setUserJID, } from '@converse/headless/utils/init.js'; import { settings_api } from '@converse/headless/shared/settings/api.js'; import { stx , toStanza } from './stanza.js'; @@ -125,12 +124,12 @@ u.getLongestSubstring = function (string, candidates) { return candidates.reduce(reducer, ''); } -u.isValidJID = function (jid) { +export function isValidJID (jid) { if (typeof jid === 'string') { return compact(jid.split('@')).length === 2 && !jid.startsWith('@') && !jid.endsWith('@'); } return false; -}; +} u.isValidMUCJID = function (jid) { return !jid.startsWith('@') && !jid.endsWith('@'); @@ -553,18 +552,6 @@ export function setUnloadEvent () { } } -export async function getLoginCredentialsFromBrowser () { - try { - const creds = await navigator.credentials.get({'password': true}); - if (creds && creds.type == 'password' && u.isValidJID(creds.id)) { - await setUserJID(creds.id); - return {'jid': creds.id, 'password': creds.password}; - } - } catch (e) { - log.error(e); - } -} - export function replacePromise (name) { const existing_promise = _converse.promises[name]; if (!existing_promise) { @@ -591,9 +578,10 @@ export function decodeHTMLEntities (str) { } export default Object.assign({ - prefixMentions, - isEmptyMessage, getUniqueId, - toStanza, + isEmptyMessage, + isValidJID, + prefixMentions, stx, + toStanza, }, u); diff --git a/src/headless/utils/init.js b/src/headless/utils/init.js index a589927eb..8b1cb0408 100644 --- a/src/headless/utils/init.js +++ b/src/headless/utils/init.js @@ -9,7 +9,7 @@ import { Connection } from '@converse/headless/shared/connection/index.js'; import { Model } from '@converse/skeletor/src/model.js'; import { Strophe } from 'strophe.js/src/strophe'; import { createStore, initStorage } from '@converse/headless/utils/storage.js'; -import { getLoginCredentialsFromBrowser } from '@converse/headless/utils/core.js'; +import { isValidJID } from './core.js'; export function initPlugins (_converse) { @@ -87,17 +87,6 @@ export async function initSessionStorage (_converse) { }; } -export async function initScramStorage (_converse) { - _converse.storage = { - ..._converse.storage, - 'scramStorage': Storage.localForage.createInstance({ - 'name': 'converse-scram', - 'description': 'SCRAM storage driver', - 'driver': Storage.localForage.INDEXEDDB - }) - }; -} - function initPersistentStorage (_converse, store_name) { if (_converse.api.settings.get('persistent_store') === 'sessionStorage') { return; @@ -130,6 +119,7 @@ function initPersistentStorage (_converse, store_name) { _converse.storage['persistent'] = Storage.localForage.createInstance(config); } + function saveJIDtoSession (_converse, jid) { jid = _converse.session.get('jid') || jid; if (_converse.api.settings.get("authentication") !== _converse.ANONYMOUS && !Strophe.getResourceFromJid(jid)) { @@ -169,6 +159,7 @@ function saveJIDtoSession (_converse, jid) { */ export async function setUserJID (jid) { await initSession(_converse, jid); + /** * Triggered whenever the user's JID has been updated * @event _converse#setUserJID @@ -177,6 +168,7 @@ export async function setUserJID (jid) { return jid; } + export async function initSession (_converse, jid) { const is_shared_session = _converse.api.settings.get('connection_options').worker; @@ -212,6 +204,7 @@ export async function initSession (_converse, jid) { } } + export function registerGlobalEventHandlers (_converse) { document.addEventListener("visibilitychange", _converse.saveWindowState); _converse.saveWindowState({'type': document.hidden ? "blur" : "focus"}); // Set initial state @@ -233,6 +226,7 @@ function unregisterGlobalEventHandlers (_converse) { api.trigger('unregisteredGlobalEventHandlers'); } + // Make sure everything is reset in case this is a subsequent call to // converse.initialize (happens during tests). export async function cleanup (_converse) { @@ -248,23 +242,6 @@ export async function cleanup (_converse) { } } -async function getLoginCredentials () { - let credentials; - let wait = 0; - while (!credentials) { - try { - credentials = await fetchLoginCredentials(wait); // eslint-disable-line no-await-in-loop - } catch (e) { - log.error('Could not fetch login credentials'); - log.error(e); - } - // If unsuccessful, we wait 2 seconds between subsequent attempts to - // fetch the credentials. - wait = 2000; - } - return credentials; -} - function fetchLoginCredentials (wait=0) { return new Promise( @@ -296,6 +273,50 @@ function fetchLoginCredentials (wait=0) { ); } + +async function getLoginCredentialsFromURL () { + let credentials; + let wait = 0; + while (!credentials) { + try { + credentials = await fetchLoginCredentials(wait); // eslint-disable-line no-await-in-loop + } catch (e) { + log.error('Could not fetch login credentials'); + log.error(e); + } + // If unsuccessful, we wait 2 seconds between subsequent attempts to + // fetch the credentials. + wait = 2000; + } + return credentials; +} + + +async function getLoginCredentialsFromBrowser () { + try { + const creds = await navigator.credentials.get({'password': true}); + if (creds && creds.type == 'password' && isValidJID(creds.id)) { + await setUserJID(creds.id); + return {'jid': creds.id, 'password': creds.password}; + } + } catch (e) { + log.error(e); + } +} + + +async function getLoginCredentialsFromSCRAMKeys () { + const jid = localStorage.getItem('conversejs-session-jid'); + if (!jid) return null; + + await setUserJID(jid); + + const login_info = await savedLoginInfo(jid); + const scram_keys = login_info.get('scram_keys'); + return scram_keys ? { jid , 'password': scram_keys } : null; +} + + export async function attemptNonPreboundSession (credentials, automatic) { const { api } = _converse; if (api.settings.get("authentication") === _converse.LOGIN) { @@ -306,18 +327,24 @@ export async function attemptNonPreboundSession (credentials, automatic) { // automatically setting up a new session (``auto_login``). // So we can't do the check (!automatic || _converse.api.settings.get("auto_login")) here. if (credentials) { - connect(credentials); + return connect(credentials); } else if (api.settings.get("credentials_url")) { // We give credentials_url preference, because // _converse.connection.pass might be an expired token. - connect(await getLoginCredentials()); + return connect(await getLoginCredentialsFromURL()); } else if (_converse.jid && (api.settings.get("password") || _converse.connection.pass)) { - connect(); - } else if (!_converse.isTestEnv() && 'credentials' in navigator) { - connect(await getLoginCredentialsFromBrowser()); - } else { - !_converse.isTestEnv() && log.warn("attemptNonPreboundSession: Couldn't find credentials to log in with"); + return connect(); } + + if (api.settings.get('reuse_scram_keys')) { + const credentials = await getLoginCredentialsFromSCRAMKeys(); + if (credentials) return connect(credentials); + } + + if (!_converse.isTestEnv() && 'credentials' in navigator) { + return connect(await getLoginCredentialsFromBrowser()); + } + !_converse.isTestEnv() && log.warn("attemptNonPreboundSession: Couldn't find credentials to log in with"); } else if ( [_converse.ANONYMOUS, _converse.EXTERNAL].includes(api.settings.get("authentication")) && (!automatic || api.settings.get("auto_login")) @@ -338,7 +365,26 @@ export function getConnectionServiceURL () { } -function connect (credentials) { +/** + * Fetch the stored SCRAM keys for the given JID, if available. + * + * The user's plaintext password is not stored, nor any material from which + * the user's plaintext password could be recovered. + * + * @param { String } JID - The XMPP address for which to fetch the SCRAM keys + * @returns { Promise } A promise which resolves once we've fetched the previously + * used login keys. + */ +export async function savedLoginInfo (jid) { + const id = `converse.scram-keys-${Strophe.getBareJidFromJid(jid)}`; + const login_info = new Model({ id }); + initStorage(login_info, id, 'persistent'); + await new Promise(f => login_info.fetch({'success': f, 'error': f})); + return login_info; +} + + +async function connect (credentials) { const { api } = _converse; if ([_converse.ANONYMOUS, _converse.EXTERNAL].includes(api.settings.get("authentication"))) { if (!_converse.jid) { @@ -369,26 +415,19 @@ function connect (credentials) { let callback; - if (api.settings.get("save_scram_keys") && !password.ck) { - // Don't save the SCRAM data if we already logged in with SCRAM - const login_info = await _converse.api.savedLoginInfo(); + // Save the SCRAM data if we're not already logged in with SCRAM + if ( + _converse.config.get('trusted') && + _converse.jid && + api.settings.get("reuse_scram_keys") && + !password?.ck + ) { + // Store scram keys in scram storage + const login_info = await savedLoginInfo(_converse.jid); - callback = async (status) => { - // Store scram keys in scram storage - if (!_converse?.storage?.scramStorage) { - await initScramStorage(_converse); - } - - const newScramKeys = _converse.connection.scramKeys; - if (newScramKeys) { - try { - const new_users_info = login_info.users ?? { }; - new_users_info[_converse.connection.authzid] = newScramKeys; - login_info.save({'users': new_users_info }); - } catch (e) { // Could not find local storage } - log.error("No storage method found: ", e); - } - } + callback = (status) => { + const { scram_keys } = _converse.connection; + if (scram_keys) login_info.save({ scram_keys }); _converse.connection.onConnectStatusChanged(status); }; }