From a421a03d2be16670dc8d9076d26f4a7eac669986 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Mon, 28 Sep 2026 21:14:10 +0200 Subject: fix: bound pending admin challenges and sign every value an op acts on Any member could make a node hold unbounded challenge requests; a connection now keeps at most 8, 64 KiB each. root_add, group_attach, invite_create and tmdb_config signed less than they did; their subjects are now canonical JSON of every value (the TMDB token by SHA-256). MNP 5.0, floor kept at 4.0. Co-Authored-By: Claude Opus 5.5 --- .../meshbay-common/src/meshbay_common/__init__.py | 13 +- .../meshbay-common/src/meshbay_common/adminop.py | 43 ++++++ .../meshbay-common/src/meshbay_common/handshake.py | 2 + .../tests/test_admin_subject_parity.py | 145 +++++++++++++++++++++ 4 files changed, 202 insertions(+), 1 deletion(-) create mode 100644 packages/meshbay-common/tests/test_admin_subject_parity.py (limited to 'packages/meshbay-common') diff --git a/packages/meshbay-common/src/meshbay_common/__init__.py b/packages/meshbay-common/src/meshbay_common/__init__.py index 842c52d..fbfdd4d 100644 --- a/packages/meshbay-common/src/meshbay_common/__init__.py +++ b/packages/meshbay-common/src/meshbay_common/__init__.py @@ -249,5 +249,16 @@ __version__ = "0.16.0" # API. A pre-4.0 client presents the session token and is refused at the # handshake — there is no compatibility branch, because leaving one would keep # the disclosure reachable on every node. So the floor moves with it. -MNP_VERSION = "4.0" +# +# 5.0 (2026-09-28) is a MAJOR — four signed operations now sign everything they +# do. `root_add` signed its path and not whether every member may write there; +# `group_attach` signed a group's name and not the directory it exposes; +# `invite_create` did not sign the name it records; `tmdb_config` did not bind +# the token (it now names its SHA-256). Each subject is canonical JSON of every +# value the node acts on (`adminop.structured_subject`). A 4.x client refuses to +# sign the new subjects, and a 5.0 client the old ones — so those four fail, with +# a refusal, across the break. The break is confined to them, so the floor stays +# at 4.0: everything else a 4.x peer does still works, and nothing is left +# unsigned on either side — no node accepts the old subjects. +MNP_VERSION = "5.0" MHP_VERSION = "0.1" diff --git a/packages/meshbay-common/src/meshbay_common/adminop.py b/packages/meshbay-common/src/meshbay_common/adminop.py index c679718..4379519 100644 --- a/packages/meshbay-common/src/meshbay_common/adminop.py +++ b/packages/meshbay-common/src/meshbay_common/adminop.py @@ -30,6 +30,9 @@ fields it received, the node from the state it stored. They are compared by producing the same bytes, never by trusting a value off the wire. """ +import hashlib +import json + ADMIN_TRANSCRIPT_PREFIX = b"meshbay:admin:v1" # Operations that require node-operator authority. @@ -135,6 +138,46 @@ OP_GROUP_DETACH = "group_detach" ADMIN_CHALLENGE_TTL = 120 # seconds +def structured_subject(fields: dict) -> str: + """ + The subject of an operation whose effect is more than one value. + + Every value the executor acts on is in here, because the signature covers the + subject and nothing else of the request: a root's path alone left whether + every member may write there unsigned. Canonical JSON — sorted keys, no + whitespace, UTF-8 — so `null`, `""` and a value stay distinct, and the + browser's `adminSubject` (static/crypto.js) produces the same bytes. + """ + return json.dumps(fields, sort_keys=True, separators=(",", ":"), ensure_ascii=False) + + +def secret_digest(value: str | None) -> str | None: + """A secret named in a subject without being written there: `None` (leave it + unchanged) and `""` (clear it) as themselves, anything else as its SHA-256.""" + if not value: + return value + return "sha256:" + hashlib.sha256(value.encode()).hexdigest() + + +def root_add_subject(path: str, name: str, kind: str, writable: bool, + removable: bool) -> str: + return structured_subject({"path": path, "name": name, "kind": kind, + "writable": writable, "removable": removable}) + + +def group_attach_subject(name: str, shared_dir: str, writable: bool) -> str: + return structured_subject({"name": name, "shared_dir": shared_dir, + "writable": writable}) + + +def invite_create_subject(user_id: str, username: str) -> str: + return structured_subject({"user_id": user_id, "username": username}) + + +def tmdb_config_subject(token: str | None, language: str | None) -> str: + return structured_subject({"token": secret_digest(token), "language": language}) + + def admin_transcript( op: str, node_pk_b64: str, diff --git a/packages/meshbay-common/src/meshbay_common/handshake.py b/packages/meshbay-common/src/meshbay_common/handshake.py index 992fe8a..c3d5b94 100644 --- a/packages/meshbay-common/src/meshbay_common/handshake.py +++ b/packages/meshbay-common/src/meshbay_common/handshake.py @@ -89,6 +89,8 @@ CHALLENGE_PREFIX = b"meshbay:mnp:challenge:v1" # hub session token (MNP_VERSION note). A pre-4.0 peer presents the session # token, which this node now refuses — so the floor moves to 4.0 rather than # leaving a branch that would keep a hub credential reachable by every node. +# 5.0 (2026-09-28) does not move it: the break is confined to four signed +# operations, which a peer across it refuses to sign (MNP_VERSION note). MNP_MIN_SUPPORTED = "4.0" ROLE_CLIENT = "client" diff --git a/packages/meshbay-common/tests/test_admin_subject_parity.py b/packages/meshbay-common/tests/test_admin_subject_parity.py new file mode 100644 index 0000000..7a229a9 --- /dev/null +++ b/packages/meshbay-common/tests/test_admin_subject_parity.py @@ -0,0 +1,145 @@ +""" +The subjects of multi-value admin operations are byte-identical in the browser and +in Python. + +The subject is what the operator's signature covers of a request, and each side +builds it on its own — the node from the request it stored, the client from what +the person asked for. A one-byte disagreement does not weaken anything (the client +refuses to sign), but it makes the operation impossible from a browser, and nothing +else in the suite crosses this boundary. + +Skipped when node is unavailable; that is a coverage gap, not a pass. +""" + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest +from meshbay_common.adminop import ( + group_attach_subject, + invite_create_subject, + root_add_subject, + secret_digest, + structured_subject, + tmdb_config_subject, +) + +CRYPTO_JS = (Path(__file__).resolve().parents[2] + / "meshbay-hub" / "src" / "meshbay_hub" / "static" / "crypto.js") + +pytestmark = pytest.mark.skipif( + shutil.which("node") is None or not CRYPTO_JS.exists(), + reason="node or crypto.js unavailable — parity cannot be checked", +) + +ROOT_ADD = [ + ("/srv/Films", "", "generic", False, False), + ("/srv/Films", "Films", "video", True, True), + ("C:\\Users\\me\\Share", "Partagé", "photo", True, False), + ('/srv/a "quoted", odd:name|x', "名前", "audio", False, True), + ("/srv/tab\there\nnewline\x01ctl", "é", "generic", True, False), +] +GROUP_ATTACH = [ + ("photos", "/srv/photos", True), + ("famille-été", "/mnt/disque externe/Photos", False), +] +INVITE_CREATE = [ + ("0f8fad5b-d9cb-469f-a165-70867728950e", ""), + ("0f8fad5b-d9cb-469f-a165-70867728950e", "Élodie \"E\" 🙂"), +] +TMDB_CONFIG = [ + (None, None), ("", None), (None, ""), ("", ""), + ("eyJhbGciOiJIUzI1NiJ9.token", "fr-FR"), + ("abc", "keep"), +] + +_HARNESS = r""" +const fs = require('fs'); +globalThis.window = {}; +const src = fs.readFileSync(process.argv[2], 'utf8'); +const M = new Function(src + '\nreturn { rootAddSubject, groupAttachSubject, ' + + 'inviteCreateSubject, tmdbConfigSubject };')(); +const v = JSON.parse(fs.readFileSync(process.argv[3], 'utf8')); +(async () => { + const out = { + root_add: v.root_add.map((a) => M.rootAddSubject(...a)), + group_attach: v.group_attach.map((a) => M.groupAttachSubject(...a)), + invite_create: v.invite_create.map((a) => M.inviteCreateSubject(...a)), + tmdb_config: [], + }; + for (const a of v.tmdb_config) out.tmdb_config.push(await M.tmdbConfigSubject(...a)); + process.stdout.write(JSON.stringify(out)); +})(); +""" + + +@pytest.fixture(scope="module") +def js(tmp_path_factory): + d = tmp_path_factory.mktemp("subject-parity") + (d / "harness.js").write_text(_HARNESS, encoding="utf-8") + (d / "vectors.json").write_text(json.dumps({ + "root_add": ROOT_ADD, "group_attach": GROUP_ATTACH, + "invite_create": INVITE_CREATE, "tmdb_config": TMDB_CONFIG, + }), encoding="utf-8") + proc = subprocess.run( + ["node", str(d / "harness.js"), str(CRYPTO_JS), str(d / "vectors.json")], + capture_output=True, text=True, encoding="utf-8", timeout=60) + if proc.returncode != 0: + pytest.fail(f"node harness failed:\n{proc.stderr}") + return json.loads(proc.stdout) + + +def _bytes(s: str) -> bytes: + return s.encode("utf-8") + + +@pytest.mark.parametrize("i,args", list(enumerate(ROOT_ADD))) +def test_root_add_subject_parity(i, args, js): + assert _bytes(js["root_add"][i]) == _bytes(root_add_subject(*args)) + + +@pytest.mark.parametrize("i,args", list(enumerate(GROUP_ATTACH))) +def test_group_attach_subject_parity(i, args, js): + assert _bytes(js["group_attach"][i]) == _bytes(group_attach_subject(*args)) + + +@pytest.mark.parametrize("i,args", list(enumerate(INVITE_CREATE))) +def test_invite_create_subject_parity(i, args, js): + assert _bytes(js["invite_create"][i]) == _bytes(invite_create_subject(*args)) + + +@pytest.mark.parametrize("i,args", list(enumerate(TMDB_CONFIG))) +def test_tmdb_config_subject_parity(i, args, js): + assert _bytes(js["tmdb_config"][i]) == _bytes(tmdb_config_subject(*args)) + + +def test_every_value_changes_the_subject(): + base = ("/srv/Films", "Films", "video", False, False) + variants = {root_add_subject(*base)} + for i, other in enumerate(("/srv/Other", "Other", "audio", True, True)): + args = list(base) + args[i] = other + variants.add(root_add_subject(*args)) + assert len(variants) == 6 + + +def test_unchanged_cleared_and_set_are_three_subjects(): + assert len({tmdb_config_subject(None, None), tmdb_config_subject("", None), + tmdb_config_subject("t", None)}) == 3 + assert len({tmdb_config_subject(None, None), tmdb_config_subject(None, ""), + tmdb_config_subject(None, "fr-FR")}) == 3 + + +def test_the_token_is_never_written_into_the_subject(): + token = "eyJhbGciOiJIUzI1NiJ9.a-real-looking-secret" + assert token not in tmdb_config_subject(token, "fr-FR") + assert secret_digest(token).startswith("sha256:") + + +def test_a_crafted_field_cannot_impersonate_another(): + # Under a naive "path|name" join these two would collide. + a = structured_subject({"path": "/a|name=b", "name": ""}) + b = structured_subject({"path": "/a", "name": "b"}) + assert a != b -- cgit v1.2.3