diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-30 11:57:03 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-30 11:57:03 +0200 |
| commit | 5612dbbac41609b3f84784f57f1de538262db9a9 (patch) | |
| tree | 571e0bbd18c31be32ca33db35ca42f8aff672cc9 /packages/meshbay-node/src/meshbay_node/transport | |
| parent | d3ad243c4ae3a273f623bd5fc631e3266aa4d0e4 (diff) | |
| download | meshbay-5612dbbac41609b3f84784f57f1de538262db9a9.tar.gz | |
fix(node): the roster, not the key alone, decides who gets a session
The handshake opened a session for anyone holding the group key with a hub
token naming the group; the roster was consulted only when wrapping the key
in a join. A member revoked or unpinned on the node but still a member on
the hub kept a full session with the key they already held — and was handed
the chat epoch their removal had just opened, since chat keys go to any
session. An honest client never met this (it asks for the key through
join_request every time); one that kept the key did not have to.
- After the proof, the node asks the roster and refuses with
`not_authorized_for_group` unless the account is an active member of the
group or the node's operator.
- A removal from any door — MNP, the node page, the CLI — now opens a new
chat epoch in each group the person could read, broadcasts it, and closes
every connection they hold (`ops.members._after_removal`). The CLI and the
node page did neither.
- Design §5.2, protocol §6.1, §6.3, §14.2.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Diffstat (limited to 'packages/meshbay-node/src/meshbay_node/transport')
| -rw-r--r-- | packages/meshbay-node/src/meshbay_node/transport/webrtc/group_ops.py | 22 | ||||
| -rw-r--r-- | packages/meshbay-node/src/meshbay_node/transport/webrtc/handshake.py | 37 |
2 files changed, 41 insertions, 18 deletions
diff --git a/packages/meshbay-node/src/meshbay_node/transport/webrtc/group_ops.py b/packages/meshbay-node/src/meshbay_node/transport/webrtc/group_ops.py index 5f5f9ed..3756eac 100644 --- a/packages/meshbay-node/src/meshbay_node/transport/webrtc/group_ops.py +++ b/packages/meshbay-node/src/meshbay_node/transport/webrtc/group_ops.py @@ -113,8 +113,9 @@ class GroupOpsMixin: self._audit("admin_auth_failed", f"member_unpin:{user_id[:8]}") return try: + # The new chat epochs and the closed sessions are the op's own + # (`ops.members._after_removal`), for every door alike. await self._run_op(ops.unpin_member, user_id) - await self._new_chat_epoch(self._group_id or "", "member_unpin") except ops.OpError as e: self._send({"type": "error", "detail": e.message}) return @@ -331,26 +332,17 @@ class GroupOpsMixin: return try: + # The op opens the new chat epoch and closes every connection the + # account holds in this group (`ops.members._after_removal`), for + # the loopback API and the CLI as much as for this door. They keep + # the key they already unwrapped; rotating it is the operator's + # call, and the ack says so. result = await self._run_op( ops.revoke_member, user_id, self._group_id or "") - await self._new_chat_epoch(self._group_id or "", "member_revoke") except ops.OpError as e: self._send({"type": "error", "detail": e.message}) return - # Anyone connected right now keeps the key they already unwrapped; what - # they lose is the next one. Rotating it is the operator's call, and the - # ack says so rather than implying this undid anything already read. - # Every connection that account holds, not "the" one: with device - # linking a person may be connected from several at once, and the - # registry is keyed per connection precisely because it cannot hold - # only one of them. - for peer in self._sessions_of(user_id): - try: - await peer.close() - except Exception: - pass - self._audit("member_revoke", user_id) self._send({ "type": MNP.MEMBER_REVOKE_ACK, "v": MNP_VERSION, diff --git a/packages/meshbay-node/src/meshbay_node/transport/webrtc/handshake.py b/packages/meshbay-node/src/meshbay_node/transport/webrtc/handshake.py index f701072..7822186 100644 --- a/packages/meshbay-node/src/meshbay_node/transport/webrtc/handshake.py +++ b/packages/meshbay-node/src/meshbay_node/transport/webrtc/handshake.py @@ -137,7 +137,8 @@ class HandshakeMixin: return {"sig": base64.b64encode(self._ctx["sk_node"].sign(transcript)).decode()} def _do_handshake_response(self, msg: dict) -> None: - if not self._gek_challenge or not hasattr(self, "_pending_sub"): + if not self._gek_challenge or not hasattr(self, "_pending_sub") \ + or getattr(self, "_admitting", False): self._send({"type": "error", "detail": "No pending handshake challenge"}) return @@ -170,8 +171,38 @@ class HandshakeMixin: self._audit_auth_failed(group_id, "GEK HMAC mismatch") return - self._complete_handshake(gek, binding) - self._gek_challenge = None + # One answer at a time: the roster is asked asynchronously, and a + # second response arriving meanwhile must not be taken as a new one. + self._admitting = True + self._spawn(self._admit(gek, binding, group_id)) + + async def _admit(self, gek: bytes, binding: bytes, group_id: str) -> None: + """ + Open the session only for someone this node's roster admits. + + The group-key proof says the peer holds the key, and the token says the + hub counts the account a member. Neither is the node's own answer — + and the roster is supposed to be the authority (§6.1). Without this, a + member revoked here, or unpinned, but still a member on the hub, kept + full access with the key they already held: files, uploads, and — since + chat keys are handed to any session — the very epoch their removal had + just opened. An honest client never met the gap, because it asks for + the key through `join_request`, which does consult the roster; a + client that kept the key did not have to. + """ + try: + roster = self._ctx.get("roster") + if roster is not None and not await roster.is_authorized( + group_id, self._pending_sub): + self._send({"type": "error", + "detail": "This node has not admitted you to this group", + "code": "not_authorized_for_group"}) + self._audit_auth_failed(group_id, "not admitted by the roster") + return + self._complete_handshake(gek, binding) + finally: + self._gek_challenge = None + self._admitting = False def _complete_handshake(self, gek: bytes, binding: bytes) -> None: # Authenticated peers may send large frames (file uploads); unauthenticated |