COMP-1149 added HttpClientAddressResolver in RateLimiterFilter#966
Open
munishchouhan wants to merge 14 commits intomasterfrom
Open
COMP-1149 added HttpClientAddressResolver in RateLimiterFilter#966munishchouhan wants to merge 14 commits intomasterfrom
munishchouhan wants to merge 14 commits intomasterfrom
Conversation
Signed-off-by: munishchouhan <hrma017@gmail.com>
Signed-off-by: munishchouhan <hrma017@gmail.com>
Signed-off-by: munishchouhan <hrma017@gmail.com>
pditommaso
reviewed
Jan 16, 2026
|
|
||
| @Inject | ||
| @Nullable | ||
| private HttpClientAddressResolver addressResolver |
Collaborator
There was a problem hiding this comment.
What's prevent to use this always ?
Member
Author
There was a problem hiding this comment.
I am concerned that, it will add performance overhead on each request, I will dig more to see what is better
Signed-off-by: munishchouhan <hrma017@gmail.com>
Signed-off-by: munishchouhan <hrma017@gmail.com>
Signed-off-by: munishchouhan <hrma017@gmail.com>
Signed-off-by: munishchouhan <hrma017@gmail.com>
Signed-off-by: munishchouhan <hrma017@gmail.com>
Signed-off-by: munishchouhan <hrma017@gmail.com>
Member
Author
|
tested in dev: |
Member
Author
|
run again: |
Signed-off-by: munishchouhan <hrma017@gmail.com>
Member
Author
|
if removed alb env: |
pditommaso
reviewed
Jan 22, 2026
Comment on lines
55
to
60
| // Header not present, use socket address | ||
| final String socketIp = request.getRemoteAddress().getAddress().getHostAddress() | ||
| if (log.isTraceEnabled()) { | ||
| log.trace "No X-Forwarded-For header, using socket address: '$socketIp'" | ||
| } | ||
| return socketIp |
Collaborator
There was a problem hiding this comment.
would not make sense to fallback to super.resolve(request) ?
Member
Author
There was a problem hiding this comment.
i did not check that, but yes we can use super.resolve(request)
i will make the changes
Member
Author
There was a problem hiding this comment.
i did not check that, but yes we can use super.resolve(request)
i will make the changes
Signed-off-by: munishchouhan <hrma017@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixed IP address spoofing vulnerability where attackers could bypass rate limiting by sending arbitrary
X-Forwarded-Forheaders. The rate limiter now uses socket addresses by default (secure) with opt-in support for trusted proxy headers when deployed behind AWS ALB.Vulnerability
The rate limiter previously trusted client-supplied
X-Forwarded-Forheaders without validation, allowing attackers to bypass rate limits by spoofing different IP addresses:Solution
Implemented custom
SecureHttpClientAddressResolverthat takes the rightmost IP from comma-separatedX-Forwarded-Forheaders:X-Forwarded-For(the one ALB added)Key Implementation
Why Rightmost IP?
When behind ALB, the X-Forwarded-For header contains:
Example:
Changes
Core Files
SecureHttpClientAddressResolver.groovy (NEW) (
src/main/groovy/io/seqera/wave/util/SecureHttpClientAddressResolver.groovy):ContainerController.groovy & RegistryProxyController.groovy:
HttpClientAddressResolverRateLimiterFilter.groovy (
src/main/groovy/io/seqera/wave/filter/RateLimiterFilter.groovy):HttpClientAddressResolverConfiguration
src/main/resources/application-alb.yml):client-address-header: X-Forwarded-ForTests
src/test/groovy/io/seqera/wave/filter/RateLimiterFilterTest.groovy):Documentation
Security Benefits
Default Mode (Secure by Default)
ALB Mode (When Configured)