From 9da3cfc5d31bc4e1b9c4ea70f56e07b9aed8bb85 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sun, 13 Sep 2026 16:29:36 +0200 Subject: fix(node): the node's own controls take no authority from a hub token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_is_node_admin()` is `self._user_id == node_user_id`, and `_user_id` is the `sub` of a JWT the hub issued. Six node-wide controls were gated on that alone: `node_status` — which lists every group on the machine with each root's **absolute path** — plus `node_settings_set`, `roster_read`, `denylist_read`, `denylist_clear` and `node_reload`. So the answer to "are you the operator of this node" was "the hub says so", which NS4 and M3 rule out in as many words: operator authority comes from the node's roster and from nowhere else, and asking the hub is how the hub installs itself as node administrator. The reach is bounded — a completed handshake also needs the group key — but an active hub obtains one legitimately in an open-join group, which §3.5 concedes, and from there it could read the operator's directory layout or clear the denylist, which is the persisted revocation H4 exists to keep. `_operator_device()` requires both halves now: the account is the one the node belongs to, *and* the device on this connection has proved a key the roster holds as an operator. `device_hello` is signed over a transcript naming the node, the group and this connection's nonce, and `operator_pks()` is rebuilt from the roster on each call, so an unpinned browser and a revoked one are both refused at once. The hub holds no user keys and cannot countersign a device. Keeping the account check as well is deliberate: dropping it would widen these node-wide controls to any paired operator of any group on the machine, which is a separate decision. `_is_node_admin()` stays as what it is in the handshake ack — a hint telling a client whether to offer the Node page — and says so. Nothing changes for a paired operator: `device_hello` runs unconditionally after the ack, and anyone using the Node page's controls is already paired, since `root_add` and every other signed op has always verified against `operator_pks()`. A browser that never paired now reads nothing there, which is the state in which it could already write nothing. test_node_status.py's fixture set the account and not the device, which is how it went on passing; it now wires the device the way `device_hello` leaves it. The adversary itself is in test_security_regressions.py — a token naming the owner's account with no proved device, which the previous source answered with `node_status_ack`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UMxEQadpzPkYLFf5CYKhpW --- docs/MESHBAY_DESIGN.md | 13 +++ .../src/meshbay_node/transport/webrtc_server.py | 90 +++++++++++---- packages/meshbay-node/tests/test_node_status.py | 7 ++ .../tests/test_security_regressions.py | 121 +++++++++++++++++++++ 4 files changed, 212 insertions(+), 19 deletions(-) diff --git a/docs/MESHBAY_DESIGN.md b/docs/MESHBAY_DESIGN.md index 9f045c4..d1630f0 100644 --- a/docs/MESHBAY_DESIGN.md +++ b/docs/MESHBAY_DESIGN.md @@ -1468,6 +1468,19 @@ issues invitations and reads the audit log. There is no server-rendered dashboar the desktop client's Node page and the CLI are the two consumers, and each operation endpoint is one `_op(...)` line onto `ops.py` (§5.4). +**Over MNP, the node's own controls need a proved operator device.** The +node-wide surface — `node_status`, which lists every group on the machine with +each root's absolute path, plus `node_settings_set`, `roster_read`, +`denylist_read`, `denylist_clear` and `node_reload` — is reachable when two +things hold: the account is the one the node belongs to, *and* the device on the +connection has proved (`device_hello`, §3.3) a key the roster holds as an +operator. The first alone is a claim in a token the hub issued, and **NS4** does +not allow it to be authority: a hub that can name the operator is a hub that can +be one. The second is what it cannot forge, since it holds no user keys and +cannot countersign a device — the same property device linking rests on. A +browser that has never been paired therefore reads nothing here, exactly as it +can already sign nothing (§5.4). + **The accepted cost, recorded as a choice:** on a headless server the only admin path is the CLI. The CLI covers every operation, so this is acceptable — but it is a real capability reduction, not an oversight. diff --git a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py index d36b812..59198d5 100644 --- a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py +++ b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py @@ -2656,12 +2656,18 @@ class WebRTCPeerSession: # ── Node management (D5) ───────────────────────────────────────────────── async def _do_node_status(self, msg: dict) -> None: - """All groups, roots, peers — the operator's overview.""" + """All groups, roots, peers — the operator's overview. + + Including every root's absolute path, which is why this is gated on a + proved operator device and not on an account the hub named. + """ node_uid = self._ctx.get("node_user_id") - log.info("node_status: user=%s node_user=%s admin=%s", - self._user_id, node_uid, self._is_node_admin()) - if not self._is_node_admin(): - self._send({"type": "error", "detail": "Not the node operator"}) + log.info("node_status: user=%s node_user=%s owner=%s device=%s", + self._user_id, node_uid, self._is_node_admin(), + "confirmed" if self._device_confirmed else "unidentified") + if not await self._operator_device(): + self._send({"type": "error", "detail": "Not the node operator", + "code": "not_operator"}) return try: result = await self._run_op(ops.list_groups) @@ -2673,8 +2679,9 @@ class WebRTCPeerSession: self._send({"type": "error", "detail": "Internal error"}) async def _do_node_settings_set(self, msg: dict) -> None: - if not self._is_node_admin(): - self._send({"type": "error", "detail": "Not the node operator"}) + if not await self._operator_device(): + self._send({"type": "error", "detail": "Not the node operator", + "code": "not_operator"}) return settings = msg.get("settings", {}) if not settings: @@ -2691,8 +2698,9 @@ class WebRTCPeerSession: self._send({"type": "error", "detail": "Internal error"}) async def _do_roster_read(self, msg: dict) -> None: - if not self._is_node_admin(): - self._send({"type": "error", "detail": "Not the node operator"}) + if not await self._operator_device(): + self._send({"type": "error", "detail": "Not the node operator", + "code": "not_operator"}) return group_id = str(msg.get("group_id", "")).strip() try: @@ -2705,8 +2713,9 @@ class WebRTCPeerSession: self._send({"type": "error", "detail": "Internal error"}) async def _do_denylist_read(self, msg: dict) -> None: - if not self._is_node_admin(): - self._send({"type": "error", "detail": "Not the node operator"}) + if not await self._operator_device(): + self._send({"type": "error", "detail": "Not the node operator", + "code": "not_operator"}) return try: result = await self._run_op(ops.read_denylist) @@ -2718,8 +2727,9 @@ class WebRTCPeerSession: self._send({"type": "error", "detail": "Internal error"}) async def _do_denylist_clear(self, msg: dict) -> None: - if not self._is_node_admin(): - self._send({"type": "error", "detail": "Not the node operator"}) + if not await self._operator_device(): + self._send({"type": "error", "detail": "Not the node operator", + "code": "not_operator"}) return subject = str(msg.get("subject", "")).strip() try: @@ -2815,8 +2825,9 @@ class WebRTCPeerSession: log.error("Reload after group_detach failed: %s", e) async def _do_node_reload(self, msg: dict) -> None: - if not self._is_node_admin(): - self._send({"type": "error", "detail": "Not the node operator"}) + if not await self._operator_device(): + self._send({"type": "error", "detail": "Not the node operator", + "code": "not_operator"}) return state = self._ctx.get("daemon_state") reload_fn = state.get("reload_fn") if state else None @@ -5173,15 +5184,56 @@ class WebRTCPeerSession: def _is_node_admin(self) -> bool: """ - Whether the peer on this connection is the node's operator. + Whether the **account** on this connection is the one the node belongs to. - Was written out twice — once in the handshake ack and once at the gate - below it — which is how the two come to disagree. From the node's own - record of who it belongs to, never from a hub claim. + This is a display hint and half of a check — never authority on its own. + `self._user_id` is the `sub` of a JWT the hub issued, so read alone it + says "the hub says you are the owner", which is the one thing NS4 and + M3 rule out: a hub that can name the operator can install itself as + node administrator. It rides the handshake ack so a client knows whether + to offer the Node page at all, and every operation is gated on + `_operator_device()` below. """ node_user_id = self._ctx.get("node_user_id") return bool(node_user_id and self._user_id == node_user_id) + async def _operator_device(self) -> bool: + """ + Whether this connection may run the node's own controls. + + Two things, and the second is the one that cannot be forged: + + - the account is the one this node belongs to (`_is_node_admin`), which + is what keeps node-wide controls with the machine's owner rather than + with every paired operator of every group on it; and + - **the device on this connection proved a key the node pinned as an + operator**. `device_hello` is signed over a transcript naming this + node, this group and this connection's nonce, and `operator_pks()` is + rebuilt from the roster on each call, so an unpinned browser and a + revoked one are both refused at once. + + The second clause is the fix for the door this used to leave open. + `node_status`, `node_settings_set`, `roster_read`, `denylist_read`, + `denylist_clear` and `node_reload` were gated on the account id alone — + a value the hub chooses. An active hub that can also reach the group key + (which §3.5 concedes it can in an open-join group) could therefore mint + a token for the owner's account and read `node_status`, which lists + every group on the node with the operator's **absolute paths**, or clear + the denylist, which is the persisted revocation H4 exists to keep. + + It holds no user keys and cannot countersign anything, so it cannot + produce a `device_hello` — which is the same property device linking + rests on (§3.3), applied to the node's own surface. + """ + if not self._is_node_admin(): + return False + if not self._device_confirmed or not self._pinned_pk: + return False + roster = self._ctx.get("roster") + if roster is None: + return False + return self._pinned_pk in await roster.operator_pks() + def _has_admin_authority(self) -> bool: """ Cheap synchronous pre-check: is there anyone who could authorize this? diff --git a/packages/meshbay-node/tests/test_node_status.py b/packages/meshbay-node/tests/test_node_status.py index 091b1db..897be33 100644 --- a/packages/meshbay-node/tests/test_node_status.py +++ b/packages/meshbay-node/tests/test_node_status.py @@ -110,6 +110,13 @@ async def _session( session._group_id = GROUP session._user_id = "grenet" if operator else "mallory" session._username = session._user_id + # The device on the connection, as `device_hello` leaves it. Node-wide + # controls are gated on a key the node pinned as an operator and *proved* + # on this connection, not on the account id in the token — a hub can choose + # the second and cannot produce the first (NS4, M3). A fixture that sets + # only the account tests a check that is no longer the check. + session._pinned_pk = pk_op if operator else "" + session._device_confirmed = bool(operator) session._pk_user = "" session._uploads = {} session._admin_ops = {} diff --git a/packages/meshbay-node/tests/test_security_regressions.py b/packages/meshbay-node/tests/test_security_regressions.py index 988aa46..fa55ff6 100644 --- a/packages/meshbay-node/tests/test_security_regressions.py +++ b/packages/meshbay-node/tests/test_security_regressions.py @@ -837,3 +837,124 @@ def test_node_control_api_serves_no_html(): for gone in ("_render_page", "_render_audit_page", "_render_roster", "_AUDIT_HTML"): assert not hasattr(ui_app, gone), f"{gone} came back — HTML surface" + + +# ── NS4 / M3: the node's own controls take no authority from the hub ───────── + +async def _owner_session(tmp_path, roster, *, device: str = "", + confirmed: bool = False): + """A session whose token says it is the account this node belongs to. + + Which is all a hub can decide: `_user_id` is the `sub` of a JWT it issued, + so this is what an active hub forging a token arrives holding. Whether the + *device* on the connection is one the node pinned as an operator is the + other half, and no token can assert it. + """ + from meshbay_node.indexer.group_index import GroupIndex + + shared = tmp_path / "shared" + shared.mkdir(exist_ok=True) + index = GroupIndex(group_id="g" * 32, sk_node=Ed25519PrivateKey.generate()) + session = WebRTCPeerSession.__new__(WebRTCPeerSession) + session._ctx = { + "roots": one_root(shared), "index": index, "sk_node": index.sk_node, + "gek": generate_gek(), "roster": roster, + "node_user_id": "the-owner", + "daemon_state": {"groups_ctx": {}, "reload_fn": None}, + } + session._group_id = "g" * 32 + session._user_id = "the-owner" + session._username = "the-owner" + session._pinned_pk = device + session._device_confirmed = confirmed + session._pk_user = "" + session.sent = [] + session._send = session.sent.append + session._audit = lambda *a, **k: None + return session + + +@pytest.mark.asyncio +async def test_a_token_naming_the_owner_is_not_node_authority(tmp_path): + """ + The hub holds no user keys, so it cannot countersign a device — but it does + choose what a token says. Six node-wide controls used to be gated on the + account id alone, which is the hub's to decide: `node_status` (every group + on the machine, with the operator's absolute paths), `node_settings_set`, + `roster_read`, `denylist_read`, `denylist_clear` (the persisted revocation + H4 exists to keep) and `node_reload`. + + An active hub reaches a completed handshake wherever it can also obtain the + group key, which §3.5 concedes it can in an open-join group. From there, + "the hub says you are the owner" was the whole of the check. + """ + from meshbay_node.roster import Roster + + roster = Roster(db_path=tmp_path / "roster.db") + await roster.open() + try: + forged = await _owner_session(tmp_path, roster) + await forged._do_node_status({}) + assert forged.sent[-1]["type"] == "error" + assert forged.sent[-1]["code"] == "not_operator" + + forged.sent.clear() + await forged._do_denylist_read({}) + assert forged.sent[-1]["type"] == "error" + + forged.sent.clear() + await forged._do_denylist_clear({"subject": ""}) + assert forged.sent[-1]["type"] == "error", ( + "clearing the denylist undoes a revocation every node enforces") + finally: + await roster.close() + + +@pytest.mark.asyncio +async def test_an_identified_device_that_is_not_an_operator_is_refused(tmp_path): + """Being a pinned member of the group is not being the node's operator. + + The device proof is real here; what it proves is an ordinary member's key, + and `operator_pks()` is rebuilt from the roster on every call so a revoked + one stops working at once. + """ + from meshbay_node.roster import Roster + from meshbay_common.crypto import pk_to_b64 + + roster = Roster(db_path=tmp_path / "roster.db") + await roster.open() + try: + sk = Ed25519PrivateKey.generate() + member_pk = pk_to_b64(sk.public_key()) + await roster.pin_identity("the-owner", "the-owner", member_pk, + member_pk, "code") + session = await _owner_session(tmp_path, roster, device=member_pk, + confirmed=True) + await session._do_node_status({}) + assert session.sent[-1]["type"] == "error" + finally: + await roster.close() + + +@pytest.mark.asyncio +async def test_a_paired_operator_device_is_what_opens_it(tmp_path): + """The positive case, so the test above is about authority and not about + everything being refused.""" + from meshbay_node.roster import Roster + from meshbay_common.join import ROLE_OPERATOR + from meshbay_common.crypto import pk_to_b64 + + roster = Roster(db_path=tmp_path / "roster.db") + await roster.open() + try: + sk = Ed25519PrivateKey.generate() + pk = pk_to_b64(sk.public_key()) + await roster.pin_identity("the-owner", "the-owner", pk, pk, "code") + await roster.set_member("", "the-owner", ROLE_OPERATOR, "active", + "local-cli") + session = await _owner_session(tmp_path, roster, device=pk, + confirmed=True) + await session._do_denylist_read({}) + assert session.sent[-1]["type"] != "error", session.sent[-1] + finally: + await roster.close() -- cgit v1.2.3