diff --git a/.env.example b/.env.example index fc8f1ca..3510092 100644 --- a/.env.example +++ b/.env.example @@ -20,6 +20,9 @@ DDNS_TTL=60 # Run a single Uvicorn worker by default. Rate-limit counters and per-host update locks are process-local. # DDNS_TRUSTED_HOSTS=karldns.de,www.karldns.de # DDNS_DNS_ZONES=karldns.de +# Trust CF-Connecting-IP / X-Forwarded-For when the direct peer is a private/local reverse proxy. +# This is the normal Docker + Nginx Proxy Manager setup. Set to 0 if the app port is reachable by untrusted LAN clients. +# DDNS_TRUST_PRIVATE_PROXY_HEADERS=1 # DDNS_TRUSTED_PROXY_IPS=127.0.0.1,::1 # DDNS_RATE_LIMIT_PER_MINUTE=60 # DDNS_ADMIN_RATE_LIMIT_PER_MINUTE=20 diff --git a/README.md b/README.md index 4f0be24..230a8dd 100644 --- a/README.md +++ b/README.md @@ -337,7 +337,8 @@ The default container command runs one Uvicorn worker. Keep that deployment shap - Set `DDNS_PUBLIC_BASE_URL` to the real public URL. KarlDNS uses it for generated links, code examples, API docs, canonical URLs, and the default trusted host. - Use a least-privilege DNS API token or TSIG key. - Set `DDNS_TRUSTED_HOSTS` only when the app should answer for hostnames beyond `DDNS_PUBLIC_BASE_URL`. -- Configure `DDNS_TRUSTED_PROXY_IPS` only for reverse proxies that strip user-supplied forwarding headers. +- KarlDNS automatically trusts `CF-Connecting-IP`, `X-Forwarded-For`, and `X-Real-IP` when the direct peer is a private/local reverse proxy such as Docker or Nginx Proxy Manager. This lets the UI examples and router updates use the real public visitor IP behind Cloudflare. Set `DDNS_TRUST_PRIVATE_PROXY_HEADERS=0` if the app port is reachable by untrusted LAN clients. +- Configure `DDNS_TRUSTED_PROXY_IPS` when your reverse proxy is not on a private/local address, and only for proxies that strip user-supplied forwarding headers. - Keep `/docs` available if you want the small developer surface. It documents only the public user and router endpoints. - Leave cleanup enabled. The cleanup scheduler is stored in SQLite with a task row, next run time, status, last error, and a database lease so only one app process runs a due cleanup job. - Back up `/data/ddns.sqlite3`. diff --git a/karldns/config.py b/karldns/config.py index 7a776eb..93bcba5 100644 --- a/karldns/config.py +++ b/karldns/config.py @@ -18,6 +18,7 @@ def settings_from_env() -> DdnsSettings: 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=_csv_set("DDNS_TRUSTED_PROXY_IPS"), + trust_private_proxy_headers=_env_flag("DDNS_TRUST_PRIVATE_PROXY_HEADERS", default=True), rfc2136_server=os.getenv("DDNS_RFC2136_SERVER") or None, rfc2136_zone=os.getenv("DDNS_RFC2136_ZONE") or None, rfc2136_key_name=os.getenv("DDNS_RFC2136_KEY_NAME") or None, @@ -46,8 +47,9 @@ def _csv_set(name: str) -> set[str]: return {item.strip().lower() for item in os.getenv(name, "").split(",") if item.strip()} -def _env_flag(name: str) -> bool: - return os.getenv(name, "0").strip().lower() not in {"0", "false", "no"} +def _env_flag(name: str, *, default: bool = False) -> bool: + default_value = "1" if default else "0" + return os.getenv(name, default_value).strip().lower() not in {"0", "false", "no"} def _env_int(name: str, default: int) -> int: diff --git a/karldns/security.py b/karldns/security.py index 314fedb..06654e7 100644 --- a/karldns/security.py +++ b/karldns/security.py @@ -15,6 +15,19 @@ from .settings import DdnsSettings DASHBOARD_UNLOCK_COOKIE = "ddns_dashboard_unlock" DASHBOARD_UNLOCK_DEFAULT_SECONDS = 24 * 60 * 60 DASHBOARD_UNLOCK_STAY_SIGNED_IN_SECONDS = 30 * 24 * 60 * 60 +PRIVATE_PROXY_NETWORKS = tuple( + ip_network(network) + for network in ( + "10.0.0.0/8", + "172.16.0.0/12", + "192.168.0.0/16", + "127.0.0.0/8", + "169.254.0.0/16", + "::1/128", + "fc00::/7", + "fe80::/10", + ) +) def credentials_from_basic_auth(header: str) -> tuple[str | None, str | None]: @@ -146,11 +159,10 @@ def require_admin_csrf(settings: DdnsSettings, supplied: str) -> None: def client_ip(request: Request, settings: DdnsSettings | None = None) -> str: peer = request.client.host if request.client else "unknown" - if settings and _trusted_proxy(peer, settings.trusted_proxy_ips): - for header_name in ("cf-connecting-ip", "x-forwarded-for", "x-real-ip"): - candidate = request.headers.get(header_name, "") - if candidate: - return candidate.split(",", 1)[0].strip() + if _should_trust_proxy_headers(peer, settings): + forwarded_ip = _client_ip_from_headers(request) + if forwarded_ip: + return forwarded_ip return peer @@ -181,3 +193,64 @@ def _trusted_proxy(peer: str, trusted: set[str]) -> bool: except ValueError: continue return False + + +def _should_trust_proxy_headers(peer: str, settings: DdnsSettings | None) -> bool: + if settings and _trusted_proxy(peer, settings.trusted_proxy_ips): + return True + if settings and settings.trust_private_proxy_headers: + return _private_proxy_peer(peer) + return False + + +def _private_proxy_peer(peer: str) -> bool: + try: + peer_ip = ip_address(peer) + except ValueError: + return False + return any(peer_ip in network for network in PRIVATE_PROXY_NETWORKS) + + +def _client_ip_from_headers(request: Request) -> str | None: + for header_name in ("cf-connecting-ip", "true-client-ip", "x-real-ip"): + candidate = _valid_header_ip(request.headers.get(header_name, "")) + if candidate: + return candidate + for candidate in request.headers.get("x-forwarded-for", "").split(","): + parsed = _valid_header_ip(candidate) + if parsed: + return parsed + return _client_ip_from_forwarded(request.headers.get("forwarded", "")) + + +def _client_ip_from_forwarded(value: str) -> str | None: + for forwarded_pair in value.split(";"): + key, separator, candidate = forwarded_pair.strip().partition("=") + if separator and key.lower() == "for": + parsed = _valid_header_ip(candidate) + if parsed: + return parsed + return None + + +def _valid_header_ip(value: str) -> str | None: + cleaned = _strip_header_ip(value) + if not cleaned: + return None + try: + return str(ip_address(cleaned)) + except ValueError: + return None + + +def _strip_header_ip(value: str) -> str: + cleaned = value.strip().strip('"').strip("'") + if not cleaned or cleaned.lower() == "unknown": + return "" + if cleaned.startswith("[") and "]" in cleaned: + return cleaned[1 : cleaned.index("]")] + if cleaned.count(":") == 1: + host, port = cleaned.rsplit(":", 1) + if port.isdigit(): + return host + return cleaned diff --git a/karldns/settings.py b/karldns/settings.py index 8b424f2..a411410 100644 --- a/karldns/settings.py +++ b/karldns/settings.py @@ -20,6 +20,7 @@ class DdnsSettings(BaseModel): 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) + trust_private_proxy_headers: bool = Field(default=True) rfc2136_server: str | None = None rfc2136_zone: str | None = None rfc2136_key_name: str | None = None diff --git a/karldns/web/components.py b/karldns/web/components.py index a6c4586..dd1dfde 100644 --- a/karldns/web/components.py +++ b/karldns/web/components.py @@ -31,30 +31,30 @@ def _event_label(status: object) -> str: def _copy_row(label: str, value: str, sensitivity: str | None = None, *, mask_secret: bool = True) -> SafeHtml: safe_label = escape(label) safe_value = escape(value) - badge = "" + attr_label = attr(label) + attr_value = attr(value) is_secret = bool(sensitivity and sensitivity.lower() == "secret") - if sensitivity: - safe_sensitivity = escape(sensitivity) - badge_class = "text-bg-warning" if is_secret else "text-bg-secondary" - badge = f'{safe_sensitivity}' should_mask = is_secret and mask_secret value_field = ( - f'' + f'' if should_mask - else f'{safe_value}' + else f'{safe_value}' ) reveal_button = ( - f'' + f'' if should_mask else "" ) + row_class = "copy-field copy-field-secret" if is_secret else "copy-field copy-field-public" return trusted( f""" -
- {safe_label}{badge} - {value_field} - {reveal_button} -
diff --git a/karldns/web/styles.py b/karldns/web/styles.py index ce1ee82..6f956b0 100644 --- a/karldns/web/styles.py +++ b/karldns/web/styles.py @@ -618,52 +618,67 @@ select:focus-visible { } .copy-field { + display: grid; + grid-template-columns: minmax(0, 1fr) auto; + align-items: stretch; + gap: 12px; min-width: 0; max-width: 100%; } -.copy-field .input-group-text { - min-width: 150px; - border-color: var(--rp-line); - color: var(--rp-muted); - font-size: 13px; - background: var(--rp-surface-alt); -} - -.copy-label { - display: inline-flex; - align-items: center; - justify-content: space-between; - gap: 8px; -} - -.copy-field .form-control { +.copy-pill { + display: flex; min-width: 0; - border-color: var(--rp-line); + min-height: 58px; + align-items: center; + gap: 10px; + padding: 9px 16px; + border: 1px solid var(--rp-line); + border-radius: 9999px; background: var(--rp-field); } +.copy-field-secret .copy-pill { + border-color: var(--rp-danger); +} + +.copy-pill-label { + display: inline-flex; + flex: 0 0 auto; + align-items: center; + color: var(--rp-muted); + font-weight: 600; + white-space: nowrap; +} + .copy-field .copy-value { + flex: 1 1 auto; min-width: 0; - overflow-wrap: anywhere; -} - -.copy-field code.form-control { - width: 1%; - flex: 1 1 0; overflow: hidden; - white-space: nowrap; + color: var(--rp-ink); text-overflow: ellipsis; + white-space: nowrap; } -.copy-field code { - min-height: 42px; +.copy-field code, +.copy-field input { margin: 0; - color: var(--rp-ink); + border: 0; + outline: 0; + background: transparent; font-family: ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, monospace; - font-size: 13px; - line-height: 1.6; - background: var(--rp-field); + font-size: 16px; + line-height: 1.45; +} + +.copy-field input { + padding: 0; +} + +.copy-reveal-button { + flex: 0 0 auto; + min-height: 36px; + padding-inline: 14px; } .copy-button { @@ -671,6 +686,9 @@ select:focus-visible { align-items: center; justify-content: center; gap: 6px; + min-width: 122px; + min-height: 58px; + font-weight: 600; } .copy-button .copy-icon, @@ -973,12 +991,15 @@ select:focus-visible { .copy-field { display: grid; grid-template-columns: 1fr; + gap: 10px; } - .copy-field .input-group-text { - min-width: 0; - border-radius: 14px 14px 0 0; - white-space: normal; + .copy-pill { + min-height: 50px; + align-items: flex-start; + flex-wrap: wrap; + border-radius: 24px; + padding: 12px 14px; } .copy-field .copy-value, @@ -986,23 +1007,14 @@ select:focus-visible { width: 100%; white-space: normal; overflow-wrap: anywhere; - border-radius: 0; - } - - .copy-field code.form-control { - flex: 1 1 auto; - width: 100%; overflow: visible; text-overflow: clip; } - .copy-field .btn { + .copy-field .copy-button { width: 100%; - border-radius: 0; - } - - .copy-field .btn:last-child { - border-radius: 0 0 14px 14px; + min-height: 50px; + border-radius: 9999px; } .card .copy-icon-button, diff --git a/karldns/web/templates_management.py b/karldns/web/templates_management.py index 7a05576..079e58c 100644 --- a/karldns/web/templates_management.py +++ b/karldns/web/templates_management.py @@ -180,10 +180,6 @@ def _render_pending_management_page(

Pending setup

{hostname}

Save these values, then register the hostname. Pending setups are cleaned up after {escape(cleanup_text)}.

-
- - -
{message_html} diff --git a/tests/test_security.py b/tests/test_security.py index fa62c10..2ab6645 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -268,6 +268,67 @@ def test_trusted_proxy_uses_cloudflare_connecting_ip(tmp_path: Path) -> None: assert response.json()["ip"] == "198.51.100.20" +def test_private_reverse_proxy_uses_cloudflare_connecting_ip_by_default(tmp_path: Path) -> None: + settings = DdnsSettings( + database_path=tmp_path / "ddns.sqlite3", + admin_password="admin", + ) + app = make_app(settings) + + @app.get("/debug-ip", include_in_schema=False) + def debug_ip(request: Request) -> dict[str, str]: + return {"ip": client_ip(request, settings)} + + client = TestClient(app, client=("172.18.0.5", 12345)) + response = client.get( + "/debug-ip", + headers={ + "cf-connecting-ip": "8.8.8.8", + "x-forwarded-for": "1.1.1.1", + }, + ) + + assert response.status_code == 200 + assert response.json()["ip"] == "8.8.8.8" + + +def test_private_reverse_proxy_uses_forwarded_for_without_cloudflare_header(tmp_path: Path) -> None: + settings = DdnsSettings( + database_path=tmp_path / "ddns.sqlite3", + admin_password="admin", + ) + app = make_app(settings) + + @app.get("/debug-ip", include_in_schema=False) + def debug_ip(request: Request) -> dict[str, str]: + return {"ip": client_ip(request, settings)} + + client = TestClient(app, client=("172.18.0.5", 12345)) + response = client.get("/debug-ip", headers={"x-forwarded-for": "8.8.4.4, 172.18.0.5"}) + + assert response.status_code == 200 + assert response.json()["ip"] == "8.8.4.4" + + +def test_private_reverse_proxy_trust_can_be_disabled(tmp_path: Path) -> None: + settings = DdnsSettings( + database_path=tmp_path / "ddns.sqlite3", + admin_password="admin", + trust_private_proxy_headers=False, + ) + app = make_app(settings) + + @app.get("/debug-ip", include_in_schema=False) + def debug_ip(request: Request) -> dict[str, str]: + return {"ip": client_ip(request, settings)} + + client = TestClient(app, client=("172.18.0.5", 12345)) + response = client.get("/debug-ip", headers={"cf-connecting-ip": "8.8.8.8"}) + + assert response.status_code == 200 + assert response.json()["ip"] == "172.18.0.5" + + def test_untrusted_proxy_headers_are_ignored(tmp_path: Path) -> None: settings = DdnsSettings( database_path=tmp_path / "ddns.sqlite3", diff --git a/tests/test_ui.py b/tests/test_ui.py index 45c0be5..6207484 100644 --- a/tests/test_ui.py +++ b/tests/test_ui.py @@ -89,8 +89,8 @@ def test_hostname_without_login_generates_update_and_dashboard_links(tmp_path: P assert "Copy setup values" not in response.text assert "Paste this exact URL into the FRITZ!Box" in response.text assert "Copy manual update commands" in response.text - assert response.text.count("Secret") >= 3 - assert response.text.count("Public") >= 3 + assert response.text.count("copy-field-secret") >= 3 + assert response.text.count("copy-field-public") >= 2 assert ".ddns.example.net" in response.text assert 'type="password"' not in response.text @@ -116,6 +116,27 @@ def test_public_examples_use_visitor_public_ip(tmp_path: Path) -> None: assert "myip=8.8.8.8" in created.text +def test_public_examples_use_cloudflare_ip_from_private_reverse_proxy(tmp_path: Path) -> None: + app = make_app( + DdnsSettings( + database_path=tmp_path / "ddns.sqlite3", + admin_password="admin", + hostname_suffix="ddns.example.net", + public_base_url="https://karldns.de", + ) + ) + client = TestClient(app, client=("172.18.0.5", 12345)) + + home = client.get("/", headers={"cf-connecting-ip": "8.8.4.4"}) + created = client.post("/", headers={"x-forwarded-for": "8.8.4.4, 172.18.0.5"}) + + assert home.status_code == 200 + assert created.status_code == 200 + assert "myip=8.8.4.4" in home.text + assert "good 8.8.4.4" in home.text + assert "myip=8.8.4.4" in created.text + + def test_generated_hostname_must_be_activated_before_updates_work(tmp_path: Path) -> None: database_path = tmp_path / "ddns.sqlite3" app = make_app(