From 7988da2b937841f48a112e142a127571b2485cc9 Mon Sep 17 00:00:00 2001 From: ducoterra Date: Sun, 1 Nov 2026 17:47:31 -0500 Subject: [PATCH] feat: decide who may connect at all, by source address MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mirror has no accounts, so on a real network the address a connection arrives from is not a nicety — it is the entire access control. Before this change the answer was "everyone who can open a TCP connection", which is fine for a laptop demo and alarming for a box someone leaves running. `mirror serve --allow-cidr 10.2.0.0/22` (or MIRROR_ALLOW_CIDRS) now restricts pages, the JSON API and the signaling socket to the networks named, and it is strict where it needs to be: a malformed CIDR exits 2 with one line instead of starting a server whose list silently failed to parse, because the two outcomes of that typo are "nobody can connect" and "everybody can connect". Empty remains the only permissive state, on purpose. The rules that matter live in src/mirror/access.py and are the ones that get either outcome: * X-Forwarded-For is believed only from MIRROR_TRUSTED_PROXIES (loopback by default), and only its left-most value. An untrusted peer that sends the header gets judged on its own address — there is a test for exactly that, because it is the shape of a bypass. * The socket is closed before it accepts, so a refused address never creates a roster entry to clean up behind it. * An address that cannot be parsed fails closed, and a v6 address is never "inside" a v4 network (comparing the two raises TypeError, which would be a 500 on a dual-stack host). * /healthz is exempt: the container health probe comes from loopback, which a production list otherwise excludes. The refused client is told why, and which address it was judged as — "403" in front of a proxy is undiagnosable otherwise. Browsers get a page, fetches get JSON, and the page is self-contained because a refused address cannot load /static/app.css any more than it can load a page. net.js keys its own notice on the /api/config 403 and stops: without that the pill reads "offline, reconnecting in 32s" forever for a decision that will never change. The static files drop their only inline script and style attribute (the landing page's counter is /static/home.js, its fine print is .fineprint) so the deployment can send `script-src 'self'; style-src 'self'` with no 'unsafe-inline'. That policy was verified through a real nginx edge carrying the exact header set — capture through SRTP to decoded, painted frames at 1280x720, eight distinct, zero securitypolicyviolation events — which also caught the edge sending `Cache-Control: no-store, no-store` and needed proxy_hide_header, since nginx appends rather than replaces. 24 tests added; 176 pass, 2 skipped, pyright and ruff clean. LLM-Generated: true --- .env.example | 14 ++++ src/mirror/__main__.py | 40 ++++++++-- src/mirror/access.py | 146 ++++++++++++++++++++++++++++++++++ src/mirror/app.py | 102 +++++++++++++++++++++++- src/mirror/config.py | 16 ++++ src/mirror/static/app.css | 11 +++ src/mirror/static/home.js | 33 ++++++++ src/mirror/static/index.html | 34 ++------ src/mirror/static/net.js | 56 ++++++++++++- src/mirror/static/share.js | 11 ++- src/mirror/static/watch.js | 16 ++-- tests/e2e/conftest.py | 38 +++++++-- tests/e2e/test_live_mirror.py | 33 ++++++++ tests/test_access.py | 120 ++++++++++++++++++++++++++++ tests/test_app.py | 107 +++++++++++++++++++++++++ tests/test_config.py | 49 ++++++++++++ 16 files changed, 771 insertions(+), 55 deletions(-) create mode 100644 src/mirror/access.py create mode 100644 src/mirror/static/home.js create mode 100644 tests/test_access.py diff --git a/.env.example b/.env.example index d61e5d8..81ca9de 100644 --- a/.env.example +++ b/.env.example @@ -19,6 +19,20 @@ MIRROR_PING_INTERVAL=20 MIRROR_ICE_SERVERS= # MIRROR_ICE_SERVERS=[{"urls":"stun://192.168.1.1:3478"}] +# Who may connect. Unset (the default) accepts any address, which is right for a +# laptop demo and wrong for anything other people can reach. Once one network is +# listed, everything else is refused — including this machine's own address, so +# when a proxy sits in front, allow the client networks here AND in nginx. +# Loopback is exempt from all of this only for /healthz, which is what the +# container health check probes. +MIRROR_ALLOW_CIDRS= +# MIRROR_ALLOW_CIDRS=10.2.0.0/22,fd00::/64 + +# Which peers may speak for a client through X-Forwarded-For. Default is loopback, +# correct when nginx runs on the same host and *overwrites* the header +# (proxy_set_header X-Forwarded-For $remote_addr). Only add a proxy you control. +MIRROR_TRUSTED_PROXIES= + # Developer only: attach a debugpy listener before serving. DEBUGPY=0 DEBUGPY_PORT=5678 diff --git a/src/mirror/__main__.py b/src/mirror/__main__.py index d69dd39..529ad2a 100644 --- a/src/mirror/__main__.py +++ b/src/mirror/__main__.py @@ -11,6 +11,7 @@ from dataclasses import replace from typing import Any, get_args from . import __version__, protocol +from .access import ConfigError, format_cidrs, parse_cidrs def _positive_int(raw: str) -> int: @@ -28,7 +29,8 @@ def build_parser() -> argparse.ArgumentParser: prog="mirror", description="Device-agnostic screen mirroring, served entirely from the browser.", epilog="Environment: MIRROR_HOST, MIRROR_PORT, MIRROR_ICE_SERVERS, " - "MIRROR_MAX_BITRATE, MIRROR_MAX_MIRRORS, DEBUGPY.", + "MIRROR_MAX_BITRATE, MIRROR_MAX_MIRRORS, MIRROR_ALLOW_CIDRS, " + "MIRROR_TRUSTED_PROXIES, DEBUGPY.", ) parser.add_argument("--version", action="version", version=f"mirror {__version__}") sub = parser.add_subparsers(dest="command") @@ -41,6 +43,15 @@ def build_parser() -> argparse.ArgumentParser: help="Bind address. Defaults to $MIRROR_HOST, or 0.0.0.0, which reaches other devices.", ) serve.add_argument("--port", type=_positive_int, default=None, help="Bind port.") + serve.add_argument( + "--allow-cidr", + action="append", + default=None, + metavar="NET", + help="Only serve connections from this address or CIDR network, e.g. " + "10.2.0.0/22. Repeatable, and it replaces $MIRROR_ALLOW_CIDRS. " + "Unset means unrestricted.", + ) serve.add_argument( "--role-hint", choices=list(get_args(protocol.Role)), @@ -63,12 +74,22 @@ def cmd_serve(args: argparse.Namespace) -> int: from .config import Settings, lan_addresses from .debug import maybe_enable_debugpy - settings = Settings.from_env() overrides: dict[str, Any] = {} - if args.host is not None: - overrides["host"] = args.host - if args.port is not None: - overrides["port"] = args.port + try: + settings = Settings.from_env() + if args.host is not None: + overrides["host"] = args.host + if args.port is not None: + overrides["port"] = args.port + if args.allow_cidr is not None: + # Replacing rather than appending: two sources of truth about who may + # connect is how a server ends up open by accident. + overrides["allow_cidrs"] = parse_cidrs(",".join(args.allow_cidr), source="--allow-cidr") + except ConfigError as exc: + # One line, no traceback: this is a typo in a deployment file, not a bug. + print(f"mirror: {exc}", file=sys.stderr) + return 2 + if overrides: settings = replace(settings, **overrides) @@ -84,6 +105,13 @@ def cmd_serve(args: argparse.Namespace) -> int: print(f" from another device: http://{addr}:{settings.port}", flush=True) print(" share a screen -> /mirror", flush=True) print(" watch everything -> /watch", flush=True) + if settings.allow_cidrs: + print(f" open to: {format_cidrs(settings.allow_cidrs)}", flush=True) + else: + print( + " open to: any address (restrict with --allow-cidr / MIRROR_ALLOW_CIDRS)", + flush=True, + ) uvicorn.run( app, diff --git a/src/mirror/access.py b/src/mirror/access.py new file mode 100644 index 0000000..dfcb276 --- /dev/null +++ b/src/mirror/access.py @@ -0,0 +1,146 @@ +"""Source-address admission control. + +mirror has no accounts — the point of the tool is that anyone in the room can +share and watch, and an account system would defeat that. What makes that safe +is *where* it runs: the address a connection arrives from is the access control, +and this module is the one place that decides it, for pages, the JSON API and +the signaling socket alike. + +Two rules are worth stating because they are the ones that get someone locked +out or let something in: + +* **An empty allow list means no restriction.** That is the development default, + and it is deliberately the *only* permissive state. Once a single network is + listed everything else is refused, including this machine's own address. +* **A proxy is only believed when it is on the trusted list**, and then only the + left-most ``X-Forwarded-For`` value is used. In the fleet deployment nginx + runs on the same host as this container and sets the header to + ``$remote_addr`` (overwriting, never appending), so loopback is the only + thing that can honestly claim to have seen the real client and a browser + cannot invent its own history. + +Reach the media is not: WebRTC runs browser-to-browser and never touches this +process, so admitting a peer to the roster lets it *arrange* a stream, which is +the same thing the roster is for. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from ipaddress import ( + IPv4Address, + IPv4Network, + IPv6Address, + IPv6Network, + ip_address, + ip_network, +) + +Address = IPv4Address | IPv6Address +Network = IPv4Network | IPv6Network + +#: Proxies we believe when they speak for a client. See the module docstring. +DEFAULT_TRUSTED_PROXIES = "127.0.0.0/8, ::1/128" + +#: Close code for "you may not use this service" (RFC 6455 policy violation). +POLICY_VIOLATION = 1008 + + +class ConfigError(ValueError): + """A CIDR list was unusable. + + Raised while parsing configuration, never while serving a request: a typo + here means either "nobody can connect" or "everybody can connect", and + neither is something to discover quietly at runtime. + """ + + +def _address(value: str | None) -> Address | None: + """Parse an address, or None when it is not one.""" + if not value: + return None + try: + return ip_address(value.strip()) + except ValueError: + return None + + +def parse_cidrs(raw: str | None, *, source: str = "MIRROR_ALLOW_CIDRS") -> tuple[Network, ...]: + """Parse a comma or whitespace separated list of networks. + + Accepts bare addresses (``10.2.1.5`` becomes ``10.2.1.5/32``) and + normalises a host part that does not fit its mask (``10.2.1.5/22`` becomes + the ``10.2.0.0/22`` it was meant to be) because that is what someone + pasting their laptop's address and netmask means. Anything else raises. + """ + out: list[Network] = [] + for chunk in (raw or "").replace(";", ",").replace("\n", ",").split(","): + chunk = chunk.strip() + if not chunk: + continue + try: + network = ip_network(chunk, strict=False) + except ValueError as exc: + raise ConfigError( + f"{source}: {chunk!r} is not an IP address or CIDR network " + "(e.g. 10.2.0.0/22, fd00::/64, 10.2.1.5)" + ) from exc + if network not in out: + out.append(network) + return tuple(out) + + +def format_cidrs(networks: tuple[Network, ...]) -> str: + """A short, readable rendering for status pages and error messages.""" + return ", ".join(str(network) for network in networks) + + +@dataclass(frozen=True, slots=True) +class Gate: + """Decides whether a peer address may use this server.""" + + allow: tuple[Network, ...] = () + trusted_proxies: tuple[Network, ...] = () + + @property + def enabled(self) -> bool: + """False when any address is welcome (nothing was listed).""" + return bool(self.allow) + + @staticmethod + def _inside(ip: Address | None, networks: tuple[Network, ...]) -> bool: + if ip is None: + return False + # Version matters: an IPv6 address is never "inside" an IPv4 network, and + # comparing the two is a TypeError rather than a False. + return any(ip.version == network.version and ip in network for network in networks) + + def client_address(self, peer: str | None, forwarded: str | None = None) -> str | None: + """The address to judge, unwrapping one trusted proxy hop. + + The socket peer wins unless it is a trusted proxy *and* a usable address + arrived in ``X-Forwarded-For``, in which case the left-most value is the + client. An unparseable header is ignored rather than fatal: a broken + proxy should not lock an otherwise valid client out of its own tool. + """ + peer_address = _address(peer) + if not self._inside(peer_address, self.trusted_proxies) or not forwarded: + return peer + for part in forwarded.split(","): + if _address(part) is not None: + return part.strip() + return peer + + def permits(self, address: str | None) -> bool: + """Whether ``address`` may connect. Fails closed when unknown.""" + if not self.allow: + return True + ip = _address(address) + if ip is None: + return False + return self._inside(ip, self.allow) + + def check(self, peer: str | None, forwarded: str | None = None) -> tuple[bool, str | None]: + """``(allowed, judged_address)`` — the shape the HTTP and WS paths want.""" + address = self.client_address(peer, forwarded) + return self.permits(address), address diff --git a/src/mirror/app.py b/src/mirror/app.py index 43c78c8..969eda4 100644 --- a/src/mirror/app.py +++ b/src/mirror/app.py @@ -8,6 +8,7 @@ from __future__ import annotations import json from contextlib import suppress +from html import escape from pathlib import Path from typing import Any @@ -18,6 +19,7 @@ from pydantic import ValidationError from starlette.websockets import WebSocketDisconnected from . import __version__, protocol +from .access import POLICY_VIOLATION, Gate, format_cidrs from .config import Settings from .registry import Envelope, Registry, RegistryError @@ -27,11 +29,73 @@ STATIC_DIR = Path(__file__).parent / "static" #: anything bigger is noise or an attack, and this server has no authentication. MAX_MESSAGE_BYTES = 64 * 1024 +#: A roster changes second to second, and a refusal must never be cached by an +#: intermediary on behalf of an address that is allowed. +_NO_STORE = {"Cache-Control": "no-store"} + def _page(name: str) -> FileResponse: return FileResponse(STATIC_DIR / name, media_type="text/html") +def _refusal_html(address: str | None, gate: Gate) -> str: + """A self-contained refusal page. + + It cannot link to /static/app.css — a refused address cannot fetch it any + more than it can fetch a page — so the styling is inline. It says which + address was judged, because "you are not allowed" without that is undiagnosable + when a proxy is in front of you and has rewritten it. + """ + allowed = format_cidrs(gate.allow) + you = escape(address or "unknown") + return f""" + + + + + mirror — not open to this address + + + +
+

This mirror is not open to your address

+

+ It accepts connections from {escape(allowed)}. Your request was + judged as {you}. +

+

+ If you reached it through a proxy, the address shown is what the proxy reported + — check its allow/deny list and the + X-Forwarded-For it sends. +

+
+ + +""" + + class Hub: """Owns the sockets and delivers the roster's outbound envelopes.""" @@ -201,6 +265,7 @@ def create_app(settings: Settings | None = None) -> FastAPI: settings = settings or Settings.from_env() registry = Registry(max_streams=settings.max_streams) hub = Hub(registry) + gate = settings.gate() app = FastAPI( title="mirror", @@ -253,13 +318,46 @@ def create_app(settings: Settings | None = None) -> FastAPI: @app.websocket("/ws") async def signaling(websocket: WebSocket) -> None: + if not gate.permits( + gate.client_address( + websocket.client.host if websocket.client else None, + websocket.headers.get("x-forwarded-for"), + ) + ): + # Close before accepting, so there is no session to tear down and no + # roster entry to clean behind it. The page explains itself: the same + # address already got a 403 from /api/config. + await websocket.close(code=POLICY_VIOLATION, reason="address not allowed") + return await hub.serve(websocket) PAGES = ("/", "/mirror", "/share", "/watch") @app.middleware("http") - async def no_store_for_pages(request: Request, call_next): # type: ignore[no-untyped-def] - """A mirror roster changes second to second; never let a browser cache it.""" + async def admission_and_cache_headers(request: Request, call_next): # type: ignore[no-untyped-def] + """Refuse addresses outside the allowed space; never let a roster be cached. + + `/healthz` is exempt: the container health check comes from loopback and a + probe that reports counts is not information an address filter is for. + """ + if gate.enabled and request.url.path != "/healthz": + allowed, address = gate.check( + request.client.host if request.client else None, + request.headers.get("x-forwarded-for"), + ) + if not allowed: + payload = { + "error": "forbidden", + "message": "This mirror only accepts connections from " + f"{format_cidrs(gate.allow)}.", + "you": address, + } + if "text/html" in request.headers.get("accept", ""): + return HTMLResponse( + _refusal_html(address, gate), status_code=403, headers=_NO_STORE + ) + return JSONResponse(payload, status_code=403, headers=_NO_STORE) + response = await call_next(request) if request.url.path in PAGES: response.headers["Cache-Control"] = "no-store" diff --git a/src/mirror/config.py b/src/mirror/config.py index aae5268..b71370d 100644 --- a/src/mirror/config.py +++ b/src/mirror/config.py @@ -8,6 +8,8 @@ import socket from dataclasses import dataclass, field from typing import Any +from .access import DEFAULT_TRUSTED_PROXIES, Gate, Network, parse_cidrs + DEFAULT_HOST = "0.0.0.0" DEFAULT_PORT = 8055 DEFAULT_MAX_BITRATE_BPS = 12_000_000 @@ -56,6 +58,15 @@ class Settings: max_bitrate_bps: int = DEFAULT_MAX_BITRATE_BPS max_streams: int = 0 ping_interval: float = 20.0 + #: Source networks that may use this server. Empty means unrestricted. + allow_cidrs: tuple[Network, ...] = () + trusted_proxies: tuple[Network, ...] = parse_cidrs( + DEFAULT_TRUSTED_PROXIES, source="MIRROR_TRUSTED_PROXIES" + ) + + def gate(self) -> Gate: + """The admission rule for this server, derived from the settings.""" + return Gate(allow=self.allow_cidrs, trusted_proxies=self.trusted_proxies) @classmethod def from_env(cls) -> Settings: @@ -72,6 +83,11 @@ class Settings: max_bitrate_bps=_env_int("MIRROR_MAX_BITRATE", DEFAULT_MAX_BITRATE_BPS), max_streams=_env_int("MIRROR_MAX_MIRRORS", 0), ping_interval=float(_env_int("MIRROR_PING_INTERVAL", 20)), + allow_cidrs=parse_cidrs(os.environ.get("MIRROR_ALLOW_CIDRS")), + trusted_proxies=parse_cidrs( + os.environ.get("MIRROR_TRUSTED_PROXIES") or DEFAULT_TRUSTED_PROXIES, + source="MIRROR_TRUSTED_PROXIES", + ), ) def client_config(self) -> dict[str, Any]: diff --git a/src/mirror/static/app.css b/src/mirror/static/app.css index 3f3b0d7..c7a101c 100644 --- a/src/mirror/static/app.css +++ b/src/mirror/static/app.css @@ -88,6 +88,9 @@ a { color: var(--accent); } .net[data-state="online"] i { background: #37d67a; box-shadow: 0 0 8px rgba(55, 214, 122, 0.7); } .net[data-state="connecting"] i { animation: pulse 1.1s ease-in-out infinite; } .net[data-state="offline"] i { background: var(--live); } +/* Refused by the server's address list. Deliberately not pulsing: pulsing means + "we are trying", and this is a decision, not an outage. */ +.net[data-state="blocked"] i { background: var(--warn); box-shadow: 0 0 8px rgba(245, 165, 36, 0.55); } @keyframes pulse { 50% { opacity: 0.25; } } @@ -204,6 +207,14 @@ a { color: var(--accent); } font-size: 14px; } +/* A class rather than a style attribute, so `style-src 'self'` needs no + 'unsafe-inline'. */ +.fineprint { + margin-top: 26px; + color: var(--fg-dim); + font-size: 13.5px; +} + /* ------------------------------------------------------------------ share */ .share { diff --git a/src/mirror/static/home.js b/src/mirror/static/home.js new file mode 100644 index 0000000..f7bf8dd --- /dev/null +++ b/src/mirror/static/home.js @@ -0,0 +1,33 @@ +/* The landing page's live counter. + * + * This is a separate file rather than an inline + diff --git a/src/mirror/static/net.js b/src/mirror/static/net.js index 3a9e9c1..9a7a856 100644 --- a/src/mirror/static/net.js +++ b/src/mirror/static/net.js @@ -20,6 +20,20 @@ export async function loadConfig() { const fallback = { iceServers: [], maxBitrateBps: 12000000, pingIntervalMs: 20000, version: "?" }; try { const res = await fetch("/api/config", { cache: "no-store" }); + if (res.status === 403) { + // The server refused this address. Nothing else on the page can work — the + // signaling socket is closed before it accepts — so carry the reason + // instead of letting the page look healthy and retry forever. + const detail = await res.json().catch(() => ({})); + cachedConfig = { + ...fallback, + refused: true, + refusedMessage: + detail.message || "This mirror only accepts connections from addresses it trusts.", + refusedYou: detail.you || null, + }; + return cachedConfig; + } cachedConfig = res.ok ? { ...fallback, ...(await res.json()) } : fallback; } catch { cachedConfig = fallback; @@ -27,6 +41,28 @@ export async function loadConfig() { return cachedConfig; } +/** + * A `showNotice`-shaped explanation of a refusal, or null when we were accepted. + * + * @param {Object} config the value loadConfig() resolved with + */ +export function refusalNotice(config) { + if (!config || !config.refused) return null; + return { + ok: false, + code: "address_not_allowed", + title: "This mirror is not open to your address", + body: + escapeHtml(config.refusedMessage || "") + + (config.refusedYou ? ` You appear to be ${escapeHtml(config.refusedYou)}.` : ""), + bullets: [ + "Ask whoever runs this mirror to add your address or network to its allow list.", + "Behind a proxy, the address shown is the one the proxy reported — check its " + + "allow/deny list.", + ], + }; +} + /** PeerConnection configuration. On a LAN we deliberately ship no STUN/TURN: * host candidates reach each other directly, and we do not want to announce * internal addresses to an external STUN server. */ @@ -107,10 +143,11 @@ const BACKOFF_MAX_MS = 8000; /** * Open the signaling socket and keep it open. - * @param {{onMessage: Function, onOpen?: Function, onClose?: Function, onStatus?: Function}} handlers + * @param {{onMessage: Function, onOpen?: Function, onClose?: Function, onStatus?: Function, + * refused?: boolean, refusedReason?: string}} handlers */ export function connectSignaling(handlers) { - const { onMessage, onOpen, onClose, onStatus } = handlers; + const { onMessage, onOpen, onClose, onStatus, refused, refusedReason } = handlers; let ws = null; let attempts = 0; let closedByUs = false; @@ -152,6 +189,21 @@ export function connectSignaling(handlers) { }; } + // A refused address is not a transient outage: opening the socket would only + // produce a close we would retry through, burying the reason in backoff noise. + if (refused) { + status("blocked", refusedReason || "this address is not allowed"); + return { + send() { + return false; + }, + get open() { + return false; + }, + close() {}, + }; + } + open(); return { diff --git a/src/mirror/static/share.js b/src/mirror/static/share.js index 509eb88..4481556 100644 --- a/src/mirror/static/share.js +++ b/src/mirror/static/share.js @@ -11,6 +11,7 @@ import { deviceLabel, escapeHtml, loadConfig, + refusalNotice, rtcConfig, showNotice, toast, @@ -365,6 +366,8 @@ function hello() { } state.ws = connectSignaling({ + refused: !!config.refused, + refusedReason: config.refusedMessage, onOpen() { hello(); // A reconnect means the server forgot us; republish whatever is live. @@ -420,7 +423,13 @@ document.addEventListener("keydown", (event) => { if (event.key === "Escape" && state.stream) stop(); }); -if (!captureSupport().ok) { +// A refused address outranks every other diagnosis: capture would work and the +// viewers would never arrive, so say the true thing first. +const refusal = refusalNotice(config); +if (refusal) { + showNotice(ui.notice, refusal); + ui.start.disabled = true; +} else if (!captureSupport().ok) { showNotice(ui.notice, captureSupport()); ui.start.disabled = true; } diff --git a/src/mirror/static/watch.js b/src/mirror/static/watch.js index b11b485..208ef29 100644 --- a/src/mirror/static/watch.js +++ b/src/mirror/static/watch.js @@ -2,14 +2,7 @@ * stage — the tile element itself moves into the stage container, so the * RTCPeerConnection and the