From: Michael Tremer <michael.tremer@ipfire.org>
Date: Tue, 30 Apr 2019 09:56:05 +0000 (+0100)
Subject: firewall: Fix source/destination interface settings
X-Git-Tag: v2.23-core133~171^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=249839b0ca06f81eaf3b75b03ac41ab2f7b6c352;p=ipfire-2.x.git

firewall: Fix source/destination interface settings

When a forwarding rule is being created, we sometimes create
INPUT/OUTPUT rules, too. Those were slightly invalid because
the source and destination interfaces where passed, too.

This could render some rules in certain circumstances useless.

This patch fixes this and only adds -i for INPUT and -o for
OUTPUT rules.

Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
---

diff --git a/config/firewall/rules.pl b/config/firewall/rules.pl
index a87fc5274a..d2971566c8 100644
--- a/config/firewall/rules.pl
+++ b/config/firewall/rules.pl
@@ -383,6 +383,19 @@ sub buildrules {
 						push(@destination_options, ("-d", $destination));
 					}
 
+					# Add source and destination interface to the filter rules.
+					# These are supposed to help filtering forged packets that originate
+					# from BLUE with an IP address from GREEN for instance.
+					my @source_intf_options = ();
+					if ($source_intf) {
+						push(@source_intf_options, ("-i", $source_intf));
+					}
+
+					my @destination_intf_options = ();
+					if ($destination_intf) {
+						push(@destination_intf_options, ("-o", $destination_intf));
+					}
+
 					# Add time constraint options.
 					push(@options, @time_options);
 
@@ -467,10 +480,7 @@ sub buildrules {
 						} elsif ($NAT_MODE eq "SNAT") {
 							my @nat_options = @options;
 
-							if ($destination_intf) {
-								push(@nat_options, ("-o", $destination_intf));
-							}
-
+							push(@nat_options, @destination_intf_options);
 							push(@nat_options, @source_options);
 							push(@nat_options, @destination_options);
 
@@ -481,25 +491,14 @@ sub buildrules {
 						}
 					}
 
-					# Add source and destination interface to the filter rules.
-					# These are supposed to help filtering forged packets that originate
-					# from BLUE with an IP address from GREEN for instance.
-					if ($source_intf) {
-						push(@source_options, ("-i", $source_intf));
-					}
-
-					if ($destination_intf) {
-						push(@destination_options, ("-o", $destination_intf));
-					}
-
 					push(@options, @source_options);
 					push(@options, @destination_options);
 
 					# Insert firewall rule.
 					if ($LOG && !$NAT) {
-						run("$IPTABLES -A $chain @options @log_limit_options -j LOG --log-prefix '$chain '");
+						run("$IPTABLES -A $chain @options @source_intf_options @destination_intf_options @log_limit_options -j LOG --log-prefix '$chain '");
 					}
-					run("$IPTABLES -A $chain @options -j $target");
+					run("$IPTABLES -A $chain @options @source_intf_options @destination_intf_options -j $target");
 
 					# Handle forwarding rules and add corresponding rules for firewall access.
 					if ($chain eq $CHAIN_FORWARD) {
@@ -508,17 +507,17 @@ sub buildrules {
 						# for the firewall, too.
 						if ($firewall_is_in_destination_subnet && ($target ~~ @special_input_targets)) {
 							if ($LOG && !$NAT) {
-								run("$IPTABLES -A $CHAIN_INPUT @options @log_limit_options -j LOG --log-prefix '$CHAIN_INPUT '");
+								run("$IPTABLES -A $CHAIN_INPUT @options @source_intf_options @log_limit_options -j LOG --log-prefix '$CHAIN_INPUT '");
 							}
-							run("$IPTABLES -A $CHAIN_INPUT @options -j $target");
+							run("$IPTABLES -A $CHAIN_INPUT @options @source_intf_options -j $target");
 						}
 
 						# Likewise.
 						if ($firewall_is_in_source_subnet && ($target ~~ @special_output_targets)) {
 							if ($LOG && !$NAT) {
-								run("$IPTABLES -A $CHAIN_OUTPUT @options @log_limit_options -j LOG --log-prefix '$CHAIN_OUTPUT '");
+								run("$IPTABLES -A $CHAIN_OUTPUT @options @destination_intf_options @log_limit_options -j LOG --log-prefix '$CHAIN_OUTPUT '");
 							}
-							run("$IPTABLES -A $CHAIN_OUTPUT @options -j $target");
+							run("$IPTABLES -A $CHAIN_OUTPUT @options @destination_intf_options -j $target");
 						}
 					}
 				}