#322 √ resolved
Mark Imbriaco

Don't return 500 if Client-IP and X-Forwarded-For agree.

Reported by Mark Imbriaco | June 3rd, 2008 @ 08:28 PM

If Client-IP and X-Forwarded-For are both set, a 500 is returned regardless of what they're set to. If they're both set but they agree, we can safely use either value.

Comments and changes to this ticket

  • Jeremy Kemper

    Jeremy Kemper June 3rd, 2008 @ 08:35 PM

    This is due to

          if @env.include? 'HTTP_CLIENT_IP'
            if @env.include? 'HTTP_X_FORWARDED_FOR'
              # We don't know which came from the proxy, and which from the user
              raise ActionControllerError.new(<<EOM)
    IP spoofing attack?!
    HTTP_CLIENT_IP=#{@env['HTTP_CLIENT_IP'].inspect}
    HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}
    EOM
            end
    

    but I'm not sure about its motivation.

  • Mark Imbriaco

    Mark Imbriaco June 3rd, 2008 @ 08:38 PM

    I'm not sure about the motivation either but if we use the comment as a guide, it seems safe to skip the 500 if they match since we can determine the remote IP. That's all my tiny little patch does.

  • DHH

    DHH June 3rd, 2008 @ 10:10 PM

    I like that patch, but let's get a test in there to ensure that we have this functionality tested in general.

  • Brad Folkens

    Brad Folkens June 3rd, 2008 @ 11:13 PM

    Had this issue this morning with Yahoo's Slurp! bot with the following case:

    HTTP_CLIENT_IP="74.6.29.94"

    HTTP_X_FORWARDED_FOR="74.6.29.94, 74.6.8.122"

    So, patch updated to reflect this (and test added too).

  • DHH

    DHH June 4th, 2008 @ 12:16 AM

    • → State changed from “new” to “resolved”

    Closed by edfa195e2ace7b4fb8195333c6e44e6bf8986c11

  • adi_azar

    adi_azar June 7th, 2008 @ 09:12 PM

    DHH,

    This is is NOT fixed yet. I am running an application from edge rails, and it is returning 500 for Yahoo Slurp.

    Thanks

  • Jeremy Kemper

    Jeremy Kemper June 7th, 2008 @ 09:26 PM

    adi_azar, please provide a failing test case.

  • adi_azar

    adi_azar June 7th, 2008 @ 09:44 PM

    I have my server logs:

    74.6.18.216 - - [07/Jun/2008:20:40:55 +0000] "www.onlinecityflorist.com/LA/baskin-flowers/" 500 60 "-" "Mozilla/5.0 (compatible; Yahoo! Slurp; http://help.yahoo.com/help/us/ys...)"

    74.6.18.216 - - [07/Jun/2008:20:40:59 +0000] "www.onlinecityflorist.com/GA/macon-flowers/" 500 60 "-" "Mozilla/5.0 (compatible; Yahoo! Slurp; http://help.yahoo.com/help/us/ys...)"

    74.6.18.216 - - [07/Jun/2008:20:41:10 +0000] "www.onlinecityflorist.com/FL/dade-city-flowers/" 500 60 "-" "Mozilla/5.0 (compatible; Yahoo! Slurp; http://help.yahoo.com/help/us/ys...)"

    74.6.18.216 - - [07/Jun/2008:20:41:41 +0000] "www.onlinecityflorist.com/LA/iota-flowers/" 500 60 "-" "Mozilla/5.0 (compatible; Yahoo! Slurp; http://help.yahoo.com/help/us/ys...)"

    74.6.18.216 - - [07/Jun/2008:20:41:43 +0000] "www.onlinecityflorist.com/LA/baskin-flowers/" 500 60 "-" "Mozilla/5.0 (compatible; Yahoo! Slurp; http://help.yahoo.com/help/us/ys...)"

    74.6.18.216 - - [07/Jun/2008:20:41:47 +0000] "www.onlinecityflorist.com/GA/macon-flowers/" 500 60 "-" "Mozilla/5.0 (compatible; Yahoo! Slurp; http://help.yahoo.com/help/us/ys...)"

    74.6.18.216 - - [07/Jun/2008:20:41:58 +0000] "www.onlinecityflorist.com/FL/dade-city-flowers/" 500 60 "-" "Mozilla/5.0 (compatible; Yahoo! Slurp; http://help.yahoo.com/help/us/ys...)"

    It returns 500 only for Yahoo slurp!

    Is that helpful?

  • Sebastian Friedrich

    Sebastian Friedrich June 16th, 2008 @ 08:21 AM

    adi_azar, are you using Edge? This patch did not make it into 2.1. I had this problem with 2.1 but it works for me when applying the attached patch (which is now in Edge).

  • Ryan Bates

    Ryan Bates June 28th, 2008 @ 06:49 AM

    • → Tag changed from “” to “patch”

    I'm running into this problem as well. Hoping 2.1.1 will be released soon and will include this patch.

  • sbwoodside

    sbwoodside August 11th, 2008 @ 05:46 AM

    This patch doesn't resolve it for me. I'm doing testing where the client is a mobile phone on the DeviceAnywhere system. I get HTTP_X_FORWARDED_FOR="66.94.9.52, ::1" and HTTP_CLIENT_IP="10.176.38.35" and it gives me the "IP spoofing attack" error (with or without the patch). If I return @env['HTTP_X_FORWARDED_FOR'][0] instead of raising the error, it works fine.

  • Sean Hussey

    Sean Hussey August 13th, 2008 @ 10:46 PM

    I can confirm that sbwoodside's suggestion worked for me when the patch alone did not. Standard Apache to mongrel proxying behind a hardware load balancer.

  • sbwoodside

    sbwoodside August 14th, 2008 @ 01:10 AM

    The IP spoofing test here doesn't make sense. Both HTTP_X_FORWARDED_FOR and HTTP_CLIENT_IP are proxy headers, both equally untrustworthy. If they differ, it's probably just the proxy being either clever or wrong. (Or a dumb spoofer.)

    There doesn't seem to be any sure way to tell if X-Forwarded-For is being spoofed or legit. The only real solution is to add a mechanism for the users to know if the result of remote_ip is trustable or not. Then they can make their own decisions. For example, you could add a method unspoofable_remote_ip which only returns REMOTE_ADDR. Or perhaps some kind of configuration variable to set whether you want to be trusting or not. Mainly you'd only care about spoofing if you're using the method to authenticate users by IP white-listing.

    As for knowing which value to use to respond to the user, according to Wikipedia the first address in X-Forwarded-For is always the original machine, so that would be the correct address to use when there's ambiguity. So, @env['HTTP_X_FORWARDED_FOR'].first is I think the correct action.

  • thomas morgan

    thomas morgan August 16th, 2008 @ 04:53 AM

    The patch here seems to rely on a certain subset of real-world behavior to determine what's valid.

    It assumes that a) there is a trusted proxy or load balancer immediately upstream b) that said proxy uses X-Forwarded-For c) that Client-IP was legitimately added by some other, more upstream proxy

    Given that X-Forwarded-For is the more popular header and is used by Apache, and rails apps are often behind a proxy of some sort, it's not bad.

    However, when the deployment situation is outside the above, a spoofed address or a 500 Server Error are both possibilities.

    A better approach might be to rely on the sysadmin or developer to explicitly help Rails know something about the environment so Rails can trust the right data.

    A few possibilities (all variants on the same idea):

    a) Have yet another header, Rails-specific, trump the others and configure the proxy or load balancer to use it. X-Rails-Remote-IP or something. Con: Could still be spoofed when not configured on a given deployment box.

    b) Have an environment variable set locally to tell Rails which header to use. This is similar to the approach of how to tell Rails when it is inside an https connection.

    c) Let the proxy forcibly remove the non-used header. Apache example: add this line to the virtual host config: RequestHeader unset Client-IP

    B or C seem like better choices to me. The IP spoofing exception message could include language pointing the admin to info on how to resolve it.

    They have the added benefit of not being clever but instead relying on specific information about which header Rails should consider trustworthy.

    C is what I'm trying now and preliminary tests suggest it will work although I'm still waiting on Slurp to come by and actually load a page.

    --

    As an aside, the IP spoofing attack message may generate a server error, but it's still reporting Status: 200 OK to apache. A review of Apache's log would not suggest there is any problem at all.

  • Eric Allen

    Eric Allen September 11th, 2008 @ 01:12 PM

    Apparently T-Mobile's WAP gateway (apn.voicestream.com) makes the mistake of setting HTTP_CLIENT_IP and HTTP_X_FORWARDED_FOR to different values. This means that I am unable to access my site from any phone on the T-Zones internet plan. Correct behavior?

  • Orion Delwaterman

    Orion Delwaterman October 9th, 2008 @ 07:04 PM

    I also have a site that T-mobile phones need to hit. This fix will not work for T-Mo phones

  • sbwoodside

    sbwoodside October 11th, 2008 @ 12:18 AM

    I've opened a new bug #1200 for the mobile issue, since this bug has been marked resolved (and deals with a different issue).

Please Login or create a free account to add a new comment.

You can update this ticket by sending an email to from your email client. (help)

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Source available from github

The Git repository resides at http://github.com/rails

Check out the current development trunk (Edge Rails) with:

git clone git://github.com/rails/rails.git

Creating or reviewing a patch

See the contributor guide.

Creating a feature request

Please don't. If you want a new feature in Rails, you'll have to pull up your sleeves and get busy yourself. Or convince someone else to do it. See the contributor guide on how to get going. But posting them here is just going to lead to ticket root.

Creating a bug report

When creating a bug report, be sure to include as much relevant information as possible. Post the code sample that causes the problem. Preferably, alter the unit tests and show through either changed or added tests how the expected behavior is not occuring.

Security vulnerabilities should be reported via an email to security@rubyonrails.org, do not use trac for reporting security vulnerabilities. All content in trac is publicly available as soon as it is posted.

Then don't get your hopes up. Unless you have a "Code Red, Mission Critical, The World is Coming to an End" kinda bug, you're creating this ticket in the hope that others with the same problem will be able to collaborate with you on solving it. Do not expect that the ticket automatically will see any activity or that others will jump to fix it. Creating a ticket like this is mostly to help yourself start on the path of fixing the problem and for others to sign on to with a "I'm having this problem too".

Shared Ticket Bins

Tags