Skip to content

Commit f9eb3bd

Browse files
committed
Load/store FAST keys for auto_login.
Also clean up FAST *and* SCRAM keys on log out; otherwise, the credentials are still in the browser, and could be stolen, or reused simply by someone who knows to redefined conversejs-session-jid in localStorage. Depends on strophe/strophejs#839 TODO: * [ ] This *renames* reuse_scram_keys to reuse_keys to cover both FAST and SCRAM, so it should probably get a backwards-compatibility shim for the old name. * [ ] Drop my development environment edit to package.json (without there's no way to test because both repos need to be in sync)
1 parent 3351481 commit f9eb3bd

File tree

9 files changed

+59
-29
lines changed

9 files changed

+59
-29
lines changed

docs/source/configuration.rst

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1887,25 +1887,26 @@ Based on the OGP metadata Converse will render a URL preview (also known as an
18871887
the ``show_images_inline``, ``embed_audio`` and ``embed_videos`` settings.
18881888

18891889

1890-
reuse_scram_keys
1891-
----------------
1890+
reuse_keys
1891+
----------
18921892

18931893
* Default: ``true``
18941894

18951895
Most XMPP servers enable the Salted Challenge Response Authentication Mechanism
1896-
or SCRAM for short. This allows the user and the server to mutually
1897-
authenticate *without* the need to transmit the user's password in plaintext.
1896+
or SCRAM for short. Newer servers also support Fast Authentication Streamlining Tokens.
1897+
These allow the user and the server to mutually authenticate *without* the need
1898+
to transmit the user's password in plaintext.
18981899

18991900
Assuming the server does not alter the user's password or the
1900-
storage parameters, we can authenticate with the same SCRAM key multiple times.
1901+
storage parameters, we can authenticate with the same key multiple times.
19011902

19021903
This opens an opportunity: we can store the user's login credentials in the
19031904
browser without storing the sensitive plaintext password, or the
19041905
need to set up complicated third party backends, like OAuth.
19051906

1906-
Enabling this option will let Converse save a user's SCRAM keys upon successful
1907+
Enabling this option will let Converse save a user's keys upon successful
19071908
login, and next time Converse is loaded the user will be automatically logged in
1908-
with those SCRAM keys.
1909+
with those keys.
19091910

19101911

19111912
.. _`roomconfig_whitelist`:

docs/source/session.rst

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,12 @@ We've purposefully not put this functionality in Converse.js due to the
196196
security implications of storing plaintext passwords in localStorage.
197197

198198

199-
Storing the SASL SCRAM-SHA1 hash in IndexedDB
200-
---------------------------------------------
199+
Storing the SASL SCRAM-SHA1 hash or FAST token in IndexedDB
200+
-----------------------------------------------------------
201201

202202
Another suggestion that's been suggested is to store the SCRAM-SHA1 computed
203203
``clientKey`` in localStorage and to use that upon page reload to log the user in again.
204204

205-
This has been implemented since version 10, see documentation on `reuse_scram_keys <https://conversejs.org/docs/html/configuration.html#reuse-scram-keys>`_
205+
In more modern terms, we can store the FAST token if supported by the server.
206+
207+
This has been implemented since version 10, see documentation on `reuse_keys <https://conversejs.org/docs/html/configuration.html#reuse-keys>`_

package-lock.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,8 @@
151151
"pluggable.js": "^3.0.1",
152152
"prettier": "^3.2.5",
153153
"sizzle": "^2.3.5",
154-
"sprintf-js": "^1.1.2"
154+
"sprintf-js": "^1.1.2",
155+
"strophe.js": "file:../strophejs"
155156
},
156157
"resolutions": {
157158
"autoprefixer": "10.4.5"

src/headless/shared/api/user.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import _converse from '../_converse.js';
55
import presence_api from './presence.js';
66
import connection_api from '../connection/api.js';
77
import { replacePromise } from '../../utils/session.js';
8-
import { attemptNonPreboundSession, setUserJID } from '../../utils/init.js';
8+
import { attemptNonPreboundSession, setUserJID, savedLoginInfo } from '../../utils/init.js';
99
import { getOpenPromise } from '@converse/openpromise';
1010
import { user_settings_api } from '../settings/user/api.js';
1111
import { LOGOUT } from '../constants.js';
@@ -108,8 +108,15 @@ const api = {
108108
// Recreate all the promises
109109
Object.keys(_converse.promises).forEach((p) => replacePromise(_converse, p));
110110

111-
// Remove the session JID, otherwise the user would just be logged
111+
// Remove the session JID/keys, otherwise the user would just be logged
112112
// in again upon reload. See #2759
113+
const jid = localStorage.getItem("conversejs-session-jid");
114+
if (jid) {
115+
savedLoginInfo(jid).then((login_info) => {
116+
// XXX TODO: is there a cleaner call? .remove()? .clear()?
117+
login_info.save({ fast: null, scram_keys: null });
118+
})
119+
}
113120
localStorage.removeItem('conversejs-session-jid');
114121

115122
/**

src/headless/shared/connection/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ export class Connection extends Strophe.Connection {
400400
condition = __('Your XMPP address and/or password is incorrect. Please try again.');
401401
}
402402
this.setConnectionStatus(status, condition);
403-
this.setDisconnectionCause(status, condition, true);
403+
this.setDisconnectionCause(status, condition, true); // XXX DISABLE this to enable SASL fallback
404404
this.onDisconnected();
405405

406406
} else if (status === Strophe.Status.CONNFAIL) {

src/headless/shared/settings/constants.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* @property {String} [assets_path='/dist']
77
* @property {('login'|'prebind'|'anonymous'|'external')} [authentication='login']
88
* @property {Boolean} [auto_login=false] - Currently only used in connection with anonymous login
9-
* @property {Boolean} [reuse_scram_keys=true] - Save SCRAM keys after login to allow for future auto login
9+
* @property {Boolean} [reuse_keys=true] - Save SCRAM/FAST keys after login to allow for future auto login
1010
* @property {Boolean} [auto_reconnect=true]
1111
* @property {Array<String>} [blacklisted_plugins]
1212
* @property {Boolean} [clear_cache_on_logout=false]
@@ -50,7 +50,7 @@ export const DEFAULT_SETTINGS = {
5050
geouri_replacement: 'https://www.openstreetmap.org/?mlat=$1&mlon=$2#map=18/$1/$2',
5151
i18n: undefined,
5252
jid: undefined,
53-
reuse_scram_keys: true,
53+
reuse_keys: true,
5454
keepalive: true,
5555
loglevel: 'info',
5656
locales: [

src/headless/types/shared/settings/constants.d.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export namespace DEFAULT_SETTINGS {
1616
let geouri_replacement: string;
1717
let i18n: any;
1818
let jid: any;
19-
let reuse_scram_keys: boolean;
19+
let reuse_keys: boolean;
2020
let keepalive: boolean;
2121
let loglevel: string;
2222
let locales: string[];
@@ -46,9 +46,9 @@ export type ConfigurationSettings = {
4646
*/
4747
auto_login?: boolean;
4848
/**
49-
* - Save SCRAM keys after login to allow for future auto login
49+
* - Save keys after login to allow for future auto login
5050
*/
51-
reuse_scram_keys?: boolean;
51+
reuse_keys?: boolean;
5252
auto_reconnect?: boolean;
5353
blacklisted_plugins?: Array<string>;
5454
clear_cache_on_logout?: boolean;
@@ -77,4 +77,4 @@ export type ConfigurationSettings = {
7777
websocket_url?: string;
7878
whitelisted_plugins?: Array<string>;
7979
};
80-
//# sourceMappingURL=constants.d.ts.map
80+
//# sourceMappingURL=constants.d.ts.map

src/headless/utils/init.js

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -348,15 +348,25 @@ async function getLoginCredentialsFromBrowser() {
348348
}
349349
}
350350

351-
async function getLoginCredentialsFromSCRAMKeys() {
351+
async function getLoginCredentialsFromStorage() {
352352
const jid = localStorage.getItem("conversejs-session-jid");
353353
if (!jid) return null;
354354

355355
await setUserJID(jid);
356356

357357
const login_info = await savedLoginInfo(jid);
358+
359+
const fast_credential = login_info.get("fast");
360+
if (fast_credential) {
361+
return { jid, password: fast_credential }
362+
}
363+
358364
const scram_keys = login_info.get("scram_keys");
359-
return scram_keys ? { jid, password: scram_keys } : null;
365+
if (scram_keys) {
366+
return { jid, password: scram_keys }
367+
}
368+
369+
return null
360370
}
361371

362372
/**
@@ -393,8 +403,13 @@ export async function attemptNonPreboundSession(credentials, automatic) {
393403
return connect();
394404
}
395405

396-
if (api.settings.get("reuse_scram_keys")) {
397-
const credentials = await getLoginCredentialsFromSCRAMKeys();
406+
if (api.settings.get("reuse_keys")) {
407+
// XXX if connecting with FAST, we need to present the same user-agent and client-id as before
408+
// This is currently implemented by storing the client ID as part of the fast keys
409+
// and having Strophe generate it if unset.
410+
// but it makes more sense for the client, Converse, to handle that.
411+
// and it should probably coordinate with the OMEMO plugin.
412+
const credentials = await getLoginCredentialsFromStorage();
398413
if (credentials) return connect(credentials);
399414
}
400415

@@ -423,7 +438,7 @@ export async function attemptNonPreboundSession(credentials, automatic) {
423438
* used login keys.
424439
*/
425440
export async function savedLoginInfo(jid) {
426-
const id = `converse.scram-keys-${Strophe.getBareJidFromJid(jid)}`;
441+
const id = `converse.keys-${Strophe.getBareJidFromJid(jid)}`;
427442
if (_converse.state.login_info?.get("id") === id) {
428443
return _converse.state.login_info;
429444
}
@@ -479,20 +494,23 @@ async function connect(credentials) {
479494

480495
let callback;
481496
// Save the SCRAM data if we're not already logged in with SCRAM
482-
if (_converse.state.config.get("trusted") && jid && api.settings.get("reuse_scram_keys") && !password?.ck) {
483-
// Store scram keys in scram storage
497+
if (_converse.state.config.get("trusted") && jid && api.settings.get("reuse_keys") && !password?.ck) {
498+
// Store scram/fast keys in storage
499+
// XXX seems like we could move the *loading* of keys from storage here, since login_info contains them
484500
const login_info = await savedLoginInfo(jid);
485501
callback =
486502
/**
487503
* @param {string} status
488504
* @param {string} message
489505
*/
490506
(status, message) => {
491-
const { scram_keys } = connection;
507+
const { scram_keys, fast } = connection;
492508
if (scram_keys) login_info.save({ scram_keys });
509+
if (fast?.credential?.token) login_info.save('fast', fast.credential);
493510
connection.onConnectStatusChanged(status, message);
494511
};
495512
}
513+
496514
connection.connect(jid, password, callback);
497515
}
498516
}

0 commit comments

Comments
 (0)