Merge pull request perlbal#3 from abh/master

support multiple netmasks for trusted_upstream_proxies
athomason · Aug 17, 2011 · 10557ca · 10557ca
2 parents ce75605 + 72c0e46
commit 10557ca
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 12 deletions.
diff --git a/devtools/gendocs.pl b/devtools/gendocs.pl
@@ -1,4 +1,4 @@
-#!/usr/bin/perl
+#!/usr/bin/env perl
 #
 
 use strict;

diff --git a/doc/hacking/todo.txt b/doc/hacking/todo.txt
@@ -70,9 +70,6 @@
 * getter commands to retrieve the running config (GET?) or DUMP/SHOW/LIST
   http://rt.livejournal.org/Ticket/Display.html?id=2783
 
-* add tests for 'trusted_upstream_proxies' and 'always_trusted'
-  http://rt.livejournal.org/Ticket/Display.html?id=2784
-
 * get rid of httpres vs. res distinction in HTTPHeaders
   http://rt.livejournal.org/Ticket/Display.html?id=2785
 

diff --git a/doc/service-parameters.txt b/doc/service-parameters.txt
@@ -75,6 +75,10 @@ For all services:
 |                           |    |                     |service that maps onto     |
 |                           |    |                     |other services.            |
 |---------------------------+----+---------------------+---------------------------|
+|                           |    |                     |Path to directory          |
+|ssl_ca_path                |    |                     |containing certificates for|
+|                           |    |                     |SSL.                       |
+|---------------------------+----+---------------------+---------------------------|
 |ssl_cert_file              |    |certs/server-cert.pem|Path to certificate PEM    |
 |                           |    |                     |file for SSL.              |
 |---------------------------+----+---------------------+---------------------------|
@@ -83,7 +87,10 @@ For all services:
 |ssl_key_file               |    |certs/server-key.pem |Path to private key PEM    |
 |                           |    |                     |file for SSL.              |
 |---------------------------+----+---------------------+---------------------------|
-|                           |    |                     |A Net::Netmask filter (e.g.|
+|ssl_verify_mode            |int |0                    |SSL verification mode      |
+|---------------------------+----+---------------------+---------------------------|
+|                           |    |                     |A comma separated list of  |
+|                           |    |                     |Net::Netmask filters (e.g. |
 |                           |    |                     |10.0.0.0/24, see           |
 |                           |    |                     |Net::Netmask) that         |
 |trusted_upstream_proxies   |    |                     |determines whether upstream|

diff --git a/lib/Perlbal/Manual/Internals.pod b/lib/Perlbal/Manual/Internals.pod
@@ -775,7 +775,7 @@ Int, 0-100; % chance to take a standard priority request when we're in pressure
 
 =item trusted_upstream_proxies
 
-L<Net::Netmask> object containing netmasks for trusted upstreams.
+Array of L<Net::Netmask> objects containing netmasks for trusted upstreams.
 
 
 =item always_trusted

diff --git a/lib/Perlbal/Manual/ReverseProxy.pod b/lib/Perlbal/Manual/ReverseProxy.pod
@@ -289,7 +289,7 @@ Default is C<certs/server-key.pem>.
 
 =item B<trusted_upstream_proxies> = Net::Netmask filter
 
-A L<Net::Netmask> filter (e.g. 10.0.0.0/24, see L<Net::Netmask>) that determines whether upstream clients are trusted or not, where trusted means their X-Forwarded-For/etc headers are not munged.
+A comma separated list of L<Net::Netmask> filters (e.g. 10.0.0.0/24, see L<Net::Netmask>) that determines whether upstream clients are trusted or not, where trusted means their X-Forwarded-For/etc headers are not munged.
 
 
 =item B<upload_status_listeners> = comma separated list of hosts

diff --git a/lib/Perlbal/Service.pm b/lib/Perlbal/Service.pm
@@ -376,7 +376,7 @@ our $tunables = {
     },
 
     'trusted_upstream_proxies' => {
-        des => "A Net::Netmask filter (e.g. 10.0.0.0/24, see Net::Netmask) that determines whether upstream clients are trusted or not, where trusted means their X-Forwarded-For/etc headers are not munged.",
+        des => "A comma separated list of Net::Netmask filters (e.g. 10.0.0.0/24, see Net::Netmask) that determines whether upstream clients are trusted or not, where trusted means their X-Forwarded-For/etc headers are not munged.",
         check_role => "*",
         check_type => sub {
             my ($self, $val, $errref) = @_;
@@ -385,9 +385,23 @@ our $tunables = {
                 return 0;
             }
 
-            return 1 if $self->{trusted_upstream_proxies} = Net::Netmask->new2($val);
-            $$errref = "Error defining trusted upstream proxies: " . Net::Netmask::errstr();
-            return 0;
+            my @val = split /\s*,\s*/, $val;
+            my @trusted_upstreams = ();
+
+            for my $ip (@val) {
+                my $net = Net::Netmask->new2($ip);
+                unless ($net) {
+                    $$errref = "Error defining trusted upstream proxies: " . Net::Netmask::errstr();
+                    return 0;
+                }
+                push @trusted_upstreams, $net;
+            }
+
+            unless (@trusted_upstreams) {
+                $$errref = "Error defining trusted upstream proxies: None found";
+                return 0;
+            }
+            $self->{trusted_upstream_proxies} = \@trusted_upstreams;
         },
         setter => sub {
             my ($self, $val, $set, $mc) = @_;
@@ -1456,7 +1470,9 @@ sub trusted_ip {
     return 0 unless $tmap;
 
     # try to use it as a Net::Netmask object
-    return 1 if eval { $tmap->match($ip); };
+    for my $tmap (@{ $self->{trusted_upstream_proxies} }) {
+        return 1 if eval { $tmap->match($ip); };
+    }
     return 0;
 }
 

diff --git a/t/90-accesscontrol.t b/t/90-accesscontrol.t
@@ -72,6 +72,11 @@ ok(manage("SET trusted_upstream_proxies = 127.0.0.1"), "Turning trusted upstream
 ok(!check(), "Denied");
 ok(check(["X-Forwarded-For" => "1.1.1.1"]), "Allowed with XFF header");
 
+ok(manage("SET trusted_upstream_proxies = 10.0.0.0/24, 127.0.0.1"), "Turning trusted upstream proxies on for multiple netmasks");
+
+ok(!check(), "Denied");
+ok(check(["X-Forwarded-For" => "1.1.1.1"]), "Allowed with XFF header");
+
 ok(manage("SET test.AccessControl.use_observed_ip = 0"), "Turning off observed IP");
 ok(!check(["X-Forwarded-For" => "1.1.1.1"]), "Denied with XFF header");