diff options
Diffstat (limited to 'packages')
| -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 |
5 files changed, 243 insertions, 3 deletions
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) |