[pve-devel] [PATCH firewall] only allow icmp names in the destination port field

[Wolfgang Bumiller]

We generate ICMP rules from the destination port field,
so allowing them in the source port field only confuses
people.
---
 src/PVE/Firewall.pm | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index d14380b..46a322a 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -995,7 +995,7 @@ sub parse_address_list {
 }
 
 sub parse_port_name_number_or_range {
-    my ($str) = @_;
+    my ($str, $dport) = @_;
 
     my $services = PVE::Firewall::get_etc_services();
     my $count = 0;
@@ -1011,9 +1011,9 @@ sub parse_port_name_number_or_range {
 	    my $port = $1;
 	    die "invalid port '$port'\n" if $port > 65535;
 	} else {
-	    if ($icmp_type_names->{$item}) {
+	    if ($dport && $icmp_type_names->{$item}) {
 		$icmp_port = 1;
-	    } elsif ($icmpv6_type_names->{$item}) {
+	    } elsif ($dport && $icmpv6_type_names->{$item}) {
 		$icmp_port = 1;
 	    } else {
 		die "invalid port '$item'\n" if !$services->{byname}->{$item};
@@ -1026,11 +1026,20 @@ sub parse_port_name_number_or_range {
     return $count;
 }
 
-PVE::JSONSchema::register_format('pve-fw-port-spec', \&pve_fw_verify_port_spec);
-sub pve_fw_verify_port_spec {
+PVE::JSONSchema::register_format('pve-fw-sport-spec', \&pve_fw_verify_sport_spec);
+sub pve_fw_verify_sport_spec {
+   my ($portstr) = @_;
+
+   parse_port_name_number_or_range($portstr, 0);
+
+   return $portstr;
+}
+
+PVE::JSONSchema::register_format('pve-fw-dport-spec', \&pve_fw_verify_dport_spec);
+sub pve_fw_verify_dport_spec {
    my ($portstr) = @_;
 
-   parse_port_name_number_or_range($portstr);
+   parse_port_name_number_or_range($portstr, 1);
 
    return $portstr;
 }
@@ -1152,11 +1161,11 @@ my $rule_properties = {
 	optional => 1,
     },
     sport => {
-	type => 'string', format => 'pve-fw-port-spec',
+	type => 'string', format => 'pve-fw-sport-spec',
 	optional => 1,
     },
     dport => {
-	type => 'string', format => 'pve-fw-port-spec',
+	type => 'string', format => 'pve-fw-dport-spec',
 	optional => 1,
     },
     comment => {
@@ -1360,14 +1369,14 @@ sub verify_rule {
     }
 
     if ($rule->{dport}) {
-	eval { parse_port_name_number_or_range($rule->{dport}); };
+	eval { parse_port_name_number_or_range($rule->{dport}, 1); };
 	&$add_error('dport', $@) if $@;
 	&$add_error('proto', "missing property - 'dport' requires this property")
 	    if !$rule->{proto};
     }
 
     if ($rule->{sport}) {
-	eval { parse_port_name_number_or_range($rule->{sport}); };
+	eval { parse_port_name_number_or_range($rule->{sport}, 0); };
 	&$add_error('sport', $@) if $@;
 	&$add_error('proto', "missing property - 'sport' requires this property")
 	    if !$rule->{proto};
@@ -1622,8 +1631,8 @@ sub ruleset_generate_cmdstr {
 
     die "unable to emit macro - internal error" if $rule->{macro}; # should not happen
 
-    my $nbdport = defined($rule->{dport}) ? parse_port_name_number_or_range($rule->{dport}) : 0;
-    my $nbsport = defined($rule->{sport}) ? parse_port_name_number_or_range($rule->{sport}) : 0;
+    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 @cmd = ();
 
-- 
2.1.4