From 903ea022e918a05c7c8cb43d95e46d82368566f1 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sat, 12 Sep 2026 12:14:40 +0200 Subject: fix(hub): MHP binds its audience, and the hub knows its own name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Federation has never worked between two hubs, and the tests said so without anyone reading it that way. `federation.py` did `from meshbay_hub.auth import _hub_id, _hub_sk_pem` at import — which is before `load_hub_keypair` runs. So it held the key as `None` and the identity as the module default: `_issue_mhp_token` could only raise, and `/mhp/info`, the directory export and every token announced this instance as `meshbay.org` whatever it was configured as. Read through accessors now, at call time. And `_verify_mhp_token` named no audience while `_issue_mhp_token` sets one. PyJWT refuses a token carrying `aud` when decode is given none, so every token this hub issues was rejected by every hub running this code. Naming the audience fixes that and makes the binding real: a token minted for one peer is refused by another, which is what stops a captured request being replayed at a third hub. The comment claiming audience binding was unavailable because "the sending side is unbuilt" was describing a function four lines below it. Both were already written down. `test_federation.py` built envelopes by hand without an `aud`; `test_public_groups_toggle.py` signed its own token with a comment saying `_issue_mhp_token` "binds `_hub_sk_pem` at import time, before the lifespan loads it, so it cannot be used from a test", and another saying PyJWT rejects a token carrying `aud` when decode is given none. Both observations were exactly right, and both were treated as facts to route around. When a test has to work around the code to run, the thing it worked around is the finding. Those helpers now go through the real issuer, and two tests pin the identity and the audience refusal. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01T4YmK41VsEURWFdop4EEeT --- .../meshbay-hub/src/meshbay_hub/api/federation.py | 48 +++++++++++----- packages/meshbay-hub/src/meshbay_hub/auth.py | 20 +++++++ packages/meshbay-hub/tests/test_federation.py | 64 +++++++++++++++++++++- .../meshbay-hub/tests/test_public_groups_toggle.py | 29 ++++------ 4 files changed, 128 insertions(+), 33 deletions(-) (limited to 'packages') diff --git a/packages/meshbay-hub/src/meshbay_hub/api/federation.py b/packages/meshbay-hub/src/meshbay_hub/api/federation.py index b1e0e30..327102e 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/federation.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/federation.py @@ -33,7 +33,8 @@ from sqlalchemy.ext.asyncio import AsyncSession from meshbay_common import MHP_VERSION from meshbay_hub import __version__, hub_settings from meshbay_hub.api.deps import require_admin -from meshbay_hub.auth import _hub_id, _hub_sk_pem, hub_public_key_pem +from meshbay_hub.auth import ( + hub_id, hub_private_key_pem, hub_public_key_pem) from meshbay_hub.db.engine import get_db from meshbay_hub.db.models import FederatedGroup, Group, HubPeer, User @@ -45,26 +46,36 @@ router = APIRouter(prefix="/mhp", tags=["federation"]) MAX_FEDERATED_GROUPS_PER_PUSH = 500 MAX_FEDERATED_GROUPS_PER_PEER = 2000 -# Seen `jti` values for the state-changing MHP endpoints, pruned lazily. The -# sending side that would set an `aud` claim is unbuilt, so audience binding is -# not available; this stops a captured POST /mhp/directory or /mhp/revoke from -# being replayed inside the token's short TTL. GET /mhp/directory is idempotent -# and not covered. +# Seen `jti` values for the state-changing MHP endpoints, pruned lazily. This +# stops a captured POST /mhp/directory or /mhp/revoke from being replayed at +# *this* hub inside the token's short TTL; the `aud` check in +# `_verify_mhp_token` is what stops it being replayed at a different one. GET +# /mhp/directory is idempotent and not covered. +# +# The comment here used to say the sending side sets no `aud` claim, so +# audience binding was unavailable. `_issue_mhp_token` four lines below has +# always set one. _seen_mhp_jti: dict[str, float] = {} def _issue_mhp_token(target_hub_id: str) -> str: - """Issue a short-lived JWT for authenticating to a peer hub.""" + """Issue a short-lived JWT for authenticating to a peer hub. + + Read through the accessors, not from names bound at import: this module + held `_hub_sk_pem` as it was *before* startup, which is `None`, so this + function could only ever have raised — and `_hub_id` as `meshbay.org`, + whatever the instance was actually called. + """ now = int(time.time()) return jwt.encode({ - "iss": _hub_id, - "sub": _hub_id, + "iss": hub_id(), + "sub": hub_id(), "aud": target_hub_id, "jti": str(uuid.uuid4()), "iat": now, "exp": now + 300, "mhp": MHP_VERSION, - }, _hub_sk_pem, algorithm="EdDSA") + }, hub_private_key_pem(), algorithm="EdDSA") async def _verify_mhp_token( @@ -84,10 +95,19 @@ async def _verify_mhp_token( if not peer: raise PermissionError(f"Unknown hub: {sender_id!r}. Register as peer first.") + # `audience` is what makes the `aud` claim mean anything — and omitting it + # did not merely leave the binding unenforced. PyJWT refuses a token that + # carries `aud` when the caller names no audience, so **every token this + # hub issues was rejected by every hub running this code**: MHP could not + # complete a single authenticated request between two peers. Nothing + # noticed because `test_federation.py` builds its envelopes by hand, + # without an `aud`, so the suite exercised a token shape production never + # emits — the fixture was not the thing. decoded = jwt.decode( token, peer.pk_hub_pem.encode(), algorithms=["EdDSA"], - options={"require": ["exp", "iss"]}, + audience=hub_id(), + options={"require": ["exp", "iss", "aud"]}, ) if single_use: @@ -109,7 +129,7 @@ async def _verify_mhp_token( async def mhp_info(): """Return this hub's identity for peer registration.""" return { - "hub_id": _hub_id, + "hub_id": hub_id(), "mhp_version": MHP_VERSION, "hub_version": __version__, "pk_hub_pem": hub_public_key_pem().decode(), @@ -139,7 +159,7 @@ async def export_directory( groups = result.scalars().all() return { - "hub_id": _hub_id, + "hub_id": hub_id(), "mhp_version": MHP_VERSION, "groups": [ { @@ -147,7 +167,7 @@ async def export_directory( "name": g.name, "join_policy": g.join_policy, "created_at": g.created_at.isoformat(), - "hub_id": _hub_id, + "hub_id": hub_id(), } for g in groups ], diff --git a/packages/meshbay-hub/src/meshbay_hub/auth.py b/packages/meshbay-hub/src/meshbay_hub/auth.py index 34baf45..c5ea34d 100644 --- a/packages/meshbay-hub/src/meshbay_hub/auth.py +++ b/packages/meshbay-hub/src/meshbay_hub/auth.py @@ -88,6 +88,26 @@ def hub_public_key_pem() -> bytes: return _hub_pk_pem +def hub_private_key_pem() -> bytes: + if _hub_sk_pem is None: + raise RuntimeError("Hub keypair not loaded — call load_hub_keypair() first") + return _hub_sk_pem + + +def hub_id() -> str: + """This hub's configured identity. + + An accessor, not the module global, because `load_hub_keypair` runs at + startup and every one of these is set *after* import. A module that wrote + `from meshbay_hub.auth import _hub_id` captured the default and kept it: + `federation.py` did, so it signed with a `None` key and announced itself + as `meshbay.org` whatever its configuration said. Reading through a + function is what makes "call once at startup" true for readers as well as + for the writer. + """ + return _hub_id + + # ── Password ────────────────────────────────────────────────────────────────── def hash_password(password: str) -> tuple[bytes, bytes]: diff --git a/packages/meshbay-hub/tests/test_federation.py b/packages/meshbay-hub/tests/test_federation.py index 6035b0c..54f86a7 100644 --- a/packages/meshbay-hub/tests/test_federation.py +++ b/packages/meshbay-hub/tests/test_federation.py @@ -48,10 +48,23 @@ class Peer: serialization.Encoding.PEM, serialization.PrivateFormat.PKCS8, serialization.NoEncryption()) - def envelope(self, jti: str | None = None) -> str: + def envelope(self, jti: str | None = None, audience: str | None = None) -> str: + """The shape `federation._issue_mhp_token` actually emits. + + This used to omit `aud`, and so tested a token production never sends: + the verifier names an audience, and PyJWT refuses a token carrying + `aud` when the caller names none — which meant MHP could not complete + one authenticated request between two real hubs while this file stayed + green. A fixture narrower than the real thing tests the fixture. + """ + # Read from the module rather than written down here: the receiving + # hub's id is what `aud` has to carry, and a copy of it in a test is a + # second place for it to drift from. + from meshbay_hub.auth import _hub_id + now = int(time.time()) return jwt.encode( - {"iss": self.hub_id, "sub": self.hub_id, + {"iss": self.hub_id, "sub": self.hub_id, "aud": audience or _hub_id, "jti": jti or str(uuid.uuid4()), "iat": now, "exp": now + 300}, self._sk_pem(), algorithm="EdDSA") @@ -177,3 +190,50 @@ async def test_federation_cannot_revoke_a_user(client): r = await client.post("/mhp/revoke", headers=peer.header(), json={"token": peer.revocation("user", "some-user-id")}) assert r.status_code == 202 and r.json()["pruned"] == 0 + + +# ── The hub's own identity, and the audience it binds ──────────────────────── +# +# MHP could not complete one authenticated request between two real hubs, and +# nothing said so: `_issue_mhp_token` sets `aud`, `_verify_mhp_token` named no +# audience, and PyJWT refuses a token carrying `aud` when the caller names +# none. This file stayed green because its envelopes were built by hand +# without one — a fixture narrower than what production emits tests the +# fixture. Underneath that, `federation.py` bound `_hub_id` and `_hub_sk_pem` +# at import, which is *before* `load_hub_keypair` runs: it signed with `None` +# and called itself `meshbay.org` whatever the instance was named. + + +def test_the_hub_issues_tokens_under_its_configured_identity(app): + """Driven through the real issuer, which is the half no test called.""" + from meshbay_hub.api.federation import _issue_mhp_token + from meshbay_hub.auth import hub_id + + assert hub_id() == "test-hub", "the fixture hub is not configured" + + token = jwt.decode(_issue_mhp_token("peer-hub"), + options={"verify_signature": False}) + assert token["iss"] == "test-hub", ( + "the hub announced an identity that is not its own") + assert token["aud"] == "peer-hub" + + +@pytest.mark.asyncio +async def test_a_token_for_another_hub_is_refused(client): + """What the audience binding is for: a token captured here must not be + replayable at a peer, and one minted for a peer must not work here.""" + admin = await _admin(client) + peer = Peer("peer-elsewhere.test") + await _register_peer(client, admin, peer) + + r = await client.post( + "/mhp/directory", + headers={"Authorization": + f"Bearer {peer.envelope(audience='some-other-hub.test')}"}, + json={"hub_id": peer.hub_id, "groups": []}) + assert r.status_code == 401, r.text + + r = await client.post( + "/mhp/directory", headers=peer.header(), + json={"hub_id": peer.hub_id, "groups": []}) + assert r.status_code == 202, r.text diff --git a/packages/meshbay-hub/tests/test_public_groups_toggle.py b/packages/meshbay-hub/tests/test_public_groups_toggle.py index 0d36b99..e52fa85 100644 --- a/packages/meshbay-hub/tests/test_public_groups_toggle.py +++ b/packages/meshbay-hub/tests/test_public_groups_toggle.py @@ -212,24 +212,19 @@ async def test_disabled_hands_a_non_member_no_node(client, db_session): def _mhp_token(hub_id): - """A peer-hub JWT, signed with the running hub's own key. - - `federation._issue_mhp_token` binds `_hub_sk_pem` at import time, before the - lifespan loads it, so it cannot be used from a test. This signs directly. + """A peer-hub JWT — from the hub's own issuer, which is the point. + + This used to sign by hand, and its two comments recorded, accurately, the + reasons it had to: `_issue_mhp_token` bound `_hub_sk_pem` at import, before + the lifespan loads it, so it signed with `None`; and it sets an `aud` that + the verifier named no audience for, which PyJWT refuses outright. Both were + written down here as facts to route around rather than as the defects they + were — between them MHP could not complete one authenticated request + between two real hubs. Fixed at the source, so this can call it. """ - import time - import uuid - - import jwt - from meshbay_hub import auth as hub_auth - - now = int(time.time()) - # No `aud`: export_directory verifies without an expected audience, and PyJWT - # rejects a token that carries `aud` when decode() is given none. - return jwt.encode( - {"iss": hub_id, "sub": hub_id, - "jti": str(uuid.uuid4()), "iat": now, "exp": now + 300}, - hub_auth._hub_sk_pem, algorithm="EdDSA") + from meshbay_hub.api.federation import _issue_mhp_token + + return _issue_mhp_token(hub_id) @pytest.mark.asyncio -- cgit v1.2.3