Consult policies when constructing dnat chains; warning when zone specified on NAT-only rules

git-svn-id: https://shorewall.svn.sourceforge.net/svnroot/shorewall/trunk@8061 fbd18981-670d-0410-9b5c-8dc0c1a9a2bb

Shorewall-common/releasenotes.txt

@@ -23,8 +23,7 @@ Problems corrected in Shorewall 4.1.4.
 2)  The compilation date recorded in the firewall.conf file produced by
     Shorewall-perl was previously mangled.
 
-3)  Previously, the following situation would result in unexpected
+3)  The following situation would result in unexpected behavior.
-    behavior.
 
     /etc/shorewall/zones:
 
@@ -46,15 +45,30 @@ Problems corrected in Shorewall 4.1.4.
 
 	#ACTION   SOURCE     DEST     PROTO    DEST
 	#                                      PORT(S)
-	ACCEPT+   net        dmz      tcp      80
+	ACCEPT    net        dmz      tcp      80
 	REDIRECT  loc        3128     tcp      80
 
     The web server in the dmz (implied by the first rule) is
     inaccessible from the 'net' zone because the REDIRECT rule 
-    redirects all traffic arriving on 'ppp+' to local port 3128 (the
+    redirects all traffic arriving on 'ppp+' to local port 3128.
-    ACCEPT+ is behaving like ACCEPT).
 
-    Shorewall 4.1.4 includes a fix for this problem.
+    Shorewall 4.1.4 includes a fix for this problem that also requires
+    a configuration change.
+
+    The basic problem with the above configuration is that 'net' is a
+    sub-zone of 'loc' (since ppp0 is a subset of ppp+) but Shorewall
+    isn't able to recognize that fact. 
+
+    By changing the /etc/shorewall/zones file to make the parent/child
+    relationship explicit:
+
+    /etc/shorewall/zones:
+
+	#ZONE	     TYPE
+	fw	     firewall
+	loc	     ipv4
+	net:loc	     ipv4
+	dmz	     ipv4
 
 Other changes in Shorewall 4.1.4.
 
@@ -63,6 +77,13 @@ Other changes in Shorewall 4.1.4.
     installed. As always, the full configuration file set is installed
     in /usr/share/shorewall/configfiles.
 
+2)  Specifying a destination zone in a NAT-only rule now generates a
+    warning and the destination zone is ignored. NAT-only rules are:
+
+    	     NONAT
+	     REDIRECT-
+	     DNAT- 
+
 Migration Issues.
 
 1)  Previously, when HIGH_ROUTE_MARKS=Yes, Shorewall allowed non-zero
@@ -78,6 +99,13 @@ Migration Issues.
     b) After the -v and -q options are applied, the resulting value is
        adjusted to fall within the range -1 through 2.
 
+3)  Specifying a destination zone in a NAT-only rule now generates a
+    warning and the destination zone is ignored. NAT-only rules are:
+
+    	     NONAT
+	     REDIRECT-
+	     DNAT- 
+
 New Features in Shorewall 4.1.
 
 1)  Shorewall 4.1 contains experimental support for multiple Internet

Shorewall-perl/Shorewall/Rules.pm

@@ -978,10 +978,11 @@ sub process_rule1 ( $$$$$$$$$$$ ) {
     # Take care of irregular syntax and targets
     #
     if ( $actiontype & REDIRECT ) {
+	my $z = $actiontype & NATONLY ? '' : firewall_zone;
 	if ( $dest eq '-' ) {
-	    $dest = join( '', firewall_zone, '::' , $ports =~ /[:,]/ ? '' : $ports );
+	    $dest = join( '', $z, '::' , $ports =~ /[:,]/ ? '' : $ports );
 	} else {
-	    $dest = join( '', firewall_zone, '::', $dest ) unless $dest =~ /:/;
+	    $dest = join( '', $z, '::', $dest ) unless $dest =~ /:/;
 	}
     } elsif ( $action eq 'REJECT' ) {
 	$action = 'reject';
@@ -1006,7 +1007,7 @@ sub process_rule1 ( $$$$$$$$$$$ ) {
 	$source = ALLIPv4;
     }
 
-    if ( $dest =~ /^(.+?):(.*)/ ) {
+    if ( $dest =~ /^(.*?):(.*)/ ) {
 	$destzone = $1;
 	$dest = $2;
     } else {
@@ -1016,8 +1017,13 @@ sub process_rule1 ( $$$$$$$$$$$ ) {
 
     fatal_error "Missing source zone" if $sourcezone eq '-' || $sourcezone =~ /^:/;
     fatal_error "Unknown source zone ($sourcezone)" unless $sourceref = defined_zone( $sourcezone );
-    fatal_error "Missing destination zone" if $destzone eq '-' || $destzone =~ /^:/;
+
-    fatal_error "Unknown destination zone ($destzone)" unless $destref = defined_zone( $destzone );
+    if ( $actiontype & NATONLY ) {
+	warning_message "Destination zone ($destzone) ignored" unless $destzone eq '-' || $destzone eq '';
+    } else {
+	fatal_error "Missing destination zone" if $destzone eq '-' || $destzone eq '';
+	fatal_error "Unknown destination zone ($destzone)" unless $destref = defined_zone( $destzone );
+    }
 
     my $restriction = NO_RESTRICT;
 
@@ -1027,16 +1033,7 @@ sub process_rule1 ( $$$$$$$$$$$ ) {
 	$restriction = INPUT_RESTRICT if $destzone eq firewall_zone;
     }
 
-    #
+    my ( $chain, $chainref, $policy );
-    # Check for illegal bridge port rule
-    #
-    if ( $destref->{type} eq 'bport4' ) {
-	unless ( $sourceref->{bridge} eq $destref->{bridge} || single_interface( $sourcezone ) eq $destref->{bridge} ) {
-	    return 1 if $wildcard;
-	    fatal_error "Rules with a DESTINATION Bridge Port zone must have a SOURCE zone on the same bridge";
-	}
-    }
-
     #
     # For compatibility with older Shorewall versions
     #
@@ -1045,31 +1042,43 @@ sub process_rule1 ( $$$$$$$$$$$ ) {
     #
     # Take care of chain
     #
-    my $chain    = "${sourcezone}2${destzone}";
+    
-    my $chainref = ensure_chain 'filter', $chain;
+    unless ( $actiontype & NATONLY ) {
-    my $policy   = $chainref->{policy};
+	#
-
+	# Check for illegal bridge port rule
-    if ( $policy eq 'NONE' ) {
+	#
-	return 1 if $wildcard;
+	if ( $destref->{type} eq 'bport4' ) {
-	fatal_error "Rules may not override a NONE policy";
+	    unless ( $sourceref->{bridge} eq $destref->{bridge} || single_interface( $sourcezone ) eq $destref->{bridge} ) {
-    }
+		return 1 if $wildcard;
-
+		fatal_error "Rules with a DESTINATION Bridge Port zone must have a SOURCE zone on the same bridge";
-    #
+	    }
-    # Handle Optimization
-    #
-    if ( $optimize > 0 ) {
-	my $loglevel = $filter_table->{$chainref->{policychain}}{loglevel};
-	if ( $loglevel ne '' ) {
-	    return 1 if $target eq "${policy}:$loglevel}";
-	} else {
-	    return 1 if $basictarget eq $policy;
 	}
+
+	$chain    = "${sourcezone}2${destzone}";
+	$chainref = ensure_chain 'filter', $chain;
+	$policy   = $chainref->{policy};
+
+	if ( $policy eq 'NONE' ) {
+	    return 1 if $wildcard;
+	    fatal_error "Rules may not override a NONE policy";
+	}
+	#
+	# Handle Optimization
+	#
+	if ( $optimize > 0 ) {
+	    my $loglevel = $filter_table->{$chainref->{policychain}}{loglevel};
+	    if ( $loglevel ne '' ) {
+		return 1 if $target eq "${policy}:$loglevel}";
+	    } else {
+		return 1 if $basictarget eq $policy;
+	    }
+	}
+	#
+	# Mark the chain as referenced and add appropriate rules from earlier sections.
+	#
+	$chainref = ensure_filter_chain $chain, 1;
     }
 
-    #
-    # Mark the chain as referenced and add appropriate rules from earlier sections.
-    #
-    $chainref = ensure_filter_chain $chain, 1;
     #
     # Generate Fixed part of the rule
     #
@@ -1145,7 +1154,11 @@ sub process_rule1 ( $$$$$$$$$$$ ) {
 	} else {
 	    fatal_error "A server must be specified in the DEST column in $action rules" if $server eq '';
 
-	    validate_address $server, 0;
+	    if ( $server =~ /^(.+)-(.+)$/ ) {
+		validate_range( $1, $2 );
+	    } else {
+		validate_address $server, 0;
+	    }
 
 	    if ( $action eq 'SAME' ) {
 		fatal_error 'Port mapping not allowed in SAME rules' if $serverport;
@@ -1209,8 +1222,6 @@ sub process_rule1 ( $$$$$$$$$$$ ) {
 	#
 	fatal_error "Invalid DEST ($dest) in $action rule" if $dest =~ /:/;
 
-	$sourceref->{options}{nested} = 1;
-
 	$origdest = '' unless $origdest and $origdest ne '-';
 
 	if ( $origdest eq 'detect' ) {
@@ -1547,6 +1558,8 @@ sub generate_matrix() {
 	my $exclusions       = $zoneref->{exclusions};
 	my $frwd_ref         = 0;
 	my $chain            = 0;
+	my $dnatref          = $nat_table->{dnat_chain $zone};
+	my $nested           = $zoneref->{options}{nested};
 
 	if ( $complex ) {
 	    $frwd_ref = $filter_table->{"${zone}_frwd"};
@@ -1561,6 +1574,14 @@ sub generate_matrix() {
 	    push @rule_chains , [ $zone , firewall_zone , $chain2 ];
 	}
 
+	if ( $nested && $dnatref->{referenced} ) {
+	    for my $zone1 ( all_zones ) {
+		if ( $filter_table->{"${zone}2${zone1}"}->{policy} eq 'CONTINUE' ) {
+		    $nested = 0;
+		    last;
+		}
+	    }
+	}
 	#
 	# Take care of PREROUTING, INPUT and OUTPUT jumps
 	#
@@ -1594,15 +1615,13 @@ sub generate_matrix() {
 
 			my $source = match_source_net $net;
 
-			my $chainref = $nat_table->{dnat_chain $zone};
+			if ( $dnatref->{referenced} ) {
-
-			if ( $chainref->{referenced} ) {
 			    add_rule $preroutingref, $_ for ( @returnstack );
 			    @returnstack = ();
-			    add_rule $preroutingref, join( '', match_source_dev( $interface), $source, $ipsec_in_match, '-j ', $chainref->{name} );
+			    add_rule $preroutingref, join( '', match_source_dev( $interface), $source, $ipsec_in_match, '-j ', $dnatref->{name} );
 			}
-			
+
-			push @returnstack, join( '', match_source_dev( $interface), $source, $ipsec_in_match, '-j RETURN' ) if $zoneref->{options}{nested};
+			push @returnstack, join( '', match_source_dev( $interface), $source, $ipsec_in_match, '-j RETURN' ) if $nested;
 
 			if ( $chain2 ) {
 			    if ( @$exclusions ) {

Shorewall-perl/Shorewall/Zones.pm

@@ -299,7 +299,7 @@ sub determine_zones()
 					  in      => parse_zone_option_list( $in_options || '', $type ) ,
 					  out     => parse_zone_option_list( $out_options || '', $type ) ,
 					  complex => ($type eq 'ipsec4' || $options || $in_options || $out_options ? 1 : 0) ,
-					  nested  => 0 } ,
+					  nested  => @parents > 0 } ,
 			  interfaces => {} ,
 			  children   => [] ,
 			  hosts      => {}

manpages/shorewall-hosts.xml

@@ -211,7 +211,9 @@ vpn         ppp+:192.168.3.0/24</programlisting></para>
           than <ulink
           url="shorewall-interfaces.html">shorewall-interfaces</ulink>(8) if
           there is another zone that uses a fixed PPP interface (for example,
-          if the 'net' zone always interfaces through ppp0).</para>
+          if the 'net' zone always interfaces through ppp0). See <ulink
+          url="shorewall-nesting.html">shorewall-nesting</ulink>(8) for
+          additional information.</para>
         </listitem>
       </varlistentry>
     </variablelist>

manpages/shorewall-nesting.xml

@@ -108,12 +108,7 @@
     nesting occurs as a result of the use of wildcard interfaces (interface
     names ends in '+').</para>
 
-    <para>Here's an example.
+    <para>Here's an example. <filename>/etc/shorewall/zones</filename>:</para>
-    <filename>/etc/shorewall/zones</filename>:<programlisting>        #ZONE    TYPE        OPTION
-        fw       firewall
-        net      ipv4
-        loc      ipv4
-        dmz      ipv4</programlisting></para>
 
     <para><filename>/etc/shorewall/interfaces</filename>:<programlisting>        #ZONE    INTERFACE      BROADCAST        OPTIONS
         net      ppp0
@@ -152,8 +147,17 @@
     port rewritten to 3128. Hence, the web server running in the DMZ will be
     inaccessible from the web.</para>
 
-    <para>The above problem can be corrected in a couple of ways. The first is
+    <para>The above problem can be corrected in several ways.</para>
-    to rewrite the DNAT rule (assume that the local zone is entirely within
+
+    <para>If you are running Shorewall version 4.1.4 or later, the preferred
+    way is to simply make the nested zones explicit:<programlisting>        #ZONE    TYPE        OPTION
+        fw       firewall
+        loc      ipv4
+        net:loc  ipv4
+        dmz      ipv4</programlisting></para>
+
+    <para>When using other Shorewall versions, the first way is to rewrite the
+    DNAT rule (assume that the local zone is entirely within
     192.168.2.0/23):<programlisting>        #ACTION     SOURCE                 DEST      PROTO      DEST
         #                                                       PORT(S)
         ACCEPT      net                    dmz       tcp        80

manpages/shorewall-rules.xml

@@ -645,6 +645,10 @@
           affected. When <emphasis role="bold">all+</emphasis> is used,
           intra-zone traffic is affected.</para>
 
+          <para>Beginning with Shorewall 4.1.4, the
+          <replaceable>zone</replaceable> should be omitted in DNAT-,
+          REDIRECT- and NONAT rules.</para>
+
           <para>If the DEST <replaceable>zone</replaceable> is a bport zone,
           then either:<orderedlist numeration="loweralpha">
               <listitem>