diff options
Diffstat (limited to 'packages')
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/api/users.py | 51 | ||||
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/mail.py | 119 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/conftest.py | 7 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_mail_is_not_a_relay.py | 313 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_recovery_email.py | 9 |
5 files changed, 486 insertions, 13 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/api/users.py b/packages/meshbay-hub/src/meshbay_hub/api/users.py index 05bf075..702d687 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/users.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/users.py @@ -130,6 +130,10 @@ class RefreshRequest(BaseModel): # ── Endpoints ───────────────────────────────────────────────────────────────── +# Between two verification mails to one pending account. +VERIFICATION_RESEND_COOLDOWN = 120 + + @router.post("/register", status_code=201) @limiter.limit("5/minute") async def register( @@ -147,9 +151,26 @@ async def register( if found.status == "pending" and found.email_hash == eh: # Same person retrying before validation — resend a code. # No captcha: the initial registration already passed it. - await _create_and_send_verification( - db, found, body.email, eh, body.recovery_key) - await db.commit() + # + # Which made this the widest of the three mail doors: no token, no + # captcha, and the username and address are the caller's own from + # a moment ago. Registering a victim's address once bought the + # right to mail them at the endpoint's rate limit for ever. + # `mail.py` bounds the recipient regardless, and this stops the + # door being hammered — the answer is the same either way, which + # is the one this endpoint has always given. + recent = await db.execute( + select(EmailVerification).where( + EmailVerification.user_id == found.id, + EmailVerification.purpose == "registration", + EmailVerification.created_at + > datetime.now(timezone.utc) + - timedelta(seconds=VERIFICATION_RESEND_COOLDOWN), + )) + if not recent.first(): + await _create_and_send_verification( + db, found, body.email, eh, body.recovery_key) + await db.commit() return {"user_id": found.id, "email_verification_required": True} raise HTTPException(status_code=409, detail="Username already taken") @@ -640,6 +661,7 @@ class UpdateProfileRequest(BaseModel): # the account's, not the caller's IP: the point is the mailbox on the receiving # end, and an IP is not what fills it. EMAIL_CHANGE_COOLDOWN = 60 +MAX_EMAIL_CHANGES_PER_DAY = 3 @router.patch("/me") @@ -680,6 +702,27 @@ async def update_profile( status_code=429, detail="A code was just sent. Wait a minute before asking again.") + # And a ceiling per day, because the cooldown alone still allows one + # stranger a minute — 1440 a day, from one account, on a hub where + # registration is open. This is the only path where a signed-in + # account chooses the recipient, so it is the only one that needs it. + # + # Counted from the IP log, not from EmailVerification: the block below + # deletes this account's unverified rows before writing a new one, so + # counting those would have counted one, always. (Which is what the + # first version of this did.) The log entry is worth having for its + # own sake — this endpoint wrote none, alone among the ones that mail. + today = await db.execute( + select(func.count()).select_from(IPLog).where( + IPLog.user_id == current_user.id, + IPLog.event == "email_change_request", + IPLog.timestamp > datetime.now(timezone.utc) - timedelta(days=1), + )) + if (today.scalar() or 0) >= MAX_EMAIL_CHANGES_PER_DAY: + raise HTTPException( + status_code=429, + detail="Too many address changes today. Try again tomorrow.") + # Check that no other active/pending account uses this email dup = await db.execute( select(User).where( @@ -709,6 +752,8 @@ async def update_profile( user_id=current_user.id, expires_at=datetime.now(timezone.utc) + timedelta(seconds=VERIFICATION_TTL), )) + db.add(IPLog(user_id=current_user.id, event="email_change_request", + ip_address=client_ip(request))) await db.flush() await mail.send_off_loop(mail.send_email_change_code, new_email, code) pending_email = new_email diff --git a/packages/meshbay-hub/src/meshbay_hub/mail.py b/packages/meshbay-hub/src/meshbay_hub/mail.py index 8373204..5d13b8d 100644 --- a/packages/meshbay-hub/src/meshbay_hub/mail.py +++ b/packages/meshbay-hub/src/meshbay_hub/mail.py @@ -3,15 +3,107 @@ MeshBay Hub — email sending via localhost Postfix. Postfix listens on loopback only (inet_interfaces = loopback-only), so no authentication is needed. See docs/MAIL-SERVER.md for the full setup. + +**No authentication is needed** is exactly why everything below exists. The +hub can emit mail from its own domain to anywhere, and three API paths reach +that ability — two of them at an address the caller types. Unbounded, that is +an open relay wearing the instance's reputation, so the bounds are here, in +the one function every send passes through, rather than at the call sites +where the next one added would forget them (`AV9`–`AV10`, §13.5b). """ import asyncio +import hashlib import logging import smtplib +import time from email.message import EmailMessage log = logging.getLogger(__name__) + +class MailRefused(Exception): + """The gate below declined to send. Never carries the address.""" + + +# The complete list of reasons this hub will ever send mail. A `send_*` +# function that names anything else does not send — and one that names nothing +# is a TypeError, because `purpose` is keyword-required. +# +# registration confirm an address at sign-up +# password_reset a code to the address already on file for that account +# invite tell a registered member they were invited to a group +# email_change confirm a new address before it replaces the old one +# +# Only `registration` and `email_change` can reach an address this hub has no +# prior relationship with. Both are necessary; both are why the per-destination +# bound below is keyed on the recipient rather than on who asked. +ALLOWED_PURPOSES = frozenset({ + "registration", "password_reset", "invite", "email_change"}) + +# Per recipient, across every purpose, every account and every IP. This is the +# bound that matters: it is what a person being mail-bombed actually +# experiences, and no combination of accounts, addresses or endpoints moves it. +DESTINATION_COOLDOWN_SECONDS = 120 +DESTINATION_DAILY_CAP = 5 + +# Instance-wide ceiling. Registration is open, so "per account" is a bound an +# attacker buys more of; this one cannot be bought. Sized far above a real +# hub's traffic — meshbay.org sends single-digit mails a day — and low enough +# that being used as a relay is not worth the trouble. +HOURLY_BUDGET = 200 + +# Single-process state, like `_connected_nodes` and `_node_groups` next door: +# the hub serves on one worker (`server.workers` defaults to 1) and the +# signaling registries already require it. A restart clears these, which costs +# at most one burst and is not a security decision — unlike the denylist, +# which persists for exactly that reason (S3). +_destinations: dict[str, tuple[float, int, float]] = {} # key → (last, day count, day start) +_hour: tuple[float, int] = (0.0, 0) # (window start, count) + + +def _destination_key(address: str) -> str: + """A stable handle for one recipient that is not the address itself. + + Hashed because this dict is the one place in the hub that would otherwise + hold a list of plaintext addresses in memory — the rest of the codebase + goes to the trouble of encrypting them at rest (S2). + """ + return hashlib.sha256(address.strip().lower().encode()).hexdigest()[:32] + + +def _budget_or_refuse(purpose: str, address: str) -> None: + """Raise MailRefused unless this hub may send this, to this person, now.""" + global _hour + + if purpose not in ALLOWED_PURPOSES: + raise MailRefused(f"not a purpose this hub sends mail for: {purpose!r}") + + now = time.monotonic() + start, count = _hour + if now - start >= 3600: + start, count = now, 0 + if count >= HOURLY_BUDGET: + _hour = (start, count) + raise MailRefused("the hub's hourly mail budget is spent") + + key = _destination_key(address) + last, sent_today, day_start = _destinations.get(key, (0.0, 0, now)) + if now - day_start >= 86400: + sent_today, day_start = 0, now + if last and now - last < DESTINATION_COOLDOWN_SECONDS: + raise MailRefused("too soon since the last mail to this recipient") + if sent_today >= DESTINATION_DAILY_CAP: + raise MailRefused("this recipient has had its daily allowance") + + if len(_destinations) > 10_000: + for k, (_, _, started) in list(_destinations.items()): + if now - started >= 86400: + _destinations.pop(k, None) + + _destinations[key] = (now, sent_today + 1, day_start) + _hour = (start, count + 1) + _hub_domain: str = "meshbay.org" _hub_url: str = "https://meshbay.org" @@ -22,14 +114,27 @@ def configure(hub_id: str) -> None: _hub_url = f"https://{hub_id}" -def _send(msg: EmailMessage) -> bool: - """Blocking. Every caller in an async handler must use `send_off_loop`.""" +def _send(msg: EmailMessage, *, purpose: str) -> bool: + """Blocking. Every caller in an async handler must use `send_off_loop`. + + The gate is here rather than in `send_off_loop` so that it cannot be + stepped around: a new `send_*` helper, a script, a test — everything that + puts a message on the wire comes through this function, and `purpose` is + keyword-required so forgetting it is a TypeError rather than an + unrestricted send. + """ + try: + _budget_or_refuse(purpose, msg["To"] or "") + except MailRefused as refusal: + # Never the address: this line goes to the journal. + log.warning("Mail refused (%s): %s", purpose, refusal) + return False try: with smtplib.SMTP("localhost", 25, timeout=10) as s: s.send_message(msg) return True except Exception: - log.exception("Failed to send email to %s", msg["To"]) + log.exception("Failed to send email to %s", _mask_email(msg["To"] or "")) return False @@ -86,7 +191,7 @@ def send_verification_code(to: str, code: str, recovery_key: str | None = None) msg["To"] = to msg["Subject"] = f"MeshBay — Your verification code: {code}" msg.set_content(body) - _send(msg) + _send(msg, purpose="registration") log.info("Verification code sent to %s (recovery_key=%s)", _mask_email(to), bool(recovery_key)) @@ -106,7 +211,7 @@ def send_email_change_code(to: str, code: str) -> None: "\n" f"{_hub_url}\n" ) - _send(msg) + _send(msg, purpose="email_change") log.info("Email change code sent to %s", _mask_email(to)) @@ -132,7 +237,7 @@ def send_password_reset_code(to: str, code: str) -> None: "\n" f"{_hub_url}\n" ) - _send(msg) + _send(msg, purpose="password_reset") log.info("Passphrase reset code sent to %s", _mask_email(to)) @@ -153,7 +258,7 @@ def send_invite_notification( "\n" f"{_hub_url}\n" ) - _send(msg) + _send(msg, purpose="invite") log.info("Invite notification sent to %s", _mask_email(to)) diff --git a/packages/meshbay-hub/tests/conftest.py b/packages/meshbay-hub/tests/conftest.py index cb595c5..69c3c5b 100644 --- a/packages/meshbay-hub/tests/conftest.py +++ b/packages/meshbay-hub/tests/conftest.py @@ -95,7 +95,12 @@ def _skip_email_verification(monkeypatch): monkeypatch.setattr( "meshbay_hub.api.users._create_and_send_verification", _noop) - monkeypatch.setattr("meshbay_hub.mail._send", lambda msg: True) + # `**_` because `_send` takes a required keyword `purpose` — the gate + # that keeps the hub off the open-relay list. A stub with the old + # signature turns every send into a TypeError, which looks like a bug + # in the handler. `test_mail_is_not_a_relay.py` opts out of this and + # drives the real thing. + monkeypatch.setattr("meshbay_hub.mail._send", lambda msg, **_: True) @pytest.fixture(autouse=True) diff --git a/packages/meshbay-hub/tests/test_mail_is_not_a_relay.py b/packages/meshbay-hub/tests/test_mail_is_not_a_relay.py new file mode 100644 index 0000000..a56ab17 --- /dev/null +++ b/packages/meshbay-hub/tests/test_mail_is_not_a_relay.py @@ -0,0 +1,313 @@ +""" +The hub can send mail from its own domain, to anywhere, with no authentication +to its local Postfix. That is an open relay unless something bounds it, and +what bounds it has to be one place, or the next `send_*` helper added will not +have it. + +Three API paths reach that ability and two of them mail an address the caller +types: + + POST /v1/users/register an address nobody has verified + PATCH /v1/users/me an address nobody has verified, signed in + POST /v1/users/password/reset only the address already on file + POST /v1/groups/{id}/invite-notify only a registered member's address + +The widest was the register *resend* branch: no token, no captcha, and the +username and address are the caller's own from a moment ago, so registering a +victim's address once bought the right to mail them at the endpoint's rate +limit indefinitely. + +The bound that matters is per **recipient**, because that is what a person +being mail-bombed experiences, and no combination of accounts, addresses or +endpoints moves it. Per-account and per-IP limits sit above it and bound a +caller, which is a different and weaker thing. +""" + +import base64 + +import pytest +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey +from cryptography.hazmat.primitives.asymmetric.x25519 import X25519PrivateKey + +from meshbay_common.crypto import pk_to_b64 +from meshbay_hub import mail as mail_mod +from meshbay_hub.api import users as users_mod + +# Captured at import, before conftest's autouse fixture replaces them for every +# other test in the suite: one stubs `_send` out entirely and the other skips +# the registration mail. This file is about the code they stub. +_REAL_SEND = mail_mod._send +_REAL_CREATE_AND_SEND = users_mod._create_and_send_verification + + +@pytest.fixture(autouse=True) +def _real_mail_path(monkeypatch): + """Undo conftest's stubs, for the tests that go through the API. + + The direct tests below call `_REAL_SEND` rather than `mail_mod._send`: + depending on a fixture to *un*-patch a module global is a coupling that + passes alone and fails in a full run, which is how this file first behaved. + """ + monkeypatch.setattr(mail_mod, "_send", _REAL_SEND) + monkeypatch.setattr(users_mod, "_create_and_send_verification", + _REAL_CREATE_AND_SEND) + assert mail_mod._send is _REAL_SEND + + +@pytest.fixture(autouse=True) +def _fresh_budget(): + """Each test starts with the hub's allowance untouched.""" + mail_mod._destinations.clear() + mail_mod._hour = (0.0, 0) + yield + mail_mod._destinations.clear() + mail_mod._hour = (0.0, 0) + + +@pytest.fixture +def wire(monkeypatch): + """Everything that actually reaches Postfix, captured at the socket.""" + sent = [] + + class _FakeSMTP: + def __init__(self, *a, **kw): + pass + + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + def send_message(self, msg): + sent.append((msg["To"], msg["Subject"])) + + monkeypatch.setattr(mail_mod.smtplib, "SMTP", _FakeSMTP) + return sent + + +def _msg(to: str): + from email.message import EmailMessage + m = EmailMessage() + m["From"] = "noreply@test" + m["To"] = to + m["Subject"] = "test" + m.set_content("test") + return m + + +# ── The gate itself ────────────────────────────────────────────────────────── + +def test_a_send_must_name_a_purpose(wire): + """Keyword-required, so a helper that forgets is a TypeError rather than + an unrestricted send.""" + with pytest.raises(TypeError): + _REAL_SEND(_msg("someone@example.test")) + + +def test_only_the_four_purposes_send(wire): + assert _REAL_SEND(_msg("a@example.test"), purpose="registration") + assert not _REAL_SEND(_msg("b@example.test"), purpose="marketing") + assert not _REAL_SEND(_msg("c@example.test"), purpose="") + assert [to for to, _ in wire] == ["a@example.test"] + + +def test_one_recipient_has_a_cooldown_and_a_daily_allowance(wire): + """Across every purpose — the recipient is the thing being protected, not + the sender.""" + victim = "victim@example.test" + assert _REAL_SEND(_msg(victim), purpose="registration") + assert not _REAL_SEND(_msg(victim), purpose="password_reset"), ( + "a second purpose walked around the cooldown") + + # Past the cooldown, up to the day's allowance, then no more. + for _ in range(mail_mod.DESTINATION_DAILY_CAP - 1): + mail_mod._destinations[mail_mod._destination_key(victim)] = ( + 0.0, + mail_mod._destinations[mail_mod._destination_key(victim)][1], + mail_mod._destinations[mail_mod._destination_key(victim)][2]) + assert _REAL_SEND(_msg(victim), purpose="invite") + + mail_mod._destinations[mail_mod._destination_key(victim)] = ( + 0.0, mail_mod.DESTINATION_DAILY_CAP, + mail_mod._destinations[mail_mod._destination_key(victim)][2]) + assert not _REAL_SEND(_msg(victim), purpose="invite") + assert len(wire) == mail_mod.DESTINATION_DAILY_CAP + + +def test_the_same_recipient_is_one_recipient_however_it_is_written(wire): + """Case and surrounding space are not a new person.""" + assert _REAL_SEND(_msg("Victim@Example.Test"), purpose="registration") + assert not _REAL_SEND(_msg(" victim@example.test "), purpose="registration") + assert len(wire) == 1 + + +def test_the_instance_has_a_ceiling_no_account_can_buy_past(wire): + """Registration is open, so "per account" is a bound an attacker buys more + of. This one cannot be bought.""" + for i in range(mail_mod.HOURLY_BUDGET): + assert _REAL_SEND(_msg(f"u{i}@example.test"), purpose="registration") + assert not _REAL_SEND(_msg("one-more@example.test"), purpose="registration") + assert len(wire) == mail_mod.HOURLY_BUDGET + + +def test_a_refusal_never_names_the_address(): + """This line goes to the journal, which is not where addresses live. + + Listens on the module's own logger rather than through `caplog`: the hub + configures logging when an app is built, so whether pytest's capture sees + anything depends on which tests ran first. In the full suite it saw + nothing, and the assertion that had to hold — no address in the line — + was never reached. + """ + import logging + + records: list[str] = [] + + class _Capture(logging.Handler): + def emit(self, record): + records.append(record.getMessage()) + + # The refusal itself, which is what the log line is built from. + with pytest.raises(mail_mod.MailRefused) as refusal: + mail_mod._budget_or_refuse("marketing", "private@example.test") + assert "private@example.test" not in str(refusal.value) + + # And the line as emitted. Level and `logging.disable` are both forced: + # attaching a handler is not enough when an earlier test has raised the + # logger's level, which is why the first version of this saw nothing in + # the full suite and everything on its own. + handler = _Capture() + previous_level, previously_disabled = mail_mod.log.level, mail_mod.log.disabled + mail_mod.log.addHandler(handler) + mail_mod.log.setLevel(logging.WARNING) + # `.disabled` as well as the level and the manager-wide switch. Something + # earlier in a full run leaves this logger disabled — pytest's own capture + # plugin toggles loggers between tests — and a disabled logger drops the + # record before any handler sees it. Alone, none of this is needed; the + # first two versions of this test passed alone and failed in the suite for + # two different reasons, which is the whole argument for not reading a + # global's state in a test without pinning it first. + mail_mod.log.disabled = False + logging.disable(logging.NOTSET) + try: + _REAL_SEND(_msg("private@example.test"), purpose="marketing") + finally: + mail_mod.log.removeHandler(handler) + mail_mod.log.setLevel(previous_level) + mail_mod.log.disabled = previously_disabled + + assert records, "a refusal must leave a trace" + assert not any("private@example.test" in line for line in records) + assert any("marketing" in line for line in records), ( + "the trace has to say what was refused to be worth writing") + + +# ── The doors, driven through the API ──────────────────────────────────────── + +async def _register(client, username: str, email: str, captcha=None): + sk_ed, sk_x = Ed25519PrivateKey.generate(), X25519PrivateKey.generate() + return await client.post("/v1/users/register", json={ + "username": username, + "email": email, + "auth_key": base64.b64encode(b"k" * 32).decode(), + "pk_user_ed25519": pk_to_b64(sk_ed.public_key()), + "pk_user_x25519": pk_to_b64(sk_x.public_key()), + }) + + +@pytest.mark.asyncio +async def test_the_register_resend_branch_cannot_be_hammered(client, wire): + """ + No token and no captcha reach this branch. Registering a victim's address + once used to buy the right to mail them at the endpoint's rate limit for + as long as the account stayed pending. + """ + victim = "bombing-target@example.test" + r = await _register(client, "relay_a", victim) + assert r.status_code == 201, r.text + assert len(wire) == 1 + + for _ in range(5): + r = await _register(client, "relay_a", victim) + assert r.status_code == 201, r.text + assert len(wire) == 1, f"{len(wire)} mails to one address from one sign-up" + + +async def _signed_in(client, db_session, username: str, email: str) -> dict: + """Register, activate, sign in. + + Activated by hand because this file restores the real verification mail, + which conftest stubs out for the rest of the suite — so the account is + genuinely `pending` here, exactly as it would be in production, and a + pending account cannot log in. + """ + from sqlalchemy import select, update + + from meshbay_hub.db.models import User + + r = await _register(client, username, email) + assert r.status_code == 201, r.text + await db_session.execute( + update(User).where(User.username == username).values(status="active")) + await db_session.commit() + assert (await db_session.execute( + select(User.status).where(User.username == username))).scalar() == "active" + + r = await client.post("/v1/users/login", json={ + "username": username, + "auth_key": base64.b64encode(b"k" * 32).decode()}) + assert r.status_code == 200, r.text + return {"Authorization": f"Bearer {r.json()['access_token']}"} + + +@pytest.mark.asyncio +async def test_a_signed_in_account_cannot_mail_a_stranger_repeatedly( + client, db_session, wire, monkeypatch): + """ + `PATCH /v1/users/me` is the one path where a signed-in account chooses the + recipient. A cooldown alone still allowed one stranger a minute — 1440 a + day — so there is a ceiling per day above it. + """ + monkeypatch.setattr( + "meshbay_hub.api.users.EMAIL_CHANGE_COOLDOWN", 0) + + headers = await _signed_in(client, db_session, "relay_b", + "relay_b@example.test") + before = len(wire) + + accepted = 0 + for i in range(8): + r = await client.patch("/v1/users/me", headers=headers, + json={"email": f"stranger{i}@example.test"}) + if r.status_code == 200: + accepted += 1 + else: + assert r.status_code == 429, r.text + + from meshbay_hub.api.users import MAX_EMAIL_CHANGES_PER_DAY + assert accepted == MAX_EMAIL_CHANGES_PER_DAY, ( + f"{accepted} strangers mailed by one account in one day") + assert len(wire) - before == MAX_EMAIL_CHANGES_PER_DAY + + +@pytest.mark.asyncio +async def test_the_daily_ceiling_survives_the_row_being_deleted( + client, db_session, wire, monkeypatch): + """ + The change path deletes this account's unverified verification rows before + writing a new one, so a ceiling counted from that table counts one, always + — which is what the first version of this counted. It comes off the IP log. + """ + monkeypatch.setattr("meshbay_hub.api.users.EMAIL_CHANGE_COOLDOWN", 0) + headers = await _signed_in(client, db_session, "relay_c", + "relay_c@example.test") + + for i in range(3): + await client.patch("/v1/users/me", headers=headers, + json={"email": f"c-stranger{i}@example.test"}) + + r = await client.patch("/v1/users/me", headers=headers, + json={"email": "c-one-more@example.test"}) + assert r.status_code == 429, ( + "the ceiling was counted from a table the handler empties") diff --git a/packages/meshbay-hub/tests/test_recovery_email.py b/packages/meshbay-hub/tests/test_recovery_email.py index e4288a1..c6dab97 100644 --- a/packages/meshbay-hub/tests/test_recovery_email.py +++ b/packages/meshbay-hub/tests/test_recovery_email.py @@ -31,7 +31,12 @@ def _skip_email_verification(monkeypatch): run so the e-mail is actually built. Capture it instead of sending. """ sent = [] - monkeypatch.setattr("meshbay_hub.mail._send", lambda msg: sent.append(msg) or True) + # `**_` swallows the `purpose` keyword `_send` now requires — the gate + # that keeps this hub off the open-relay list. What this module is + # about is the body of the message, so the gate is stubbed away with + # the socket; `test_mail_is_not_a_relay.py` is where it is exercised. + monkeypatch.setattr("meshbay_hub.mail._send", + lambda msg, **_: sent.append(msg) or True) return sent @@ -84,7 +89,7 @@ async def test_the_recovery_key_is_not_persisted( def test_mail_body_with_and_without_the_key(monkeypatch): captured = [] monkeypatch.setattr("meshbay_hub.mail._send", - lambda msg: captured.append(msg) or True) + lambda msg, **_: captured.append(msg) or True) mail.send_verification_code("x@example.com", "123456", recovery_key="MY-RECOVERY-KEY") |