diff options
6 files changed, 352 insertions, 3 deletions
diff --git a/docs/MESHBAY_DESIGN.md b/docs/MESHBAY_DESIGN.md index 3aeb660..f1c0a67 100644 --- a/docs/MESHBAY_DESIGN.md +++ b/docs/MESHBAY_DESIGN.md @@ -2908,6 +2908,8 @@ had already been asked. | **AV24** | **A node registered for no group is refused signaling, not exempted from it** (§7.2). The membership check was written as "if the node claims any group", so it skipped itself — membership, group status and the public-group gate together — for the node AV1 made commonplace: the unconfigured one, which is also the one least able to absorb the work | | **AV25** | **Which nodes host a group is answered to its members** (§7.3). Only the public case checked, so a private group told any authenticated account that knew its id which machines hosted it — and an ex-member knows that id for ever | | **AV26** | **A sign-in lockout refuses passphrase sign-in and nothing else** (§7.7). It is keyed by username, usernames are public, and so anyone can spend somebody else's attempts. Open sessions, renewal and device sign-in are untouched and a reset code ends it, which bounds what a stranger buys to one forced sign-in. The lockout is a DoS primitive by construction; this is the ceiling on it | +| **AV27** | **A free-text third-party search is bounded per member and per node** (§6.5). `tmdb_search_req` spends the *operator's* credential, which TMDB rates and the whole group's automatic matching depends on, so one member holding a search box degrades the library for everyone. Per member and not per connection — three tabs is one person — and kept in the group context so a reconnect does not reset it. The refusal is an error, because an empty result list is what "no such film" looks like | +| **AV28** | **How many node keys one account may announce is bounded** (§7.2). Each is a row plus an IP-log row under a one-year retention, so an account in a loop writes a year of storage on the operator's disk having paid only for signatures. Proof of possession (**M8**) settles whose key it is and not how many. Counted only where a row is added: re-announcing a key already held keeps working at the ceiling, or a node that reached it could never refresh its address again | ### 13.6 Chat design findings @@ -3077,8 +3079,6 @@ process runs it — `systemctl --user` on Linux, Task Scheduler on Windows. | The exact-hash content check | Structural, not functional (§7.5) | | **QUIC** | Off by default, and **not at parity**: it serves the index and file chunks with no transfer lease, no leaseless ceiling and no root-availability check, does its file I/O on the event loop, and returns exception text to the peer (**L3**). No client speaks it. Either it comes to parity or it goes; until then §5.1's "chat is the only gap" is the one sentence here that overstates the code | | **The relay registry** | **Closed in the code**: `relay.RELAYS_ENABLED` is False and every `/v1/relays` route answers 503, as federation does. Nothing in the tree calls them, node or client, and §11.1 measured two ISPs with no TURN relay needed. Kept code that nothing calls is what **L7** says not to keep; it stays only as the proof-of-possession design (**AV6**) until a node needs a relay or it is deleted | -| **Free-text third-party search** | `tmdb_search_req` takes a member's query and spends the operator's per-credential quota with no rate limit and no per-member bound, where link previews carry both. §6.5's standing rule — a bound and a named adversary in the same commit — was not applied here | -| **Node announcements are not bounded** | One account may announce unlimited distinct node keys, each a row plus an IP-log row under a one-year retention. Proof of possession is checked (**M8**); the count is not | | **Migrations run on SQLite only** | The chain reaches head and agrees with the models there (§12), which is not where it ships. **The exposure is one revision deep, not the whole chain**: every revision behind the first packaged release was development that no installation ever ran, so nothing replays them on PostgreSQL. What is unguarded is the *next* migration — a default, an index type or a constraint PostgreSQL refuses reaches a deploy without the suite saying so | --- diff --git a/packages/meshbay-hub/src/meshbay_hub/api/nodes.py b/packages/meshbay-hub/src/meshbay_hub/api/nodes.py index 67e65f2..83b60f2 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/nodes.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/nodes.py @@ -8,7 +8,7 @@ from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PublicKey from cryptography.exceptions import InvalidSignature from fastapi import APIRouter, Depends, HTTPException, Request from pydantic import BaseModel -from sqlalchemy import select +from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession from meshbay_hub.auth import issue_access_token @@ -89,6 +89,22 @@ class NodeAnnounceRequest(BaseModel): signature: str | None = None # base64 Ed25519 over the announce message +# How many distinct node keys one account may announce. +# +# M8 closed the half of this that was about *whose* key it is: the announcer now +# proves possession. What it did not close is *how many*. Each new key is a row +# in `nodes` plus a row in the IP log, and the IP log is kept for a year — so an +# account in a loop writes a year of storage on somebody else's disk, having paid +# only for the signatures. +# +# Ten is past what the feature is for. A node is a machine left running: a +# desktop, a laptop, a box in a cupboard, a second home. Someone who genuinely +# reaches it deletes one, which is a thing the operator surface already does — +# and an account that wants an eleventh *identity* rather than an eleventh +# machine is the case this refuses. +MAX_NODES_PER_ACCOUNT = 10 + + @router.post("/announce", status_code=201) async def announce_node( body: NodeAnnounceRequest, @@ -146,6 +162,21 @@ async def announce_node( await db.commit() return {"node_id": node.id} + # Counted only where a row is actually added: re-announcing a key this + # account already holds takes the branch above and must keep working at the + # ceiling, or a node that has reached it can never refresh its address again. + held = (await db.execute( + select(func.count()).select_from(Node) + .where(Node.user_id == current_user.id))).scalar() or 0 + if held >= MAX_NODES_PER_ACCOUNT: + db.add(IPLog(user_id=current_user.id, event="node_announce_refused", + ip_address=seen_from, detail=f"{held} nodes")) + await db.commit() + raise HTTPException( + status_code=409, + detail=f"This account already has {held} nodes, which is the limit of " + f"{MAX_NODES_PER_ACCOUNT}. Remove one you no longer run.") + node = Node( user_id=current_user.id, pk_node=body.pk_node, diff --git a/packages/meshbay-hub/tests/test_availability_between_members.py b/packages/meshbay-hub/tests/test_availability_between_members.py index d1a4dcb..2be7c03 100644 --- a/packages/meshbay-hub/tests/test_availability_between_members.py +++ b/packages/meshbay-hub/tests/test_availability_between_members.py @@ -718,3 +718,70 @@ async def test_a_private_groups_node_list_is_for_its_members(client): finally: rev._connected_nodes.pop(node_id, None) rev._node_groups.pop(node_id, None) + + +async def _announce_key(client, user: dict, sk) -> int: + """Announce a *distinct* node key, and return the status code.""" + from meshbay_common.crypto import pk_to_b64 + + pk = pk_to_b64(sk.public_key()) + ts = int(time.time()) + msg = f"meshbay:node_announce:{user['user_id']}:{pk}:{ts}".encode() + r = await client.post("/v1/nodes/announce", json={ + "pk_node": pk, "endpoint_hint": "test", "timestamp": ts, + "signature": base64.b64encode(sk.sign(msg)).decode(), + }, headers={"Authorization": f"Bearer {user['token']}"}) + return r.status_code + + +async def test_one_account_cannot_announce_unlimited_nodes(client, monkeypatch): + """ + Each new node key is a row in `nodes` and a row in the IP log, and the IP log + is kept for a year. Proof of possession (M8) settles *whose* key it is and + says nothing about how many: an account in a loop wrote a year of storage on + the operator's disk having paid only for signatures. + + Two accounts, because the ceiling has to be per account. One that is shared + would let a single member deny every other member the ability to bring a + machine online, which is the same defect with better manners. + """ + from meshbay_hub.api import nodes as nodes_api + + monkeypatch.setattr(nodes_api, "MAX_NODES_PER_ACCOUNT", 3) + alice = await _make_user(client, "av_nodecap_alice") + bob = await _make_user(client, "av_nodecap_bob") + + keys = [Ed25519PrivateKey.generate() for _ in range(4)] + for sk in keys[:3]: + assert await _announce_key(client, alice, sk) == 201 + + assert await _announce_key(client, alice, keys[3]) == 409, ( + "an account announced past the ceiling") + + # Bob has announced nothing and must be unaffected. + assert await _announce_key(client, bob, Ed25519PrivateKey.generate()) == 201, ( + "one account's ceiling was charged to another's" + ) + + +async def test_a_node_at_the_ceiling_can_still_refresh_its_address(client, monkeypatch): + """ + The ceiling counts rows, so it must be checked only where a row is added. + Applied to every announce, it would freeze the address of every node an + account already runs the moment it reached the limit — and a node that + cannot re-announce is a node nobody can reach after their ISP renumbers + them, which is an outage caused by the protection. + """ + from meshbay_hub.api import nodes as nodes_api + + monkeypatch.setattr(nodes_api, "MAX_NODES_PER_ACCOUNT", 2) + alice = await _make_user(client, "av_nodecap_refresh") + + keys = [Ed25519PrivateKey.generate() for _ in range(2)] + for sk in keys: + assert await _announce_key(client, alice, sk) == 201 + assert await _announce_key(client, alice, Ed25519PrivateKey.generate()) == 409 + + for sk in keys: + assert await _announce_key(client, alice, sk) == 201, ( + "a node already known could not re-announce at the ceiling") 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 4540f2f..773d3dc 100644 --- a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py +++ b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py @@ -213,6 +213,23 @@ _LINK_PREVIEW_RATE_WINDOW = 60.0 _LINK_PREVIEW_RATE_PER_CONN = 15 _LINK_PREVIEW_RATE_NODE = 60 +# A free-text TMDB search spends the *operator's* credential, which is rated by +# TMDB and shared by everyone in the group: one member typing in the search box +# can exhaust what every other member's automatic matching depends on, and the +# operator is the one who has to notice. §6.5's rule is a bound and a named +# adversary in the same commit; this one arrived without either. +# +# Per member rather than per connection, unlike link previews above: three tabs +# is one person, and a ceiling a tab can multiply is not a ceiling. Kept in the +# group context so it survives a reconnect, which is the other thing a per-session +# count cannot do. +# +# Generous next to what a person types — ten searches a minute is a search every +# six seconds, sustained — and small next to a loop. +_TMDB_SEARCH_WINDOW = 60.0 +_TMDB_SEARCH_PER_MEMBER = 10 +_TMDB_SEARCH_NODE = 30 + # Chat limits. A message is a member-supplied write onto the operator's disk # (`chat.db`, where retention is a manual CLI command — §6.6), relayed from there # to every other connected member and turned into a notification for every member @@ -4471,6 +4488,21 @@ class WebRTCPeerSession: "query": query, "media_type": media_type, "results": []}) return + # Refused out loud, not as an empty result: "no matches" is what the + # client draws for an empty list, and telling somebody their film is + # unknown when the node simply declined to ask is a worse answer than + # the truth. `video-app.js`'s `runSearch` puts `detail` on screen. + if not self._tmdb_search_rate_ok(): + log.info("tmdb_search_req: rate-limited (user=%s)", (self._user_id or "")[:8]) + self._send({ + "type": "error", + "detail": "Too many searches in the last minute. This spends the " + "operator's search quota, which everyone in the group " + "shares — try again shortly.", + "code": "tmdb_search_rate_limited", + }) + return + raw = (await tmdb_client.search_movie_results(query) if media_type == "movie" else await tmdb_client.search_tv_results(query)) results = [] @@ -5103,6 +5135,33 @@ class WebRTCPeerSession: if isinstance(m.payload, bytes) else m.payload) return row + def _tmdb_search_rate_ok(self) -> bool: + """ + True when this search is within both the member's window and the node's; + records it when so, and trims both to the window on every call so neither + list can grow without bound. + + Both are checked because they answer different questions: the member's + keeps one person from spending everyone's quota, and the node's keeps a + group of them from doing it together. + """ + now = time.monotonic() + w = _TMDB_SEARCH_WINDOW + ctx = self._group_ctx() + by_member = ctx.setdefault("tmdb_search_hits", {}) + who = self._user_id or "" + mine = [t for t in by_member.get(who, []) if now - t < w] + node = [t for t in self._ctx.get("tmdb_search_hits_node", []) if now - t < w] + if len(mine) >= _TMDB_SEARCH_PER_MEMBER or len(node) >= _TMDB_SEARCH_NODE: + by_member[who] = mine + self._ctx["tmdb_search_hits_node"] = node + return False + mine.append(now) + node.append(now) + by_member[who] = mine + self._ctx["tmdb_search_hits_node"] = node + return True + def _link_preview_rate_ok(self) -> bool: """ True when this preview fetch is within both the per-connection and the diff --git a/packages/meshbay-node/tests/test_season_and_search_requests.py b/packages/meshbay-node/tests/test_season_and_search_requests.py index 9dcc6ce..4141d13 100644 --- a/packages/meshbay-node/tests/test_season_and_search_requests.py +++ b/packages/meshbay-node/tests/test_season_and_search_requests.py @@ -22,6 +22,12 @@ def _session(media_cache=None, tmdb_client=None) -> WebRTCPeerSession: session = WebRTCPeerSession.__new__(WebRTCPeerSession) session._ctx = {"media_cache": media_cache, "tmdb_client": tmdb_client} session._group_id = None + # Set because production always has one: `_dispatch_message` refuses every + # message until the handshake settles `_user_id`, so a session reaching any + # of these handlers without it does not exist. Left out, this fixture was + # narrower than the node and the per-member search ceiling could not be + # exercised by it at all. + session._user_id = "u1" session.sent = [] session._send = session.sent.append return session diff --git a/packages/meshbay-node/tests/test_tmdb_search_bound.py b/packages/meshbay-node/tests/test_tmdb_search_bound.py new file mode 100644 index 0000000..486a2c2 --- /dev/null +++ b/packages/meshbay-node/tests/test_tmdb_search_bound.py @@ -0,0 +1,186 @@ +""" +One member's typing must not spend what the whole group depends on. + +`tmdb_search_req` takes a member's free text and calls TMDB with the +**operator's** credential. That credential is rated by TMDB and shared: the +automatic matching every other member sees runs on it too. So a member holding +down a search box — or a script doing it — degrades the library for everyone and +costs the operator their quota, and the node had no ceiling of any kind on it. +§6.5's standing rule is a bound and a named adversary in the same commit; this +handler shipped with neither. + +Two members in every test here, which is the point: a ceiling that one person +can exhaust for another is not a ceiling, it is a queue. The per-member window +is what keeps them apart, and the node-wide one is what keeps them together +from emptying the operator's quota — they answer different questions and both +are checked. + +The refusal is an error rather than an empty result. An empty list is what "no +such film" looks like, and telling somebody their film is unknown when the node +simply declined to ask is a worse answer than the truth. +""" + +import pytest +from meshbay_node.transport import webrtc_server +from meshbay_node.transport.webrtc_server import WebRTCPeerSession + +GROUP = "g" * 32 + + +class _FakeTmdb: + """Counts what would have been spent.""" + + def __init__(self): + self.calls = 0 + + async def search_movie_results(self, query): + self.calls += 1 + return [{"id": 1, "title": "Some Saga", "release_date": "1999-01-01", + "poster_path": None}] + + async def search_tv_results(self, query): + self.calls += 1 + return [] + + +class _FakeMediaCache: + async def get_thumb_hash_by_file_id(self, _file_id): + return None + + +@pytest.fixture +def group(): + """One group's context, shared by every session in it, as a node has.""" + return { + "gek": b"k" * 32, + "tmdb_enabled": True, + } + + +@pytest.fixture +def node(group): + tmdb = _FakeTmdb() + ctx = { + "groups": {GROUP: group}, + "media_cache": _FakeMediaCache(), + "tmdb_client": tmdb, + } + return ctx, tmdb + + +def _member(ctx, user_id: str) -> WebRTCPeerSession: + s = WebRTCPeerSession.__new__(WebRTCPeerSession) + s._ctx = ctx + s._group_id = GROUP + s._user_id = user_id + s._peer_id = user_id + s.sent = [] + s._send = s.sent.append + s._audit = lambda *a, **k: None + return s + + +async def _search(session, query="a film"): + await session._do_tmdb_search_request( + {"query": query, "media_type": "movie"}) + + +def _refusals(session): + return [m for m in session.sent + if m.get("code") == "tmdb_search_rate_limited"] + + +async def test_a_member_at_the_ceiling_does_not_stop_another_one(node, monkeypatch): + """ + The property a one-member test cannot state. + + Alice exhausts her own window; Bob, who has typed nothing, must be served + exactly as if she had not been there. + """ + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_PER_MEMBER", 3) + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_NODE", 100) + ctx, tmdb = node + + alice = _member(ctx, "alice") + for i in range(4): + await _search(alice, f"film {i}") + assert tmdb.calls == 3, "the ceiling did not stop the fourth search" + assert len(_refusals(alice)) == 1 + + bob = _member(ctx, "bob") + await _search(bob, "something else") + assert tmdb.calls == 4 + assert _refusals(bob) == [] + + +async def test_one_member_cannot_spend_the_whole_node_quota(node, monkeypatch): + """ + And the other half: two members together still meet a node-wide ceiling, + because the operator's credential is one credential however many people + hold the search box down. + """ + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_PER_MEMBER", 100) + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_NODE", 2) + ctx, tmdb = node + + alice, bob = _member(ctx, "alice"), _member(ctx, "bob") + await _search(alice) + await _search(bob) + await _search(bob) + + assert tmdb.calls == 2 + assert len(_refusals(bob)) == 1 + + +async def test_a_members_count_survives_their_reconnection(node, monkeypatch): + """ + Kept in the group context, not on the session: otherwise the ceiling is one + reconnect wide, and a client that drops its DataChannel between searches has + no ceiling at all. + """ + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_PER_MEMBER", 2) + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_NODE", 100) + ctx, tmdb = node + + first = _member(ctx, "alice") + await _search(first, "one") + await _search(first, "two") + + reconnected = _member(ctx, "alice") # same person, new connection + await _search(reconnected, "three") + + assert tmdb.calls == 2, "a reconnect reset the member's window" + assert len(_refusals(reconnected)) == 1 + + +async def test_a_refusal_is_said_out_loud_and_not_drawn_as_no_matches(node, monkeypatch): + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_PER_MEMBER", 0) + ctx, _ = node + + alice = _member(ctx, "alice") + await _search(alice) + + (msg,) = alice.sent + assert msg["type"] == "error" + assert msg["code"] == "tmdb_search_rate_limited" + assert msg.get("results") is None, ( + "a refusal that carries an empty result list reads as 'no such film'") + + +async def test_the_windows_do_not_grow_without_bound(node, monkeypatch): + """ + The lists are trimmed on every call, so the thing that bounds a member also + bounds what remembering them costs. + """ + monkeypatch.setattr(webrtc_server, "_TMDB_SEARCH_WINDOW", 0.0) + ctx, tmdb = node + + alice = _member(ctx, "alice") + for i in range(12): + await _search(alice, f"film {i}") + + # Every entry ages out before the next call, so nothing is refused, and what + # is kept is the one just recorded rather than one per search ever made. + assert tmdb.calls == 12 + assert len(ctx["groups"][GROUP]["tmdb_search_hits"]["alice"]) == 1 + assert len(ctx["tmdb_search_hits_node"]) == 1 |