From 5e6c00561cb64fa33d3d43745b4683e06fdc3b69 Mon Sep 17 00:00:00 2001
From: teastep <teastep@fbd18981-670d-0410-9b5c-8dc0c1a9a2bb>
Date: Thu, 19 Oct 2006 20:18:22 +0000
Subject: [PATCH] A little cleanup and some comments regarding redundant rule
 removal

git-svn-id: https://shorewall.svn.sourceforge.net/svnroot/shorewall/trunk@4709 fbd18981-670d-0410-9b5c-8dc0c1a9a2bb
---
 Shorewall/compiler | 61 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 54 insertions(+), 7 deletions(-)

diff --git a/Shorewall/compiler b/Shorewall/compiler
index a13dde6f5..7c8959d97 100755
--- a/Shorewall/compiler
+++ b/Shorewall/compiler
@@ -4264,7 +4264,11 @@ activate_rules()
 		run_iptables -A OUTPUT -o $interface -d 224.0.0.0/4     -j $chain1
 	    done
 	fi
-
+	#
+	# The following code attempts to eliminate redundant sequences of jumps to 
+	# all2all or <source zone>2all. It does so by combining all trailing 
+	# jumps to the same policy-only chain.
+	#
 	dest_zones=
 	temp_zones=
 	last_chain=
@@ -4275,9 +4279,6 @@ activate_rules()
 
 	    [ "$policy" = NONE ] && continue
 
-	    eval dest_hosts=\$${zone1}_hosts
-	    eval exclusions1=\"\$${zone1}_exclusions\"
-
 	    chain="$(rules_chain $zone $zone1)"
 
 	    [ -z "$chain" ] && continue # CONTINUE policy and there is no canonical chain.
@@ -4306,27 +4307,54 @@ activate_rules()
 
 	    case $chain in
 		*2all)
+		    #
+		    # Rules chain is a Policy-only chain that could be used more than once (all2all or ${zone}2all
+		    #
 		    if [ -n "$last_chain" ]; then
+			#
+			# And the last rules chain was a policy-only chain
+			#
 			if [ "$chain" != "$last_chain" ]; then
+			    #
+			    # But it was a different one -- back to square 1
+			    #
 			    last_chain=$chain
 			    dest_zones="$dest_zones $temp_zones"
 			    temp_zones=$zone1
 			else
+			    #
+			    # Same chain -- add this dest zone to the running list of
+			    #               zones using the same rules chain
+			    #
 			    temp_zones="$temp_zones $zone1"
 			fi
+		    elif [ $policy = ACCEPT ]; then
+			#
+			# We don't wild-card ACCEPT policies -- could open up security holes through interfaces
+			# that aren't described in /etc/shorewall/interfaces
+			#
+			dest_zones="$dest_zones $zone1"
 		    else
+			#
+			# First in a potential run of rules using this chain
+			#
 			last_chain=$chain
 			temp_zones=$zone1
 		    fi
 		    ;;
 		*)
+		    #
+		    # Not a policy chain -- add accumulated sequence of dest zones to those needing processing
+		    #
 		    dest_zones="$dest_zones $temp_zones $zone1"
 		    temp_zones=
 		    last_chain=
 		    ;;
 	    esac
 	done
-
+	#
+	# $dest_zones is now the (possibly condensed) list of destination zones that we need to handle from this source zone
+	#
 	for zone1 in $dest_zones; do
 
 	    eval policy=\$${zone}2${zone1}_policy
@@ -4368,17 +4396,32 @@ activate_rules()
 	    fi
 
 	    if [ -n "$exclusions1" ]; then
+		#
+		# We handle exlusions in the dest zone by inserting RETURN rules at the front of
+		# each rules chain where the zone is the destination
+		#
 		case $chain in
 		    all2$zone1)
+			#
+			# We only want to add the exclusions once
+			#
 		        if eval test -z \"\$${chain}_exclusions\"; then
 			    eval ${chain}_exclusions=Yes
 			    insert_exclusions filter $chain $exclusions1
 			fi
 			;;
 		    *2all)
+			#
+			# A policy-only chain -- we create one exclusion chain for this
+			#                        dest zone/chain combination, and re-use
+			#                        it if the occasion presents itself
+			#
 			eval chain1=\$${chain}_${zone1}_ex
 
 			if [ -z "$chain1" ]; then
+			    #
+			    # Must create the chain
+			    #
 			    chain1=excl_${EXCLUSION_SEQ}
 			    EXCLUSION_SEQ=$(( $EXCLUSION_SEQ + 1 ))
 			    eval ${chain}_${zone}_ex=$chain1
@@ -4386,7 +4429,9 @@ activate_rules()
 			    add_exclusions filter $chain1 $exclusions1
 			    run_iptables -A $chain1 -j $chain
 			fi
-
+			#
+			# We must jump to the exclusion chain rather than to the policy chain
+			#
 			chain=$chain1
 			;;
 		    *)
@@ -4425,7 +4470,9 @@ activate_rules()
 		done
 	    fi
 	done
-
+	#
+	# Now add (an) unconditional jump(s) to the last unique policy-only chain determined above, if any
+	#
 	if [ -n "$last_chain" ]; then
 	    if [ -n "$complex" ]; then
 		run_iptables -A $frwd_chain -j $last_chain