This project is archived and is in readonly mode.

#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 set 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).

  • mnot (at mnot)

    mnot (at mnot) August 6th, 2009 @ 06:04 PM

    Both X-Forwarded-For and Client-IP are non-standard (i.e., implementation-specific) headers, so relying on them to have consistent syntax and semantics is risky.

    In particular, Squid was the first implementation to use X-Forwarded-For, but it's been copied to other implementations, and some may behave differently.

    As a result, it's very possible -- and legitimate -- to get a request with both headers present but containing different values. This isn't "IP spoofing," and rails should NOT refuse such requests.

    As has been noted, it's pretty much impossible to determine whether these headers are legitimate or faked; at best they're a hint that indicates what part of the request chain might have been.

    I think the real problem here is that remote_ip is called as part of the logging code (AFAICT; I'm not really a ruby person), so that ANY request with both headers will be effectively refused by Rails.

    Most Web servers log the actual, on-wire next-hop client IP address by default, with the ability to log the address from headers if explicitly configured. I'd suggest that this is also what Rails should do (while at the same time making the logic more resilient; e.g., if it can't determine the address from headers, fall back to the on-wire address).

  • Ryan Bigg

    Ryan Bigg October 9th, 2010 @ 09:46 PM

    • Tag cleared.

    Automatic cleanup of spam.

  • Jeff Kreeftmeijer
  • bingbing

Create your profile

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

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Referenced by

Pages