feat: add HP_TRUSTED_PROXY_IPS env option (#53)

* feat: add HP_TRUSTED_PROXY_IPS env option

Signed-off-by: Oleksander Piskun <[email protected]>

Author: oleksandr-nc

Dockerfile

@@ -37,6 +37,7 @@ ENV HP_EXAPPS_ADDRESS="0.0.0.0:8780" \
     HP_TIMEOUT_CLIENT="30s" \
     HP_TIMEOUT_SERVER="1800s" \
     NC_INSTANCE_URL="" \
+    HP_TRUSTED_PROXY_IPS="" \
     HP_LOG_LEVEL="warning"
 
 # NOTE: We do NOT define HP_SHARED_KEY or HP_SHARED_KEY_FILE here

README.md

@@ -136,6 +136,11 @@ HaRP is configured via several environment variables. Here are the key variables
     - `HP_EXAPPS_HTTPS_ADDRESS="0.0.0.0:8781"`
   - **Note:** Must be reachable by your reverse proxy.
 
+- **`HP_TRUSTED_PROXY_IPS`**
+  - **Description:** A comma-separated list of trusted reverse proxy IP addresses or CIDR ranges. When HaRP is behind another reverse proxy (like NGINX), set this to the IP of that proxy to allow HaRP to correctly identify the true client IP from `X-Forwarded-For` or `X-Real-IP` headers.
+  - **Default:** `""` (disabled)
+  - **Example:** `"172.18.0.0/16,127.0.0.1"`
+
 - **`HP_FRP_ADDRESS`**
   - **Description:** IP:Port for the FRP (TCP) frontends.
   - **Default:** `HP_FRP_ADDRESS="0.0.0.0:8782"`
@@ -336,6 +341,7 @@ docker run \
   -e NC_INSTANCE_URL="http://nextcloud.local" \
   -e HP_LOG_LEVEL="debug" \
   -e HP_VERBOSE_START="1" \
+  -e HP_TRUSTED_PROXY_IPS="192.168.0.0/16" \
   -v /var/run/docker.sock:/var/run/docker.sock \
   -v `pwd`/certs:/certs \
   --name appapi-harp -h appapi-harp \
@@ -346,6 +352,9 @@ docker run \
   -d nextcloud-appapi-harp:local
 ```
 
+> [!important]
+> Be mindful of checking and changing the environment variables `HP_SHARED_KEY`, `NC_INSTANCE_URL`, and `HP_TRUSTED_PROXY_IPS` in the above command to suit your environment and setup.
+
 #### Debugging HaRP
 
 ##### One time initializing steps:

haproxy_agent.py

@@ -44,6 +44,23 @@
 SPOA_AGENT = SpoaServer()
 DOCKER_API_HOST = "127.0.0.1"
 
+TRUSTED_PROXIES_STR = os.environ.get("HP_TRUSTED_PROXY_IPS", "")
+TRUSTED_PROXIES = []
+if TRUSTED_PROXIES_STR:
+    try:
+        TRUSTED_PROXIES = [
+            ipaddress.ip_network(proxy.strip()) for proxy in TRUSTED_PROXIES_STR.split(",") if proxy.strip()
+        ]
+        LOGGER.info("Trusting reverse proxies for client IP detection: %s", [str(p) for p in TRUSTED_PROXIES])
+    except ValueError as e:
+        LOGGER.error(
+            "Invalid value for HP_TRUSTED_PROXY_IPS: %s. Client IP detection from headers is disabled. "
+            "The X-Forwarded-For and X-Real-IP headers will not be respected. "
+            "This can lead to the outer proxy's IP being blocked during a bruteforce attempt instead of the actual client's IP.",
+            e,
+        )
+        TRUSTED_PROXIES = []
+
 ###############################################################################
 # Definitions
 ###############################################################################
@@ -155,6 +172,36 @@ class InstallCertificatesPayload(ExAppName):
 ###############################################################################
 
 
+def get_true_client_ip(
+    direct_ip: ipaddress.IPv4Address | ipaddress.IPv6Address, headers: dict[str, str]
+) -> ipaddress.IPv4Address | ipaddress.IPv6Address:
+    """Determine the true client IP by inspecting headers from trusted proxies."""
+    if not TRUSTED_PROXIES:
+        return direct_ip
+
+    is_trusted = any(direct_ip in network for network in TRUSTED_PROXIES)
+    if not is_trusted:
+        return direct_ip
+
+    # The request is from a trusted proxy, so we can check the headers.
+    # X-Forwarded-For can be a list: client, proxy1, proxy2. We want the first one.
+    x_forwarded_for = headers.get("x-forwarded-for")
+    if x_forwarded_for:
+        true_ip_str = x_forwarded_for.split(",")[0].strip()
+        try:
+            return ipaddress.ip_address(true_ip_str)
+        except ValueError:
+            LOGGER.warning("Could not parse IP from X-Forwarded-For header: %s", true_ip_str)
+
+    x_real_ip = headers.get("x-real-ip")
+    if x_real_ip:
+        try:
+            return ipaddress.ip_address(x_real_ip)
+        except ValueError:
+            LOGGER.warning("Could not parse IP from X-Real-IP header: %s", x_real_ip)
+    return direct_ip  # If headers are present but invalid, fall back to the direct IP of the proxy
+
+
 async def record_ip_failure(ip_address: str | IPv4Address | IPv6Address) -> None:
     """Record a failed request attempt for this IP using BLACKLIST_CACHE."""
     ip_str = str(ip_address)
@@ -165,7 +212,7 @@ async def record_ip_failure(ip_address: str | IPv4Address | IPv6Address) -> None
         attempts = [ts for ts in attempts if now - ts < BLACKLIST_REQUEST_WINDOW]
         attempts.append(now)
         BLACKLIST_CACHE[ip_str] = attempts
-        LOGGER.debug("Recorded failure for IP %s. Failures in window: %d", ip_str, len(attempts))
+        LOGGER.warning("Recorded failure for IP %s. Failures in window: %d", ip_str, len(attempts))
 
 
 async def is_ip_banned(ip_address: str | IPv4Address | IPv6Address) -> bool:
@@ -219,8 +266,10 @@ async def get_session(pass_cookie: str) -> NcUser | None:
 async def exapps_msg(
     path: str, headers: str, client_ip: ipaddress.IPv4Address | ipaddress.IPv6Address, pass_cookie: str
 ) -> AckPayload:
-    client_ip_str = str(client_ip)
     reply = AckPayload()
+    request_headers = parse_headers(headers)
+    client_ip_str = str(get_true_client_ip(client_ip, request_headers))
+    reply = reply.set_txn_var("true_client_ip", client_ip_str)
     LOGGER.debug("Incoming request to ExApp: path=%s, headers=%s, ip=%s", path, headers, client_ip_str)
 
     # Check if the IP is banned based on failed attempts in BLACKLIST_CACHE.
@@ -238,8 +287,6 @@ async def exapps_msg(
     target_path = path.removeprefix(f"/exapps/{exapp_id}")
     reply = reply.set_txn_var("target_path", target_path)
 
-    request_headers = parse_headers(headers)
-
     # Special handling for AppAPI requests
     if exapp_id == "app_api":
         return await handle_app_api_request(target_path, request_headers, client_ip_str, reply)
@@ -366,18 +413,15 @@ async def exapps_msg(
 
 
 @SPOA_AGENT.handler("exapps_response_status_msg")
-async def exapps_response_status_msg(
-    status: int, client_ip: ipaddress.IPv4Address | ipaddress.IPv6Address, statuses_to_trigger_bp: str
-) -> AckPayload:
+async def exapps_response_status_msg(status: int, client_ip: str, statuses_to_trigger_bp: str) -> AckPayload:
     reply = AckPayload()
     if not statuses_to_trigger_bp:
         return reply.set_txn_var("bp_triggered", 0)
     statuses = json.loads(statuses_to_trigger_bp)
     if status not in statuses:
         return reply.set_txn_var("bp_triggered", 0)
-    str_client_ip = str(client_ip)
-    LOGGER.warning("Bruteforce protection(status=%s) triggered IP=%s.", status, str_client_ip)
-    await record_ip_failure(str_client_ip)
+    LOGGER.warning("Bruteforce protection(status=%s) triggered IP=%s.", status, client_ip)
+    await record_ip_failure(client_ip)
     return reply.set_txn_var("bp_triggered", 1)
 
 

spoe-agent.conf

@@ -26,4 +26,4 @@ spoe-agent exapps-agent
 
 spoe-message exapps_response_status_msg
     event on-http-response
-    args status=status client_ip=src statuses_to_trigger_bp=var(txn.exapps.statuses_to_trigger_bp)
+    args status=status client_ip=var(txn.exapps.true_client_ip) statuses_to_trigger_bp=var(txn.exapps.statuses_to_trigger_bp)