You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tomcat.apache.org by Brett Delle Grazie <br...@gmail.com> on 2010/12/10 14:03:54 UTC
RemoteIP valve and multiple X-Forwarded-For headers
Hi,
We're using:
RHEL5 (fully up to date)
Tomcat 6.0.29 (from apache.org)
JVM 1.6.0_22
We use HAproxy (1.4.8) as a front end to Tomcat, HAproxy uses the 'option
forwardfor' which adds an additional X-Forwarded-For header
to the request.
Everything works fine except if the client has an X-Forwarded-For header
_already_ in the request (perhaps due to Squid in forward proxy on client
side).
Thus offending request looks like:
Headers (fake IP addresses used):
X-Forwarded-For: 192.168.0.4 (client side added)
... (some other headers) ...
X-Forwarded-For: 224.212.128.2 (added by HAproxy - this is the actual IP of
the client's squid proxy).
... (some other headers) ...
Now Tomcat's RemoteIP valve doesn't appear to handle this situation
correctly - it returns 192.168.0.4 instead of the expected 224.212.128.2
Should HAproxy be extending the existing header to:
e.g. X-Forwarded-For: 192.168.0.4, 224.212.128.2
Or should Tomcat's RemoteIP valve handle this situation?
I'm also not sure which situation is 'correct' according to standards
anyway...
Any ideas?
Thanks,
--
Best Regards,
Brett Delle Grazie
Re: RemoteIP valve and multiple X-Forwarded-For headers
Posted by Brett Delle Grazie <br...@gmail.com>.
Hi,
On 10 December 2010 14:42, Jim Riggs <ap...@riggs.me> wrote:
> On Dec 10, 2010, at 7:59 AM, Mark Thomas wrote:
>
>>> Looks like a bug,
>>>
>>> Please add it to bugzilla, as Mark suggested.
>>>
>>> BTW, I think that the following change can fix it:
>>> (for current tc6.0.x, not tested!)
>>
>> I don't think so. I think the problem is further up on line 558:
>> String[] remoteIpHeaderValue =
>> commaDelimitedListToStringArray(request.getHeader(remoteIpHeader));
>>
>> I'm pretty sure that needs to be using request.getHeaders() but I
>> haven't tested it either ;)
>
> It does indeed need to be using the Enumeration from request.getHeaders() instead of the single value from getHeader(). I have started work on a patch, unless someone beats me to it...
>
>
I've manually compiled 6.0.29 with the patch and applied it to our
test and then production systems (we couldn't wait for 6.0.30).
This works perfectly.
Thank you very much to all who helped.
--
Best Regards,
Brett Delle Grazie
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: RemoteIP valve and multiple X-Forwarded-For headers
Posted by Jim Riggs <ap...@riggs.me>.
On Dec 10, 2010, at 7:59 AM, Mark Thomas wrote:
>> Looks like a bug,
>>
>> Please add it to bugzilla, as Mark suggested.
>>
>> BTW, I think that the following change can fix it:
>> (for current tc6.0.x, not tested!)
>
> I don't think so. I think the problem is further up on line 558:
> String[] remoteIpHeaderValue =
> commaDelimitedListToStringArray(request.getHeader(remoteIpHeader));
>
> I'm pretty sure that needs to be using request.getHeaders() but I
> haven't tested it either ;)
It does indeed need to be using the Enumeration from request.getHeaders() instead of the single value from getHeader(). I have started work on a patch, unless someone beats me to it...
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: RemoteIP valve and multiple X-Forwarded-For headers
Posted by Brett Delle Grazie <br...@gmail.com>.
On 10 December 2010 13:59, Mark Thomas <ma...@apache.org> wrote:
> On 10/12/2010 13:54, Konstantin Kolinko wrote:
> > 2010/12/10 Brett Delle Grazie <br...@gmail.com>:
> >> (...)
> >>
> >> Everything works fine except if the client has an X-Forwarded-For header
> >> _already_ in the request (perhaps due to Squid in forward proxy on
> client
> >> side).
> >>
> >> Thus offending request looks like:
> >>
> >> Headers (fake IP addresses used):
> >> X-Forwarded-For: 192.168.0.4 (client side added)
> >> ... (some other headers) ...
> >> X-Forwarded-For: 224.212.128.2 (added by HAproxy - this is the actual IP
> of
> >> the client's squid proxy).
> >> ... (some other headers) ...
> >>
> >> Now Tomcat's RemoteIP valve doesn't appear to handle this situation
> >> correctly - it returns 192.168.0.4 instead of the expected 224.212.128.2
> >>
> >
> > Looks like a bug,
> >
> > Please add it to bugzilla, as Mark suggested.
>
Done:
https://issues.apache.org/bugzilla/show_bug.cgi?id=50453
> >
> > BTW, I think that the following change can fix it:
> > (for current tc6.0.x, not tested!)
>
> I don't think so. I think the problem is further up on line 558:
> String[] remoteIpHeaderValue =
> commaDelimitedListToStringArray(request.getHeader(remoteIpHeader));
>
> I'm pretty sure that needs to be using request.getHeaders() but I
> haven't tested it either ;)
>
> Fortunately, the contribution of this valve and the matching filter came
> with an extensive set of unit tests. It should be easy to extend the
> tests to cover multiple headers.
>
> Mark
>
> >
> > Index: java/org/apache/catalina/valves/RemoteIpValve.java
> > ===================================================================
> > --- java/org/apache/catalina/valves/RemoteIpValve.java (revision
> 1044342)
> > +++ java/org/apache/catalina/valves/RemoteIpValve.java (working
> copy)
> > @@ -564,12 +564,12 @@
> > // loop on remoteIpHeaderValue to find the first trusted
> > remote ip and to build the proxies chain
> > for (idx = remoteIpHeaderValue.length - 1; idx >= 0; idx--)
> {
> > String currentRemoteIp = remoteIpHeaderValue[idx];
> > - remoteIp = currentRemoteIp;
> > if (matchesOne(currentRemoteIp, internalProxies)) {
> > // do nothing, internalProxies IPs are not appended
> to the
> > } else if (matchesOne(currentRemoteIp, trustedProxies))
> {
> > proxiesHeaderValue.addFirst(currentRemoteIp);
> > } else {
> > + remoteIp = currentRemoteIp;
> > idx--; // decrement idx because break statement
> > doesn't do it
> > break;
> > }
> >
> > Best regards,
> > Konstantin Kolinko
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
> > For additional commands, e-mail: users-help@tomcat.apache.org
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: users-help@tomcat.apache.org
>
>
--
Best Regards,
Brett Delle Grazie
Re: RemoteIP valve and multiple X-Forwarded-For headers
Posted by Mark Thomas <ma...@apache.org>.
On 10/12/2010 13:54, Konstantin Kolinko wrote:
> 2010/12/10 Brett Delle Grazie <br...@gmail.com>:
>> (...)
>>
>> Everything works fine except if the client has an X-Forwarded-For header
>> _already_ in the request (perhaps due to Squid in forward proxy on client
>> side).
>>
>> Thus offending request looks like:
>>
>> Headers (fake IP addresses used):
>> X-Forwarded-For: 192.168.0.4 (client side added)
>> ... (some other headers) ...
>> X-Forwarded-For: 224.212.128.2 (added by HAproxy - this is the actual IP of
>> the client's squid proxy).
>> ... (some other headers) ...
>>
>> Now Tomcat's RemoteIP valve doesn't appear to handle this situation
>> correctly - it returns 192.168.0.4 instead of the expected 224.212.128.2
>>
>
> Looks like a bug,
>
> Please add it to bugzilla, as Mark suggested.
>
> BTW, I think that the following change can fix it:
> (for current tc6.0.x, not tested!)
I don't think so. I think the problem is further up on line 558:
String[] remoteIpHeaderValue =
commaDelimitedListToStringArray(request.getHeader(remoteIpHeader));
I'm pretty sure that needs to be using request.getHeaders() but I
haven't tested it either ;)
Fortunately, the contribution of this valve and the matching filter came
with an extensive set of unit tests. It should be easy to extend the
tests to cover multiple headers.
Mark
>
> Index: java/org/apache/catalina/valves/RemoteIpValve.java
> ===================================================================
> --- java/org/apache/catalina/valves/RemoteIpValve.java (revision 1044342)
> +++ java/org/apache/catalina/valves/RemoteIpValve.java (working copy)
> @@ -564,12 +564,12 @@
> // loop on remoteIpHeaderValue to find the first trusted
> remote ip and to build the proxies chain
> for (idx = remoteIpHeaderValue.length - 1; idx >= 0; idx--) {
> String currentRemoteIp = remoteIpHeaderValue[idx];
> - remoteIp = currentRemoteIp;
> if (matchesOne(currentRemoteIp, internalProxies)) {
> // do nothing, internalProxies IPs are not appended to the
> } else if (matchesOne(currentRemoteIp, trustedProxies)) {
> proxiesHeaderValue.addFirst(currentRemoteIp);
> } else {
> + remoteIp = currentRemoteIp;
> idx--; // decrement idx because break statement
> doesn't do it
> break;
> }
>
> Best regards,
> Konstantin Kolinko
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: users-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: RemoteIP valve and multiple X-Forwarded-For headers
Posted by Konstantin Kolinko <kn...@gmail.com>.
2010/12/10 Brett Delle Grazie <br...@gmail.com>:
>(...)
>
> Everything works fine except if the client has an X-Forwarded-For header
> _already_ in the request (perhaps due to Squid in forward proxy on client
> side).
>
> Thus offending request looks like:
>
> Headers (fake IP addresses used):
> X-Forwarded-For: 192.168.0.4 (client side added)
> ... (some other headers) ...
> X-Forwarded-For: 224.212.128.2 (added by HAproxy - this is the actual IP of
> the client's squid proxy).
> ... (some other headers) ...
>
> Now Tomcat's RemoteIP valve doesn't appear to handle this situation
> correctly - it returns 192.168.0.4 instead of the expected 224.212.128.2
>
Looks like a bug,
Please add it to bugzilla, as Mark suggested.
BTW, I think that the following change can fix it:
(for current tc6.0.x, not tested!)
Index: java/org/apache/catalina/valves/RemoteIpValve.java
===================================================================
--- java/org/apache/catalina/valves/RemoteIpValve.java (revision 1044342)
+++ java/org/apache/catalina/valves/RemoteIpValve.java (working copy)
@@ -564,12 +564,12 @@
// loop on remoteIpHeaderValue to find the first trusted
remote ip and to build the proxies chain
for (idx = remoteIpHeaderValue.length - 1; idx >= 0; idx--) {
String currentRemoteIp = remoteIpHeaderValue[idx];
- remoteIp = currentRemoteIp;
if (matchesOne(currentRemoteIp, internalProxies)) {
// do nothing, internalProxies IPs are not appended to the
} else if (matchesOne(currentRemoteIp, trustedProxies)) {
proxiesHeaderValue.addFirst(currentRemoteIp);
} else {
+ remoteIp = currentRemoteIp;
idx--; // decrement idx because break statement
doesn't do it
break;
}
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org
Re: RemoteIP valve and multiple X-Forwarded-For headers
Posted by Mark Thomas <ma...@apache.org>.
On 10/12/2010 13:03, Brett Delle Grazie wrote:
> Hi,
>
> We're using:
> RHEL5 (fully up to date)
> Tomcat 6.0.29 (from apache.org)
> JVM 1.6.0_22
>
> We use HAproxy (1.4.8) as a front end to Tomcat, HAproxy uses the 'option
> forwardfor' which adds an additional X-Forwarded-For header
> to the request.
>
> Everything works fine except if the client has an X-Forwarded-For header
> _already_ in the request (perhaps due to Squid in forward proxy on client
> side).
>
> Thus offending request looks like:
>
> Headers (fake IP addresses used):
> X-Forwarded-For: 192.168.0.4 (client side added)
> ... (some other headers) ...
> X-Forwarded-For: 224.212.128.2 (added by HAproxy - this is the actual IP of
> the client's squid proxy).
> ... (some other headers) ...
>
> Now Tomcat's RemoteIP valve doesn't appear to handle this situation
> correctly - it returns 192.168.0.4 instead of the expected 224.212.128.2
>
> Should HAproxy be extending the existing header to:
> e.g. X-Forwarded-For: 192.168.0.4, 224.212.128.2
>
> Or should Tomcat's RemoteIP valve handle this situation?
It isn't absolutely clear since I can't find a precise enough definition
of the X-Forwarded-For header but the implication is that Tomcat should
handle this. Currently it doesn't.
> I'm also not sure which situation is 'correct' according to standards
> anyway...
The problem is Tomcat is only looking at the first X-Forwarded-For
header when it should probably be looking at both. Please create a
bugzilla entry for this and someone will hopefully take a look.
Mark
>
> Any ideas?
>
> Thanks,
>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tomcat.apache.org
For additional commands, e-mail: users-help@tomcat.apache.org