-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
breaking change: by default do not trust any client #3333
Conversation
89afa1e
to
b15701a
Compare
This change makes nginx-ingress not trust the IP for the The correct change to the defaults would be I 100% support setting |
@Dirbaio is there any practical security implication with this? I agree that modifying |
They're more rare, but there are indeed vulnerabilities that can be caused by the other X-Forwarded-* headers spoofing. For example: Software often uses X-Forwarded-Host, X-Forwarded-Scheme to build "links to itself" to use in emails and such. If you can mess with these, you can make these links go somewhere else, which can be bad for password reset emails, for example. Aside from vulns, disabling all |
b15701a
to
98705cf
Compare
98705cf
to
5f3b48e
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf, ElvinEfendi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
Currently we configure Nginx in a way that it trusts any client to extract true client IP address from X-Forwarded-For header using realip module. This PR makes it so that by default it does not trust any client at all.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged): fixes #Special notes for your reviewer: