Rework trusted proxies (#5549)

* Rework trusted proxies
Fix https://github.com/FreshRSS/FreshRSS/issues/5502
Follow-up of https://github.com/FreshRSS/FreshRSS/pull/3226

New environment variable `TRUSTED_PROXY`: set to 0 to disable, or to a list of trusted IP ranges compatible with https://httpd.apache.org/docs/current/mod/mod_remoteip.html#remoteiptrustedproxy

New internal environment variable `CONN_REMOTE_ADDR` to remember the true IP address of the connection (e.g. last proxy), even when using mod_remoteip.

Current working setups should not observe any significant change.

* Minor whitespace

* Safer trusted sources during install
Rework of https://github.com/FreshRSS/FreshRSS/pull/5358
https://github.com/FreshRSS/FreshRSS/issues/5357

* Minor readme

17 files changed, 100 insertions, 24 deletions

diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile
index 73073dcf1..0f0aabfa1 100644
--- a/.devcontainer/Dockerfile
+++ b/.devcontainer/Dockerfile
@@ -29,5 +29,6 @@ ENV CRON_MIN ''
 ENV DATA_PATH ''
 ENV FRESHRSS_ENV 'development'
 ENV LISTEN '0.0.0.0:8080'
+ENV TRUSTED_PROXY 0
 
 EXPOSE 8080
diff --git a/Docker/Dockerfile b/Docker/Dockerfile
index 4b6979993..239a0e067 100644
--- a/Docker/Dockerfile
+++ b/Docker/Dockerfile
@@ -58,6 +58,7 @@ ENV DATA_PATH ''
 ENV FRESHRSS_ENV ''
 ENV LISTEN ''
 ENV OIDC_ENABLED ''
+ENV TRUSTED_PROXY ''
 
 ENTRYPOINT ["./Docker/entrypoint.sh"]
 
diff --git a/Docker/Dockerfile-Alpine b/Docker/Dockerfile-Alpine
index 59142384c..1da380f81 100644
--- a/Docker/Dockerfile-Alpine
+++ b/Docker/Dockerfile-Alpine
@@ -54,6 +54,7 @@ ENV DATA_PATH ''
 ENV FRESHRSS_ENV ''
 ENV LISTEN ''
 ENV OIDC_ENABLED ''
+ENV TRUSTED_PROXY ''
 
 ENTRYPOINT ["./Docker/entrypoint.sh"]
 
diff --git a/Docker/Dockerfile-Newest b/Docker/Dockerfile-Newest
index 8c2d6eb71..c5615b512 100644
--- a/Docker/Dockerfile-Newest
+++ b/Docker/Dockerfile-Newest
@@ -57,6 +57,7 @@ ENV DATA_PATH ''
 ENV FRESHRSS_ENV ''
 ENV LISTEN ''
 ENV OIDC_ENABLED ''
+ENV TRUSTED_PROXY ''
 
 ENTRYPOINT ["./Docker/entrypoint.sh"]
 
diff --git a/Docker/Dockerfile-Oldest b/Docker/Dockerfile-Oldest
index 88d02b512..22b9cec21 100644
--- a/Docker/Dockerfile-Oldest
+++ b/Docker/Dockerfile-Oldest
@@ -56,6 +56,7 @@ ENV DATA_PATH ''
 ENV FRESHRSS_ENV ''
 ENV LISTEN ''
 ENV OIDC_ENABLED ''
+ENV TRUSTED_PROXY ''
 
 ENTRYPOINT ["./Docker/entrypoint.sh"]
 
diff --git a/Docker/Dockerfile-QEMU-ARM b/Docker/Dockerfile-QEMU-ARM
index 58459cf37..48ce56345 100644
--- a/Docker/Dockerfile-QEMU-ARM
+++ b/Docker/Dockerfile-QEMU-ARM
@@ -70,6 +70,7 @@ ENV DATA_PATH ''
 ENV FRESHRSS_ENV ''
 ENV LISTEN ''
 ENV OIDC_ENABLED ''
+ENV TRUSTED_PROXY ''
 
 ENTRYPOINT ["./Docker/entrypoint.sh"]
 
diff --git a/Docker/FreshRSS.Apache.conf b/Docker/FreshRSS.Apache.conf
index 9330a17f4..f3dc6da7c 100644
--- a/Docker/FreshRSS.Apache.conf
+++ b/Docker/FreshRSS.Apache.conf
@@ -1,14 +1,21 @@
 ServerName freshrss.localhost
 Listen 80
 DocumentRoot /var/www/FreshRSS/p/
-RemoteIPHeader X-Forwarded-For
-RemoteIPTrustedProxy 10.0.0.1/8 172.16.0.1/12 192.168.0.1/16
-LogFormat "%a %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-Agent}i\"" combined_proxy
-CustomLog "|/var/www/FreshRSS/cli/sensitive-log.sh" combined_proxy
-ErrorLog /dev/stderr
 AllowEncodedSlashes On
 ServerTokens OS
 TraceEnable Off
+ErrorLog /dev/stderr
+
+# For logging the original user-agent IP instead of proxy IPs:
+<IfModule mod_remoteip.c>
+	# Can be disabled by setting the TRUSTED_PROXY environment variable to 0:
+	RemoteIPHeader X-Forwarded-For
+	# Can be overridden by the TRUSTED_PROXY environment variable:
+	RemoteIPTrustedProxy 10.0.0.1/8 172.16.0.1/12 192.168.0.1/16
+</IfModule>
+
+LogFormat "%a %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-Agent}i\"" combined_proxy
+CustomLog "|/var/www/FreshRSS/cli/sensitive-log.sh" combined_proxy
 
 <IfDefine OIDC_ENABLED>
 	<IfModule !auth_openidc_module>
diff --git a/Docker/README.md b/Docker/README.md
index 1fbc9634c..ffd1fc2d2 100644
--- a/Docker/README.md
+++ b/Docker/README.md
@@ -330,6 +330,13 @@ services:
       FRESHRSS_ENV: development
       # Optional advanced parameter controlling the internal Apache listening port
       LISTEN: 0.0.0.0:80
+      # Optional parameter, remove for automatic settings, set to 0 to disable,
+	  # or (if you use a proxy) to a space-separated list of trusted IP ranges
+      # compatible with https://httpd.apache.org/docs/current/mod/mod_remoteip.html#remoteiptrustedproxy
+      # This impacts which IP address is logged (X-Forwarded-For or REMOTE_ADDR).
+      # This also impacts external authentication methods;
+      # see https://freshrss.github.io/FreshRSS/en/admins/09_AccessControl.html
+      TRUSTED_PROXY: 172.16.0.1/12 192.168.0.1/16
       # Optional parameter, set to 1 to enable OpenID Connect (only available in our Debian image)
       # Requires more environment variables. See https://freshrss.github.io/FreshRSS/en/admins/16_OpenID-Connect.html
       OIDC_ENABLED: 0
diff --git a/Docker/entrypoint.sh b/Docker/entrypoint.sh
index 1b25026b5..6cb5a49b4 100755
--- a/Docker/entrypoint.sh
+++ b/Docker/entrypoint.sh
@@ -11,6 +11,16 @@ if [ -n "$LISTEN" ]; then
 	find /etc/apache2/ -type f -name FreshRSS.Apache.conf -exec sed -r -i "\\#^Listen#s#^.*#Listen $LISTEN#" {} \;
 fi
 
+if [ -n "$TRUSTED_PROXY" ]; then
+	if [ "$TRUSTED_PROXY" -eq 0 ]; then
+		# Disable RemoteIPHeader and RemoteIPTrustedProxy
+		find /etc/apache2/ -type f -name FreshRSS.Apache.conf -exec sed -r -i "/^\s*RemoteIP.*$/s/^/#/" {} \;
+	else
+		# Custom list for RemoteIPTrustedProxy
+		find /etc/apache2/ -type f -name FreshRSS.Apache.conf -exec sed -r -i "\\#^\s*RemoteIPTrustedProxy#s#^.*#\tRemoteIPTrustedProxy $TRUSTED_PROXY#" {} \;
+	fi
+fi
+
 if [ -n "$OIDC_ENABLED" ] && [ "$OIDC_ENABLED" -ne 0 ]; then
 	a2enmod -q auth_openidc
 fi
diff --git a/Docker/freshrss/docker-compose-proxy.yml b/Docker/freshrss/docker-compose-proxy.yml
index 980e45e67..9b4846bce 100644
--- a/Docker/freshrss/docker-compose-proxy.yml
+++ b/Docker/freshrss/docker-compose-proxy.yml
@@ -7,7 +7,7 @@ volumes:
 services:
 
   traefik:
-    image: traefik:2.6
+    image: traefik:2.10
     container_name: traefik
     restart: unless-stopped
     logging:
@@ -42,6 +42,8 @@ services:
       - traefik.enable=false
 
   freshrss:
+    environment:
+      TRUSTED_PROXY: 172.16.0.1/12
     labels:
       - traefik.enable=true
       - traefik.http.middlewares.freshrssM1.compress=true
diff --git a/Docker/freshrss/docker-compose.yml b/Docker/freshrss/docker-compose.yml
index b8956bca5..7eb23fe9c 100644
--- a/Docker/freshrss/docker-compose.yml
+++ b/Docker/freshrss/docker-compose.yml
@@ -25,3 +25,4 @@ services:
     environment:
       TZ: Europe/Paris
       CRON_MIN: '3,33'
+      TRUSTED_PROXY: 172.16.0.1/12 192.168.0.1/16
diff --git a/app/Controllers/authController.php b/app/Controllers/authController.php
index 8feebe9a3..90c9a9e03 100644
--- a/app/Controllers/authController.php
+++ b/app/Controllers/authController.php
@@ -79,7 +79,7 @@ class FreshRSS_auth_Controller extends FreshRSS_ActionController {
 					'error' => [
 						_t('feedback.access.denied'),
 						' [HTTP Remote-User=' . htmlspecialchars(httpAuthUser(false), ENT_NOQUOTES, 'UTF-8') .
-						' ; Remote IP address=' . ($_SERVER['REMOTE_ADDR'] ?? '') . ']'
+						' ; Remote IP address=' . connectionRemoteAddress() . ']'
 					]
 				], false);
 				break;
diff --git a/app/install.php b/app/install.php
index 873689054..b42771fbb 100644
--- a/app/install.php
+++ b/app/install.php
@@ -208,9 +208,14 @@ function saveStep3(): bool {
 			return false;
 		}
 
-		if (FreshRSS_Context::$system_conf->auth_type === 'http_auth' && !empty($_SERVER['REMOTE_ADDR']) && is_string($_SERVER['REMOTE_ADDR'])) {
-			// Trust by default the remote IP address (e.g. proxy) used during install to provide remote user name
-			FreshRSS_Context::$system_conf->trusted_sources = [ $_SERVER['REMOTE_ADDR'] ];
+		if (FreshRSS_Context::$system_conf->auth_type === 'http_auth' &&
+			connectionRemoteAddress() !== '' &&
+			empty($_SERVER['REMOTE_USER']) && empty($_SERVER['REDIRECT_REMOTE_USER']) &&	// No safe authentication HTTP headers
+			(!empty($_SERVER['HTTP_REMOTE_USER']) || !empty($_SERVER['HTTP_X_WEBAUTH_USER']))	// but has unsafe authentication HTTP headers
+		) {
+			// Trust by default the remote IP address (e.g. last proxy) used during install to provide remote user name via unsafe HTTP header
+			FreshRSS_Context::$system_conf->trusted_sources[] = connectionRemoteAddress();
+			FreshRSS_Context::$system_conf->trusted_sources = array_unique(FreshRSS_Context::$system_conf->trusted_sources);
 		}
 
 		// Create default user files but first, we delete previous data to
diff --git a/config.default.php b/config.default.php
index b5e3a6318..f7c4e1315 100644
--- a/config.default.php
+++ b/config.default.php
@@ -194,9 +194,12 @@ return array(
 	# Disable self-update,
 	'disable_update' => false,
 
-	# Trusted IPs that are allowed to send unsafe headers
-	# Please read the documentation, before configuring this
-	# https://freshrss.github.io/FreshRSS/en/admins/09_AccessControl.html
+	# Trusted IPs (e.g. of last proxy) that are allowed to send unsafe HTTP headers.
+	# The connection IP used during FreshRSS setup is automatically added to this list.
+	# Will be checked against CONN_REMOTE_ADDR (if available, to be robust even when using Apache mod_remoteip)
+	# or REMOTE_ADDR environment variable.
+	# This array can be overridden by the TRUSTED_PROXY environment variable.
+	# Read the documentation before configuring this https://freshrss.github.io/FreshRSS/en/admins/09_AccessControl.html
 	'trusted_sources' => [
 		'127.0.0.0/8',
 		'::1/128',
diff --git a/docs/en/admins/09_AccessControl.md b/docs/en/admins/09_AccessControl.md
index 870d3a6ad..65f78be4d 100644
--- a/docs/en/admins/09_AccessControl.md
+++ b/docs/en/admins/09_AccessControl.md
@@ -24,13 +24,15 @@ variable containing the email address of the authenticated user (e.g. `REMOTE_US
 
 ## External Authentication
 
-You may also use the `Remote-User` or `X-WebAuth-User` header to integrate with a your reverse-proxy’s authentication.
+You may also use the `Remote-User` or `X-WebAuth-User` HTTP headers to integrate with a reverse-proxy’s authentication.
 
 To enable this feature, you need to add the IP range (in CIDR notation) of your trusted proxy in the `trusted_sources` configuration option.
 To allow only one IPv4, you can use a `/32` like this: `trusted_sources => [ '192.168.1.10/32' ]`.
 Likewise to allow only one IPv6, you can use a `/128` like this: `trusted_sources => [ '::1/128' ]`.
 
-WARNING: FreshRSS will trust any IP configured in the `trusted_sources` option, if your proxy isn’t properly secured, an attacker could simply attach this header and get admin access.
+You may alternatively pass a `TRUSTED_PROXY` environment variable in a format compatible with [Apache’s `mod_remoteip` `RemoteIPTrustedProxy`](https://httpd.apache.org/docs/current/mod/mod_remoteip.html#remoteiptrustedproxy).
+
+> ☠️ WARNING: FreshRSS will trust any IP configured in the `trusted_sources` option, if your proxy isn’t properly secured, an attacker could simply attach this header and get admin access.
 
 ## No Authentication
 
diff --git a/lib/lib_rss.php b/lib/lib_rss.php
index ec00513e2..9d995c6d6 100644
--- a/lib/lib_rss.php
+++ b/lib/lib_rss.php
@@ -653,21 +653,43 @@ function checkCIDR(string $ip, string $range): bool {
 }
 
 /**
- * Check if the client is allowed to send unsafe headers
- * This uses the REMOTE_ADDR header to determine the sender’s IP
- * and the configuration option "trusted_sources" to get an array of the authorized ranges
- *
+ * Use CONN_REMOTE_ADDR (if available, to be robust even when using Apache mod_remoteip) or REMOTE_ADDR environment variable to determine the connection IP.
+ */
+function connectionRemoteAddress(): string {
+	$remoteIp = $_SERVER['CONN_REMOTE_ADDR'] ?? '';
+	if ($remoteIp == '') {
+		$remoteIp = $_SERVER['REMOTE_ADDR'] ?? '';
+	}
+	if ($remoteIp == 0) {
+		$remoteIp = '';
+	}
+	return $remoteIp;
+}
+
+/**
+ * Check if the client (e.g. last proxy) is allowed to send unsafe headers.
+ * This uses the `TRUSTED_PROXY` environment variable or the `trusted_sources` configuration option to get an array of the authorized ranges,
+ * The connection IP is obtained from the `CONN_REMOTE_ADDR` (if available, to be robust even when using Apache mod_remoteip) or `REMOTE_ADDR` environment variables.
  * @return bool, true if the sender’s IP is in one of the ranges defined in the configuration, else false
  */
 function checkTrustedIP(): bool {
 	if (FreshRSS_Context::$system_conf === null) {
 		return false;
 	}
-	if (!empty($_SERVER['REMOTE_ADDR'])) {
-		foreach (FreshRSS_Context::$system_conf->trusted_sources as $cidr) {
-			if (checkCIDR($_SERVER['REMOTE_ADDR'], $cidr)) {
-				return true;
-			}
+	$remoteIp = connectionRemoteAddress();
+	if ($remoteIp === '') {
+		return false;
+	}
+	$trusted = getenv('TRUSTED_PROXY');
+	if ($trusted != 0 && is_string($trusted)) {
+		$trusted = preg_split('/\s+/', $trusted, -1, PREG_SPLIT_NO_EMPTY);
+	}
+	if (empty($trusted)) {
+		$trusted = FreshRSS_Context::$system_conf->trusted_sources;
+	}
+	foreach (FreshRSS_Context::$system_conf->trusted_sources as $cidr) {
+		if (checkCIDR($remoteIp, $cidr)) {
+			return true;
 		}
 	}
 	return false;
diff --git a/p/.htaccess b/p/.htaccess
index 79e315c07..955b0c59d 100644
--- a/p/.htaccess
+++ b/p/.htaccess
@@ -35,3 +35,14 @@ AddDefaultCharset	UTF-8
 	</FilesMatch>
 	Header edit Set-Cookie ^(.*)$ "$1; SameSite=Lax"
 </IfModule>
+
+# Provide the true IP address of the connection (e.g. last proxy), even when using mod_remoteip
+<IfModule mod_setenvif.c>
+	SetEnvIfExpr "%{CONN_REMOTE_ADDR} =~ /(.*)/" CONN_REMOTE_ADDR=$1
+</IfModule>
+<IfModule !mod_setenvif.c>
+	<IfModule mod_rewrite.c>
+		RewriteEngine on
+		RewriteRule .* - [E=CONN_REMOTE_ADDR:%{CONN_REMOTE_ADDR}]
+	</IfModule>
+</IfModule>