Accept the proxy user header only from trusted addresses
In proxy mode, each client that reached the port could send the user header. PROXY_AUTH_TRUSTED_IPS now lists the addresses and networks that can send it. The default is the loopback and private addresses, so a request that comes straight from the internet cannot log in.
This commit is contained in:
@@ -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 |
|
| `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 |
|
| `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_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 |
|
| `PROXY_AUTH_LOGOUT_URL` | Where the Log out button goes in proxy mode | None |
|
||||||
| `BINDERY_API_KEY` | A key for scripts. See "Scripts". | None |
|
| `BINDERY_API_KEY` | A key for scripts. See "Scripts". | None |
|
||||||
| `SESSION_COOKIE_SECURE` | `true` if users reach Bindery only over HTTPS | `false` |
|
| `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,
|
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 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
|
**If the login settings are incomplete or wrong, Bindery refuses every request**, also with the API
|
||||||
key, and shows the reason.
|
key, and shows the reason.
|
||||||
|
|
||||||
|
|||||||
+33
-3
@@ -7,6 +7,7 @@ OPDS login works only with a CWA that refuses a wrong password.
|
|||||||
|
|
||||||
import hashlib
|
import hashlib
|
||||||
import hmac
|
import hmac
|
||||||
|
import ipaddress
|
||||||
import logging
|
import logging
|
||||||
import threading
|
import threading
|
||||||
import time
|
import time
|
||||||
@@ -54,6 +55,23 @@ def _valid_local_hash(value: str) -> bool:
|
|||||||
return False
|
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:
|
def local_password_version() -> str:
|
||||||
"""Invalidate local sessions when the configured password hash changes."""
|
"""Invalidate local sessions when the configured password hash changes."""
|
||||||
return hashlib.sha256(config.LOCAL_PASSWORD_HASH.encode()).hexdigest()
|
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"
|
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):
|
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"
|
return "AUTH_METHOD=cwa needs CWA_URL or CWA_DB_PATH"
|
||||||
if method == AUTH_PROXY and not config.PROXY_AUTH_USER_HEADER:
|
if method == AUTH_PROXY:
|
||||||
|
if not config.PROXY_AUTH_USER_HEADER:
|
||||||
return "AUTH_METHOD=proxy needs 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
|
return None
|
||||||
|
|
||||||
|
|
||||||
@@ -100,8 +125,10 @@ def _api_key_identity(headers: Mapping[str, str]) -> Identity | None:
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
def current_identity(headers: Mapping[str, str], session: Mapping[str, Any]) -> Identity | None:
|
def current_identity(
|
||||||
"""Who makes this request, or None when a login is needed."""
|
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():
|
if config_error():
|
||||||
return None
|
return None
|
||||||
identity = _api_key_identity(headers)
|
identity = _api_key_identity(headers)
|
||||||
@@ -112,6 +139,9 @@ def current_identity(headers: Mapping[str, str], session: Mapping[str, Any]) ->
|
|||||||
return Identity("", "none")
|
return Identity("", "none")
|
||||||
if mode == AUTH_PROXY:
|
if mode == AUTH_PROXY:
|
||||||
name = (headers.get(config.PROXY_AUTH_USER_HEADER) or "").strip()
|
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
|
return Identity(name, "proxy") if name else None
|
||||||
name = session.get("user")
|
name = session.get("user")
|
||||||
if not name or session.get("mode") != mode:
|
if not name or session.get("mode") != mode:
|
||||||
|
|||||||
@@ -75,6 +75,11 @@ LOCAL_PASSWORD_HASH = os.getenv("BINDERY_PASSWORD_HASH", "").strip()
|
|||||||
CWA_ALLOWED_USERS = _list("CWA_ALLOWED_USERS")
|
CWA_ALLOWED_USERS = _list("CWA_ALLOWED_USERS")
|
||||||
PROXY_AUTH_USER_HEADER = os.getenv("PROXY_AUTH_USER_HEADER", "X-Remote-User").strip()
|
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()
|
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()
|
API_KEY = os.getenv("BINDERY_API_KEY", "").strip()
|
||||||
SESSION_COOKIE_SECURE = _bool("SESSION_COOKIE_SECURE")
|
SESSION_COOKIE_SECURE = _bool("SESSION_COOKIE_SECURE")
|
||||||
# Take the client address from the last entry of X-Forwarded-For, which a reverse proxy adds.
|
# Take the client address from the last entry of X-Forwarded-For, which a reverse proxy adds.
|
||||||
|
|||||||
+1
-1
@@ -136,7 +136,7 @@ def _names(data: dict[str, Any], name: str) -> list[str] | None:
|
|||||||
|
|
||||||
|
|
||||||
def _identity() -> auth.Identity | 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
|
@app.before_request
|
||||||
|
|||||||
@@ -23,8 +23,11 @@ services:
|
|||||||
# URL_BASE: /bindery
|
# URL_BASE: /bindery
|
||||||
# true only if clients reach Bindery through a reverse proxy alone. The login lockout then uses X-Forwarded-For.
|
# true only if clients reach Bindery through a reverse proxy alone. The login lockout then uses X-Forwarded-For.
|
||||||
# TRUST_PROXY: "true"
|
# 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
|
# BINDERY_API_KEY: a-long-random-string
|
||||||
ports:
|
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
|
- 8085:8085
|
||||||
restart: unless-stopped
|
restart: unless-stopped
|
||||||
# /api/health needs no login, and works with and without URL_BASE.
|
# /api/health needs no login, and works with and without URL_BASE.
|
||||||
|
|||||||
@@ -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(
|
@pytest.mark.parametrize(
|
||||||
("setting", "value", "error"),
|
("setting", "value", "error"),
|
||||||
[
|
[
|
||||||
("AUTH_METHOD", "cwa", "AUTH_METHOD=cwa needs CWA_URL or CWA_DB_PATH"),
|
("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'"),
|
("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_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):
|
def test_incomplete_login_settings_refuse_every_request(client, monkeypatch, setting, value, error):
|
||||||
|
|||||||
@@ -97,6 +97,7 @@ def test_defaults(monkeypatch, reload_config):
|
|||||||
"DEFAULT_LOCKED_FIELDS",
|
"DEFAULT_LOCKED_FIELDS",
|
||||||
"LOG_LEVEL",
|
"LOG_LEVEL",
|
||||||
"PROXY_AUTH_USER_HEADER",
|
"PROXY_AUTH_USER_HEADER",
|
||||||
|
"PROXY_AUTH_TRUSTED_IPS",
|
||||||
"SESSION_COOKIE_SECURE",
|
"SESSION_COOKIE_SECURE",
|
||||||
"TRUST_PROXY",
|
"TRUST_PROXY",
|
||||||
"CWA_DB_PATH",
|
"CWA_DB_PATH",
|
||||||
@@ -113,6 +114,15 @@ def test_defaults(monkeypatch, reload_config):
|
|||||||
assert config.DEFAULT_LOCKED_FIELDS == ["title"]
|
assert config.DEFAULT_LOCKED_FIELDS == ["title"]
|
||||||
assert config.LOG_LEVEL == "INFO"
|
assert config.LOG_LEVEL == "INFO"
|
||||||
assert config.PROXY_AUTH_USER_HEADER == "X-Remote-User"
|
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.SESSION_COOKIE_SECURE is False
|
||||||
assert config.TRUST_PROXY is False
|
assert config.TRUST_PROXY is False
|
||||||
assert config.CWA_DB_PATH is None
|
assert config.CWA_DB_PATH is None
|
||||||
|
|||||||
Reference in New Issue
Block a user