Fix DNAT[-] destination list handling; add shell validation

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

Shorewall/changelog.txt

@@ -26,3 +26,9 @@ Changes since 1.4.5
 10) Added the SHOREWALL_SHELL configuraiton parameter.
 
 11) Fixed capability reporting (thanks to Simon Matter).
+
+12) Correct the implementation of destination IP list in DNAT[-] rules.
+
+13) Check for broken shells that don't support "^" in arithmetic
+    expressions or whose arithmetic support is otherwise broken.
+

Shorewall/firewall

@@ -1849,7 +1849,11 @@ add_nat_rule() {
 
     if [ -n "$serv" ]; then
 	servport="${servport:+:$servport}"
-	target1="DNAT --to-destination ${serv}${servport}"
+	serv1=
+	for srv in `separate_list $serv`; do
+	    serv1="$serv1 --to-destination ${srv}${servport}"
+	done
+	target1="DNAT $serv1"
     else
 	target1="REDIRECT --to-port $servport"
     fi
@@ -2111,26 +2115,28 @@ add_a_rule()
 
 	    if [ -z "$dnat_only" -a $chain != ${FW}2${FW} ]; then
 		if [ -n "$serv" ]; then
-		    for srv in `ip_range $serv`; do
+		    for serv1 in `separate_list $serv`; do
-			if [ -n "$addr" -a -n "$CONNTRACK_MATCH" ]; then
+			for srv in `ip_range $serv1`; do
-			    for adr in $addr; do
+			    if [ -n "$addr" -a -n "$CONNTRACK_MATCH" ]; then
+				for adr in $addr; do
+				    if [ -n "$loglevel" -a -z "$natrule" ]; then
+					log_rule $loglevel $chain $logtarget -m conntrack --ctorigdst $adr \
+					    `fix_bang $proto $sports $multiport $state $cli -d $srv $dports`
+				    fi
+
+				    run_iptables2 -A $chain $proto $multiport $state $cli $sports \
+					-d $srv $dports -m conntrack --ctorigdst $adr -j $target
+				done
+			    else
 				if [ -n "$loglevel" -a -z "$natrule" ]; then
-				    log_rule $loglevel $chain $logtarget -m conntrack --ctorigdst $adr \
+				    log_rule $loglevel $chain $logtarget \
 					`fix_bang $proto $sports $multiport $state $cli -d $srv $dports`
 				fi
 
 				run_iptables2 -A $chain $proto $multiport $state $cli $sports \
-				    -d $srv $dports -m conntrack --ctorigdst $adr -j $target
+				    -d $srv $dports -j $target
-			    done
-			else
-			    if [ -n "$loglevel" -a -z "$natrule" ]; then
-				log_rule $loglevel $chain $logtarget \
-				    `fix_bang $proto $sports $multiport $state $cli -d $srv $dports`
 			    fi
-
+			done
-			    run_iptables2 -A $chain $proto $multiport $state $cli $sports \
-				-d $srv $dports -j $target
-			fi
 		    done
 		else
 		    if [ -n "$loglevel" -a -z "$natrule" ]; then
@@ -2327,42 +2333,83 @@ process_rule() # $1 = target
 
     # Generate Netfilter rule(s)
 
-    if [ -n "$MULTIPORT" -a \
+    case $logtarget in
-	 "$ports" = "${ports%:*}" -a \
+	DNAT*)
-	 "$cports" = "${cports%:*}" -a \
+	    if [ -n "$MULTIPORT" -a \
-	 `list_count $ports` -le 15 -a \
+		"$ports" = "${ports%:*}" -a \
-	 `list_count $cports` -le 15 ]
+		"$cports" = "${cports%:*}" -a \
-    then
+		`list_count $ports` -le 15 -a \
-	#
+		`list_count $cports` -le 15 ]
-	# MULTIPORT is enabled, there are no port ranges in the rule and less than
+		then
-	# 16 ports are listed - use multiport match.
+	        #
-	#
+	        # MULTIPORT is enabled, there are no port ranges in the rule and less than
-	multioption="-m multiport"
+	        # 16 ports are listed - use multiport match.
-	for client in `separate_list ${clients:=-}`; do
+	        #
-	    for server in `separate_list ${servers:=-}`; do
+		multioption="-m multiport"
-		#
+		for client in `separate_list ${clients:=-}`; do
-		# add_a_rule() modifies these so we must set their values each time
+		    #
-		#
+		    # add_a_rule() modifies these so we must set their values each time
-		port=${ports:=-}
+		    #
-		cport=${cports:=-}
+		    server=${servers:=-}
-		add_a_rule
+		    port=${ports:=-}
-	    done
+		    cport=${cports:=-}
-	done
+		    add_a_rule
-    else
+		done
-	#
+	    else
-	# MULTIPORT is disabled or the rule isn't compatible with multiport match
+	        #
-	#
+	        # MULTIPORT is disabled or the rule isn't compatible with multiport match
-	multioption=
+	        #
-	for client in `separate_list ${clients:=-}`; do
+		multioption=
-	    for server in `separate_list ${servers:=-}`; do
+		for client in `separate_list ${clients:=-}`; do
-		for port in `separate_list ${ports:=-}`; do
+		    for port in `separate_list ${ports:=-}`; do
-		    for cport in `separate_list ${cports:=-}`; do
+			for cport in `separate_list ${cports:=-}`; do
+			    server=${servers:=-}
+			    add_a_rule
+			done
+		    done
+		done
+	    fi
+	    ;;
+	*)
+
+	    if [ -n "$MULTIPORT" -a \
+		"$ports" = "${ports%:*}" -a \
+		"$cports" = "${cports%:*}" -a \
+		`list_count $ports` -le 15 -a \
+		`list_count $cports` -le 15 ]
+		then
+	        #
+	        # MULTIPORT is enabled, there are no port ranges in the rule and less than
+	        # 16 ports are listed - use multiport match.
+	        #
+		multioption="-m multiport"
+		for client in `separate_list ${clients:=-}`; do
+		    for server in `separate_list ${servers:=-}`; do
+		        #
+		        # add_a_rule() modifies these so we must set their values each time
+		        #
+			port=${ports:=-}
+			cport=${cports:=-}
 			add_a_rule
 		    done
 		done
-	    done
+	    else
-	done
+	        #
-    fi
+	        # MULTIPORT is disabled or the rule isn't compatible with multiport match
+	        #
+		multioption=
+		for client in `separate_list ${clients:=-}`; do
+		    for server in `separate_list ${servers:=-}`; do
+			for port in `separate_list ${ports:=-}`; do
+			    for cport in `separate_list ${cports:=-}`; do
+				add_a_rule
+			    done
+			done
+		    done
+		done
+	    fi
+	    ;;
+    esac
     #
     # Report Result
     #
@@ -2872,7 +2919,7 @@ decodeaddr() {
     IFS=.
 
     for x in $1; do
-	temp=$(( $(( $temp << 8 )) | $x ))
+	temp=$(( $(( $temp $LEFTSHIFT 8 )) | $x ))
     done
 
     echo $temp
@@ -2926,8 +2973,9 @@ ip_range() {
 # Netmask from CIDR
 #
 ip_netmask() {
-    echo $(( -1 << $((32 - ${1#*/})) ))
+    echo $(( -1 $LEFTSHIFT $((32 - ${1#*/})) ))
 }
+
 #
 # Network address from CIDR
 #
@@ -4551,6 +4599,18 @@ added_param_value_no() # $1 = Parameter Name, $2 = Parameter value
 # Initialize this program
 #
 do_initialize() {
+    #
+    # Code to see if shell can support xor
+    #
+    check_xor() {
+	echo $(( 256 ^ -1 ))
+    }
+    
+    check_xor1() {
+	local x=`check_xor 2> /dev/null`
+	[ -n "$x" ] && echo $x || echo 0
+    }
+
     # Run all utility programs using the C locale
     #
     # Thanks to Vincent Planchenault for this tip #
@@ -4563,6 +4623,10 @@ do_initialize() {
     #
     terminator=startup_error
     #
+    # So that emacs doesn't get lost
+    #
+    LEFTSHIFT='<<'
+    #
     # Clear all configuration variables
     #
     version=
@@ -4750,7 +4814,15 @@ do_initialize() {
     # Determine the capabilities of the installed iptables/netfilter
     #
     determine_capabilities
+    #
+    # Check out the user's shell
+    #
+    [ -n "$SHOREWALL_SHELL" ] || SHOREWALL_SHELL=/bin/sh
 
+    temp=`decodeaddr 192.168.1.1` 
+    if [ `encodeaddr $temp` != 192.168.1.1 -o `check_xor1` -ne -257 ]; then
+	startup_error "Shell $SHOREWALL_SHELL is broken and may not be used with Shorewall"
+    fi
 }
 
 #

Shorewall/releasenotes.txt

@@ -19,6 +19,18 @@ Problems Corrected:
    the output of the capabilities report was corrupted in the case
    where the capability was not available.
 
+4) Where a list of IP addresses appears in the DEST column of a DNAT[-]
+   rule, Shorewall incorrectly created multiple DNAT rules in the nat
+   table (one for each element in the list). Shorewall now correctly
+   creates a single DNAT rule with multiple "--to-destination" clauses.
+
+Migration Considerations:
+
+   This version of Shorewall uses shell features that aren't available
+   in all shells. Before you upgrade to this version of Shorewall, you
+   should download and run the 'shellcheck.sh' script from
+   http://shorewall.net/pub/shorewall/misc.
+
 New Features:
 
 1) A 'newnotsyn' interface option has been added. This option may be