[pve-devel] [RFC firewall] fix multiport handling

[Fabian Grünbichler]

there are basically three cases:

1) only a single or no src and dst port each
2) at least one of src or dst has more than one port, but the port
   ranges are not identical
3) both src and dst have more than one port, with identical port ranges

1) can just use --dport/--sport with no multiport matching
2) needs to use '--match multiport' for src and dst separately, and use
  '--sports'/'--dports' even if either is just a single port
3) can use a single '--match multiport' and collapse the identical port
   range into one '--ports'

Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
---
this broke the default firewall setup, which contains a DROP/REJECT for SMB:
dport 1024:65535, sport 137

 src/PVE/Firewall.pm | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index bc3d9fe..79ba6fb 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1888,14 +1888,11 @@ sub ipt_rule_to_cmds {
 	    my $nbdport = defined($rule->{dport}) ? parse_port_name_number_or_range($rule->{dport}, 1) : 0;
 	    my $nbsport = defined($rule->{sport}) ? parse_port_name_number_or_range($rule->{sport}, 0) : 0;
 
-	    my $multiport = 0;
-	    $multiport++ if $nbdport > 1;
-	    $multiport++ if $nbsport > 1;
-
-	    push @match, "--match multiport" if $multiport;
-
-	    die "multiport: option '--sports' cannot be used together with '--dports'\n"
-		if ($multiport == 2) && ($rule->{dport} ne $rule->{sport});
+	    # 0 = no multiport
+	    # 1 = multiport with different src and dst port ranges
+	    # 2 = multiport with identical port ranges
+	    my $multiport = ($nbdport > 1) || ($nbsport > 1);
+	    $multiport++ if $multiport && ($rule->{dport} eq $rule->{sport});
 
 	    if ($rule->{dport}) {
 		if ($proto eq 'icmp') {
@@ -1910,24 +1907,29 @@ sub ipt_rule_to_cmds {
 		    push @match, "-m icmpv6 --icmpv6-type $rule->{dport}";
 		} elsif (!$PROTOCOLS_WITH_PORTS->{$proto}) {
 		    die "protocol $proto does not have ports\n";
-		} else {
-		    if ($nbdport > 1) {
-			if ($multiport == 2) {
-			    push @match,  "--ports $rule->{dport}";
-			} else {
-			    push @match, "--dports $rule->{dport}";
-			}
+		} elsif ($multiport) {
+		    push @match, "--match multiport";
+		    if ($multiport == 2) {
+			# handles both sports and dports
+			push @match,  "--ports $rule->{dport}";
 		    } else {
-			push @match, "--dport $rule->{dport}";
+			push @match, "--dports $rule->{dport}";
 		    }
+		} else {
+		    push @match, "--dport $rule->{dport}";
 		}
 	    }
 
 	    if ($rule->{sport}) {
 		die "protocol $proto does not have ports\n"
 		    if !$PROTOCOLS_WITH_PORTS->{$proto};
-		if ($nbsport > 1) {
-		    push @match, "--sports $rule->{sport}" if $multiport != 2;
+		if ($multiport) {
+		    if ($multiport ==2) {
+			# already handled in dsport branch
+		    } else {
+			push @match, "--match multiport";
+			push @match, "--sports $rule->{sport}";
+		    }
 		} else {
 		    push @match, "--sport $rule->{sport}";
 		}
-- 
2.14.2