-
Couldn't load subscription status.
- Fork 1.2k
Fix logging of forwarded IPs in logs #11854
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
base: main
Are you sure you want to change the base?
Conversation
…r.verify is enabled
|
@blueorangutan package |
|
@abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
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.
clgtm
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11854 +/- ##
=========================================
Coverage 17.55% 17.56%
- Complexity 15529 15531 +2
=========================================
Files 5909 5909
Lines 529012 529010 -2
Branches 64604 64603 -1
=========================================
+ Hits 92892 92909 +17
+ Misses 425671 425653 -18
+ Partials 10449 10448 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 15485 |
|
@blueorangutan package |
|
@abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15490 |
|
@blueorangutan package |
|
@abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 15495 |
|
@blueorangutan package |
|
@abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 15496 |
|
@blueorangutan package |
|
@abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 15509 |
63ce1c6 to
2ca1e10
Compare
|
@blueorangutan package |
|
@abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15513 |
|
@blueorangutan test |
|
@abh1sar a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-14708) |
|
@blueorangutan test |
|
@abh1sar a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-14710) |
Description
This PR fixes a bug due to which forwarded IPs behind a proxy won't be logged in management.log, api.log and access.log.
Access log : addForwardingCustomiser() updates request's remoteAddr to the forwarded client's IP
But ACSRequestLog.log() was looking at getHttpChannel().getEndPoint().getRemoteAddress(), which still returns the proxy's address
AddForwardinfCustomizer would only look at the first header in
proxy.header.names. If the user is sending client IP address via some other header, it won't be detected.Changing the request's remoteAddr by addFOrwardingCustomizer changes the behaviour in ApiServlet.getClientAddress() which expects request.getRemoteAddr() to return the proxy's address, so that it can compare it with the allowed proxy cidrs as set in
proxy.cidrFor the fix, I have removed addForwardingCustomizer and calling ApiServlet.getClientAddress() from ACSRequestLog. This way IPs in all the logs are consistent with each other and use the same method (getClientAddress()) to get the forwarded client's IP address.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Verified that the forwarded IP is being logged in all the logs.
Also tried the with the script test_forwareded_headers.sh as given here #11386 (review)
Settings used :

The script ./test_forwareded_headers.sh fails without the fix
How did you try to break this feature and the system with this change?