Security hardening from the assessment; testing fixes
Security (ultimo-security-report.md):
- Security headers on every response, the app shell included: they lived at
server level and nginx dropped them in any location with its own header.
CSP, framing, nosniff, referrer, permissions (docker/nginx-security.conf).
- Plain HTTP forwarded by the reverse proxy is redirected to HTTPS; HSTS on
HTTPS responses. server_tokens off.
- Cross-site state-changing /api requests refused (login CSRF), WebSocket too.
- Validation errors in the usual error shape, without the request model.
- New-password policy: max 128, common/own-address/repeated refused,
breached-password check (HIBP k-anonymity, fails open).
- Message HTML: links keep only http(s)/mailto (sanitizer v6); the HTML
endpoint carries its own no-script CSP.
- Outbound fetches pinned to the checked IP (DNS rebinding); external
IMAP/SMTP servers re-checked before each connection; CGNAT and mapped
addresses counted as non-public.
Fixes from testing:
- IDLE loop catches up before idling: mail that came while the worker was
away no longer waits up to 10 minutes.
- Restore from Spam/Trash/Archive moves only the removed messages, not your
reply in Sent.
- Narrow desktop list stacks sender/subject/snippet (subject was squeezed
to nothing at the default width), with hover actions.
- Phone: wide emails scale to fit the screen.
- Singular forms ("1 message"); no zip button for a single attachment;
app passwords no longer claim "never used" for mail apps.
- ruff format across the backend.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5.5
@@ -0,0 +1,285 @@
|
||||
"""Fixes from the 2026-10-14 security assessment (ultimo-security-report.md):
|
||||
cross-site requests refused (login CSRF), terse validation errors, the new-password
|
||||
policy, link schemes in message HTML, and DNS-rebinding-proof outbound fetches."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import uuid
|
||||
from collections.abc import AsyncIterator
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
import pytest_asyncio
|
||||
from httpx import ASGITransport
|
||||
from starlette.datastructures import Headers
|
||||
|
||||
from app.accounts import ssrf
|
||||
from app.config import get_settings
|
||||
from app.identity import password_policy
|
||||
from app.identity.password_policy import PasswordRejected, check_new_password
|
||||
from app.indexer.sanitize import sanitize_html
|
||||
from app.main import create_app
|
||||
from app.security.cross_site import is_cross_site
|
||||
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def client() -> AsyncIterator[httpx.AsyncClient]:
|
||||
async with httpx.AsyncClient(
|
||||
transport=ASGITransport(app=create_app()), base_url="http://mail.example.org"
|
||||
) as c:
|
||||
yield c
|
||||
|
||||
|
||||
def _login(client: httpx.AsyncClient, headers: dict[str, str]): # type: ignore[no-untyped-def]
|
||||
return client.post(
|
||||
"/api/auth/login", json={"handle": "nobody", "password": "x"}, headers=headers
|
||||
)
|
||||
|
||||
|
||||
# --- cross-site requests (finding 3) --------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"headers",
|
||||
[
|
||||
{"Sec-Fetch-Site": "cross-site", "Origin": "https://evil.example"},
|
||||
{"Origin": "https://evil.example"}, # an older browser: Origin only
|
||||
{"Origin": "null"}, # a sandboxed frame
|
||||
],
|
||||
)
|
||||
async def test_sign_in_from_another_site_is_refused(
|
||||
client: httpx.AsyncClient, headers: dict[str, str]
|
||||
) -> None:
|
||||
resp = await _login(client, headers)
|
||||
assert resp.status_code == 403
|
||||
assert resp.json()["error"]["code"] == "cross_site"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"headers",
|
||||
[
|
||||
{"Sec-Fetch-Site": "same-origin", "Origin": "http://mail.example.org"},
|
||||
{"Origin": "http://mail.example.org"},
|
||||
{"Origin": "https://mail.example.org:8443"}, # the port does not matter
|
||||
{"Sec-Fetch-Site": "same-site"}, # our own subdomain
|
||||
{"Sec-Fetch-Site": "none"}, # typed into the address bar
|
||||
{}, # not a browser (scripts, mail apps)
|
||||
],
|
||||
)
|
||||
async def test_sign_in_from_our_own_pages_goes_through(
|
||||
client: httpx.AsyncClient, headers: dict[str, str]
|
||||
) -> None:
|
||||
resp = await _login(client, headers)
|
||||
assert resp.status_code == 401 # evaluated: wrong credentials, not refused
|
||||
assert resp.json()["error"]["code"] == "invalid_credentials"
|
||||
|
||||
|
||||
async def test_reads_from_another_site_are_not_refused(client: httpx.AsyncClient) -> None:
|
||||
resp = await client.get("/api/auth/registration", headers={"Sec-Fetch-Site": "cross-site"})
|
||||
assert resp.status_code == 200
|
||||
|
||||
|
||||
def test_origin_on_our_own_domain_passes_whatever_the_host_header() -> None:
|
||||
domain = get_settings().app_domain
|
||||
headers = Headers({"origin": f"https://{domain}", "host": "localhost:8088"})
|
||||
assert is_cross_site(headers) is False
|
||||
sub = Headers({"origin": f"https://mail.{domain}", "host": "localhost:8088"})
|
||||
assert is_cross_site(sub) is False
|
||||
lookalike = Headers({"origin": f"https://evil{domain}", "host": "localhost:8088"})
|
||||
assert is_cross_site(lookalike) is True
|
||||
|
||||
|
||||
def test_origin_matches_the_name_a_reverse_proxy_saw() -> None:
|
||||
headers = Headers(
|
||||
{"origin": "https://ultimo.email", "host": "web:80", "x-forwarded-host": "ultimo.email"}
|
||||
)
|
||||
assert is_cross_site(headers) is False
|
||||
|
||||
|
||||
# --- validation errors (finding 5) ----------------------------------------------------
|
||||
|
||||
|
||||
async def test_validation_errors_do_not_spell_out_the_request_model(
|
||||
client: httpx.AsyncClient,
|
||||
) -> None:
|
||||
resp = await client.post("/api/auth/login", json={})
|
||||
assert resp.status_code == 422
|
||||
body = resp.json()
|
||||
assert "detail" not in body
|
||||
assert body["error"]["code"] == "validation_error"
|
||||
assert body["error"]["message"].startswith("handle:")
|
||||
|
||||
|
||||
async def test_a_too_short_password_says_so(client: httpx.AsyncClient) -> None:
|
||||
resp = await client.post("/api/auth/register", json={"localpart": "abcde", "password": "123"})
|
||||
assert resp.status_code == 422
|
||||
assert "password" in resp.json()["error"]["message"]
|
||||
|
||||
|
||||
async def test_new_passwords_are_capped_at_128(client: httpx.AsyncClient) -> None:
|
||||
resp = await client.post(
|
||||
"/api/auth/register", json={"localpart": "abcde", "password": "x" * 129}
|
||||
)
|
||||
assert resp.status_code == 422
|
||||
|
||||
|
||||
# --- new-password policy (finding 6) --------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"password",
|
||||
["password123", "Password123", "12345678", "aaaaaaaaaa", "abababab", "jsmith2026"],
|
||||
)
|
||||
async def test_guessable_passwords_are_refused(password: str) -> None:
|
||||
with pytest.raises(PasswordRejected):
|
||||
await check_new_password(password, "jsmith@example.org")
|
||||
|
||||
|
||||
async def test_a_decent_password_passes() -> None:
|
||||
await check_new_password("correct horse battery", "jsmith@example.org")
|
||||
|
||||
|
||||
async def test_breached_passwords_are_refused(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setattr(get_settings(), "password_breach_check", True)
|
||||
sha = "A" * 40
|
||||
|
||||
async def breached(_password: str) -> bool:
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(password_policy, "_breached", breached)
|
||||
with pytest.raises(PasswordRejected) as err:
|
||||
await check_new_password("long unusual phrase " + sha, "jsmith@example.org")
|
||||
assert err.value.code == "breached_password"
|
||||
|
||||
|
||||
async def test_the_breach_service_being_down_does_not_block_sign_up(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
def unreachable(request: httpx.Request) -> httpx.Response:
|
||||
raise httpx.ConnectError("down", request=request)
|
||||
|
||||
real_client = httpx.AsyncClient
|
||||
|
||||
def client_factory(*args, **kwargs): # type: ignore[no-untyped-def]
|
||||
return real_client(transport=httpx.MockTransport(unreachable))
|
||||
|
||||
monkeypatch.setattr(password_policy.httpx, "AsyncClient", client_factory)
|
||||
assert await password_policy._breached("anything at all") is False
|
||||
|
||||
|
||||
async def test_sign_up_with_a_common_password_is_refused(client: httpx.AsyncClient) -> None:
|
||||
resp = await client.post(
|
||||
"/api/auth/register",
|
||||
json={"localpart": f"pw{uuid.uuid4().hex[:8]}", "password": "password123"},
|
||||
)
|
||||
assert resp.status_code == 400
|
||||
assert resp.json()["error"]["code"] == "weak_password"
|
||||
|
||||
|
||||
# --- message HTML (V2) ----------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"html",
|
||||
[
|
||||
'<a href="data:text/html,<script>alert(1)</script>">x</a>',
|
||||
'<a href="javascript:alert(1)">x</a>',
|
||||
'<a href="jav	ascript:alert(1)">x</a>',
|
||||
'<a href="cid:part1">x</a>',
|
||||
"<img src=x onerror=alert(1)>",
|
||||
"<svg onload=alert(1)><circle/></svg>",
|
||||
"<math><mtext><table><mglyph><style><img src=x onerror=alert(1)>",
|
||||
'<noscript><p title="</noscript><img src=x onerror=alert(1)>">',
|
||||
'<iframe srcdoc="<script>alert(1)</script>"></iframe>',
|
||||
'<div style="width:expression(alert(1))">x</div>',
|
||||
'<meta http-equiv="refresh" content="0;url=https://evil.example">',
|
||||
'<base href="https://evil.example/">',
|
||||
'<form action="https://evil.example"><input type=password></form>',
|
||||
],
|
||||
)
|
||||
def test_no_active_content_survives(html: str) -> None:
|
||||
out = sanitize_html(html, uuid.uuid4()).html.lower()
|
||||
for marker in (
|
||||
"<script", "onerror", "onload", "javascript:", "data:text", "cid:", "expression(",
|
||||
"<iframe", "<meta", "<base", "<form", "<svg", "<math",
|
||||
): # fmt: skip
|
||||
assert marker not in out, (html, out)
|
||||
|
||||
|
||||
def test_ordinary_links_stay() -> None:
|
||||
out = sanitize_html(
|
||||
'<a href="https://example.com/x">w</a><a href="mailto:a@b.c">m</a>', uuid.uuid4()
|
||||
).html
|
||||
assert 'href="https://example.com/x"' in out and 'href="mailto:a@b.c"' in out
|
||||
|
||||
|
||||
async def test_the_message_document_carries_its_own_strict_policy() -> None:
|
||||
from app.api import messages
|
||||
|
||||
resp = messages._html("<p>x</p>")
|
||||
csp = resp.headers["content-security-policy"]
|
||||
assert "script-src 'none'" in csp and "sandbox" in csp
|
||||
|
||||
|
||||
# --- outbound fetches (V3) ------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"ip", ["100.64.0.1", "::ffff:127.0.0.1", "fd00::1", "169.254.169.254", "fe80::1%eth0"]
|
||||
)
|
||||
def test_more_non_public_addresses_are_recognised(ip: str) -> None:
|
||||
assert ssrf._is_public_ip(ip) is False
|
||||
|
||||
|
||||
async def test_requests_go_to_the_address_that_was_checked(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
answers = iter([["93.184.216.34"], ["127.0.0.1"]]) # a rebinding DNS server
|
||||
|
||||
async def resolve(_host: str) -> list[str]:
|
||||
return next(answers)
|
||||
|
||||
monkeypatch.setattr(ssrf, "_resolve_all", resolve)
|
||||
url, headers, extensions = await ssrf._pinned("https://cal.example.com:8443/dav/?a=1")
|
||||
assert url == "https://93.184.216.34:8443/dav/?a=1"
|
||||
assert headers == {"Host": "cal.example.com:8443"}
|
||||
assert extensions == {"sni_hostname": "cal.example.com"}
|
||||
|
||||
|
||||
async def test_pinning_prefers_ipv4(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
async def resolve(_host: str) -> list[str]:
|
||||
return ["2606:4700::6810:84e5", "104.16.132.229"]
|
||||
|
||||
monkeypatch.setattr(ssrf, "_resolve_all", resolve)
|
||||
url, _, _ = await ssrf._pinned("https://example.com/")
|
||||
assert url == "https://104.16.132.229/"
|
||||
|
||||
|
||||
async def test_mail_servers_are_checked_again_at_every_connection(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
from app.providers.base import ProviderConnectError
|
||||
from app.providers.imap_smtp import ImapSmtpProvider, ProviderConfig
|
||||
|
||||
async def resolve(_host: str) -> list[str]:
|
||||
return ["10.0.0.5"] # the name now points inside the network
|
||||
|
||||
monkeypatch.setattr(ssrf, "_resolve_all", resolve)
|
||||
provider = ImapSmtpProvider(
|
||||
ProviderConfig(
|
||||
imap_host="imap.example.com",
|
||||
imap_port=993,
|
||||
imap_tls="ssl",
|
||||
smtp_host="smtp.example.com",
|
||||
smtp_port=465,
|
||||
smtp_tls="ssl",
|
||||
username="u",
|
||||
password="p",
|
||||
public_only=True,
|
||||
)
|
||||
)
|
||||
with pytest.raises(ProviderConnectError):
|
||||
await asyncio.wait_for(provider.verify(), 5)
|
||||
with pytest.raises(ProviderConnectError):
|
||||
await asyncio.wait_for(provider.send(b"x", "u@example.com", ["v@example.com"]), 5)
|
||||
Reference in New Issue
Block a user