On 03/16/2013 06:21 PM, Mr Dash Four wrote:

> Now, the bug I found:
> 
> routes
> ~~~~
> main 10.0.0.0/8 blackhole
> main 10.0.0.0/8 prohibit
> 
> generates:
> 
> run_ip route add blackhole 10.0.0.0/8 table 254
> run_ip route add prohibit 10.0.0.0/8 table 254
> 
> That is not going to work (ip will complain that the route already 
> exists). A much cleaner solution to this would be if shorewall could 
> spot the overlap during compile time (don't know how doable that would 
> be), or, if that is not possible, to change "add" with replace, in which 
> case the latest route added will take precedence.
> 
> In this respect, if I also have 
> NULL_ROUTE_RFC1918=Yes|unreachable|prohibit|blackhole, then the 
> generated statements are with "ip route replace", which will 
> overlap/replace the one I defined in "routes" using my example above, so 
> if this can be detected, at least a warning by shorewall should be issued.
> 

The attached patch detects duplicate route destinations. A duplicate is
fatal in the routes file. If a routes entry matches an RFC1918 network
and NULL_ROUTE_RFC1918=Yes, then a warning message is issued and the
duplicate route is suppressed.

-Tom
-- 
Tom Eastep        \ When I die, I want to go like my Grandfather who
Shoreline,         \ died peacefully in his sleep. Not screaming like
Washington, USA     \ all of the passengers in his car
http://shorewall.net \________________________________________________
diff --git a/Shorewall/Perl/Shorewall/Providers.pm b/Shorewall/Perl/Shorewall/Providers.pm
index c23be2c..ae5580f 100644
--- a/Shorewall/Perl/Shorewall/Providers.pm
+++ b/Shorewall/Perl/Shorewall/Providers.pm
@@ -105,11 +105,11 @@ sub initialize( $ ) {
     $maxload                = 0;
     $tproxies               = 0;
 
-    %providers  = ( local   => { number => LOCAL_TABLE   , mark => 0 , optional => 0 ,routes => [], rules => [] } ,
-		    main    => { number => MAIN_TABLE    , mark => 0 , optional => 0 ,routes => [], rules => [] } ,
-		    default => { number => DEFAULT_TABLE , mark => 0 , optional => 0 ,routes => [], rules => [] } ,
-		    balance => { number => BALANCE_TABLE , mark => 0 , optional => 0 ,routes => [], rules => [] } ,
-		    unspec  => { number => UNSPEC_TABLE  , mark => 0 , optional => 0 ,routes => [], rules => [] } );
+    %providers  = ( local   => { number => LOCAL_TABLE   , mark => 0 , optional => 0 ,routes => [], rules => [] , routedests => {} } ,
+		    main    => { number => MAIN_TABLE    , mark => 0 , optional => 0 ,routes => [], rules => [] , routedests => {} } ,
+		    default => { number => DEFAULT_TABLE , mark => 0 , optional => 0 ,routes => [], rules => [] , routedests => {} } ,
+		    balance => { number => BALANCE_TABLE , mark => 0 , optional => 0 ,routes => [], rules => [] , routedests => {} } ,
+		    unspec  => { number => UNSPEC_TABLE  , mark => 0 , optional => 0 ,routes => [], rules => [] , routedests => {} } );
     @providers = ();
 }
 
@@ -625,6 +625,7 @@ sub process_a_provider( $ ) {
 			   what        => $what ,
 			   rules       => [] ,
 			   routes      => [] ,
+			   routedests  => {} ,
 			 };
 
     $provider_interfaces{$interface} = $table unless $shared;
@@ -1142,9 +1143,18 @@ sub add_a_route( ) {
     my $number = $providerref->{number};
     my $physical = $device eq '-' ? $providers{$provider}{physical} : physical_name( $device );
     my $routes = $providerref->{routes};
+    my $routedests = $providerref->{routedests};
 
     fatal_error "You may not add routes to the $provider table" if $number == LOCAL_TABLE || $number == UNSPEC_TABLE;
 
+    $dest .= join( '', '/', $family == 4 ? '32' : '128' ) unless $dest =~ '/';
+
+    if ( $routedests->{$dest} ) {
+	fatal_error "Duplicate DEST ($dest) in table ($provider)";
+    } else {
+	$routedests->{$dest} = 1;
+    }
+
     if ( $gateway ne '-' ) {
 	if ( $device ne '-' ) {
 	    push @$routes, qq(run_ip route replace $dest via $gateway dev $physical table $number);
@@ -1171,10 +1181,14 @@ sub setup_null_routing() {
     save_progress_message "Null Routing the RFC 1918 subnets";
     emit "> \${VARDIR}/undo_rfc1918_routing\n";
     for ( rfc1918_networks ) {
-	emit( qq(if ! \$IP -4 route ls | grep -q '^$_.* dev '; then),
-	      qq(    run_ip route replace $type $_),
-	      qq(    echo "\$IP -4 route del $type $_ > /dev/null 2>&1" >> \${VARDIR}/undo_rfc1918_routing),
-	      qq(fi\n) );
+	if ( $providers{main}{routedests}{$_} ) {
+	    warning_message "No NULL_ROUTE_RFC1918 route added for $_; there is already a route to that network defined in the routes file";
+	} else {
+	    emit( qq(if ! \$IP -4 route ls | grep -q '^$_.* dev '; then),
+		  qq(    run_ip route replace $type $_),
+		  qq(    echo "\$IP -4 route del $type $_ > /dev/null 2>&1" >> \${VARDIR}/undo_rfc1918_routing),
+		  qq(fi\n) );
+	}
     }
 }
 

Attachment: signature.asc
Description: OpenPGP digital signature

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
Shorewall-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/shorewall-devel

Reply via email to