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

[John Shields]

Hmm...

I have another patch at the end of this email that might work. After
considering the points above, it seems to me that it needs to be up to
the Catalyst administrator as to which IP to select from the
"X-Forwarded-For" header. Either the end, somewhere in the middle, or
the first. My new patch allows you to select which IP you want by
specifying how many proxy servers you have in front of your Catalyst
application. One nice thing about this is that it does not change
existing behavior at all nor requires any new configuration (assuming
you specify "1" instead of "true"). It re-purposes the
"using_frontend_proxy" configuration variable to specify the number of
proxies that are in front of your Catalyst application and then uses
that to select which IP to pull from the list. If you specify "1" or
don't define it, the code will pick the last IP in the list (i.e.,
existing behavior). If you specify a positive integer then it will
move up the list from the end that many places. If you specify a
number larger than the number of entries in the list then it will
effectively pick the first entry (i.e., you could specify 99 to always
pick the first).

One problem I see with this code in my environment is that the
"X-Forwarded-For" header seems to have an extra IP at the end. In
other words, the last two IPs are always the IP of my "lite" front end
apache instance when I'd expect there to only be the one. (The
"X-Forwarded-Host" header only has entries for each proxy server as
I'd expect.) Is that normal behavior or is that something unique to my
environment?

I don't know how everyone feels about reusing the
"using_frontend_proxy" configuration variable or if you would rather a
new variable be introduced. I could go either way on that. You may
also want separate configuration variables, one for the IP selection
and one for the host.

Also, I'd like to point out that my main goal in bringing this all up
in the first place is to properly select the *host* when there are
multiple hosts in the "X-Forwarded-Host" header. The current code will
fail in that case. Whether we pick the first, last, or whatever, it
does need to handle the case where there are multiple. So, at a
minimum it would be nice to just select the last "X-Forwarded-Host"
entry to fix the bug. (Frankly, the X-Forwarded-For header isn't that
useful for me as we don't do anything with the IP.)

Thanks!

- John


Index: Apache.pm
===================================================================
--- Apache.pm
+++ Apache.pm
@@ -30,17 +30,21 @@

     PROXY_CHECK:
     {
-        my $headers = $self->apache->headers_in;
-        unless ( $c->config->{using_frontend_proxy} ) {
+        my $headers     = $self->apache->headers_in;
+        my $proxy_count = $c->config->{using_frontend_proxy};
+        unless ( $proxy_count ) {
             last PROXY_CHECK if $c->request->address ne '127.0.0.1';
             last PROXY_CHECK if $c->config->{ignore_frontend_proxy};
         }
         last PROXY_CHECK unless $headers->{'X-Forwarded-For'};
+
+        $proxy_count ||= 1;

         # 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]+)$/;
-        $c->request->address( $ip );
+        # as 127.0.0.1. Select the specified upstream IP
+        my @ips   = reverse( split( /,\s+/, $headers->{'X-Forwarded-For'} ) );
+        my $index = ( $proxy_count <= @ips ) ? $proxy_count : scalar(@ips);
+        $c->request->address( $ips[$index - 1] );
     }

     $c->request->hostname( $self->apache->connection->remote_host );
@@ -85,14 +89,21 @@
     # If we are running as a backend proxy, get the true hostname
     PROXY_CHECK:
     {
-        unless ( $c->config->{using_frontend_proxy} ) {
+        my $headers     = $self->apache->headers_in;
+        my $proxy_count = $c->config->{using_frontend_proxy};
+        unless ( $proxy_count ) {
             last PROXY_CHECK if $host !~ /localhost|127.0.0.1/;
             last PROXY_CHECK if $c->config->{ignore_frontend_proxy};
         }
-        last PROXY_CHECK unless $c->request->header( 'X-Forwarded-Host' );
+        last PROXY_CHECK unless $headers->{'X-Forwarded-Host'};

-        $host = $c->request->header( 'X-Forwarded-Host' );
+        $proxy_count ||= 1;

+        # choose the original host (first) in the list
+        my @hosts = reverse( split( /,\s+/, $headers->{'X-Forwarded-Host'} ) );
+        my $index = ( $proxy_count <= @hosts ) ? $proxy_count : scalar(@hosts);
+        $host     = $hosts[$index - 1];
+
         if ( $host =~ /^(.+):(\d+)$/ ) {
             $host = $1;
             $port = $2;