diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-12 12:14:40 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-12 16:36:54 +0200 |
| commit | 903ea022e918a05c7c8cb43d95e46d82368566f1 (patch) | |
| tree | b6cb4eb8fbac87f512948f5005e2ed48f26629c9 /packages/meshbay-hub/src/meshbay_hub/api | |
| parent | 6260825bf6d8340549de53905de3bd0b84d97d0a (diff) | |
| download | meshbay-903ea022e918a05c7c8cb43d95e46d82368566f1.tar.gz | |
fix(hub): MHP binds its audience, and the hub knows its own name
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4YmK41VsEURWFdop4EEeT
Diffstat (limited to 'packages/meshbay-hub/src/meshbay_hub/api')
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/api/federation.py | 48 |
1 files changed, 34 insertions, 14 deletions
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 ], |