Incorrect trusted proxy match test in mod_remoteip
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
| Apache2 Web Server |
Fix Released
|
Medium
|
||
| apache2 (Ubuntu) |
Undecided
|
Unassigned | ||
| Trusty |
Medium
|
Wesley Wiedenmeier |
Bug Description
Hi,
I checked and the latest version in trusty-updates is missing the patch for PR 54651 (link below):
https:/
This fixes the case where mod_remoteip trusts multiple IP addresses in X-Forwarded-For if the client IP is trusted. This allows anyone to spoof the remote address by sending an X-Forwarded-For header to a trusted proxy (which will append its own IP to it).
Is it possible to ship this patch in trusty-updates?
To answer the common questions:
$ lsb_release -rd
Description: Ubuntu 14.04.3 LTS
Release: 14.04
$ apt-cache policy apache2-bin
apache2-bin:
Installed: 2.4.7-1ubuntu4.8
Candidate: 2.4.7-1ubuntu4.8
Version table:
*** 2.4.7-1ubuntu4.8 0
500 http://
100 /var/lib/
2.
500 http://
2.4.7-1ubuntu4 0
500 http://
Expected to happen: mod_remoteip should use the rightmost X-Forwarded-For entry if the client IP is in the trusted proxy list. It should then use the second rightmost entry if the rightmost entry is in the trusted proxy list, and so on.
What happened instead: mod_remoteip always checks the client IP against the trusted proxy list as it goes down the X-Forwarded-For entries. It will always set the remote IP to the leftmost entry in X-Forwarded-For if the client IP is trusted.
Regards,
William
|
#5 |
Hello Eugene,
Thanks for pointing out this bug report on the Apache httpd dev mailing list.
It answers a mystery I had with regard to bug 55635.
As you can see in comment 1 (of 55635), I submitted results that were somewhat different from those of the bug reporter.
In comment 3, I gave an explanation of the bug reporter's results.
But that did not explain my own results.
Once I applied your patch from this bug, my results matched those of the bug reporter in 55635.
It appears that the bug reporter in 55635 had the patch for 54651 applied.
So I confirm that your patch is indeed valid and useful.
Since I am a relatively new developer (1 year) for the Apache httpd project, I do not have committer access.
Perhaps there is a committer who is interested in mod_remoteip that will consider committing the 54651 patch to trunk.
Thanks again,
Mike Rumph
|
#6 |
Created attachment 31174
Patch to use correct IP address for trusted proxy comparison.
Worked the patch in the bug description as an attachment.
This patch is an essential fix for mod_remoteip to function properly.
This fix should be receiving some attention.
Without this fix the remoteip_
The first pass of the while will work correctly,
but the subsequent passes will not.
|
#7 |
Committed to trunk in https:/
|
#8 |
Backported to httpd 2.4.8 by r1569006.
summary: |
- Please include patch for Apache PR 54651 in trusty-updates + Incorrect trusted proxy match test in mod_remoteip |
Robie Basak (racb) wrote : | #1 |
Thank you for taking the time to report this bug and helping to make Ubuntu better.
Looks like this was fixed upstream in 2.4.8 so it is fixed in Vivid, Wily and Xenial, but affects Trusty. It looks like this should be trivial to cherry-pick to Trusty. I will put this on the server team's backlog.
In the meantime, please could you provide exact steps to reproduce this issue so that we are able to verify that the fix works?
And if you'd like to help further, the full procedure is documented at https:/
tags: | added: bitesize |
Changed in apache2 (Ubuntu Trusty): | |
status: | New → Triaged |
importance: | Undecided → Medium |
Changed in apache2 (Ubuntu): | |
status: | New → Fix Released |
Robie Basak (racb) wrote : | #2 |
The patch applied to the upstream 2.4 branch is https:/
William Shallum (william) wrote : | #3 |
Steps to reproduce (not exact):
apache config:
LoadModule rewrite_module modules/
LoadModule remoteip_module modules/
Listen 18000
<VirtualHost *:18000>
RewriteRule ^/?(.*) http://
</VirtualHost>
Let's assume we are a proxy on 127.0.0.1.
If a connection comes from 1.2.3.4 without an existing header we will set X-Forwarded-For: 1.2.3.4 and Apache should trust us.
curl -vH 'X-Forwarded-For: 1.2.3.4' 'http://
...
< Location: http://
...
This is OK as the connection comes from 127.0.0.1 and it is trusted to present the IP 1.2.3.4
If a connection comes from 1.2.3.4 with an existing "X-Forwarded-For: 5.6.7.8", we should add the IP 1.2.3.4 at the end, like so:
curl -vH 'X-Forwarded-For: 5.6.7.8, 1.2.3.4' 'http://
...
< Location: http://
...
This shows that Apache thinks the REMOTE_ADDR should be 5.6.7.8. This is not OK as the IP 5.6.7.8 comes from 1.2.3.4 and 1.2.3.4 is not trusted.
Expected:
After the patch is applied
curl -vH 'X-Forwarded-For: 5.6.7.8, 1.2.3.4' 'http://
...
< Location: http://
...
Changed in apache2: | |
importance: | Unknown → Medium |
status: | Unknown → Fix Released |
Changed in apache2 (Ubuntu Trusty): | |
assignee: | nobody → Wesley Wiedenmeier (wesley-wiedenmeier) |
Here's an updated debdiff to apply this patch, with better formatting for the changelog entry
It looks to me like the patched version resolves the bug. If anyone else wants to test before uploading there is a deb in my ppa here:
ppa:wesley-
Before this is uploaded it would be good if anyone who has been experienceing this bug could that the patched version resolves the issue for them, thanks.
The updated deb is available here:
https:/
There was recently a security update for apache2 that went in to trusty-updates, so I need to remake the debdiff based off of the updated version in trusty-proposed as the security update took the package version that the above debdiff had. When that is done I will update the version in ppa if anyone wants to test.
Here is the updated debdiff. A build is running with it, which I will test soon. It will be available in ppa:wesley-
It uses the version number '2.4.7-
The build of apache 2.4.7-1ubuntu4.14 in the ppa appears to work for me.
It would be good if the original bug reporter could test the build in the ppa in order to verify it before it goes into trusty-proposed though, thanks.
Here is the test case I used:
root@ubuntu:~$ apt-cache show apache2 | grep Version
Version: 2.4.7-1ubuntu4.13
root@ubuntu:
root@ubuntu:~$ cat /etc/apache2/
<VirtualHost *:80>
RemoteIPH
RemoteIPT
RewriteEngine on
RewriteRule ^/?(.*) http://
</VirtualHost>
root@ubuntu:
root@ubuntu:
root@ubuntu:
root@ubuntu:
< Location: http://
root@ubuntu:
< Location: http://
root@ubuntu:
root@ubuntu:
root@ubuntu:
root@ubuntu:
Version: 2.4.7-1ubuntu4.14
root@ubuntu:
root@ubuntu:
< Location: http://
root@ubuntu:
< Location: http://
Since a SRU for (LP: #1534538) was recently uploaded to trusty-proposed using the same version string as the current debdiff, I have redone the patch on top of the version of apache currently in trusty-proposed. The updated debdiff is attached.
A package with the updated debdiff is currently building in ppa:wesley-
Thanks
I have confirmed a bug in mod_remoteip.c's remoteip_ modify_ request function.
This bug was reported by <email address hidden> in 2012 in this thread:
http:// mail-archives. apache. org/mod_ mbox/httpd- users/201210. mbox/%<email address hidden>%3E
The bug appears to still be in httpd/trunk.
The bug here is that, even though temp_sa gets assigned to a new IP with every iteration of the while-loop, the apr_ipsubnet_test continues to check the list of proxy match_ip against the same connection IP (using c->client_addr) over and over again. Thus, if c->client_addr matches, the code always walks to the very beginning of the X-Forwarded-For header.
--- modules/ metadata/ mod_remoteip. c (revision 1407459) metadata/ mod_remoteip. c (working copy)
+++ modules/
@@ -246,16 +246,16 @@
temp_sa = c->client_addr;
while (remote) {
- /* verify c->client_addr is trusted if there is a trusted proxy list >proxymatch_ ip) {
remoteip_ proxymatch_ t *match; proxymatch_ t *)config- >proxymatch_ ip->elts; >proxymatch_ ip->nelts; ++i) { test(match[ i].ip, c->client_addr)) { test(match[ i].ip, temp_sa)) {
internal = match[i].internal;
break;
+ /* verify temp_sa is trusted if there is a trusted proxy list
*/
if (config-
int i;
match = (remoteip_
for (i = 0; i < config-
- if (apr_ipsubnet_
+ if (apr_ipsubnet_
}
}
The fix is to replace apr_ipsubnet_ test(match[ i].ip, c->client_addr) with apr_ipsubnet_ test(match[ i].ip, temp_sa) , and to correct the mention of c->client_addr comment. Once fixed, the module works great.
To reproduce this bug, you have to setup mod_remoteip with these directives:
RemoteIPHeader X-Forwarded-For lProxy 127.0.0.1
RemoteIPInterna
Then, hit make two requests:
1) curl --header 'X-Forwarded-For: 1.2.3.4' http:// localhost: 80/ localhost: 80/
2) curl --header 'X-Forwarded-For: 1.2.3.4, 5.6.7.8' http://
For (1) the r->useragent_ip logged is expected to be 1.2.3.4 . The code behaves correctly for this case.
For (2) the r->useragent_ip logged should be 5.6.7.8 . The current code logs 1.2.3.4 still. This is not the behavior as documented because 5.6.7.8 is not configured to be "trusted".
EugeneL