diff options
| -rw-r--r-- | docs/captcha.md | 59 | ||||
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/api/users.py | 1 | ||||
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/captcha.py | 28 | ||||
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/config.py | 11 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_captcha_host_check.py | 83 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_register_captcha.py | 3 | ||||
| -rw-r--r-- | packaging/conf/hub.toml.example | 16 |
7 files changed, 168 insertions, 33 deletions
diff --git a/docs/captcha.md b/docs/captcha.md index 84dadc4..4054e9f 100644 --- a/docs/captcha.md +++ b/docs/captcha.md @@ -415,7 +415,9 @@ solutions" on the key, and check the origin on the hub, where it belongs: [captcha] site_key = "6Le..." secret_key = "6Le..." -allowed_hosts = ["meshbay.org", "localhost", "meshbay"] +allowed_hosts = ["meshbay.org", "localhost"] +# Only with the desktop client. See below — this is the loose one. +allow_unattributed_host = true ``` `verify_captcha` then refuses a solve whose reported hostname is not in that @@ -430,20 +432,37 @@ 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. +### The hostname a desktop solve reports is empty, not `meshbay` -**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. +Built first as an allowlist entry, on the assumption that Google would report +the host component of the origin. It does not, and registration from the +client failed with `captcha_failed` while the checkbox was green — a worse +symptom than the one being fixed, because the widget now looked fine. The log +line said it outright: + +``` +captcha solved on an unexpected host ''; allowed: ['localhost', 'meshbay', 'meshbay.org'] +``` + +A solve Google cannot attribute to a domain reports an **empty** hostname. No +allowlist entry can match that, and an empty entry is not the answer either: +a blank in a TOML list is a typo far more often than an intention, and +`load_config` drops blanks for that reason. `allow_unattributed_host` is a +named flag instead, so the trade is stated where it is made. + +**What it admits, plainly.** Every non-web client, not only ours — a `file://` +page or somebody else's Electron application report the same nothing. That is +the same bar the client's own origin would have been (`main.js` already records +that `app://meshbay` is not a credential; any application can claim it), and it +*is* a bar: the captcha still has to be solved, per token, in something that +can render it. What is given up is the origin restriction for non-web clients, +not the captcha. A hub that does not ship the desktop client should leave the +flag off. + +**Reading the value yourself.** Any refusal is logged at WARNING, with the +hostname spelled out and the allowed list beside it when there is one. That is +how the empty hostname was found, and it is the way to check what a given +client actually reports rather than guess — which is what went wrong here. --- @@ -508,7 +527,9 @@ from Google — no npm package. site_key = "6Le..." secret_key = "6Le..." # Required whenever the console's origin check is off, and only then. - allowed_hosts = ["meshbay.org", "localhost", "meshbay"] + allowed_hosts = ["meshbay.org", "localhost"] + # Only with the desktop client — §6 says what it gives up. + allow_unattributed_host = true ``` 3. Deploy the new hub code (`deploy-hub.sh` — runs `alembic upgrade head` + restart; no migration needed for this change). @@ -517,6 +538,8 @@ 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. +6. Verify the desktop client by actually registering from it. Two distinct + failures, and the first hides the second: "Invalid domain for site key" + inside the widget means the console's origin check is still on, while a + green checkbox followed by `captcha_failed` means the hub refused it — the + WARNING in the journal says which host, or that there was none. diff --git a/packages/meshbay-hub/src/meshbay_hub/api/users.py b/packages/meshbay-hub/src/meshbay_hub/api/users.py index ece98cd..56208a0 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/users.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/users.py @@ -69,6 +69,7 @@ async def _verify_captcha_or_raise(token: str | None, request: Request) -> None: token, request.client.host if request.client else None, _cfg.captcha.host_check, # type: ignore[union-attr] + _cfg.captcha.allow_unattributed_host, # 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 74a77b6..cf33d52 100644 --- a/packages/meshbay-hub/src/meshbay_hub/captcha.py +++ b/packages/meshbay-hub/src/meshbay_hub/captcha.py @@ -14,6 +14,7 @@ async def verify_captcha( token: str, remote_ip: str | None = None, allowed_hosts: frozenset[str] | set[str] | None = None, + allow_unattributed: bool = False, ) -> bool: """True when Google accepts the token, and it came from a host we expect. @@ -33,6 +34,21 @@ async def verify_captcha( `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. + + `allow_unattributed` covers the case that made the desktop client fail + anyway. A solve from a page Google cannot attribute to a domain comes back + with an **empty** hostname, not the host part of the origin — measured + live: `app://meshbay` reports `''`, never `'meshbay'`. So an allowlist + entry can never match it, and an empty string cannot be an allowlist entry + either: a blank in a TOML list is a typo far more often than it is an + intention, and the config parser drops blanks for that reason. + + What it admits is every non-web client, not only ours — a `file://` page or + somebody else's Electron application report the same nothing. That is the + same bar the desktop client's own origin would have been (`app://meshbay` + is not a credential; any application can claim it), and it is a bar: the + captcha still has to be *solved*, per token. What is given up is the origin + restriction for non-web clients, not the captcha. """ payload: dict[str, str] = {"secret": secret_key, "response": token} if remote_ip: @@ -48,9 +64,17 @@ async def verify_captcha( 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. + # whose origin they have not seen before. It is how the empty + # one was found. host = result.get("hostname") or "" - if host not in allowed_hosts: + if not host: + if not allow_unattributed: + log.warning( + "captcha solved on a host Google did not attribute; " + "set captcha.allow_unattributed_host to admit " + "non-web clients such as the desktop application") + return False + elif host not in allowed_hosts: log.warning( "captcha solved on an unexpected host %r; allowed: %s", host, sorted(allowed_hosts)) diff --git a/packages/meshbay-hub/src/meshbay_hub/config.py b/packages/meshbay-hub/src/meshbay_hub/config.py index 39fdc52..e61e69e 100644 --- a/packages/meshbay-hub/src/meshbay_hub/config.py +++ b/packages/meshbay-hub/src/meshbay_hub/config.py @@ -74,6 +74,13 @@ class CaptchaConfig: # 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) + # A solve from a page Google cannot attribute to a domain reports an empty + # hostname — `app://meshbay` does, measured live, and so does any other + # non-web client. No `allowed_hosts` entry can match that, and a blank + # entry is not the answer: the parser drops blanks because in a TOML list + # a blank is a typo far more often than an intention. Hence a named flag, + # which also states the trade at the place it is made. + allow_unattributed_host: bool = False @property def enabled(self) -> bool: @@ -122,6 +129,8 @@ def load_config(path: Path | None = None) -> HubConfig: 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()] + if "allow_unattributed_host" in cap: + cfg.captcha.allow_unattributed_host = bool(cap["allow_unattributed_host"]) break # Env var overrides @@ -143,5 +152,7 @@ def load_config(path: Path | None = None) -> HubConfig: 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()] + if unattributed := os.environ.get("MESHBAY_CAPTCHA_ALLOW_UNATTRIBUTED_HOST"): + cfg.captcha.allow_unattributed_host = unattributed.lower() in ("1", "true", "yes") return cfg diff --git a/packages/meshbay-hub/tests/test_captcha_host_check.py b/packages/meshbay-hub/tests/test_captcha_host_check.py index bbd6c6d..68cad34 100644 --- a/packages/meshbay-hub/tests/test_captcha_host_check.py +++ b/packages/meshbay-hub/tests/test_captcha_host_check.py @@ -19,6 +19,13 @@ 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. + +The desktop client needed one more thing, found only by trying it. Its origin +is `app://meshbay`, and the guess was that Google would report the host part, +`meshbay`. It does not: a solve it cannot attribute to a domain comes back +with an **empty** hostname. `allow_unattributed_host` is what admits those, +and it is a named flag rather than an allowlist entry because a blank in a +TOML list is a typo far more often than an intention. """ import json @@ -79,12 +86,44 @@ async def test_a_solve_from_an_expected_host_is_accepted(google): @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) +async def test_an_unattributed_solve_is_refused_by_default(google): + """What the desktop client actually produces, and what it costs. + + `app://meshbay` does not report `meshbay`. Google attributes the solve to + no domain at all and the hostname comes back **empty** — measured live, + after the first attempt to allowlist `meshbay` failed. Refusing by default + is right: an answer with no hostname must not read as "any hostname". + """ + google(success=True, hostname="") + assert not await verify_captcha( + "s", "t", allowed_hosts=frozenset({"meshbay.org", "localhost"})) + + +@pytest.mark.asyncio +async def test_the_flag_admits_it_and_the_allowlist_is_not_how(google): + """The flag is the only way in, on purpose. + + An empty allowlist entry would be the obvious alternative and is a bad + one: a blank in a TOML list is a typo far more often than an intention, + and `load_config` drops blanks for exactly that reason. So this must not + work by accident from an allowlist alone. + """ + google(success=True, hostname="") + allowed = frozenset({"meshbay.org", "localhost"}) + assert await verify_captcha("s", "t", allowed_hosts=allowed, allow_unattributed=True) + assert not await verify_captcha("s", "t", allowed_hosts=allowed | {""}) + + +@pytest.mark.asyncio +async def test_the_flag_does_not_open_the_allowlist(google): + """It admits *unattributed* solves, not solves from somewhere else. + + A bot on its own web page still reports that page's hostname, which is + still not ours, flag or no flag. + """ + google(success=True, hostname="a-bot-farm.example") + assert not await verify_captcha( + "s", "t", allowed_hosts=frozenset({"meshbay.org"}), allow_unattributed=True) @pytest.mark.asyncio @@ -97,10 +136,12 @@ async def test_a_solve_from_somewhere_else_is_refused(google): @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".""" +async def test_an_absent_hostname_field_reads_like_an_empty_one(google): + """`hostname` missing and `hostname: ""` are the same answer.""" google(success=True) - assert not await verify_captcha("s", "t", allowed_hosts=frozenset({"meshbay.org"})) + allowed = frozenset({"meshbay.org"}) + assert not await verify_captcha("s", "t", allowed_hosts=allowed) + assert await verify_captcha("s", "t", allowed_hosts=allowed, allow_unattributed=True) @pytest.mark.asyncio @@ -143,6 +184,15 @@ async def test_a_transport_failure_is_a_refusal_not_an_exception(monkeypatch): assert not await verify_captcha("s", "t", allowed_hosts=frozenset({"meshbay.org"})) +@pytest.mark.asyncio +async def test_the_flag_alone_checks_nothing_without_an_allowlist(google): + """`allowed_hosts` empty still means "do not check", flag or no flag — + the whole block is skipped and reCAPTCHA is doing the origin check.""" + google(success=True, hostname="somewhere.example") + assert await verify_captcha("s", "t", allow_unattributed=False) + assert await verify_captcha("s", "t", allow_unattributed=True) + + 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.""" @@ -179,6 +229,21 @@ def test_a_hub_toml_without_the_key_checks_nothing(tmp_path): assert load_config(cfg_file).captcha.host_check is None +def test_the_unattributed_flag_comes_from_the_config(tmp_path, monkeypatch): + cfg_file = tmp_path / "hub.toml" + cfg_file.write_text( + '[captcha]\nsite_key = "k"\nsecret_key = "s"\n' + 'allowed_hosts = ["meshbay.org"]\nallow_unattributed_host = true\n') + assert load_config(cfg_file).captcha.allow_unattributed_host is True + + cfg_file.write_text('[captcha]\nsite_key = "k"\nsecret_key = "s"\n') + assert load_config(cfg_file).captcha.allow_unattributed_host is False, ( + "the default has to stay off — it is the looser of the two") + + monkeypatch.setenv("MESHBAY_CAPTCHA_ALLOW_UNATTRIBUTED_HOST", "true") + assert load_config(cfg_file).captcha.allow_unattributed_host is True + + 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 ,") diff --git a/packages/meshbay-hub/tests/test_register_captcha.py b/packages/meshbay-hub/tests/test_register_captcha.py index d319137..32663fe 100644 --- a/packages/meshbay-hub/tests/test_register_captcha.py +++ b/packages/meshbay-hub/tests/test_register_captcha.py @@ -17,7 +17,8 @@ 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, allowed_hosts=None): + async def fake_verify(secret, token, remote_ip=None, allowed_hosts=None, + allow_unattributed=False): return token == "good-token" monkeypatch.setattr("meshbay_hub.captcha.verify_captcha", fake_verify) diff --git a/packaging/conf/hub.toml.example b/packaging/conf/hub.toml.example index d647707..7f5ac8f 100644 --- a/packaging/conf/hub.toml.example +++ b/packaging/conf/hub.toml.example @@ -66,8 +66,18 @@ secret_key = "" # and is served from `app://meshbay`, so the hostname Google sees is not this # hub's and never can be; with the console check on, the widget shows # "Invalid domain for site key" and nothing client-side reaches that decision. -# Add the client's own host only if you distribute it — it is the weak entry, -# since any Electron application can claim the same scheme and host. # -# allowed_hosts = ["hub.example.org", "localhost", "meshbay"] +# allowed_hosts = ["hub.example.org", "localhost"] allowed_hosts = [] + +# A solve Google cannot attribute to a domain reports an *empty* hostname — +# the desktop client's `app://` origin does, and so does any other non-web +# client. No `allowed_hosts` entry matches that, hence a flag rather than a +# blank list entry. +# +# What it admits is every non-web client, not only this project's: a file:// +# page or somebody else's Electron application look identical from here. The +# captcha still has to be solved per token; what is given up is the origin +# restriction for those clients. Leave it off unless you ship the desktop +# client. See docs/captcha.md §6. +allow_unattributed_host = false |