New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add config to read the last IP address in the list of proxies rather than the first #17765
Conversation
Before merging once the config is final be good to document this security feature in https://matomo.org/faq/how-to-install/faq_98/ and maybe in https://matomo.org/docs/security-how-to/ we could as well mention that they might want to enable this config if they are using a load balancer. @mattab you will want to document this in the enterprise guide probably as well. |
config/global.ini.php
Outdated
@@ -641,6 +641,9 @@ | |||
; By enabling this flag the header HTTP_X_FORWARDED_URI will be considered for the current script name. | |||
proxy_uri_header = 0 | |||
|
|||
; If set to 1 we use the last IP in the list of proxy IPs when determining the client IP. | |||
proxy_ip_read_last_in_list = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we make 0
as default to keep bc. Or at least set this option automatically to 0
on update, to ensure it won't break any instances. Could maybe only do that if proxy_client_headers
is set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was specified to set it to 1 in the issue (I assume it adds security). cc @tsteur
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default we want to keep existing behaviour and follow the RFC so we'd set it to 0
as otherwise it would be a breaking change.
We probably wouldn't leave the default at 1 and change it for existing installs as there are often random issues with it and it would require an additional step when setting up proxy headers. I reckon we could create an issue though to do this in Matomo 5 and put it directly into the Matomo 5 milestone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi could tweak the description a bit.Like
; If set to 1, the last IP in the list of proxy IPs will be used when determining the client IP. Using the last IP can increase the security when you are using proxy headers in combination with a load balancer. By default, the first IP is read according to RFC7239 which may be required in some cases when the client sends the IP through a proxy header as well as the load balancer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI put #17202 in Matomo 5 milestone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me now
Sounds good, let me know when the user guide is updated and i'll copy the information in the PDF enterprise guide. |
@mattab added a bit to the docs |
Description:
Fixes L3-123.
Review