From 7edc79971bfdab194929327e2bd28851b33e29c1 Mon Sep 17 00:00:00 2001 From: StasonJatham Date: Wed, 20 May 2026 18:17:20 +0200 Subject: [PATCH] Harden admin operations and API docs --- .env.example | 2 + README.md | 15 +- router_dyndns/ddns.py | 322 ++++++++++++++++++++++++++++++++--- router_dyndns/ddns_models.py | 4 + router_dyndns/ddns_store.py | 87 ++++++++++ tests/test_ddns.py | 106 +++++++++++- 6 files changed, 511 insertions(+), 25 deletions(-) diff --git a/.env.example b/.env.example index dccd6d7..b359b1e 100644 --- a/.env.example +++ b/.env.example @@ -9,6 +9,8 @@ DDNS_CLOUDFLARE_API_TOKEN=change-me-cloudflare-token DDNS_CLOUDFLARE_ZONE_ID=change-me-cloudflare-zone-id DDNS_TTL=60 DDNS_RATE_LIMIT_PER_MINUTE=60 +DDNS_ADMIN_RATE_LIMIT_PER_MINUTE=20 +DDNS_MAX_REQUEST_BODY_BYTES=16384 DDNS_CLEANUP_CHALLENGE_HOURS=72 DDNS_CLEANUP_UNUSED_ACCOUNT_HOURS=168 DDNS_CLEANUP_INTERVAL_SECONDS=3600 diff --git a/README.md b/README.md index 393a96a..fdd5c5a 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ That means users always have a current hostname for VPN, remote access, self-hos | Persistence | SQLite with WAL mode | | API | FastAPI, OpenAPI, Swagger UI, ReDoc | | Auth model | No user accounts. Private bearer links for hostnames and custom-domain claims | -| Operations | Rate limiting, update logs, cleanup jobs, Docker, Caddy example | +| Operations | Rate limiting, admin audit views, update logs, cleanup jobs, Docker, Caddy example | ## Quick Start @@ -92,6 +92,8 @@ Open: Admin auth uses HTTP Basic auth. The username can be any value; the password is `DDNS_ADMIN_PASSWORD`. +The public OpenAPI schema documents the developer API under `/api/v1`. Admin/operator endpoints are intentionally omitted from Swagger UI and ReDoc. + ## Production Setup For an internet-facing DynDNS service, run behind HTTPS and require a real DNS backend: @@ -103,6 +105,9 @@ export DDNS_DATABASE='/data/ddns.sqlite3' export DDNS_REQUIRE_DNS_PROVIDER=1 export DDNS_DNS_ZONES='ddns.example.net' export DDNS_TRUSTED_PROXY_IPS='127.0.0.1,::1' +export DDNS_RATE_LIMIT_PER_MINUTE=60 +export DDNS_ADMIN_RATE_LIMIT_PER_MINUTE=20 +export DDNS_MAX_REQUEST_BODY_BYTES=16384 export DDNS_CLEANUP_CHALLENGE_HOURS=72 export DDNS_CLEANUP_UNUSED_ACCOUNT_HOURS=168 export DDNS_CLEANUP_INTERVAL_SECONDS=3600 @@ -207,14 +212,18 @@ docker compose up -d --build - Use a long random value for `DDNS_ADMIN_PASSWORD`. - Use a least-privilege DNS API token or TSIG key. - Configure `DDNS_TRUSTED_PROXY_IPS` only for reverse proxies that strip user-supplied forwarding headers. +- Keep `/docs` and `/redoc` available for API users if you want a public developer surface. Admin routes are hidden from OpenAPI and still require Basic auth. - Leave cleanup enabled. The app periodically removes abandoned TXT challenges and generated hostnames that were never updated by a router. - Back up `/data/ddns.sqlite3`. - Run one Uvicorn worker with SQLite. The per-host update lock is process-local. -Cleanup defaults: +Security and cleanup defaults: | Variable | Default | Purpose | | --- | ---: | --- | +| `DDNS_RATE_LIMIT_PER_MINUTE` | `60` | Public update/API/form requests allowed per source IP and path. | +| `DDNS_ADMIN_RATE_LIMIT_PER_MINUTE` | `20` | Admin/operator requests allowed per source IP and path. | +| `DDNS_MAX_REQUEST_BODY_BYTES` | `16384` | Maximum accepted request body for form/API endpoints. | | `DDNS_CLEANUP_CHALLENGE_HOURS` | `72` | Delete unverified DNS TXT claims after this many hours. | | `DDNS_CLEANUP_UNUSED_ACCOUNT_HOURS` | `168` | Delete generated hostnames that never received a router update after this many hours. | | `DDNS_CLEANUP_INTERVAL_SECONDS` | `3600` | Run the in-app cleanup scheduler at this interval. | @@ -243,6 +252,6 @@ Self-hosted DynDNS, self hosted DDNS, Dynamic DNS server, FRITZ!Box DynDNS, FRIT ## Security -The repository intentionally contains only placeholders in `.env.example` and documentation. Do not commit a real `.env`, DNS token, TSIG secret, admin password, session secret, database, or router-generated update URL. +The repository intentionally contains only placeholders in `.env.example` and documentation. Do not commit a real `.env`, DNS token, TSIG secret, admin password, database, magic management link, domain claim URL, or router-generated update URL. If you find a vulnerability, open a private advisory or contact the repository owner before publishing details. diff --git a/router_dyndns/ddns.py b/router_dyndns/ddns.py index 2cdd511..cc5d396 100644 --- a/router_dyndns/ddns.py +++ b/router_dyndns/ddns.py @@ -14,6 +14,7 @@ from typing import Annotated from fastapi import Depends, FastAPI, Header, HTTPException, Query, Request, Response from fastapi.responses import FileResponse, HTMLResponse, PlainTextResponse, RedirectResponse from starlette.concurrency import run_in_threadpool +from starlette.types import ASGIApp, Message, Receive, Scope, Send from .ddns_api import create_api_router from .ddns_dns import delete_dns_records, dns_backend_configured, publish_dns @@ -55,15 +56,43 @@ OPENAPI_TAGS = [ "name": "management", "description": "Bearer-link management for generated hostnames.", }, - { - "name": "admin", - "description": "Operator-only HTML and JSON endpoints protected by the admin password.", - }, ] ASSET_DIR = Path(__file__).with_name("assets") +class RequestBodyTooLargeError(Exception): + pass + + +class RequestBodyLimitMiddleware: + def __init__(self, app: ASGIApp, max_body_bytes: int) -> None: + self.app = app + self.max_body_bytes = max_body_bytes + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + if scope["type"] != "http": + await self.app(scope, receive, send) + return + + received = 0 + + async def limited_receive() -> Message: + nonlocal received + message = await receive() + if message["type"] == "http.request": + received += len(message.get("body", b"")) + if received > self.max_body_bytes: + raise RequestBodyTooLargeError + return message + + try: + await self.app(scope, limited_receive, send) + except RequestBodyTooLargeError: + response = PlainTextResponse("request body too large", status_code=413) + await response(scope, receive, send) + + def make_app(settings: DdnsSettings | None = None) -> FastAPI: settings = settings or DdnsSettings.from_env() if not settings.admin_password and settings.shared_secret: @@ -98,17 +127,29 @@ def make_app(settings: DdnsSettings | None = None) -> FastAPI: openapi_tags=OPENAPI_TAGS, lifespan=lifespan, ) + app.add_middleware(RequestBodyLimitMiddleware, max_body_bytes=settings.max_request_body_bytes) app.include_router(create_api_router(settings, store, service)) @app.middleware("http") async def security_middleware(request: Request, call_next): - if is_rate_limited_path(request.url.path): + try: + content_length = int(request.headers.get("content-length", "0") or "0") + except ValueError: + content_length = 0 + if content_length > settings.max_request_body_bytes: + return PlainTextResponse("request body too large", status_code=413) + + if is_rate_limited_path(request.url.path) or _is_admin_path(request.url.path): + limit = settings.admin_rate_limit_per_minute if _is_admin_path(request.url.path) else settings.rate_limit_per_minute key = f"{client_ip(request, settings)}:{request.url.path}" - allowed = await run_in_threadpool(store.allow_rate_limit, key, settings.rate_limit_per_minute) + allowed = await run_in_threadpool(store.allow_rate_limit, key, limit) if not allowed: return PlainTextResponse("rate limit exceeded", status_code=429) response = await call_next(request) + if _is_secret_response(request): + response.headers["Cache-Control"] = "no-store" + response.headers["Pragma"] = "no-cache" response.headers["X-Content-Type-Options"] = "nosniff" response.headers["X-Frame-Options"] = "DENY" response.headers["Referrer-Policy"] = "same-origin" @@ -169,7 +210,7 @@ def make_app(settings: DdnsSettings | None = None) -> FastAPI: @app.post("/magic", response_class=HTMLResponse) async def magic_account(request: Request) -> str: - form = _parse_form(await request.body()) + form = await _request_form(request, settings) username = _first_form_value(form, "username") or None account = await run_in_threadpool(service.create_managed_account, username) return _render_public_home(service, settings, account, None) @@ -218,7 +259,7 @@ def make_app(settings: DdnsSettings | None = None) -> FastAPI: @app.post("/request-domain", response_class=HTMLResponse) async def request_domain(request: Request) -> str: - form = _parse_form(await request.body()) + form = await _request_form(request, settings) try: challenge = await run_in_threadpool( service.create_domain_challenge, @@ -230,7 +271,7 @@ def make_app(settings: DdnsSettings | None = None) -> FastAPI: @app.post("/verify-domain", response_class=HTMLResponse) async def verify_domain(request: Request) -> str: - form = _parse_form(await request.body()) + form = await _request_form(request, settings) domain = normalize_domain(_first_form_value(form, "domain")) claim_secret = _first_form_value(form, "claim_secret") found, challenge = await run_in_threadpool(service.verify_domain, domain, claim_secret) @@ -241,7 +282,7 @@ def make_app(settings: DdnsSettings | None = None) -> FastAPI: @app.post("/accounts", response_class=HTMLResponse) async def self_service_account(request: Request) -> str: - form = _parse_form(await request.body()) + form = await _request_form(request, settings) mode = _first_form_value(form, "mode") username = _first_form_value(form, "username") or None if mode == "managed": @@ -256,13 +297,22 @@ def make_app(settings: DdnsSettings | None = None) -> FastAPI: ) return _render_public_home(service, settings, account, None) - @app.get("/admin", response_class=HTMLResponse) + @app.get("/admin", response_class=HTMLResponse, include_in_schema=False) def admin(_: None = Depends(authenticate_admin)) -> str: - return _render_admin_page(service, settings, store.list_accounts(), None) + return _render_admin_page( + service, + settings, + store.list_accounts(), + store.list_domain_challenges(), + store.list_cleanup_runs(), + store.list_update_events(25), + None, + None, + ) - @app.post("/admin/accounts") + @app.post("/admin/accounts", include_in_schema=False) async def admin_create_account(request: Request, _: None = Depends(authenticate_admin)) -> Response: - form = _parse_form(await request.body()) + form = await _request_form(request, settings) require_admin_csrf(settings, _first_form_value(form, "csrf")) hostname = normalize_hostname(_first_form_value(form, "hostname"), settings) username = _first_form_value(form, "username") or None @@ -274,11 +324,22 @@ def make_app(settings: DdnsSettings | None = None) -> FastAPI: account = store.create_account(hostname, username) except sqlite3.IntegrityError as exc: raise HTTPException(status_code=409, detail="hostname already exists") from exc - return HTMLResponse(_render_admin_page(service, settings, store.list_accounts(), account)) + return HTMLResponse( + _render_admin_page( + service, + settings, + store.list_accounts(), + store.list_domain_challenges(), + store.list_cleanup_runs(), + store.list_update_events(25), + account, + None, + ) + ) - @app.post("/admin/accounts/delete") + @app.post("/admin/accounts/delete", include_in_schema=False) async def admin_delete_account(request: Request, _: None = Depends(authenticate_admin)) -> Response: - form = _parse_form(await request.body()) + form = await _request_form(request, settings) require_admin_csrf(settings, _first_form_value(form, "csrf")) hostname = _first_form_value(form, "hostname") if hostname: @@ -288,11 +349,79 @@ def make_app(settings: DdnsSettings | None = None) -> FastAPI: return PlainTextResponse("DNS delete failed; hostname was not deleted", status_code=500) return RedirectResponse("/admin", status_code=303) - @app.get("/records") + @app.post("/admin/accounts/rotate", include_in_schema=False) + async def admin_rotate_account(request: Request, _: None = Depends(authenticate_admin)) -> Response: + form = await _request_form(request, settings) + require_admin_csrf(settings, _first_form_value(form, "csrf")) + hostname = normalize_hostname(_first_form_value(form, "hostname"), settings, expand_suffix=False) + action = _first_form_value(form, "action") + account = store.get_account_by_hostname(hostname) + if not account: + raise HTTPException(status_code=404, detail="hostname not found") + + notice = None + if action == "update": + rotated = store.rotate_update_slug(hostname) + notice = _admin_secret_panel("Update URL rotated", [_copy_row("Update-URL:", service.fritz_update_url(rotated))]) if rotated else None + elif action == "management": + rotated = store.rotate_management_slug(hostname) + notice = _admin_secret_panel("Management link rotated", [_copy_row("Magic management link:", service.magic_management_url(rotated))]) if rotated else None + elif action == "password": + rotated = store.rotate_password(hostname) + if rotated: + notice = _admin_secret_panel( + "Router password rotated", + [ + _copy_row("Domainnamen:", str(rotated["hostname"])), + _copy_row("Benutzername:", str(rotated["username"])), + _copy_row("Kennwort:", str(rotated["password"])), + ], + ) + else: + raise HTTPException(status_code=400, detail="invalid rotation action") + return HTMLResponse( + _render_admin_page( + service, + settings, + store.list_accounts(), + store.list_domain_challenges(), + store.list_cleanup_runs(), + store.list_update_events(25), + None, + notice, + ) + ) + + @app.post("/admin/cleanup", include_in_schema=False) + async def admin_run_cleanup(request: Request, _: None = Depends(authenticate_admin)) -> Response: + form = await _request_form(request, settings) + require_admin_csrf(settings, _first_form_value(form, "csrf")) + result = await run_in_threadpool(store.cleanup, settings.cleanup_challenge_hours, settings.cleanup_unused_account_hours) + notice = _admin_secret_panel( + "Cleanup completed", + [ + _copy_row("Domain claims removed", str(result["domain_challenges"])), + _copy_row("Unused hostnames removed", str(result["unused_accounts"])), + ], + ) + return HTMLResponse( + _render_admin_page( + service, + settings, + store.list_accounts(), + store.list_domain_challenges(), + store.list_cleanup_runs(), + store.list_update_events(25), + None, + notice, + ) + ) + + @app.get("/records", include_in_schema=False) def records(_: None = Depends(authenticate_admin)) -> list[dict[str, str | None]]: return store.list_records() - @app.get("/events") + @app.get("/events", include_in_schema=False) def events(_: None = Depends(authenticate_admin)) -> list[dict[str, str | None]]: return store.list_update_events() @@ -365,10 +494,30 @@ def _parse_form(body: bytes) -> dict[str, list[str]]: return urllib.parse.parse_qs(body.decode("utf-8"), keep_blank_values=True) +async def _request_form(request: Request, settings: DdnsSettings) -> dict[str, list[str]]: + body = await request.body() + if len(body) > settings.max_request_body_bytes: + raise HTTPException(status_code=413, detail="request body too large") + return _parse_form(body) + + def _first_form_value(form: dict[str, list[str]], key: str) -> str: return form.get(key, [""])[0].strip() +def _is_admin_path(path: str) -> bool: + return path == "/admin" or path.startswith("/admin/") or path in {"/records", "/events"} + + +def _is_secret_response(request: Request) -> bool: + path = request.url.path + if _is_admin_path(path) or path.startswith(("/m/", "/d/")): + return True + if path.startswith(("/api/v1/hostnames/", "/api/v1/domains/")): + return request.method in {"POST", "DELETE"} + return request.method == "POST" and path in {"/magic", "/request-domain", "/verify-domain", "/accounts"} + + def _render_public_home( service: DdnsService, settings: DdnsSettings, @@ -559,6 +708,21 @@ def _created_account_panel(service: DdnsService, created_account: dict[str, str] return _credentials_panel(service, created_account) +def _admin_secret_panel(title: str, rows: list[str]) -> str: + return f""" +
+
+

Admin action

+

{html.escape(title)}

+

Copy this now. Secret values are only shown in this response.

+
+ {"".join(rows)} +
+
+
+ """ + + def _copy_row(label: str, value: str) -> str: safe_label = html.escape(label) safe_value = html.escape(value) @@ -659,11 +823,24 @@ def _render_admin_page( service: DdnsService, settings: DdnsSettings, accounts: list[dict[str, str | int | None]], + domain_challenges: list[dict[str, str | None]], + cleanup_runs: list[dict[str, str | int]], + update_events: list[dict[str, str | None]], created_account: dict[str, str] | None, + notice_html: str | None, ) -> str: csrf = html.escape(admin_csrf_token(settings)) rows = "\n".join(_account_row(account, csrf=csrf) for account in accounts) or """ - No hostnames yet. + No hostnames yet. + """ + domain_rows = "\n".join(_domain_claim_row(challenge) for challenge in domain_challenges) or """ + No domain claims yet. + """ + cleanup_rows = "\n".join(_cleanup_row(run) for run in cleanup_runs) or """ + No cleanup runs yet. + """ + event_rows = "\n".join(_event_row(event) for event in update_events) or """ + No update events yet. """ suffix_help = ( f"Short names are expanded under {html.escape(settings.hostname_suffix)}." @@ -684,6 +861,21 @@ def _render_admin_page( {_created_account_panel(service, created_account)} + {notice_html or ""} + +
+
+
+

Service health

+

Scheduled cleanup

+

Cleanup runs every {html.escape(str(settings.cleanup_interval_seconds))} seconds. It removes unverified domain claims after {html.escape(str(settings.cleanup_challenge_hours))} hours and never-used generated hostnames after {html.escape(str(settings.cleanup_unused_account_hours))} hours.

+
+
+ + +
+
+
@@ -718,13 +910,64 @@ def _render_admin_page(
- + {rows}
DomainUserIPv4IPv6Updated
DomainUserIPv4IPv6UpdatedLinks
+ +
+
+
+
+

Domain claims

+

Verification lifecycle

+
+
+
+ + + {domain_rows} +
DomainStatusCreatedVerified
+
+
+
+ +
+
+
+
+

Cleanup

+

Recent runs

+
+
+
+ + + {cleanup_rows} +
TimeDomain claims removedUnused hostnames removed
+
+
+
+ +
+
+
+
+

Updates

+

Recent events

+
+
+
+ + + {event_rows} +
TimeDomainStatusIPv4IPv6Detail
+
+
+
""", ) @@ -739,6 +982,15 @@ def _account_row(account: dict[str, str | int | None], csrf: str = "") -> str: return f""" {hostname}{username}{ipv4}{ipv6}{updated} + +
+ + + + + +
+
@@ -750,6 +1002,31 @@ def _account_row(account: dict[str, str | int | None], csrf: str = "") -> str: """ +def _domain_claim_row(challenge: dict[str, str | None]) -> str: + domain = html.escape(str(challenge["domain"])) + created = html.escape(str(challenge.get("created_at") or "-")) + verified = html.escape(str(challenge.get("verified_at") or "-")) + status = "Verified" if challenge.get("verified_at") else "Pending" + return f"{domain}{status}{created}{verified}" + + +def _cleanup_row(run: dict[str, str | int]) -> str: + created = html.escape(str(run.get("created_at") or "-")) + domains = html.escape(str(run.get("domain_challenges_deleted") or 0)) + accounts = html.escape(str(run.get("unused_accounts_deleted") or 0)) + return f"{created}{domains}{accounts}" + + +def _event_row(event: dict[str, str | None]) -> str: + created = html.escape(str(event.get("created_at") or "-")) + hostname = html.escape(str(event.get("hostname") or "-")) + status = html.escape(str(event.get("status") or "-")) + ipv4 = html.escape(str(event.get("ipv4") or "-")) + ipv6 = html.escape(str(event.get("ipv6") or "-")) + detail = html.escape(str(event.get("detail") or "-")) + return f"{created}{hostname}{status}{ipv4}{ipv6}{detail}" + + def _page(title: str, body: str) -> str: return f""" @@ -859,6 +1136,9 @@ def _page(title: str, body: str) -> str: .empty {{ color: var(--muted); text-align: center; padding: 28px; }} .icon-button {{ width: 32px; min-height: 32px; padding: 0; background: #f5f5f7; color: var(--danger); }} .icon-button:hover {{ background: #fff1f0; }} + .table-actions {{ display: flex; gap: 6px; flex-wrap: wrap; }} + .small-button {{ min-height: 32px; padding: 0 11px; background: #f5f5f7; color: var(--blue); font-size: 12px; }} + .small-button:hover {{ background: #e8e8ed; }} .footer {{ padding: 28px 0; background: var(--bg); border-top: 1px solid var(--soft-line); }} .footer-links {{ display: flex; justify-content: center; gap: 18px; flex-wrap: wrap; }} .footer-links a {{ color: var(--muted); font-size: 13px; text-decoration: none; }} diff --git a/router_dyndns/ddns_models.py b/router_dyndns/ddns_models.py index fa07f1a..442237f 100644 --- a/router_dyndns/ddns_models.py +++ b/router_dyndns/ddns_models.py @@ -16,6 +16,8 @@ class DdnsSettings(BaseModel): hostname_suffix: str = Field(default="") verification_prefix: str = Field(default="_router_dyndns-ddns") rate_limit_per_minute: int = Field(default=60, ge=1, le=10_000) + admin_rate_limit_per_minute: int = Field(default=20, ge=1, le=10_000) + max_request_body_bytes: int = Field(default=16_384, ge=1024, le=1_048_576) trusted_proxy_ips: set[str] = Field(default_factory=set) rfc2136_server: str | None = None rfc2136_zone: str | None = None @@ -56,6 +58,8 @@ class DdnsSettings(BaseModel): hostname_suffix=os.getenv("DDNS_HOSTNAME_SUFFIX", ""), verification_prefix=os.getenv("DDNS_VERIFICATION_PREFIX", "_router_dyndns-ddns"), rate_limit_per_minute=_env_int("DDNS_RATE_LIMIT_PER_MINUTE", 60), + admin_rate_limit_per_minute=_env_int("DDNS_ADMIN_RATE_LIMIT_PER_MINUTE", 20), + max_request_body_bytes=_env_int("DDNS_MAX_REQUEST_BODY_BYTES", 16_384), trusted_proxy_ips=trusted_proxy_ips, rfc2136_server=os.getenv("DDNS_RFC2136_SERVER") or None, rfc2136_zone=os.getenv("DDNS_RFC2136_ZONE") or None, diff --git a/router_dyndns/ddns_store.py b/router_dyndns/ddns_store.py index b760a7f..25f5151 100644 --- a/router_dyndns/ddns_store.py +++ b/router_dyndns/ddns_store.py @@ -122,6 +122,21 @@ class DdnsStore: ).fetchall() return [dict(row) for row in rows] + def get_account_by_hostname(self, hostname: str) -> dict[str, str | int | None] | None: + with self.connect() as conn: + conn.row_factory = sqlite3.Row + account = conn.execute( + """ + SELECT accounts.hostname, accounts.username, accounts.update_slug, + accounts.created_at, accounts.disabled, records.ipv4, records.ipv6, records.updated_at + FROM accounts + LEFT JOIN records ON records.hostname = accounts.hostname + WHERE accounts.hostname = ? + """, + (hostname,), + ).fetchone() + return dict(account) if account else None + def get_account_by_slug(self, update_slug: str) -> dict[str, str | int | None] | None: with self.connect() as conn: conn.row_factory = sqlite3.Row @@ -170,6 +185,32 @@ class DdnsStore: conn.execute("DELETE FROM accounts WHERE hostname = ?", (hostname,)) conn.execute("DELETE FROM records WHERE hostname = ?", (hostname,)) + def rotate_update_slug(self, hostname: str) -> dict[str, str | int | None] | None: + update_slug = secrets.token_urlsafe(32) + with self.connect() as conn: + conn.execute("UPDATE accounts SET update_slug = ? WHERE hostname = ?", (update_slug, hostname)) + return self.get_account_by_hostname(hostname) + + def rotate_management_slug(self, hostname: str) -> dict[str, str | int | None] | None: + management_slug = secrets.token_urlsafe(32) + management_hash = hash_lookup_token(management_slug) + with self.connect() as conn: + conn.execute("UPDATE accounts SET management_hash = ? WHERE hostname = ?", (management_hash, hostname)) + account = self.get_account_by_hostname(hostname) + if account: + account["management_slug"] = management_slug + return account + + def rotate_password(self, hostname: str) -> dict[str, str | int | None] | None: + token = secrets.token_urlsafe(24) + token_hash = hash_secret(token) + with self.connect() as conn: + conn.execute("UPDATE accounts SET token_hash = ? WHERE hostname = ?", (token_hash, hostname)) + account = self.get_account_by_hostname(hostname) + if account: + account["password"] = token + return account + def create_domain_challenge(self, domain: str) -> dict[str, str]: token = f"ff-ddns-{secrets.token_urlsafe(24)}" claim_secret = secrets.token_urlsafe(32) @@ -229,6 +270,20 @@ class DdnsStore: result["claim_secret"] = claim_secret return result + def list_domain_challenges(self, limit: int = 100) -> list[dict[str, str | None]]: + with self.connect() as conn: + conn.row_factory = sqlite3.Row + rows = conn.execute( + """ + SELECT domain, created_at, verified_at + FROM domain_challenges + ORDER BY created_at DESC + LIMIT ? + """, + (limit,), + ).fetchall() + return [dict(row) for row in rows] + def is_domain_verified(self, domain: str, claim_secret: str | None = None) -> bool: with self.connect() as conn: sql = "SELECT verified_at FROM domain_challenges WHERE domain = ?" @@ -277,6 +332,20 @@ class DdnsStore: ).fetchall() return [dict(row) for row in rows] + def list_cleanup_runs(self, limit: int = 20) -> list[dict[str, str | int]]: + with self.connect() as conn: + conn.row_factory = sqlite3.Row + rows = conn.execute( + """ + SELECT created_at, domain_challenges_deleted, unused_accounts_deleted + FROM cleanup_runs + ORDER BY created_at DESC + LIMIT ? + """, + (limit,), + ).fetchall() + return [dict(row) for row in rows] + def allow_rate_limit(self, key: str, limit: int) -> bool: now = int(time.time()) window = now // 60 @@ -318,6 +387,13 @@ class DdnsStore: (unused_account_cutoff,), ) conn.execute("DELETE FROM rate_limits WHERE window < ?", (int(time.time()) - 3600,)) + conn.execute( + """ + INSERT INTO cleanup_runs(created_at, domain_challenges_deleted, unused_accounts_deleted) + VALUES (?, ?, ?) + """, + (datetime.now(UTC).isoformat(), challenge_cursor.rowcount, account_cursor.rowcount), + ) return { "domain_challenges": challenge_cursor.rowcount, "unused_accounts": account_cursor.rowcount, @@ -352,6 +428,17 @@ class DdnsStore: """ ) conn.execute("CREATE INDEX IF NOT EXISTS idx_update_events_created_at ON update_events(created_at)") + conn.execute( + """ + CREATE TABLE IF NOT EXISTS cleanup_runs ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + created_at TEXT NOT NULL, + domain_challenges_deleted INTEGER NOT NULL, + unused_accounts_deleted INTEGER NOT NULL + ) + """ + ) + conn.execute("CREATE INDEX IF NOT EXISTS idx_cleanup_runs_created_at ON cleanup_runs(created_at)") conn.execute( """ CREATE TABLE IF NOT EXISTS rate_limits ( diff --git a/tests/test_ddns.py b/tests/test_ddns.py index 13d4469..42cb073 100644 --- a/tests/test_ddns.py +++ b/tests/test_ddns.py @@ -6,6 +6,7 @@ from fastapi.testclient import TestClient import router_dyndns.ddns_service as ddns_service from router_dyndns.ddns import DdnsSettings, DdnsStore, make_app +from router_dyndns.ddns_security import admin_csrf_token def test_update_accepts_fritzbox_placeholders(tmp_path: Path) -> None: @@ -152,16 +153,24 @@ def test_generated_account_cannot_update_other_hostname(tmp_path: Path) -> None: def test_admin_page_shows_account_form(tmp_path: Path) -> None: - app = make_app(DdnsSettings(database_path=tmp_path / "ddns.sqlite3", admin_password="admin")) + database_path = tmp_path / "ddns.sqlite3" + store = DdnsStore(database_path) + store.create_domain_challenge("example.net") + app = make_app(DdnsSettings(database_path=database_path, admin_password="admin")) client = TestClient(app) response = client.get("/admin", auth=("admin", "admin")) assert response.status_code == 200 + assert response.headers["cache-control"] == "no-store" assert "Update-URL" in response.text assert "Domainnamen" in response.text assert "Benutzername" in response.text assert "Kennwort" in response.text + assert "Scheduled cleanup" in response.text + assert "Domain claims" in response.text + assert "Recent events" in response.text + assert "example.net" in response.text def test_admin_posts_require_csrf(tmp_path: Path) -> None: @@ -186,6 +195,63 @@ def test_admin_password_query_string_is_rejected(tmp_path: Path) -> None: assert response.status_code == 401 +def test_admin_routes_are_rate_limited(tmp_path: Path) -> None: + app = make_app( + DdnsSettings( + database_path=tmp_path / "ddns.sqlite3", + admin_password="admin", + admin_rate_limit_per_minute=1, + ) + ) + client = TestClient(app) + + first = client.get("/admin", auth=("admin", "wrong")) + second = client.get("/admin", auth=("admin", "wrong")) + + assert first.status_code == 401 + assert second.status_code == 429 + + +def test_admin_can_rotate_links_and_password(tmp_path: Path) -> None: + settings = DdnsSettings( + database_path=tmp_path / "ddns.sqlite3", + admin_password="admin", + public_base_url="http://ddns.example.net", + ) + store = DdnsStore(settings.database_path) + account = store.create_account("home.example.net", "router") + app = make_app(settings) + client = TestClient(app) + csrf = admin_csrf_token(settings) + + update = client.post( + "/admin/accounts/rotate", + auth=("admin", "admin"), + data={"csrf": csrf, "hostname": "home.example.net", "action": "update"}, + ) + assert update.status_code == 200 + assert "Update URL rotated" in update.text + assert client.get(f"/u/{account['update_slug']}", params={"myip": "203.0.113.9"}).status_code == 404 + + management = client.post( + "/admin/accounts/rotate", + auth=("admin", "admin"), + data={"csrf": csrf, "hostname": "home.example.net", "action": "management"}, + ) + assert management.status_code == 200 + assert "Management link rotated" in management.text + assert "/m/" in management.text + + password = client.post( + "/admin/accounts/rotate", + auth=("admin", "admin"), + data={"csrf": csrf, "hostname": "home.example.net", "action": "password"}, + ) + assert password.status_code == 200 + assert "Router password rotated" in password.text + assert "Kennwort:" in password.text + + def test_self_service_managed_hostname_generates_random_account(tmp_path: Path) -> None: app = make_app( DdnsSettings( @@ -325,7 +391,11 @@ def test_api_docs_include_versioned_ddns_api(tmp_path: Path) -> None: assert redoc.status_code == 200 assert "/api/v1/hostnames/magic" in schema["paths"] assert "/api/v1/updates/{update_slug}" in schema["paths"] + assert "/admin" not in schema["paths"] + assert "/records" not in schema["paths"] + assert "/events" not in schema["paths"] assert any(tag["name"] == "hostnames" for tag in schema["tags"]) + assert all(tag["name"] != "admin" for tag in schema["tags"]) def test_api_magic_hostname_lifecycle(tmp_path: Path) -> None: @@ -341,6 +411,7 @@ def test_api_magic_hostname_lifecycle(tmp_path: Path) -> None: created = client.post("/api/v1/hostnames/magic", json={"username": "router"}) assert created.status_code == 201 + assert created.headers["cache-control"] == "no-store" body = created.json() assert body["hostname"].endswith(".ddns.example.net") assert body["update_url"].startswith("http://ddns.example.net/u/") @@ -438,6 +509,39 @@ def test_api_creation_is_rate_limited(tmp_path: Path) -> None: assert second.status_code == 429 +def test_secret_html_responses_are_not_cached(tmp_path: Path) -> None: + app = make_app( + DdnsSettings( + database_path=tmp_path / "ddns.sqlite3", + admin_password="admin", + hostname_suffix="ddns.example.net", + ) + ) + client = TestClient(app) + + created = client.post("/magic", data={"username": "router"}) + + assert created.status_code == 200 + assert created.headers["cache-control"] == "no-store" + assert created.headers["pragma"] == "no-cache" + + +def test_oversized_request_body_is_rejected(tmp_path: Path) -> None: + app = make_app( + DdnsSettings( + database_path=tmp_path / "ddns.sqlite3", + admin_password="admin", + hostname_suffix="ddns.example.net", + max_request_body_bytes=1024, + ) + ) + client = TestClient(app) + + response = client.post("/magic", data={"username": "x" * 2000}) + + assert response.status_code == 413 + + def test_untrusted_forwarded_for_does_not_bypass_rate_limit(tmp_path: Path) -> None: app = make_app( DdnsSettings(