diff options
| -rw-r--r-- | docs/captcha.md | 110 | ||||
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/api/users.py | 1 | ||||
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/captcha.py | 37 | ||||
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/config.py | 18 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_captcha_host_check.py | 188 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_register_captcha.py | 2 |
6 files changed, 314 insertions, 42 deletions
diff --git a/docs/captcha.md b/docs/captcha.md index c908a5c..84dadc4 100644 --- a/docs/captcha.md +++ b/docs/captcha.md @@ -12,7 +12,9 @@ reCAPTCHA v2 with the "I'm not a robot" checkbox. Reasons: - Binary pass/fail — no score threshold to tune or monitor. - The user is already filling a form; one checkbox is negligible friction. -- Works in both the web SPA and the Electron client (both run Chromium). +- Works in the web SPA. It works in the Electron client too, but not for the + reason "both run Chromium" — reCAPTCHA validates the *domain*, not the + rendering engine, and the desktop client's is not the hub's. See §6. - v3 (invisible, score-based) is an option later if the checkbox proves annoying; the server-side verification call is identical, only the client widget differs. @@ -20,6 +22,9 @@ reCAPTCHA v2 with the "I'm not a robot" checkbox. Reasons: `https://www.google.com/recaptcha/admin`. Register the hub's domain(s) — `meshbay.org` and `localhost` for development. This produces a **site key** (public, embedded in HTML) and a **secret key** (server-only, in `hub.toml`). +If the desktop client is in use, also turn *off* "Verify the origin of +reCAPTCHA solutions" on that key and set `allowed_hosts` — §6 says why, and +what is given up. --- @@ -376,49 +381,69 @@ captcha.failed: "Captcha verification failed — please try again" --- -## 6. Desktop client (Electron) +## 6. Desktop client (Electron), and the domain problem -The Electron client loads its UI from the package (`app://meshbay`), not from -the hub. Its CSP forbids external scripts by design — the renderer executes no -code that did not ship in the package. Loading Google's reCAPTCHA script -(`https://www.google.com/recaptcha/api.js`) would be a **new category of -trust**: third-party executable code in a renderer that currently runs none. +**This section replaced two earlier designs, and both are worth naming because +the reasoning that produced them is the trap.** -Two approaches: +The first said: open the CSP to Google's reCAPTCHA domains, or skip the captcha +for native clients — and recommended skipping it, keyed on `auth_key` being +present. That carve-out shipped and was a hole: *every* real client sends +`auth_key`, the browser included (it is the password split), so the gate was +off for everybody and a bot skipped it by including the field. It is gone; +`users.py` gates on `captcha.enabled` alone, and says so at the call site. -- **Open the CSP to Google's reCAPTCHA domains.** Add - `https://www.google.com/recaptcha/` and `https://www.gstatic.com/recaptcha/` - to `script-src`, `frame-src` and `connect-src`. Functional, but undermines - the principle that the renderer runs only packaged code — the reCAPTCHA - script is fetched live and changes without the operator's knowledge. -- **Skip the captcha for native clients** (recommended). The Electron client - already raises the bar against automated account creation: it requires - installation, generates a device Ed25519 key pair stored in `safeStorage`, - and authenticates to the hub via `POST /v1/users/auth` with a signed - challenge. A bot automating that path must install and drive a full Electron - app, which is a harder problem than filling a web form — and the reCAPTCHA - exists to solve the web-form problem. +The second is the sentence in §1 above: "works in the Electron client too, both +run Chromium". The CSP was opened (`RECAPTCHA_SRC` in `main.js`, covering +`script-src`, `img-src` and `frame-src`) and the widget does render. It renders +**"ERROR for site owner: Invalid domain for site key"**. -**How to skip server-side.** The web SPA sends `password` (legacy) in the -registration body; the Electron client sends `auth_key` (PBKDF2-derived via -`window.MeshBayKeys.registerUser`). The server requires `captcha_token` only -when `auth_key` is absent — i.e. the web path. This is not a security -boundary: a bot that derives `auth_key` itself bypasses the check, but it also -proves it can run the PBKDF2 derivation, which is the same cost as solving the -captcha. The real gate for native-path abuse is the rate limiter (`5/minute`) -and the email verification step. +**Why.** reCAPTCHA validates the hostname of the page the widget is rendered +in, against the domain list on the site key. The desktop client's interface +ships inside the package and is served from `app://meshbay` (`main.js`: +`win.loadURL('app://meshbay/index.html')`). Not a preference — `file://` +breaks ES modules and IndexedDB, and the hub must never become the document +origin, which is enforced by the `will-navigate` handler. So the hostname +Google sees is `meshbay`, it is not on the key's list, and it never can be: +the check happens on Google's servers and no client-side configuration reaches +it. Widening the CSP does not help, because the CSP was never what refused. -```python -# In register(), captcha gate adjusted: -if _cfg and _cfg.captcha.enabled and not body.auth_key: - if not body.captcha_token: - raise HTTPException(400, "captcha_required") - ... +**What is done instead.** Turn *off* "Verify the origin of reCAPTCHA +solutions" on the key, and check the origin on the hub, where it belongs: + +```toml +[captcha] +site_key = "6Le..." +secret_key = "6Le..." +allowed_hosts = ["meshbay.org", "localhost", "meshbay"] ``` -On the Electron side: `registerUser()` in `keyderive.js` does not send -`captcha_token`, and the server does not ask for one. No CSP change, no -Google script loaded, no new trust boundary. +`verify_captcha` then refuses a solve whose reported hostname is not in that +list. The hostname comes from `siteverify` — it is what Google *observed*, not +something the caller asserts — so this is a real check and not a formality: the +site key is public, and the thing turning the origin check off opens is a bot +rendering the widget on a page of its own, which this refuses on the hostname +it actually served from. + +Empty (the default) means "do not check", so a hub that never touched this +setting keeps the behaviour it has, with reCAPTCHA doing the origin check +itself. **The two settings go together**: turning the console check off without +setting `allowed_hosts` leaves no origin check anywhere. + +**The last entry is the desktop client's own, and it is the weak one.** Any +Electron application can claim the same scheme and host — `main.js` already +records that `app://meshbay` is not a credential, which is why the hub's API is +reachable from no web origin at all and every call leaves from the main +process. So `meshbay` in that list is spoofable by someone who builds an +equivalent application. What it still costs them is a per-token captcha solve +inside a real Chromium, rather than a token farmed from any web page. That is +the trade, stated plainly; a hub that does not ship the desktop client should +leave the entry out. + +**Confirming the hostname.** `meshbay` is the host component of +`app://meshbay`. If a solve is refused, `captcha.py` logs it at WARNING with +the hostname spelled out and the allowed list beside it, which is how to read +the value a given client actually reports rather than guess at it. --- @@ -475,11 +500,15 @@ from Google — no npm package. ## 10. Deployment steps 1. Obtain reCAPTCHA v2 keys from Google (register `meshbay.org` + `localhost`). + For a deployment with the desktop client, also turn off "Verify the origin + of reCAPTCHA solutions" on the key — §6. 2. Add to `/etc/meshbay/hub.toml` on the production server: ```toml [captcha] - site_key = "6Le..." - secret_key = "6Le..." + site_key = "6Le..." + secret_key = "6Le..." + # Required whenever the console's origin check is off, and only then. + allowed_hosts = ["meshbay.org", "localhost", "meshbay"] ``` 3. Deploy the new hub code (`deploy-hub.sh` — runs `alembic upgrade head` + restart; no migration needed for this change). @@ -488,3 +517,6 @@ from Google — no npm package. solving the captcha. 5. Verify reset: open `https://meshbay.org/#/reset`, confirm the checkbox appears. Request a reset code, confirm email arrives only after solving it. +6. Verify the desktop client: register from it and confirm the widget solves + rather than showing "Invalid domain for site key". A refusal logged as + `captcha solved on an unexpected host` names the hostname to add. diff --git a/packages/meshbay-hub/src/meshbay_hub/api/users.py b/packages/meshbay-hub/src/meshbay_hub/api/users.py index 9b70b59..ece98cd 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/users.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/users.py @@ -68,6 +68,7 @@ async def _verify_captcha_or_raise(token: str | None, request: Request) -> None: _cfg.captcha.secret_key, # type: ignore[union-attr] token, request.client.host if request.client else None, + _cfg.captcha.host_check, # type: ignore[union-attr] ) if not ok: raise HTTPException(status_code=400, detail="captcha_failed") diff --git a/packages/meshbay-hub/src/meshbay_hub/captcha.py b/packages/meshbay-hub/src/meshbay_hub/captcha.py index faf7907..74a77b6 100644 --- a/packages/meshbay-hub/src/meshbay_hub/captcha.py +++ b/packages/meshbay-hub/src/meshbay_hub/captcha.py @@ -10,8 +10,30 @@ VERIFY_URL = "https://www.google.com/recaptcha/api/siteverify" async def verify_captcha( - secret_key: str, token: str, remote_ip: str | None = None, + secret_key: str, + token: str, + remote_ip: str | None = None, + allowed_hosts: frozenset[str] | set[str] | None = None, ) -> bool: + """True when Google accepts the token, and it came from a host we expect. + + `allowed_hosts` is the answer to a reCAPTCHA key whose own origin check is + turned off. That check compares the *page's hostname* against the key's + domain list, and the desktop client's page is served from `app://meshbay` + — Chromium refuses a service worker on a custom scheme and the interface + ships inside the package, so it cannot be `https://<hub>` and the hostname + cannot be the hub's. Google answers "Invalid domain for site key" inside + its own widget, which is not something the client can configure away. + + Turning the key's origin check off moves that decision here, where the hub + knows which hosts are its own. `siteverify` reports the hostname it + observed, not one the caller asserts, so this is a real check and not a + formality — a bot embedding the site key on a page of its own is refused + on the hostname it actually served from. + + `None` (the default) skips it, which is what a deployment leaving the + origin check with Google wants: it is then already done, one layer up. + """ payload: dict[str, str] = {"secret": secret_key, "response": token} if remote_ip: payload["remoteip"] = remote_ip @@ -22,7 +44,18 @@ async def verify_captcha( result = resp.json() if not result.get("success"): log.info("captcha rejected: %s", result.get("error-codes", [])) - return result.get("success", False) + return False + if allowed_hosts is not None: + # Logged at warning with the hostname spelled out: this is also + # how an operator finds what to allow after adding a client + # whose origin they have not seen before. + host = result.get("hostname") or "" + if host not in allowed_hosts: + log.warning( + "captcha solved on an unexpected host %r; allowed: %s", + host, sorted(allowed_hosts)) + return False + return True except Exception: log.exception("captcha verification request failed") return False diff --git a/packages/meshbay-hub/src/meshbay_hub/config.py b/packages/meshbay-hub/src/meshbay_hub/config.py index 48d5a6e..39fdc52 100644 --- a/packages/meshbay-hub/src/meshbay_hub/config.py +++ b/packages/meshbay-hub/src/meshbay_hub/config.py @@ -65,11 +65,25 @@ class JWTConfig: class CaptchaConfig: site_key: str = "" secret_key: str = "" + # Hostnames a solved captcha may have come from, checked against the one + # `siteverify` reports. Empty means "do not check", which is right while + # the reCAPTCHA key does its own origin check — it is then already done. + # + # Set this when that check is turned off in the reCAPTCHA console, which is + # what the desktop client needs: its page is served from `app://meshbay`, + # so the hostname Google sees is not the hub's and never can be. See + # docs/captcha.md §6. + allowed_hosts: list[str] = field(default_factory=list) @property def enabled(self) -> bool: return bool(self.site_key and self.secret_key) + @property + def host_check(self) -> frozenset[str] | None: + """The set to hand `verify_captcha`, or None for "do not check".""" + return frozenset(self.allowed_hosts) if self.allowed_hosts else None + @dataclass class HubConfig: @@ -106,6 +120,8 @@ def load_config(path: Path | None = None) -> HubConfig: if cap := raw.get("captcha", {}): cfg.captcha.site_key = cap.get("site_key", cfg.captcha.site_key) cfg.captcha.secret_key = cap.get("secret_key", cfg.captcha.secret_key) + if hosts := cap.get("allowed_hosts"): + cfg.captcha.allowed_hosts = [str(h).strip() for h in hosts if str(h).strip()] break # Env var overrides @@ -125,5 +141,7 @@ def load_config(path: Path | None = None) -> HubConfig: cfg.captcha.site_key = captcha_site if captcha_secret := os.environ.get("MESHBAY_CAPTCHA_SECRET_KEY"): cfg.captcha.secret_key = captcha_secret + if captcha_hosts := os.environ.get("MESHBAY_CAPTCHA_ALLOWED_HOSTS"): + cfg.captcha.allowed_hosts = [h.strip() for h in captcha_hosts.split(",") if h.strip()] return cfg diff --git a/packages/meshbay-hub/tests/test_captcha_host_check.py b/packages/meshbay-hub/tests/test_captcha_host_check.py new file mode 100644 index 0000000..bbd6c6d --- /dev/null +++ b/packages/meshbay-hub/tests/test_captcha_host_check.py @@ -0,0 +1,188 @@ +"""A solved captcha has to have been solved on one of our own pages. + +reCAPTCHA normally does this itself: the widget reports the hostname of the +page it is rendered in, and Google compares it against the key's domain list. +That is the check the desktop client cannot pass. Its interface ships inside +the package and is served from `app://meshbay` — not a preference, Chromium +refuses a service worker on a custom scheme and the hub must never become the +document origin — so the hostname Google sees is not the hub's and never can +be. The widget renders "ERROR for site owner: Invalid domain for site key", +which no amount of client-side configuration reaches, because the decision is +made on Google's servers. + +Turning the key's own origin check off moves the decision here. `siteverify` +then reports the hostname it *observed* — not one the caller asserts — so this +stays a real check: a bot that embeds the site key on a page of its own is +refused on the hostname it actually served from, which is the attack that +turning the check off would otherwise open. + +`allowed_hosts` empty means "do not check", which is what a deployment that +left the origin check with Google wants. That is the default, so an existing +hub upgrades without its captcha changing behaviour. +""" + +import json + +import pytest + +from meshbay_hub.captcha import verify_captcha +from meshbay_hub.config import CaptchaConfig, load_config + + + +class _Resp: + def __init__(self, payload): + self._payload = payload + + def raise_for_status(self): + return None + + def json(self): + return json.loads(json.dumps(self._payload)) + + +class _Client: + """Stands in for httpx.AsyncClient, and records what was sent.""" + + sent: list[dict] = [] + + def __init__(self, payload): + self._payload = payload + + def __call__(self, *a, **kw): + return self + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc): + return False + + async def post(self, url, data=None): + type(self).sent.append(dict(data or {})) + return _Resp(self._payload) + + +@pytest.fixture +def google(monkeypatch): + """`siteverify` answers whatever the test says it answers.""" + def _answer(**payload): + _Client.sent = [] + monkeypatch.setattr("httpx.AsyncClient", _Client(payload)) + return _answer + + +@pytest.mark.asyncio +async def test_a_solve_from_an_expected_host_is_accepted(google): + google(success=True, hostname="meshbay.org") + assert await verify_captcha("s", "t", allowed_hosts=frozenset({"meshbay.org"})) + + +@pytest.mark.asyncio +async def test_the_desktop_client_s_own_host_is_just_another_entry(google): + """`app://meshbay` is what the reported hostname comes from — the reason + any of this exists. Nothing about it is special to the check.""" + google(success=True, hostname="meshbay") + allowed = frozenset({"meshbay.org", "localhost", "meshbay"}) + assert await verify_captcha("s", "t", allowed_hosts=allowed) + + +@pytest.mark.asyncio +async def test_a_solve_from_somewhere_else_is_refused(google): + """The attack that turning off the key's origin check would open: the site + key is public, so a bot can render the widget on a page of its own.""" + google(success=True, hostname="a-bot-farm.example") + assert not await verify_captcha( + "s", "t", allowed_hosts=frozenset({"meshbay.org", "meshbay"})) + + +@pytest.mark.asyncio +async def test_a_missing_hostname_is_refused_when_checking(google): + """An answer with no hostname at all must not read as "any hostname".""" + google(success=True) + assert not await verify_captcha("s", "t", allowed_hosts=frozenset({"meshbay.org"})) + + +@pytest.mark.asyncio +async def test_no_allowed_hosts_means_google_is_still_the_one_checking(google): + """The default, and what every hub that never touched this config does. + + The hostname is whatever it is and this does not look: the key's own origin + check is on, so the answer would not have come back successful otherwise. + """ + google(success=True, hostname="somewhere.example") + assert await verify_captcha("s", "t") + assert await verify_captcha("s", "t", allowed_hosts=None) + + +@pytest.mark.asyncio +async def test_a_failed_solve_is_refused_whatever_the_hostname(google): + """Order matters: `success` first, and the host check never rescues a no.""" + google(success=False, hostname="meshbay.org", **{"error-codes": ["timeout-or-duplicate"]}) + assert not await verify_captcha( + "s", "t", allowed_hosts=frozenset({"meshbay.org"})) + + +@pytest.mark.asyncio +async def test_the_remote_ip_still_rides_along(google): + """Unchanged behaviour, pinned because the signature grew a parameter.""" + google(success=True, hostname="meshbay.org") + await verify_captcha("s", "t", "203.0.113.7", frozenset({"meshbay.org"})) + assert _Client.sent == [{"secret": "s", "response": "t", "remoteip": "203.0.113.7"}] + + +@pytest.mark.asyncio +async def test_a_transport_failure_is_a_refusal_not_an_exception(monkeypatch): + """Unchanged, and worth keeping: a captcha that cannot be checked is not a + captcha that passed.""" + class _Boom(_Client): + async def post(self, url, data=None): + raise RuntimeError("no route to host") + + monkeypatch.setattr("httpx.AsyncClient", _Boom({})) + assert not await verify_captcha("s", "t", allowed_hosts=frozenset({"meshbay.org"})) + + +def test_the_config_hands_over_a_set_or_nothing(): + """`host_check` is what the caller passes straight through, so "unset" has + to arrive as None rather than an empty set that refuses everything.""" + assert CaptchaConfig().host_check is None + assert CaptchaConfig(allowed_hosts=[]).host_check is None + assert CaptchaConfig(allowed_hosts=["meshbay.org", "meshbay"]).host_check == frozenset( + {"meshbay.org", "meshbay"}) + + +def test_hub_toml_carries_the_allowed_hosts(tmp_path): + """An operator who turns off the key's origin check sets this, and it is + the only place they can: the value depends on the deployment.""" + cfg_file = tmp_path / "hub.toml" + cfg_file.write_text( + '[captcha]\n' + 'site_key = "6Lsite"\n' + 'secret_key = "6Lsecret"\n' + 'allowed_hosts = ["meshbay.org", " meshbay ", "", "localhost"]\n') + + cfg = load_config(cfg_file) + + assert cfg.captcha.enabled + assert cfg.captcha.host_check == frozenset({"meshbay.org", "meshbay", "localhost"}), ( + "entries are stripped and blanks dropped — a stray space in a TOML " + "list would otherwise refuse every solve from that host") + + +def test_a_hub_toml_without_the_key_checks_nothing(tmp_path): + """The upgrade path. A hub that never heard of this setting keeps the + behaviour it has, with reCAPTCHA doing the origin check.""" + cfg_file = tmp_path / "hub.toml" + cfg_file.write_text('[captcha]\nsite_key = "6Lsite"\nsecret_key = "6Lsecret"\n') + + assert load_config(cfg_file).captcha.host_check is None + + +def test_the_environment_can_override_the_list(monkeypatch, tmp_path): + """Same shape as MESHBAY_ADMIN_USERS, for a container that has no file.""" + monkeypatch.setenv("MESHBAY_CAPTCHA_ALLOWED_HOSTS", "meshbay.org, meshbay ,") + + cfg = load_config(tmp_path / "absent.toml") + + assert cfg.captcha.host_check == frozenset({"meshbay.org", "meshbay"}) diff --git a/packages/meshbay-hub/tests/test_register_captcha.py b/packages/meshbay-hub/tests/test_register_captcha.py index befc1e2..d319137 100644 --- a/packages/meshbay-hub/tests/test_register_captcha.py +++ b/packages/meshbay-hub/tests/test_register_captcha.py @@ -17,7 +17,7 @@ def captcha_on(client, monkeypatch): monkeypatch.setattr(_cfg.captcha, "site_key", "test-site") monkeypatch.setattr(_cfg.captcha, "secret_key", "test-secret") - async def fake_verify(secret, token, remote_ip=None): + async def fake_verify(secret, token, remote_ip=None, allowed_hosts=None): return token == "good-token" monkeypatch.setattr("meshbay_hub.captcha.verify_captcha", fake_verify) |