diff options
4 files changed, 194 insertions, 20 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transport-devices.js b/packages/meshbay-hub/src/meshbay_hub/static/transport-devices.js index f079f80..3d56f69 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transport-devices.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transport-devices.js @@ -68,28 +68,39 @@ extendTransport(class { return 'unknown'; } const known = await _readPinnedAccount(this.nodePk, userId); - const entry = roster.byAccount.get(userId); - if (known && known.includes(devicePk)) return 'pinned'; - if (!known) { - // First sight, so **everything the node says** is pinned — not only what - // a chain reaches. There is nothing to compare against yet: that is what - // trust-on-first-use means, and pinning only the verified subset would - // raise "key changed" on a legitimate second device whose - // countersignature simply predates it being kept. What TOFU buys is that - // a substitution *later* is visible; it cannot buy anything now. - if (entry) await _writePinnedAccount(this.nodePk, userId, entry.all); - return entry && entry.all.includes(devicePk) ? 'first' : 'changed'; + let verdict = _judgeDevice(known, roster.byAccount.get(userId), devicePk); + // The roster is this connection's copy, read once. Somebody who joined the + // group, or added a device, since it was read is absent from it, and their + // first message raised "key changed" — found live, twice in one + // conversation, on a member invited after the page was opened. So the + // roster is read again before that is said, once per account and device + // on this connection: a node substituting keys gets one more read for each + // key it makes up, and the same answer. + if (verdict.status === 'changed') { + try { + roster = await this._rosterRecheckedFor(userId, devicePk); + verdict = _judgeDevice(known, roster.byAccount.get(userId), devicePk); + } catch { /* the verdict on the copy we had stands */ } } - if (entry && entry.verified.includes(devicePk) - && entry.chain.get(devicePk) - && known.includes(entry.chain.get(devicePk))) { - // Countersigned by a key we already trust for this account: a second - // device of someone we know, admitted without anybody comparing digits. - await _writePinnedAccount(this.nodePk, userId, - [...new Set([...known, devicePk])]); - return 'linked'; + if (verdict.pin) await _writePinnedAccount(this.nodePk, userId, verdict.pin); + return verdict.status; + } + + /** + * The roster as re-read for this key, read once per connection. Held as the + * promise, not as "done": live messages are opened concurrently, and two from + * a device that just joined must both wait for the one read, not have the + * second answer "changed" while the first is still asking. + */ + _rosterRecheckedFor(userId, devicePk) { + if (!this._rosterRechecks) this._rosterRechecks = new Map(); + const key = `${userId} ${devicePk}`; + let read = this._rosterRechecks.get(key); + if (!read) { + read = this.groupRoster({ fresh: true }); + this._rosterRechecks.set(key, read); } - return 'changed'; + return read; } /** diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transport-roster.js b/packages/meshbay-hub/src/meshbay_hub/static/transport-roster.js index cac3b52..d2e8c94 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transport-roster.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transport-roster.js @@ -103,6 +103,36 @@ async function _writePinnedAccount(nodePk, userId, keys) { } /** + * How `devicePk` stands against what is pinned for its account (`known`, null + * at first sight) and what the roster lists for it (`entry`): `{ status, pin }`, + * `pin` being the keys to keep if that answer stands. Decides and writes + * nothing, so a verdict reached on a stale roster can be taken again on a fresh + * one without having pinned half a set first. The statuses are + * `accountDeviceStatus`'s. + */ +function _judgeDevice(known, entry, devicePk) { + if (known && known.includes(devicePk)) return { status: 'pinned', pin: null }; + if (!known) { + // First sight, so **everything the node says** is pinned — not only what + // a chain reaches. There is nothing to compare against yet: that is what + // trust-on-first-use means, and pinning only the verified subset would + // raise "key changed" on a legitimate second device whose + // countersignature simply predates it being kept. What TOFU buys is that + // a substitution *later* is visible; it cannot buy anything now. + const pin = entry ? entry.all : null; + return { status: entry && entry.all.includes(devicePk) ? 'first' : 'changed', pin }; + } + if (entry && entry.verified.includes(devicePk) + && entry.chain.get(devicePk) + && known.includes(entry.chain.get(devicePk))) { + // Countersigned by a key we already trust for this account: a second + // device of someone we know, admitted without anybody comparing digits. + return { status: 'linked', pin: [...new Set([...known, devicePk])] }; + } + return { status: 'changed', pin: null }; +} + +/** * A wire payload as text. * * A plaintext message arrives as a string from the node; msgpack `bin` arrives diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transport.js b/packages/meshbay-hub/src/meshbay_hub/static/transport.js index 9dcea4f..fa5d28b 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transport.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transport.js @@ -1247,6 +1247,7 @@ class MeshBayTransport { this._chatKeysInFlight = null; this._roster = null; this._rosterInFlight = null; + this._rosterRechecks = null; // Tell the node which of this account's devices is on this connection, // after the ack and unconditionally: a peer `check_version` admitted // speaks this message, and identifying the device is what makes chat diff --git a/packages/meshbay-hub/tests/test_key_change_recheck.py b/packages/meshbay-hub/tests/test_key_change_recheck.py new file mode 100644 index 0000000..52b9b69 --- /dev/null +++ b/packages/meshbay-hub/tests/test_key_change_recheck.py @@ -0,0 +1,132 @@ +""" +"Key changed" is said on a fresh roster, never on a stale one. + +The roster is read once per connection. A member invited after it was read was +absent from it, and each of their messages raised "This account is using a key +you have not seen before" until the page was reloaded: found live, twice in one +conversation. These tests load the real `transport-roster.js` and +`transport-devices.js` into node, with the roster the node would give before and +after the member joined, and check what the chat would show. + +What must not move: a key the fresh roster cannot evidence is still "changed", +and a node that makes keys up gets one extra read per key, not one per message. +""" + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" + +pytestmark = pytest.mark.skipif(shutil.which("node") is None, reason="node is not available") + +HARNESS = """ +import vm from 'node:vm'; +import fs from 'node:fs'; + +const store = new Map(); +const ctx = { + console, + localStorage: { + getItem: (k) => (store.has(k) ? store.get(k) : null), + setItem: (k, v) => store.set(k, String(v)), + }, + window: {}, + extendTransport(part) { ctx.Part = part; }, +}; +vm.createContext(ctx); +for (const f of %(files)s) vm.runInContext(fs.readFileSync(f, 'utf8'), ctx); + +const entry = (all, verified = all, chain = []) => + ({ all, verified, chain: new Map(chain) }); +const roster = (accounts) => ({ byAccount: new Map(Object.entries(accounts)) }); + +function transport(stale, fresh) { + const t = Object.create(ctx.Part.prototype); + t.nodePk = 'NODE'; + t.freshReads = 0; + t.groupRoster = async ({ fresh: wantFresh = false } = {}) => { + if (!wantFresh) return stale; + t.freshReads++; + await new Promise((r) => setTimeout(r, 5)); + if (fresh instanceof Error) throw fresh; + return fresh; + }; + return t; +} +const pins = (user) => JSON.parse(store.get(`meshbay_account_pins:NODE:${user}`) || 'null'); +""" + + +def _run(tmp_path, scenario): + files = [str(STATIC / "transport-roster.js"), str(STATIC / "transport-devices.js")] + script = tmp_path / "recheck.mjs" + script.write_text(HARNESS % {"files": json.dumps(files)} + scenario, encoding="utf-8") + proc = subprocess.run(["node", str(script)], capture_output=True, text=True, + encoding="utf-8") + assert proc.returncode == 0, proc.stderr + return json.loads(proc.stdout) + + +def test_a_member_who_joined_after_the_roster_was_read_is_a_first_sight(tmp_path): + out = _run(tmp_path, """ +const t = transport(roster({}), roster({ bob: entry(['B']) })); +const first = await t.accountDeviceStatus('bob', 'B'); +const second = await t.accountDeviceStatus('bob', 'B'); +console.log(JSON.stringify({ first, second, reads: t.freshReads, pins: pins('bob') })); +""") + assert out == {"first": "first", "second": "pinned", "reads": 1, "pins": ["B"]} + + +def test_two_messages_at_once_share_the_one_read(tmp_path): + out = _run(tmp_path, """ +const t = transport(roster({}), roster({ bob: entry(['B']) })); +const both = await Promise.all([t.accountDeviceStatus('bob', 'B'), + t.accountDeviceStatus('bob', 'B')]); +console.log(JSON.stringify({ both, reads: t.freshReads })); +""") + assert "changed" not in out["both"], out + assert out["reads"] == 1 + + +def test_a_device_added_after_the_roster_was_read_is_linked(tmp_path): + out = _run(tmp_path, """ +store.set('meshbay_account_pins:NODE:bob', JSON.stringify(['A'])); +const t = transport(roster({ bob: entry(['A']) }), + roster({ bob: entry(['A', 'B'], ['A', 'B'], [['B', 'A']]) })); +const status = await t.accountDeviceStatus('bob', 'B'); +console.log(JSON.stringify({ status, pins: pins('bob') })); +""") + assert out == {"status": "linked", "pins": ["A", "B"]} + + +def test_a_key_the_fresh_roster_cannot_evidence_is_still_changed(tmp_path): + out = _run(tmp_path, """ +store.set('meshbay_account_pins:NODE:bob', JSON.stringify(['A'])); +const t = transport(roster({ bob: entry(['A']) }), roster({ bob: entry(['A', 'X'], ['A']) })); +const first = await t.accountDeviceStatus('bob', 'X'); +const second = await t.accountDeviceStatus('bob', 'X'); +console.log(JSON.stringify({ first, second, reads: t.freshReads, pins: pins('bob') })); +""") + assert out == {"first": "changed", "second": "changed", "reads": 1, "pins": ["A"]} + + +def test_a_failed_re_read_keeps_the_warning(tmp_path): + out = _run(tmp_path, """ +const t = transport(roster({}), new Error('node gone')); +const status = await t.accountDeviceStatus('bob', 'B'); +console.log(JSON.stringify({ status })); +""") + assert out == {"status": "changed"} + + +def test_a_roster_that_already_knows_the_key_is_not_read_again(tmp_path): + out = _run(tmp_path, """ +const t = transport(roster({ bob: entry(['B']) }), roster({})); +const status = await t.accountDeviceStatus('bob', 'B'); +console.log(JSON.stringify({ status, reads: t.freshReads })); +""") + assert out == {"status": "first", "reads": 0} |