diff --git a/README.md b/README.md index 443bca9..a5900cd 100644 --- a/README.md +++ b/README.md @@ -199,6 +199,9 @@ export STACKCHAIN_TRUSTED_PROXY_CIDRS='127.0.0.0/8' # three individual alerts followed by one private digest that opens Updates. A later # comment on an already-delivered thread triggers a fresh alert when Gitea advances # that thread's updated_at revision; unchanged and older snapshots remain silent. +# Browser push services must resolve exclusively to public IP addresses. Stackchain +# validates endpoints at enrollment and again before delivery, rejects redirects, +# and removes legacy subscriptions that resolve to private or reserved networks. export STACKCHAIN_VAPID_PUBLIC_KEY='' export STACKCHAIN_VAPID_PRIVATE_KEY='' export STACKCHAIN_VAPID_SUBJECT='mailto:ops@example.com' diff --git a/src/main.py b/src/main.py index 51dda31..e823f49 100644 --- a/src/main.py +++ b/src/main.py @@ -48,6 +48,7 @@ from src.live_snapshot_store import ( from src.models import Issue, Milestone, PullRequest, Repo, User from src.passkey_store import PasskeyStore from src.push_notifications import PushConfiguration, dispatch_unread_updates +from src.push_endpoint_policy import UnsafePushEndpoint, validate_public_push_endpoint from src.push_subscription_store import PushSubscriptionStore from src.request_boundary import RequestBodyLimitMiddleware, request_body_limit from src.security_event_store import SecurityEventStore, SecurityEventStoreError @@ -1808,6 +1809,10 @@ async def push_status(request: Request): async def subscribe_push(payload: PushSubscriptionPayload, request: Request): if not _push_configuration().enabled: raise HTTPException(status_code=503, detail="Push notifications are not configured") + try: + await validate_public_push_endpoint(payload.endpoint) + except UnsafePushEndpoint as error: + raise HTTPException(status_code=422, detail=str(error)) from error device_id = await dashboard_auth.session_management_id( request.state.dashboard_session ) diff --git a/src/push_endpoint_policy.py b/src/push_endpoint_policy.py new file mode 100644 index 0000000..a9cca10 --- /dev/null +++ b/src/push_endpoint_policy.py @@ -0,0 +1,62 @@ +import asyncio +import ipaddress +import socket +from collections.abc import Awaitable, Callable +from urllib.parse import urlsplit + + +class UnsafePushEndpoint(ValueError): + """Raised when a Web Push endpoint could reach a non-public service.""" + + +Resolver = Callable[[str, int], Awaitable[list[str]]] + + +async def _resolve(host: str, port: int) -> list[str]: + loop = asyncio.get_running_loop() + records = await loop.getaddrinfo( + host, + port, + family=socket.AF_UNSPEC, + type=socket.SOCK_STREAM, + ) + return sorted({record[4][0] for record in records}) + + +async def validate_public_push_endpoint( + endpoint: str, + *, + resolver: Resolver = _resolve, + timeout_seconds: float = 2.0, +) -> str: + """Return a canonical public HTTPS push endpoint or fail closed.""" + parsed = urlsplit(endpoint) + try: + port = parsed.port + except ValueError as error: + raise UnsafePushEndpoint( + "Endpoint must be a canonical public Web Push service URL" + ) from error + if ( + parsed.scheme != "https" + or not parsed.hostname + or parsed.username is not None + or parsed.password is not None + or parsed.fragment + or port not in {None, 443} + ): + raise UnsafePushEndpoint("Endpoint must be a canonical public Web Push service URL") + try: + addresses = await asyncio.wait_for( + resolver(parsed.hostname, port or 443), + timeout=max(0.01, timeout_seconds), + ) + except (OSError, asyncio.TimeoutError) as error: + raise UnsafePushEndpoint("Endpoint must resolve to a public Web Push service") from error + try: + public = addresses and all(ipaddress.ip_address(address).is_global for address in addresses) + except ValueError as error: + raise UnsafePushEndpoint("Endpoint must resolve to a public Web Push service") from error + if not public: + raise UnsafePushEndpoint("Endpoint must resolve to a public Web Push service") + return endpoint diff --git a/src/push_notifications.py b/src/push_notifications.py index 128a52d..99a05f2 100644 --- a/src/push_notifications.py +++ b/src/push_notifications.py @@ -6,6 +6,7 @@ from dataclasses import dataclass from typing import Awaitable, Callable from src.push_subscription_store import PushSubscriptionStore +from src.push_endpoint_policy import UnsafePushEndpoint, validate_public_push_endpoint @dataclass(frozen=True) @@ -22,8 +23,12 @@ class PushConfiguration: async def send_web_push( subscription: dict, payload: str, configuration: PushConfiguration ) -> None: + import requests from pywebpush import webpush + session = requests.Session() + session.max_redirects = 0 + await asyncio.to_thread( webpush, subscription_info=subscription, @@ -31,6 +36,8 @@ async def send_web_push( vapid_private_key=configuration.private_key, vapid_claims={"sub": configuration.subject}, ttl=300, + timeout=10, + requests_session=session, ) @@ -45,6 +52,7 @@ async def dispatch_unread_updates( send_timeout_seconds: float = 10.0, max_concurrency: int = 8, max_individual_notifications: int = 3, + endpoint_validator: Callable[[str], Awaitable[str]] | None = None, ) -> int: if not configuration.enabled: return 0 @@ -90,6 +98,15 @@ async def dispatch_unread_updates( async with semaphore: if ownership_lost.is_set(): return 0 + validate_endpoint = endpoint_validator + if validate_endpoint is None and send is None: + validate_endpoint = validate_public_push_endpoint + try: + if validate_endpoint is not None: + await validate_endpoint(delivery.subscription["endpoint"]) + except UnsafePushEndpoint: + await asyncio.to_thread(store.delete_session, delivery.session_id) + return 0 count = 0 digest_pending = set(delivery.digest_revisions) new_revisions = tuple( diff --git a/tests/test_push_endpoint_policy.py b/tests/test_push_endpoint_policy.py new file mode 100644 index 0000000..cba8ba7 --- /dev/null +++ b/tests/test_push_endpoint_policy.py @@ -0,0 +1,37 @@ +import pytest + +from src.push_endpoint_policy import UnsafePushEndpoint, validate_public_push_endpoint + + +@pytest.mark.anyio +@pytest.mark.parametrize( + "address", + ["127.0.0.1", "::1", "10.0.0.4", "169.254.169.254", "fc00::1"], +) +async def test_push_endpoint_rejects_every_non_public_resolved_address(address): + async def resolve(_host, _port): + return [address] + + with pytest.raises(UnsafePushEndpoint, match="public Web Push service"): + await validate_public_push_endpoint( + "https://push.example/device", resolver=resolve + ) + + +@pytest.mark.anyio +@pytest.mark.parametrize( + "endpoint", + [ + "http://push.example/device", + "https://user@push.example/device", + "https://push.example:8443/device", + "https://push.example:not-a-port/device", + "https://push.example/device#fragment", + ], +) +async def test_push_endpoint_rejects_noncanonical_authorities_without_resolving(endpoint): + async def must_not_resolve(_host, _port): + raise AssertionError("invalid URL reached DNS") + + with pytest.raises(UnsafePushEndpoint, match="canonical public Web Push"): + await validate_public_push_endpoint(endpoint, resolver=must_not_resolve) \ No newline at end of file diff --git a/tests/test_push_notifications.py b/tests/test_push_notifications.py index 4966971..babda53 100644 --- a/tests/test_push_notifications.py +++ b/tests/test_push_notifications.py @@ -10,6 +10,15 @@ import pytest from src import dashboard_auth, gitea_proxy, main from src.push_notifications import PushConfiguration, dispatch_unread_updates from src.push_subscription_store import PushSubscriptionStore +from src.push_endpoint_policy import UnsafePushEndpoint + + +@pytest.fixture(autouse=True) +def resolve_test_push_endpoints_publicly(monkeypatch): + async def accept(endpoint): + return endpoint + + monkeypatch.setattr(main, "validate_public_push_endpoint", accept) def test_dispatch_lease_is_exclusive_recoverable_and_owner_fenced(tmp_path): @@ -508,6 +517,32 @@ async def test_dispatch_removes_an_expired_push_endpoint(tmp_path): assert store.is_subscribed("session-a") is False +@pytest.mark.anyio +async def test_dispatch_removes_an_endpoint_that_rebinds_private_without_contacting_it(tmp_path): + store = PushSubscriptionStore(tmp_path / "push.sqlite3") + store.upsert("unsafe-device", { + "endpoint": "https://push.example/rebound", + "keys": {"p256dh": "public-key", "auth": "auth-secret"}, + }) + contacted = [] + + async def unread(): + return {"items": [{"id": 8}]} + + async def reject_rebound(_endpoint): + raise UnsafePushEndpoint("Endpoint must resolve to a public Web Push service") + + async def send(subscription, _payload): + contacted.append(subscription["endpoint"]) + + config = PushConfiguration("public", "private", "mailto:ops@example.com") + assert await dispatch_unread_updates( + store, config, unread, send, endpoint_validator=reject_rebound + ) == 0 + assert contacted == [] + assert store.is_subscribed("unsafe-device") is False + + @pytest.mark.anyio async def test_transient_endpoint_failure_does_not_block_healthy_devices(tmp_path): store = PushSubscriptionStore(tmp_path / "push.sqlite3") @@ -722,6 +757,37 @@ async def test_authenticated_device_can_subscribe_report_status_and_unsubscribe( assert (await main.push_status(request))["subscribed"] is False +@pytest.mark.anyio +async def test_subscription_rejects_an_unsafe_endpoint_before_persistence(tmp_path, monkeypatch): + store = PushSubscriptionStore(tmp_path / "push.sqlite3") + monkeypatch.setattr(main, "_push_subscription_store", store) + monkeypatch.setattr( + main, "_push_configuration", + lambda: PushConfiguration("public", "private", "mailto:ops@example.com"), + ) + + async def management_id(_session): + return "unsafe-device" + + async def reject_private(_endpoint): + raise UnsafePushEndpoint("Endpoint must resolve to a public Web Push service") + + monkeypatch.setattr(main.dashboard_auth, "session_management_id", management_id) + monkeypatch.setattr(main, "validate_public_push_endpoint", reject_private) + request = SimpleNamespace(state=SimpleNamespace(dashboard_session=object())) + payload = main.PushSubscriptionPayload( + endpoint="https://push.example/device-a", + keys={"p256dh": "public-key", "auth": "auth-secret"}, + ) + + with pytest.raises(main.HTTPException) as rejected: + await main.subscribe_push(payload, request) + + assert rejected.value.status_code == 422 + assert rejected.value.detail == "Endpoint must resolve to a public Web Push service" + assert store.is_subscribed("unsafe-device") is False + + @pytest.mark.anyio async def test_subscription_baselines_every_revision_from_the_complete_snapshot(tmp_path, monkeypatch): store = PushSubscriptionStore(tmp_path / "push.sqlite3")