[Catalyst-dev] Re: Catalyst::Engine::Apache X-Forwarded-* Handling

Andy Grundman andy at hybridized.org
Wed May 23 20:11:11 GMT 2007


On May 23, 2007, at 2:42 PM, John Shields wrote:

> All,
>
> First, I wasn't sure where to send this since I couldn't find a
> discussion forum for this module, so I'm sending it directly to you
> all. If there is a good place to post this then please let me know and
> I'd be happy to resend to a mailing list, etc.

Thanks, the proper place would be the catalyst-dev mailing list, but  
this is fine.  I've cc'ed the list and attached your patch for others  
to see.

-------------- next part --------------
Index: Apache.pm
===================================================================
--- Apache.pm   (revision 2819)
+++ Apache.pm   (revision 2821)
@@ -38,8 +38,8 @@
         last PROXY_CHECK unless $headers->{'X-Forwarded-For'};
 
         # If we are running as a backend server, the user will always appear
-        # as 127.0.0.1. Select the most recent upstream IP (last in the list)
-        my ($ip) = $headers->{'X-Forwarded-For'} =~ /([^,\s]+)$/;
+        # as 127.0.0.1. Select the original upstream IP (first in the list)
+        my ($ip) = $headers->{'X-Forwarded-For'} =~ /^([^,\s]+)/;
         $c->request->address( $ip );
     }
 
@@ -91,7 +91,8 @@
         }
         last PROXY_CHECK unless $c->request->header( 'X-Forwarded-Host' );
         
-        $host = $c->request->header( 'X-Forwarded-Host' );
+        # choose the original host (first) in the list
+        ($host) = $c->request->header( 'X-Forwarded-Host' ) =~ /^([^,\s]+)/;
 
         if ( $host =~ /^(.+):(\d+)$/ ) {
             $host = $1;
-------------- next part --------------

>
> I've attached a patch to Catalyst/Engine/Apache.pm (1.11) to handle
> the Apache "X-Forwarded-*" headers correctly (as I understand them).
> The patch does two things:
>
> 1. Changed the current IP address determination to use the *first* IP
> in the "X-Forwarded-For" header rather than the last. My understanding
> of the header is that the first IP address will be the first client IP
> and the last will be the closest proxy server IP. My understanding of
> the $c->req->address field is that this should indicate the original
> client IP, not the closest proxy server.

I think we need to stick with the current X-Forwarded-For  
implementation because you can't trust any proxy server outside of  
your own network.  So using the last IP address means you get the IP  
of what your own proxy server saw, not the actual IP of the client,  
which may even be a useless private IP address.

See http://www.openinfo.co.uk/apache/index.html for example, they  
discuss this more.

>
> 2. Added the same processing for the "X-Forwarded-Host" header as this
> can also have a comma separated list. In our environment we proxy
> twice and the existing code creates a URI that has a host name of
> "www.myhost.com, www.myhost.com". In this case as well, I think the
> correct behavior is to pick up the first host name as this should be
> the original requested host name.

Shouldn't we also use the last forwarded host in this list too?

-Andy




More information about the Catalyst-dev mailing list