diff --git a/.env.example b/.env.example index bffa1b6..7b0d8a5 100644 --- a/.env.example +++ b/.env.example @@ -10,6 +10,11 @@ DOOROPENER_PORT=6532 FLASK_SECRET_KEY=your-secret-key-here FLASK_DEBUG=false +# Interface the container port is published on (docker-compose). Keep 127.0.0.1 behind a reverse proxy. +DOOROPENER_BIND=127.0.0.1 +# Number of reverse proxies in front of the app whose X-Forwarded-For is trusted (0 = none, direct access) +DOOROPENER_TRUSTED_PROXIES=1 + # When running behind HTTPS (reverse proxy), keep this true so cookies are secure. # For local HTTP testing only, set to false so the browser will send session cookies. SESSION_COOKIE_SECURE=true diff --git a/README.md b/README.md index 21fe48c..5c82083 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ services: container_name: dooropener env_file: .env ports: - - "${DOOROPENER_PORT:-6532}:${DOOROPENER_PORT:-6532}" + - "${DOOROPENER_BIND:-127.0.0.1}:${DOOROPENER_PORT:-6532}:${DOOROPENER_PORT:-6532}" volumes: - ./config.ini:/app/config.ini:ro - ./users.json:/app/users.json @@ -70,7 +70,7 @@ cp .env.example .env # set FLASK_SECRET_KEY at minimum docker compose up -d ``` -Then open `http://your-server:6532`. +Then open `http://localhost:6532` on the Docker host, or the URL configured on your reverse proxy. The port is published on `127.0.0.1` by default; see `DOOROPENER_BIND` below if you need it reachable directly. ### Build locally @@ -80,7 +80,7 @@ docker run -d --env-file .env \ -v $(pwd)/config.ini:/app/config.ini:ro \ -v $(pwd)/users.json:/app/users.json \ -v $(pwd)/logs:/app/logs \ - -p 6532:6532 dooropener:latest + -p 127.0.0.1:6532:6532 dooropener:latest ``` ### Without Docker @@ -99,6 +99,8 @@ python app.py ```bash FLASK_SECRET_KEY=change-me-to-something-long-and-random # required DOOROPENER_PORT=6532 # default 6532 +DOOROPENER_BIND=127.0.0.1 # interface docker-compose publishes the port on (keep local behind a reverse proxy) +DOOROPENER_TRUSTED_PROXIES=1 # reverse proxies whose X-Forwarded-For is trusted; 0 if clients connect directly TZ=Europe/Amsterdam # default UTC PUID=1000 # aligns container user to your host user PGID=1000 diff --git a/app.py b/app.py index 554b478..4b2cb48 100644 --- a/app.py +++ b/app.py @@ -97,7 +97,6 @@ def get_current_time(): # --- Flask App Setup --- app = Flask(__name__) -app.wsgi_app = ProxyFix(app.wsgi_app, x_for=1, x_proto=1, x_host=1) # Prefer fixed secret from environment; fallback to temporary random (will be overridden by config.ini later if present) _env_secret = os.environ.get("FLASK_SECRET_KEY") if _env_secret: @@ -173,6 +172,19 @@ def get_effective_user_pins() -> dict: # Server Configuration server_port = int(os.environ.get("DOOROPENER_PORT", config.getint("server", "port", fallback=6532))) test_mode = config.getboolean("server", "test_mode", fallback=False) + +# Number of reverse proxies in front of the app whose X-Forwarded-* headers we trust. Every rate +# limit keys on the client IP, so trusting more hops than actually exist lets any caller who can +# reach the port directly pick their own IP via a forged X-Forwarded-For. 0 = no proxy (use the +# socket address). Default 1 matches a single reverse proxy such as Traefik/nginx/Caddy. +# Select the source first and parse only that value, so a bad INI entry can't break startup when +# the environment override is valid. +_env_proxies = os.environ.get("DOOROPENER_TRUSTED_PROXIES") +TRUSTED_PROXIES = ( + int(_env_proxies) if _env_proxies is not None else config.getint("server", "trusted_proxies", fallback=1) +) +if TRUSTED_PROXIES > 0: + app.wsgi_app = ProxyFix(app.wsgi_app, x_for=TRUSTED_PROXIES, x_proto=TRUSTED_PROXIES, x_host=TRUSTED_PROXIES) if test_mode: logging.getLogger("dooropener").warning( "TEST MODE ENABLED — the door will NOT open. " @@ -314,7 +326,8 @@ def manifest_file(): def get_client_identifier(): """Get client identifier using multiple factors for better security""" - # Use request.remote_addr as primary (can't be spoofed easily) + # request.remote_addr is the only attacker-independent factor (given a correct + # TRUSTED_PROXIES setting); everything else a client sends can be varied per request. primary_ip = request.remote_addr # Create session-based identifier if available @@ -323,12 +336,9 @@ def get_client_identifier(): session_id = secrets.token_hex(16) session["_session_id"] = session_id - # Combine multiple factors for identifier - user_agent = request.headers.get("User-Agent", "")[:100] # Limit length - accept_lang = request.headers.get("Accept-Language", "")[:50] - - # Create composite identifier (harder to spoof than just IP) - identifier = f"{primary_ip}:{hash(user_agent + accept_lang) % 10000}" + # The throttling identifier must NOT include client-controlled headers (User-Agent, + # Accept-Language): rotating them would hand an attacker a fresh failure counter per request. + identifier = primary_ip # Record activity so idle rate-limit state for these keys can be evicted later. now_mono = time.monotonic() diff --git a/config.ini.example b/config.ini.example index 8e0737c..d218d8f 100644 --- a/config.ini.example +++ b/config.ini.example @@ -26,6 +26,10 @@ admin_password = admin123 # Can be overridden by DOOROPENER_PORT environment variable port = 6532 +# Reverse proxies in front of the app whose X-Forwarded-For is trusted (0 = direct access). +# Can be overridden by DOOROPENER_TRUSTED_PROXIES. +trusted_proxies = 1 + # Test mode - when true, shows success message but doesn't actually open door test_mode = false diff --git a/docker-compose.yml b/docker-compose.yml index c2ce245..b8b4af2 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -13,7 +13,10 @@ services: - PUID=${PUID:-1000} - PGID=${PGID:-1000} ports: - - "${DOOROPENER_PORT:-6532}:${DOOROPENER_PORT:-6532}" + # Bound to localhost by default so only your reverse proxy can reach it. Rate limits key on the + # client IP taken from X-Forwarded-For, which anyone who can reach this port directly could forge. + # Set DOOROPENER_BIND=0.0.0.0 only if you accept that (and set DOOROPENER_TRUSTED_PROXIES=0). + - "${DOOROPENER_BIND:-127.0.0.1}:${DOOROPENER_PORT:-6532}:${DOOROPENER_PORT:-6532}" volumes: - ./config.ini:/app/config.ini:rw - ./logs:/app/logs diff --git a/tests/test_ip_rate_limit.py b/tests/test_ip_rate_limit.py new file mode 100644 index 0000000..726553b --- /dev/null +++ b/tests/test_ip_rate_limit.py @@ -0,0 +1,78 @@ +"""Rate limits must key on the client IP, not on headers the client controls.""" + +import pytest + +HEADERS = {"User-Agent": "pytest-client/1.0 (+https://example.test)", "Content-Type": "application/json"} + + +@pytest.fixture +def app_module(): + import app as app_module + + return app_module + + +@pytest.fixture +def client(app_module, monkeypatch): + monkeypatch.setitem(app_module.app.config, "TESTING", True) + with app_module.app.test_client() as c: + yield c + + +def test_identifier_ignores_user_agent_and_language(app_module): + seen = set() + for i in range(5): + with app_module.app.test_request_context( + "/", headers={"User-Agent": f"agent-number-{i}", "Accept-Language": f"xx-{i}"} + ): + seen.add(app_module.get_client_identifier()[2]) + assert len(seen) == 1 + + +def test_rotating_user_agent_still_gets_blocked(client, app_module, monkeypatch): + monkeypatch.setattr(app_module, "test_mode", True) + statuses = [] + for i in range(app_module.MAX_ATTEMPTS + 1): + client.delete_cookie("session") # also drop the cookie so only the IP can throttle + h = {**HEADERS, "User-Agent": f"rotating-agent-{i}-padding", "Accept-Language": f"l{i}"} + statuses.append(client.post("/open-door", json={"pin": "0000"}, headers=h).status_code) + assert statuses[-1] == 429 + + +def test_env_override_wins_even_if_ini_value_is_invalid(tmp_path): + """DOOROPENER_TRUSTED_PROXIES must be selected before the INI value is parsed. + + conftest mocks ConfigParser, so run the real thing in a subprocess against a throwaway copy. + """ + import os + import shutil + import subprocess + import sys + + root = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) + work = tmp_path / "app" + work.mkdir() + for name in os.listdir(root): + if name.endswith(".py"): + shutil.copy(os.path.join(root, name), work / name) + for d in ("templates", "static"): + shutil.copytree(os.path.join(root, d), work / d) + (work / "config.ini").write_text( + "[HomeAssistant]\nurl = http://x\ntoken = t\nswitch_entity = switch.d\n" + "[admin]\nadmin_password = a-real-password\n[server]\ntrusted_proxies = not-a-number\n" + ) + # Drop pytest-cov's env vars: otherwise its .pth hook measures this throwaway copy of the app + # and its uncovered lines drag the project's coverage below the CI gate. + env = {k: v for k, v in os.environ.items() if not k.startswith(("COV_CORE", "COVERAGE"))} + env.update(DOOROPENER_LOG_DIR=str(tmp_path / "logs"), USERS_STORE_PATH=str(tmp_path / "u.json")) + code = f"import sys; sys.path.insert(0, {str(work)!r}); import app; print(app.TRUSTED_PROXIES)" + + ok = subprocess.run( + [sys.executable, "-I", "-c", code], + cwd=work, + env={**env, "DOOROPENER_TRUSTED_PROXIES": "0"}, + capture_output=True, + text=True, + ) + assert ok.returncode == 0, ok.stderr[-1500:] + assert ok.stdout.strip().endswith("0")