diff --git a/README.md b/README.md index 69700a4..30d6895 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,7 @@ Environment variables set the configuration. **Restart the container after a cha | `BINDERY_PASSWORD_HASH` | The password hash for `AUTH_METHOD=local`. See "Local login". | None | | `CWA_ALLOWED_USERS` | The CWA users that can log in. Empty allows every CWA user. | Empty | | `PROXY_AUTH_USER_HEADER` | The header in which a reverse proxy sends the user name | `X-Remote-User` | +| `PROXY_AUTH_TRUSTED_IPS` | The addresses and networks that can send the user header. See "Logins". | Loopback and private addresses | | `PROXY_AUTH_LOGOUT_URL` | Where the Log out button goes in proxy mode | None | | `BINDERY_API_KEY` | A key for scripts. See "Scripts". | None | | `SESSION_COOKIE_SECURE` | `true` if users reach Bindery only over HTTPS | `false` | @@ -109,6 +110,13 @@ Each build without a saved CWA book ID then puts a plain ePub in that folder, in because Bindery trusts this header. If a request gets a 401 because the proxy session ended, Bindery reloads the page, so the proxy can step in again. + Bindery accepts the header only from the addresses in `PROXY_AUTH_TRUSTED_IPS`. The default is the + loopback and private addresses, such as `127.0.0.1`, `10.x`, `172.16.x` to `172.31.x` and + `192.168.x`. This stops a request that comes straight from the internet, but not one from your own + network. **Set it to the address of the proxy**, for example `PROXY_AUTH_TRUSTED_IPS=172.18.0.3`, + or publish the port of Bindery only on the host, as `127.0.0.1:8085:8085` in the Compose file. + `TRUST_PROXY` does not change this check. It is only for the lockouts. + **If the login settings are incomplete or wrong, Bindery refuses every request**, also with the API key, and shows the reason. diff --git a/bindery/auth.py b/bindery/auth.py index 0d98a16..e43062e 100644 --- a/bindery/auth.py +++ b/bindery/auth.py @@ -7,6 +7,7 @@ OPDS login works only with a CWA that refuses a wrong password. import hashlib import hmac +import ipaddress import logging import threading import time @@ -54,6 +55,23 @@ def _valid_local_hash(value: str) -> bool: return False +@lru_cache(maxsize=8) +def _networks(values: tuple[str, ...]) -> tuple[ipaddress.IPv4Network | ipaddress.IPv6Network, ...]: + """The networks of a list of addresses and networks. Raises ValueError for a value that is neither.""" + return tuple(ipaddress.ip_network(v, strict=False) for v in values) + + +def _trusted_proxy(address: str | None) -> bool: + """Whether the request comes from an address that can send the user header.""" + try: + ip = ipaddress.ip_address(address or "") + except ValueError: + return False + if ip.version == 6 and ip.ipv4_mapped: + ip = ip.ipv4_mapped + return any(ip.version == n.version and ip in n for n in _networks(tuple(config.PROXY_AUTH_TRUSTED_IPS))) + + def local_password_version() -> str: """Invalidate local sessions when the configured password hash changes.""" return hashlib.sha256(config.LOCAL_PASSWORD_HASH.encode()).hexdigest() @@ -74,8 +92,15 @@ def config_error() -> str | None: return "BINDERY_PASSWORD_HASH must be a Werkzeug scrypt or PBKDF2 hash of a nonempty password" if method == AUTH_CWA and not (config.CWA_URL or config.CWA_DB_PATH): return "AUTH_METHOD=cwa needs CWA_URL or CWA_DB_PATH" - if method == AUTH_PROXY and not config.PROXY_AUTH_USER_HEADER: - return "AUTH_METHOD=proxy needs PROXY_AUTH_USER_HEADER" + if method == AUTH_PROXY: + if not config.PROXY_AUTH_USER_HEADER: + return "AUTH_METHOD=proxy needs PROXY_AUTH_USER_HEADER" + if not config.PROXY_AUTH_TRUSTED_IPS: + return "AUTH_METHOD=proxy needs PROXY_AUTH_TRUSTED_IPS, the addresses of the proxy" + try: + _networks(tuple(config.PROXY_AUTH_TRUSTED_IPS)) + except ValueError as e: + return f"PROXY_AUTH_TRUSTED_IPS must be a list of addresses and networks: {e}" return None @@ -100,8 +125,10 @@ def _api_key_identity(headers: Mapping[str, str]) -> Identity | None: return None -def current_identity(headers: Mapping[str, str], session: Mapping[str, Any]) -> Identity | None: - """Who makes this request, or None when a login is needed.""" +def current_identity( + headers: Mapping[str, str], session: Mapping[str, Any], remote_addr: str | None = None +) -> Identity | None: + """Who makes this request, or None when a login is needed. `remote_addr` is the address that connects.""" if config_error(): return None identity = _api_key_identity(headers) @@ -112,6 +139,9 @@ def current_identity(headers: Mapping[str, str], session: Mapping[str, Any]) -> return Identity("", "none") if mode == AUTH_PROXY: name = (headers.get(config.PROXY_AUTH_USER_HEADER) or "").strip() + if name and not _trusted_proxy(remote_addr): + logger.warning(f"Ignored the user header from {remote_addr}, which is not in PROXY_AUTH_TRUSTED_IPS") + return None return Identity(name, "proxy") if name else None name = session.get("user") if not name or session.get("mode") != mode: diff --git a/bindery/config.py b/bindery/config.py index 7ea774b..843ccc5 100644 --- a/bindery/config.py +++ b/bindery/config.py @@ -75,6 +75,11 @@ LOCAL_PASSWORD_HASH = os.getenv("BINDERY_PASSWORD_HASH", "").strip() CWA_ALLOWED_USERS = _list("CWA_ALLOWED_USERS") PROXY_AUTH_USER_HEADER = os.getenv("PROXY_AUTH_USER_HEADER", "X-Remote-User").strip() PROXY_AUTH_LOGOUT_URL = os.getenv("PROXY_AUTH_LOGOUT_URL", "").strip() +# The addresses and networks that can send the user header. The default is each loopback and private address, so +# a request that comes straight from the internet cannot send it. +PROXY_AUTH_TRUSTED_IPS = _list( + "PROXY_AUTH_TRUSTED_IPS", "127.0.0.0/8, ::1, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, fc00::/7, fe80::/10" +) API_KEY = os.getenv("BINDERY_API_KEY", "").strip() SESSION_COOKIE_SECURE = _bool("SESSION_COOKIE_SECURE") # Take the client address from the last entry of X-Forwarded-For, which a reverse proxy adds. diff --git a/bindery/web.py b/bindery/web.py index a683096..297ea7d 100644 --- a/bindery/web.py +++ b/bindery/web.py @@ -136,7 +136,7 @@ def _names(data: dict[str, Any], name: str) -> list[str] | None: def _identity() -> auth.Identity | None: - return auth.current_identity(request.headers, session) + return auth.current_identity(request.headers, session, request.remote_addr) @app.before_request diff --git a/docker-compose.yml b/docker-compose.yml index 8a189ec..a1a8695 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -23,8 +23,11 @@ services: # URL_BASE: /bindery # true only if clients reach Bindery through a reverse proxy alone. The login lockout then uses X-Forwarded-For. # TRUST_PROXY: "true" + # For AUTH_METHOD=proxy: the address of the proxy. The default trusts each private address. + # PROXY_AUTH_TRUSTED_IPS: 172.18.0.3 # BINDERY_API_KEY: a-long-random-string ports: + # If a reverse proxy on this host is the only way in, publish the port only on the host: 127.0.0.1:8085:8085 - 8085:8085 restart: unless-stopped # /api/health needs no login, and works with and without URL_BASE. diff --git a/tests/test_api.py b/tests/test_api.py index 9fae276..5a5131f 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -173,12 +173,57 @@ def test_proxy_header(client, monkeypatch): } +@pytest.mark.parametrize( + ("trusted", "address", "accepted"), + [ + (None, "127.0.0.1", True), + (None, "::1", True), + (None, "172.18.0.3", True), + (None, "192.168.1.20", True), + (None, "fd00::5", True), + (None, "::ffff:10.1.2.3", True), + (None, "203.0.113.9", False), + (None, "2001:db8::1", False), + (None, "", False), + (["172.18.0.3"], "172.18.0.3", True), + (["172.18.0.3"], "172.18.0.4", False), + (["172.18.0.3"], "127.0.0.1", False), + (["10.0.0.0/8", "fd00::/64"], "fd00::7", True), + (["10.0.0.0/8", "fd00::/64"], "192.168.1.20", False), + ], +) +def test_only_a_trusted_address_can_send_the_proxy_header(client, monkeypatch, caplog, trusted, address, accepted): + monkeypatch.setattr(config, "AUTH_METHOD", "proxy") + if trusted is not None: + monkeypatch.setattr(config, "PROXY_AUTH_TRUSTED_IPS", trusted) + environ = {"REMOTE_ADDR": address} + response = client.get("/api/stories", headers={"X-Remote-User": "alice"}, environ_base=environ) + assert response.status_code == (200 if accepted else 401) + assert ( + client.get("/api/auth/check", headers={"X-Remote-User": "alice"}, environ_base=environ).json["authenticated"] + is accepted + ) + assert ("which is not in PROXY_AUTH_TRUSTED_IPS" in caplog.text) is not accepted + # A change needs the header too, from the same address. + change = client.post( + "/api/stories", json={"urls": []}, headers={**H, "X-Remote-User": "alice"}, environ_base=environ + ) + assert change.status_code == (200 if accepted else 401) + + @pytest.mark.parametrize( ("setting", "value", "error"), [ ("AUTH_METHOD", "cwa", "AUTH_METHOD=cwa needs CWA_URL or CWA_DB_PATH"), ("AUTH_METHOD", "cwa-login", "AUTH_METHOD must be none, local, cwa or proxy, not 'cwa-login'"), ("PROXY_AUTH_USER_HEADER", "", "AUTH_METHOD=proxy needs PROXY_AUTH_USER_HEADER"), + ("PROXY_AUTH_TRUSTED_IPS", [], "AUTH_METHOD=proxy needs PROXY_AUTH_TRUSTED_IPS, the addresses of the proxy"), + ( + "PROXY_AUTH_TRUSTED_IPS", + ["10.0.0.5", "proxy.local"], + "PROXY_AUTH_TRUSTED_IPS must be a list of addresses and networks: " + "'proxy.local' does not appear to be an IPv4 or IPv6 network", + ), ], ) def test_incomplete_login_settings_refuse_every_request(client, monkeypatch, setting, value, error): diff --git a/tests/test_config.py b/tests/test_config.py index 0b7b6d1..8a943fe 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -97,6 +97,7 @@ def test_defaults(monkeypatch, reload_config): "DEFAULT_LOCKED_FIELDS", "LOG_LEVEL", "PROXY_AUTH_USER_HEADER", + "PROXY_AUTH_TRUSTED_IPS", "SESSION_COOKIE_SECURE", "TRUST_PROXY", "CWA_DB_PATH", @@ -113,6 +114,15 @@ def test_defaults(monkeypatch, reload_config): assert config.DEFAULT_LOCKED_FIELDS == ["title"] assert config.LOG_LEVEL == "INFO" assert config.PROXY_AUTH_USER_HEADER == "X-Remote-User" + assert config.PROXY_AUTH_TRUSTED_IPS == [ + "127.0.0.0/8", + "::1", + "10.0.0.0/8", + "172.16.0.0/12", + "192.168.0.0/16", + "fc00::/7", + "fe80::/10", + ] assert config.SESSION_COOKIE_SECURE is False assert config.TRUST_PROXY is False assert config.CWA_DB_PATH is None