[pve-devel] [PATCH v3 firewall 1/2] fix multiport handling

[Wolfgang Bumiller]

From: Fabian Grünbichler <f.gruenbichler at proxmox.com>

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>
Acked-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
 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..54ad48a 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.11.0