Rework trusted proxies #5549
Rework trusted proxies #5549
Conversation
Fix FreshRSS#5502 Follow-up of FreshRSS#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.
|
To ease testing, I have pushed a Docker image |
|
From what I can tell, the image works fine on my system, using Caddy's Thank your for working on this! |
| if (empty($trusted)) { | ||
| $trusted = FreshRSS_Context::$system_conf->trusted_sources; | ||
| } | ||
| foreach (FreshRSS_Context::$system_conf->trusted_sources as $cidr) { |
There was a problem hiding this comment.
@Alkarex I've been trying to get this working with Authentik and staring at this logic for a bit. Am I crazy or does this completely override the $trusted logic on line 684?
On line 684 $trusted gets set, then if it's empty() on line 687 it gets overridden with FreshRSS_Context::$system_conf->trusted_sources, but is ignored for the rest of the function as it defers to FreshRSS_Context::$system_conf->trusted_sources on this line.
I currently have TRUSTED_PROXY set to 192.168.1.34/32, $trusted gets set, and then this falls through to false on line 695.
There was a problem hiding this comment.
Indeed, this should probably be something like:
| foreach (FreshRSS_Context::$system_conf->trusted_sources as $cidr) { | |
| foreach ($trusted as $cidr) { |
Do you feel like sending a PR?
Fix #5502
Follow-up of #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#remoteiptrustedproxyNew internal environment variable
CONN_REMOTE_ADDRto 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.