Really. No Joke. I’ve been spending the better part of the last three days with fixing a programming mistake that’s so stupid, it’s almost unbearable. So why did it take me that long to fix it you might ask. That’s because every PHP programmer on this beautiful planet seems to be so attracted by this mistake that every one of them wants to have his private little go with it. But maybe I should tell you about this common mistake every PHP programmer seems to make so that you won’t be the next one to make me do extra hours. So, without further ado, here we go:
Since PHP is a web programming language, there’s a big chance that the program you’re going to write is dealing with data that has to do with internet addresses. One of them being the IP address of the client who is accessing the site your script happens to run on. Now, with all of us being nosy bastards, we tend to like to know this IP address. But where are we going to find it?
Easy, you might answer. That information is provided by the superglobal variable $_SERVER. Right you are. Thus, you’ll find in almost every little PHP program a line that’ll be in the lines of:
$clientip = $_SERVER['REMOTE_ADDR']
Which, I agree, will provide the IP address of the current user most of the times. Unfortunately, it will not do so every time.
That’s because there are some folks out there who tend to run larger installations than you might have imagined when writing your script. Those installations may have proxies, caches and load balancers. Now, due to their nature, these beasts tend to answer requests on behalf of the real web server and only if they can’t do that, they’ll forward the request. What happens then is easily explained: $_SERVER['REMOTE_ADDR'] will no longer contain the address of the user but the address of the proxy, cache, whatever instead. The user’s real IP address, however, is now stored in different variables, $_SERVER['HTTP_X_FORWARDED_FOR'] being the one used most. Since most of the PHP applications out there chose to ignore this, you’ll end up with a real mess. Because most of those apps tend to identify and verify the user by his or her IP address, logins will fail and other nuisances will happen.
This could be easily avoided by using a simple replacement code:
if (isset($_SERVER['HTTP_X_FORWARDED_FOR']))
$clientip = $_SERVER['HTTP_X_FORWARDED_FOR'];
else
$clientip = $_SERVER['REMOTE_ADDR'];
You could even refine this to get to the first non-proxied address by splitting up $_SERVER['HTTP_X_FORWARDED_FOR'] (Yup, can contain more than one IP address) to find all the hops.
But no. Because by now even every little plugin tends to make use of $_SERVER['REMOTE_ADDR'], I end up fixing tons of code lines just to make some very popular apps work. These include but are not limited to:
Out of those, vBulletin really takes the cake. The folks at Jelsoft *do* have a working function (and a very ingenious at that, too) to determine the *real* IP address of any given user. They just chose to not use it. So it’s lying dormant in their code, shipped with every new version of vBulletin. Great work, guys. Keep it on. And if you find the time, please do me a favor: Just replace every call to fetch_ip() with a call to fetch_alt_ip() and get rid of the former. Thanks a lot, you douche bags.
I know your pain. At nu2m I installed the mod_extract_forwarded module for Apache2 since I don’t have time to massively correct other people’s bugs.
I can’t call the developers douche bags, though, since in my case they all are my co-workers.
But don’t you replace one problem by another here? HTTP_X_FORWARDED_FOR is the IP address of the client, right? But here it’s still possible that several different clients coming from different networks (behind different proxies) have the same (private) address… or am I missing something here?
Scott,
mod_extract_forwarded definitely looks nice. I’d have to adapt it to Apache 2.2.2 but that shouldn’t be a problem for I already did that for other modules. Did you notice any performance impact by using mod_extract_forwarded?
Charon,
As is stated on the site Scott so helpfully linked to, the X-Forwarded-For will contain a list of all IPs in the proxy chain from the client to the current server. So, yes, there’s a chance that a private IP will be in there. Thus you’d actually want to filter the X-Forwarded-For and either use the address next to the one of your local proxy or the last one that isn’t a private one.
Charon: I think you are right.
Stefan: What about two different machines in one private network accessing one application via the same proxy at the same time? Why not save and compare the full chain of X-Forwarded-For and if not present the Remote_IP?
Regards,
Ingo
Ingo,
I actually don’t really care *how* this is solved by the respective programmers. Imnsho using an IP address to identify a user is *always* a bad idea. Just think AOL, where *every* user is proxied and so has a non-unique IP address. The major problem is that the majority of (not only) PHP programmes makes a completely wrong assumption about the validity (and the correct use) of the HTTP header field “REMOTE_ADDRESS”.
Any solution -even if I’d prefer them to use the whole information of the X-Forwarded-For field – is better than just using “REMOTE_ADDRESS”. I agree that there is a possibility where two proxied users might access the same server almost simultaneously. However, taking the risk of that happening in my eyes is a lot better than to *guarantee* that all users will be seen as coming from the same address.
Also, if you take the time and look into the code of the apps I named above, you’ll notice that no programmer even considered the possibility that a HTTP header field could hold more than one IP address. Thus, your approach would be the most desireable one but unfortunately it won’t work with the current code base out there.
Stefan, just for understanding.
X_FORWARDED_FOR can contain more than one IP address, if the request gets routed through more than one proxy. Each proxy appends the sender’s IP address, so the first IP address is always the address of the original host sending the request.
Using a regexp like #([\d{1,3}\.]{1,6})# (not sure about the correctness of the syntax, but you might get the point) should extraxt all IPs and we just use the first one, right?
Oh, and by the way, the phpBB guys at least know what they do (phpBB 2.0.x):
198 // I’m removing HTTP_X_FORWARDED_FOR … this may well cause other problems such as
199 // private range IP’s appearing instead of the guilty routable IP, tough, don’t
200 // even bother complaining … go scream and shout at the idiots out there who feel
201 // “clever” is doing harm rather than good … karma is a great thing … :)
203 $client_ip = ( !empty($HTTP_SERVER_VARS['REMOTE_ADDR']) ) ? $HTTP_SERVER_VARS['REMOTE_ADDR'] : ( ( !empty($HTTP_ENV_VARS['REMOTE_ADDR']) ) ? $HTTP_ENV_VARS['REMOTE_ADDR'] : getenv(‘REMOTE_ADDR’) );
And while we are at it, in phpBB 3 Beta 1 is says:
// Why no forwarded_for et al? Well, too easily spoofed. With the results of my recent requests
// it’s pretty clear that in the majority of cases you’ll at least be left with a proxy/cache ip.
$this->ip = (!empty($_SERVER['REMOTE_ADDR'])) ? htmlspecialchars($_SERVER['REMOTE_ADDR']) : ”;
Not sure why they’re using htmlspecialchars() instead of really testing whether the field contains a IP address…
Using the complete chain as Ingo proposed is not really an option, as we do not know how long it is. Perhaps using the first 3 addresses or so?
> Oh, and by the way, the phpBB guys at least know what they do (phpBB 2.0.x):
[...]
> Not sure why they’re using htmlspecialchars() instead of really testing whether the field contains a IP address…
This really made my day.
Actually, the whole list of IP addresses can not exceed 254 entries except a router on the way resets the max_hop field in the TCP header which would be a clear violaten of current standards.
In general, you will only rarely see a case where there are more than two proxies/caches in the line of the whole communication. A rare exception would be this:
client -> local cache -> AOL cache -> your proxy -> your server
So even in the worst case, you’d see five, maybe six addresses max. Most likely, you’ll never see more than three. That’s because when being used as a local cache, most software tends to hide the private IP address completely, only giving the official IP address as the originator of a request.
Stefan, this out-of-context quotation is not fair – but funny indeed ;)
Anyway, here’s phpBB’s “response”:
> [...]
> X_Forwarded_For is tantamount to the reffered header that browsers can send. I’ve a plugin for Firefox that allows me to alter my headers, and one of the one’s I’m currently sending is an X_Forwarded-For of 1.2.3.4; spoofing X-Forwarded-For is trivial at best.
> [...]
> Remote_Addr is not only harder to fake in any capacity, but it will be correct more times than x-forwarded-for will ever be.
I think the point is, what do you need the IP information for? phpBB (and most of the systems you mentioned) use this information to handle sessions, i.e. to check whether a request from IP w.x.y.z with session id 0123456789 belongs to a logged in user or not.
What issues did you run in exactly?
Sascha,
Sorry, I’ve been too busy with other things the last couple of days. Now, what issues did I run into? Well, to start with, there’s the small problem that every user seems to come from the same IP address. This is either 127.0.0.1 if you’re running the cache/load balancer locally or the address of the machine you’re running the proxy/load balancer on if it’s a two stage setup.
Since, as you correctly mention, phpBB and other systems use this IP address to handle sessions, you’ll essentially end up with one session for all users. The result is easily described in one word: Crash! Because this in turn leads to smaller problems like no user being able to login anymore, very interesting log entries and lastly to very boring log analysis results (you’ll get a very long visit from 1 visitor who reads almost all your pages constantly). In addition, some smart systems will prevent your users to comment / write articles for they limit the rate of allowed posts per minute based on the IP address of the user … I guess I could go on and on about this and maybe Scott has some additional info.
Anyway: I agree that an X-Forwarded-For header can be easily faked. So what? See, even if’d use that feature like some phpBB programmer is obvioulsy and childishly doing, where’s *his* advantage of doing so? He may have got some additional privacy. So that’s nice for him. But it doesn’t break the application. However, the bevahiour of *his* application *is* broken in a way that renders it useless for *me* as a paying customer. Maybe *that’s* what he should be thinking about.
Oh, and lest I forget: Maybe you should forward the link to mod_extract_forwarded to this phpBB guy. This should make him reconsider his statement about REMOTE_ADDR being more relyable than X-Forwarded-For. People wouldn’t have developed such a solution if there weren’t the need for it. And why is there a need? Because of some stubborn programmers ;)
Well, I did indeed but got a negative answer. Anyway, at least *I* learnt something from this discussion ;)
If you want to learn some more, get Pound (I suggest version 1.10 from the bottom of the page) and install it with a simple config like this one. Next, configure your Apache to listen on 127.0.0.1:80, fire the whole thing up and see what happens ;)
BTW: phpBB only checks the first third bytes of the IP to avoid this problem with AOL users.
Honestly, after reading the relayed reply from some unknown phpBB guy above, I couldn’t care less in what *other* respects this piece of software is broken besides the wrong handling of REMOTE_ADDR and FORWARDED_FOR. Still, I find it quite amusing that on the one hand REMOTE_ADDR is said to be more reliable but if it is, why on the other hand only use the first and third byte, then? To make it more unreliable? The more I think about it it come to the conclusion that the headline, although meant as an eye catcher, is more fitting than I dared to believe.
First, she, not he. :)
Going further:
1) we check the first 24 bits (not the first and third as you incorrectly read) of the IP address to combat problems with AOL’s constantly changing IP’s per request.
2) I’m not sure why you wanted me to look at mod_extract_forwarded, as even their page says:
“mod_extract_forwarded can be dangerous for host based access control because X-Forwarded-For is easily spoofed.”
Plus the number of hosts actually using mod_extract_forwarded is as rare as the number of hosts that need that module.
3) the two setups that can cause problems with trusting remote_addr that you noted are rare. I would say less than 1% of all phpBB installs use such a setup. If you’re in the situation where you’re using that kind of setup, then yes, mod_extract_forwarded could be used well, if set up right.
However, X-Forwarded-For, just like HTTP-Client-IP can be quite simply faked by altering the headers which a browser sends. Indeed, if you’ve alterd your blog here to log X-Forwarded-For IP addresses, you’ll note that my post has arrived from 1.2.3.4. (If you’re trusting HTTP-Client-IP, then the post will be from 4.3.2.1)
As for the lack of validation done on the IP address in 3.0, we are looking into that, and we are looking into ways to identifiy users in the rare cases you’ve outlined where remote_addr isn’t correct, however the latter item, if included, will not be a default option as it does leave the rest of the 99% open to abuse.
(Certinally, X-Forwarded-For without validation can lead to some intresting results. whatismyip.com also checks the X-Forwarded-For, but doesn’t santitise it, and thus you can create some intresting but pointless XSS POC’s in it)
NeoThermic
> First, she, not he. :)
I stand corrected and apologize ;)
>1) we check the first 24 bits (not the first and third as you incorrectly read) of the IP address to combat problems with AOL’s constantly changing IP’s per request.
Which doesn’t make the procedure significantly more “secure”, “trustworthy”, “you name it” or even fit for reliably distinguishing users ;)
> the two setups that can cause problems with trusting remote_addr that you noted are rare
Sure. That must be why I keep hitting the same old sh*t at almost every new customer. See, in a way I should be thankful. That’s because I get paid by the hour and every hour it takes me to fix this very problem is real money for me. On the other hand, by now I’m sick and tired of doing it all over again, for every new release of … see list above.
> Indeed, if you’ve alterd your blog here to log X-Forwarded-For IP addresses, you’ll note that my post has arrived from 1.2.3.4.
Well, yes I did but no, not as stupidly as to restrict the logging to the last ip address provided in X-Forwarded-For. Thus I can see that your post indeed came from 1.2.3.4 but also from 82.34.xxx.yyy. I guess you mistakenly believe that X-Forwarded-For would only contain the address of the originating system. This isn’t the case. At least not with software that’s working the way it should. So you are able to provide a faked X-Forwarded-For entry in the header. But where’s the beef? The only achievement you’ve made is that your real IP address will be contained in the same header field as the *forwarding* system. So what? It just proves that you made an error when evaluating the header field. See headline ;) Or it proves that your software can only handle one IP address per header field. See headline ;)
>however the latter item, if included, will not be a default option as it does leave the rest of the 99% open to abuse.
Heck, what’s so hard about including an option like “Yes, dammit, I’m behind a cache, I use X-Forwarded-For and I goddamn know what I’m doing!” ? Basically, that’s all I ask for.
Also, I’d be really interested in learning what kind of abuse you’re talking about. Blocking some user – the ip address of whom I’d have to learn in advance – from entering a BBS? Well, you’re doing that *right now* by not evaluating X-Forwarded-For for *all* users of the systems I’m responsible for. Other than that I don’t see any real vector that X-Forwarded-For could be used for.
But maybe you can enlighten me. As you yourself stated: “can create some interesing but *pointless* XSS POC’s”. Yup, it’s pointless to fake X-Forwarded-For. So why should anyone actually do it? And even more: Why should anyone prevent me from a very remote danger that can’t be exploited profitably?
Cheers,
-Stefan
>Which doesn’t make the procedure significantly more “secure”, “trustworthy”, “you name it” or even fit for reliably distinguishing users ;)
It isn’t exactly used to distinguish users though, this is the main point. The IP is checked to prevent people from jumping sessions should the get hold of the session ID of another user.
>I guess you mistakenly believe that X-Forwarded-For would only contain the address of the originating system.
No, I did not. :) Also, *my* X-Forwarded-For does not contain my real address; you’ve obtained that from the remote_addr (the proof of which is something like this : http://www.neothermic.com/ipaddresses.php which shows:
“X-Forwarded-For: 1.2.3.4″ for my spoofed header)
>Blocking some user – the ip address of whom I’d have to learn in advance – from entering a BBS?
‘Learn in advanced’ is a ploy. If they are abusing your forum, thats normally done by spam posts, or just being an ass, of which they would of posted, and you would have their IP address to ban.
>Other than that I don’t see any real vector that X-Forwarded-For could be used for.
Ok, lets just assume that I post an image into a forum that trusts X-Forwarded-For over Remote_addr. The refferal line in my logs from an admin who looks at that post happens to have their session ID in it, and I’ve also got their IP in my logs due to the hit. I then just set my X-Forwarded-For to their IP, and use thier session. Boom, I’m that user. You might insist that this is just bad software design, but this kind of exploit currently exsists in another forum software which shall remain nameless. They do know about it but still havn’t done anything about it.
Or, I just make a few posts from the IP of another user (that I set in my x-forwarded-for), and the admins of the forum in question would assume that my posts are from the user who’s IP I’m using (after all, if the posts come from the same IP and the IP is a static one, it must be the same user, right? (side note: thats the average forum admin mentality, not mine)).
Thats just two tiny examples of the abuse one can do if the system is trusting X-Forwarded-For over remote_addr. Granted, trusting them *both* at the same time (in a combined fashion, in essence) just might be a decent idea. You’re right in the fact that neither of them alone is a great thing to blindly trust, but due to the way one can be faked, X-Forwareded-For is the weaker of the two.
NeoThermic
> No, I did not. :) Also, *my* X-Forwarded-For does not contain my real address; you’ve obtained that from the remote_addr (the proof of which is something like this : http://www.neothermic.com/ipaddresses.php which shows:
“X-Forwarded-For: 1.2.3.4″ for my spoofed header)
From my Apache log file:
1.2.3.4, 82.34.xxx.yyy – - [03/Aug/2006:18:10:12 +0200] “GET /archive/001228.php HTTP/1.1″ 200 13712 “-” “Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.5) Gecko/20060719 Firefox/1.5.0.5″
From my Apache httpd.conf:
LogFormat “%{X-Forwarded-For}i %l %u %t \”%r\” %>s %b \”%{Referer}i\” \”%{User-Agent}i\”" combined
Go figure. ;) Edit: No, I’m not using mod_extract_forwarded on this server, just in case.
-Stefan
Well, you certinally have something non-standard loaded. Running the tests locally with the same apache version as here shows this in my logs for the exact same LogFormat string you provided:
1.2.3.4 – - [04/Aug/2006:19:14:20 +0100] “GET / HTTP/1.1″ 200 277 “-” “Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.5) Gecko/20060719 Firefox/1.5.0.5″
Getting somone *outside* of the network to try gave this:
10.0.0.233 – - [04/Aug/2006:19:37:46 +0100] “GET / HTTP/1.1″ 200 277 “-” “Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6″
(just incase that wasn’t clear, both those IP’s are the faked X-Forwarded-For ones)
I invite you to run a quick command:
apache -L
Then list here one instance of all the .c files loaded. I’ve a very good feeling that you’re running something else which mucks about with the server vars.
NeoThermic
> I invite you to run a quick command:
> apache -L
h8399:~ # apache -L
-bash: apache: command not found
But being a nice guy, I also ran:
h8399:~ # httpd2 -L
The output of which you can download here or see here. Have fun ;)
Thinking about it a bit more, I guess I now know (edit: no, I’m 200% sure) where the fault in your setup is. But I won’t spoil your fun finding it out for yourself ;) A little hint, though: the information is all here on this very page.
-Stefan
PS: And looking at the problem from your setup, you *are* right. But that wasn’t my point from the very beginning.
No need to spoil fun. Your output seems to suggest, and so does apache’s code, that you’re doing something with mod_proxy, which is one of the two modules that cares about the X-Forwarded-For. Of course, mod_proxy is going to add the remote_addr to the x-forwarded-for and send it on its way, which is exactly *my* point; trusting X-Forwarded-For without running this rare setup will cause problems and can cause secuirty issues.
Your point might be that code isn’t always compatable with your custom setup, but I’ll point out again that the majoirty do not run such a setup. Thus any code that trusts X-Forwarded-For outright on normal setups will cater for the very big possibility of a secuirtry hole. That is why phpBB doesn’t use it by default. You may argue that all you like, but that was my point from the beginning.
Now, as noted above, we are looking into more robust ways to cope with such setups, however anything that changes this late in the game must have full backing from the development team, so do not be surprised if something doesn’t make it in.
NeoThermic
> Your output seems to suggest, and so does apache’s code, that you’re doing something with mod_proxy
No, I don’t. I run either Pound as a load balancer or Squid as a proxy cache. Both of which – as you correctly state – honor X-Forwarded-For and also put Remote_Addr in the X-Forwarded-For field – as well they should.
Now, that’s exactly what I’ve been talking about all the time: All I want is an option that allows me to tell the software that *I know* what I’m doing. This could be as easy as in “I’m running the web server behind a proxy cache (Yes/No) [If in doubt, choose "No"]“. I never said that the software should *trust* X-Forwarded-For. That’s your interpretation. I just said I want to be able to tell the software to honor the presence of an X-Forwarded-For header field. Trust was never included in my equation.
-Stefan
Also, as you might have noticed by evaluating the response headers from my site, both Pound and Squid are working transparently. That means the browser will think the content it received came directly from Apache. Which led you to believe I’d be running a pure Apache setup. So, what makes you so sure that this kind of setup is so rare? You’ve got no way of knowing whether there’s a Squid or Pound working in front of the Apache you think you’re talking to.
-Stefan
Maybe I should sum this up a bit: The problem is twofold. First, there’s the problem of “identity”. This is where your argument of *trust* comes into play. Imho, it won’t do any harm to say goodbye to the use of the user’s IP address there. Instead, a hash over REMOTE_ADDR and FORWARDED_FOR could be used. This would also solve the problem with one of the attack vectors you described: Even if I’d be able to collect the IP address and the session ID, and even if I could fake the X-Forwarded-For, the hash would be different.
The other side is logging. How do I know what IP address to log for any given user, if there’s both, REMOTE_ADDR and FORWARDED_FOR present? Well, that’s in the admin’s realm. Either (s)he tells the software to use whatever information is supplied in X-Forwarded-For, then by all means do so. But don’t restrict yourself to using only one or even the first IP address supplied in X-Forwarded-For but use all of the information that’s there for you to harvest. If the admin doesn’t tell the software to use X-Forwarded-For, do as you always did and use REMOTE_ADDR.
Actually, I don’t expect this to be included in the current development branch of phpBB (and never really did but hope dies last, doesn’t it? ;)). I’m happy that I could make you think about the problem a bit and at this point would like to thank you for actively participating.
Kind Regards
-Stefan
>So, what makes you so sure that this kind of setup is so rare?
Just to answer this one, since the rest of your points are valid enough.
Lets assume for one moment that we have an admin who notices that all of a sudden, everyone posting has the same IP. This would be a bit confusing for anyone who doesn’t know they are behind such a setup like the one you describe. What will be the first thing they do upon seeing this? They will go right to phpBB.com and post “Whoa, everyone’s using the same IP”.
In theory, if such a setup was popular, or just non-rare, we would see such postings very very frequently. I have to say you’re the first person I’ve seen to raise this issue with phpBB.
Now, in 2.x’s case, altering the way IP’s are done is straight forward. So possibly those who knew they were behind such a setup jsut went and changed the code. It would be a valid point as well. However, I would wager about 80% of phpBB users host on a shared server, which will not be using any form of squid/pound/mod_proxy load balancing. 18% would be either self-hosted or would have a dedicated server. That would leave about 2% who need any form of load balancing, and I would wager only 50% of that 2% actually load-balance the HTTP requests via pound/squid (i.e. 1% of all phpBB setups). (The most common one I see is an older box for images and static content, and a fast box for dynamic content, and seperate master/slave replication MySQL boxes)
The method you describe is something that was pitched not long after we first saw this blog post, of which we are still evaluating the critical criteria of such a modification/option.
Seeing as you’re using such a setup, you could possibly confirm something for me. You mention the maximum IP’s in the X-Forwarded-For would be 254 entries. A dot seperated IP address is 15 characters, and thus in theory, the IP’s alone is 3810 characters. From what you’ve posted, it is comma-space seperated, and thus the total size of the X-Forwarded-For should not be any larger than 4318 characters, right? However, does X-Forwarded-For cater for IPv6?
I would just like to ask that next time you go off on a rant, consider not calling the developers ‘douche bags’, as that just doesn’t look good on anyone. ;)
NeoThermic
> I would just like to ask that next time you go off on a rant, consider not calling the developers ‘douche bags’, as that just doesn’t look good on anyone. ;)
Well, what can I say? First, again, thank you for participating and reading/listening. That really was more than I expected. Second, I’ll answer your very valid questions in due course after a couple hours of sleep. Third: I really apologize for choosing this headline. However, this isn’t the first time I rant about this very topic. But on my earlier tries I didn’t get any attention. As you may have deduced from my grammar (or the TLD ;)) by now, English isn’t my native language. So I asked some friends what headline would call for more attention than “PHP Programmers are Wimps” ;). If you feel overly offended by the result I deeply apologize – but on the other hand, it served it’s purpose.
-Stefan
> Seeing as you’re using such a setup, you could possibly confirm something for me. You mention the maximum IP’s in the X-Forwarded-For would be 254 entries.
See my somewhat lenghty explanation here. In short, theroretically yes. Whether X-Forwarded-For is as well behaved for IPv6 as it is for IPv4 I don’t know for I haven’t run any tests on IPv6 as of yet. However, I’d expect it to be. Also, as outlined in the post linked to above, I wouldn’t honor more than six IP addresses in the X-Forwarded-For header for the reasons stated there. If you’re so inclined you could even make this number configurable to accomodate for future changes in the net’s infrastructure. With six valid addresses and storing them as a string, this would call for 101 bytes of total storage space needed.