X-Git-Url: https://git.proxmox.com/?p=pve-firewall.git;a=blobdiff_plain;f=src%2FPVE%2FFirewall.pm;h=ac4850744a83571a5aec1340ace98997e0dfb48d;hp=c858b853bff4dca88e15f2ee7d3dff3e042c53e7;hb=d6dd6e96e4410109ffe944d2d4f990aa6d881b91;hpb=bf2fa11471823124b257321617924aa6811aecdf

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index c858b85..ac48507 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -876,12 +876,8 @@ sub get_etc_services {
     return $etc_services;
 }
 
-my $etc_protocols;
-
-sub get_etc_protocols {
-    return $etc_protocols if $etc_protocols;
-
-    my $filename = "/etc/protocols";
+sub parse_protocol_file {
+    my ($filename) = @_;
 
     my $fh = IO::File->new($filename, O_RDONLY);
     if (!$fh) {
@@ -896,7 +892,7 @@ sub get_etc_protocols {
 	next if $line =~m/^#/;
 	next if ($line =~m/^\s*$/);
 
-	if ($line =~ m!^(\S+)\s+(\d+)\s+.*$!) {
+	if ($line =~ m!^(\S+)\s+(\d+)(?:\s+.*)?$!) {
 	    $protocols->{byid}->{$2}->{name} = $1;
 	    $protocols->{byname}->{$1} = $protocols->{byid}->{$2};
 	}
@@ -904,6 +900,16 @@ sub get_etc_protocols {
 
     close($fh);
 
+    return $protocols;
+}
+
+my $etc_protocols;
+
+sub get_etc_protocols {
+    return $etc_protocols if $etc_protocols;
+
+    my $protocols = parse_protocol_file('/etc/protocols');
+
     # add special case for ICMP v6
     $protocols->{byid}->{icmpv6}->{name} = "icmpv6";
     $protocols->{byname}->{icmpv6} = $protocols->{byid}->{icmpv6};
@@ -1035,12 +1041,13 @@ sub parse_port_name_number_or_range {
     my @elements = split(/,/, $str);
     die "extraneous commas in list\n" if $str ne join(',', @elements);
     foreach my $item (@elements) {
-	$count++;
 	if ($item =~ m/^(\d+):(\d+)$/) {
+	    $count += 2;
 	    my ($port1, $port2) = ($1, $2);
 	    die "invalid port '$port1'\n" if $port1 > 65535;
 	    die "invalid port '$port2'\n" if $port2 > 65535;
 	} elsif ($item =~ m/^(\d+)$/) {
+	    $count += 1;
 	    my $port = $1;
 	    die "invalid port '$port'\n" if $port > 65535;
 	} else {
@@ -1054,9 +1061,15 @@ sub parse_port_name_number_or_range {
 	}
     }
 
-    die "ICPM ports not allowed in port range\n" if $icmp_port && $count > 1;
+    die "ICPM ports not allowed in port range\n" if $icmp_port && $count > 0;
 
-    return $count;
+    # I really don't like to use the word number here, but it's the only thing
+    # that makes sense in a literal way. The range 1:100 counts as 2, not as
+    # one and not as 100...
+    die "too many entries in port list (> 15 numbers)\n"
+	if $count > 15;
+
+    return (scalar(@elements) > 1);
 }
 
 PVE::JSONSchema::register_format('pve-fw-sport-spec', \&pve_fw_verify_sport_spec);
@@ -1878,19 +1891,12 @@ sub ipt_rule_to_cmds {
 	if (my $proto = $rule->{proto}) {
 	    push @match, "-p $proto";
 
-	    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;
+	    my $multidport = defined($rule->{dport}) && parse_port_name_number_or_range($rule->{dport}, 1);
+	    my $multisport = defined($rule->{sport}) && parse_port_name_number_or_range($rule->{sport}, 0);
 
-	    push @match, "--match multiport" if $multiport;
+	    my $add_dport = sub {
+		return if !$rule->{dport};
 
-	    die "multiport: option '--sports' cannot be used together with '--dports'\n"
-		if ($multiport == 2) && ($rule->{dport} ne $rule->{sport});
-
-	    if ($rule->{dport}) {
 		if ($proto eq 'icmp') {
 		    # Note: we use dport to store --icmp-type
 		    die "unknown icmp-type '$rule->{dport}'\n"
@@ -1903,28 +1909,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";
+		} elsif ($multidport) {
+		    push @match, "--match multiport", "--dports $rule->{dport}";
 		} else {
-		    if ($nbdport > 1) {
-			if ($multiport == 2) {
-			    push @match,  "--ports $rule->{dport}";
-			} else {
-			    push @match, "--dports $rule->{dport}";
-			}
-		    } else {
-			push @match, "--dport $rule->{dport}";
-		    }
+		    push @match, "--dport $rule->{dport}";
 		}
-	    }
+	    };
+
+	    my $add_sport = sub {
+		return if !$rule->{sport};
 
-	    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 ($multisport) {
+		    push @match, "--match multiport", "--sports $rule->{sport}";
 		} else {
 		    push @match, "--sport $rule->{sport}";
 		}
-	    }
+	    };
+
+	    # order matters - single port before multiport!
+	    $add_dport->() if $multisport;
+	    $add_sport->();
+	    $add_dport->() if !$multisport;
 	} elsif ($rule->{dport} || $rule->{sport}) {
 	    die "destination port '$rule->{dport}', but no protocol specified\n" if $rule->{dport};
 	    die "source port '$rule->{sport}', but no protocol specified\n" if $rule->{sport};
@@ -2417,6 +2424,7 @@ sub generate_group_rules {
 
     foreach my $rule (@$rules) {
 	next if $rule->{type} ne 'in';
+	next if !$rule->{enable} || $rule->{errors};
 	next if $rule->{ipversion} && $rule->{ipversion} ne $ipversion;
 	rule_substitude_action($rule, { ACCEPT => "PVEFW-SET-ACCEPT-MARK", REJECT => "PVEFW-reject" });
 	ruleset_generate_rule($ruleset, $chain, $ipversion, $rule, $cluster_conf);
@@ -2429,6 +2437,7 @@ sub generate_group_rules {
 
     foreach my $rule (@$rules) {
 	next if $rule->{type} ne 'out';
+	next if !$rule->{enable} || $rule->{errors};
 	next if $rule->{ipversion} && $rule->{ipversion} ne $ipversion;
 	# we use PVEFW-SET-ACCEPT-MARK (Instead of ACCEPT) because we need to
 	# check also other tap rules later