[pve-devel] [PATCH firewall 3/3] introduce new icmp-type parameter

Mira Limbeck m.limbeck at proxmox.com
Wed Apr 29 15:45:25 CEST 2020


Currently icmp types are handled via 'dport'. This is not documented
anywhere except for a single line of comment in the code. To untangle
the icmp-type handling from the dport handling a new 'icmp-type'
parameter is introduced.

The valid 'icmp-type' values are limited to either the names
(icmp[v6]_type_names hash in the code, same as ip[6]tables provides) or
the combination of type and optional code (e.g. '3/0' for network-unreachable).
As both type and code can be values between 0 and 255, though not all
valid combinations, the checks limit it to range between 0/0 and
255/255.

Support for ipv6-icmp is added to icmp-type parameter handling. This makes it
possible to specify icmpv6 types via the GUI.

Signed-off-by: Mira Limbeck <m.limbeck at proxmox.com>
---
 src/PVE/API2/Firewall/Rules.pm |  4 +++
 src/PVE/Firewall.pm            | 63 ++++++++++++++++++++++++++++++++--
 2 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm
index 0e93a4a..39a0d3a 100644
--- a/src/PVE/API2/Firewall/Rules.pm
+++ b/src/PVE/API2/Firewall/Rules.pm
@@ -144,6 +144,10 @@ sub register_get_rule {
 		log => PVE::Firewall::get_standard_option('pve-fw-loglevel', {
 		    description => 'Log level for firewall rule',
 		}),
+		'icmp-type' => {
+		    type => 'string',
+		    optional => 1,
+		},
 		iface => {
 		    type => 'string',
 		    optional => 1,
diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 0cae9d8..34c2f15 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -1133,6 +1133,26 @@ sub pve_fw_verify_protocol_spec {
    return $proto;
 }
 
+PVE::JSONSchema::register_format('pve-fw-icmp-type-spec', \&pve_fw_verify_icmp_type_spec);
+sub pve_fw_verify_icmp_type_spec {
+    my ($icmp_type) = @_;
+
+    if ($icmp_type_names->{$icmp_type} ||  $icmpv6_type_names->{$icmp_type}) {
+	return $icmp_type;
+    }
+
+    if ($icmp_type =~ m!^(\d+)(?:/(\d+))?$!) {
+	die "icmp-type value '$1' > 255\n" if $1 > 255;
+	die "icmp-type value '$2' > 255\n" if $2 && $2 > 255;
+
+	return $icmp_type;
+    }
+
+    die "invalid icmp-type value '$icmp_type'\n" if $icmp_type ne '';
+
+    return $icmp_type;
+}
+
 
 # helper function for API
 
@@ -1460,6 +1480,11 @@ my $rule_properties = {
 	type => 'string',
 	optional => 1,
     },
+    'icmp-type' => {
+	description => "Specify icmp-type. Only valid if proto equals 'icmp'.",
+	type => 'string', format => 'pve-fw-icmp-type-spec',
+	optional => 1,
+    },
 };
 
 sub add_rule_properties {
@@ -1653,7 +1678,8 @@ sub verify_rule {
 	eval { pve_fw_verify_protocol_spec($rule->{proto}); };
 	&$add_error('proto', $@) if $@;
 	&$set_ip_version(4) if $rule->{proto} eq 'icmp';
- 	&$set_ip_version(6) if $rule->{proto} eq 'icmpv6';
+	&$set_ip_version(6) if $rule->{proto} eq 'icmpv6';
+	&$set_ip_version(6) if $rule->{proto} eq 'ipv6-icmp';
     }
 
     if ($rule->{dport}) {
@@ -1667,6 +1693,21 @@ sub verify_rule {
 		$proto ne 'icmp' && $proto ne 'icmpv6'; # special cases
     }
 
+    if (my $icmp_type = $rule ->{'icmp-type'}) {
+	my $proto = $rule->{proto};
+	&$add_error('proto', "missing property - 'icmp-type' requires this property")
+	    if $proto ne 'icmp' && $proto ne 'icmpv6' && $proto ne 'ipv6-icmp';
+	&$add_error('icmp-type', "'icmp-type' cannot be specified together with 'dport'")
+	    if $rule->{dport};
+	my $is_numeric = $icmp_type =~ m/\d+/;
+	if ($proto eq 'icmp' && !$is_numeric && !$icmp_type_names->{$icmp_type}) {
+	    &$add_error('icmp-type', "invalid icmp-type '$icmp_type' for proto 'icmp'");
+	} elsif (($proto eq 'icmpv6' || $proto eq 'ipv6-icmp') && !$is_numeric
+	    && !$icmpv6_type_names->{$icmp_type}) {
+	    &$add_error('icmp-type', "invalid icmp-type '$icmp_type' for proto '$proto'");
+	}
+    }
+
     if ($rule->{sport}) {
 	eval { parse_port_name_number_or_range($rule->{sport}, 0); };
 	&$add_error('sport', $@) if $@;
@@ -2040,7 +2081,7 @@ sub ipt_rule_to_cmds {
 		return if !$rule->{dport};
 
 		if ($proto eq 'icmp') {
-		    # Note: we use dport to store --icmp-type
+		    # Note: we allow dport to store --icmp-type for backwards compatibility
 		    die "unknown icmp-type '$rule->{dport}'\n"
 			if $rule->{dport} !~ /^\d+$/ && !defined($icmp_type_names->{$rule->{dport}});
 		    # values for icmp-type range between 0 and 255
@@ -2048,7 +2089,7 @@ sub ipt_rule_to_cmds {
 		    die "invalid icmp-type '$rule->{dport}'\n" if ($rule->{dport} =~ m/^(\d+)$/) && ($1 > 255);
 		    push @match, "-m icmp --icmp-type $rule->{dport}";
 		} elsif ($proto eq 'icmpv6') {
-		    # Note: we use dport to store --icmpv6-type
+		    # Note: we allow dport to store --icmpv6-type for backwards compatibility
 		    die "unknown icmpv6-type '$rule->{dport}'\n"
 			if $rule->{dport} !~ /^\d+$/ && !defined($icmpv6_type_names->{$rule->{dport}});
 		    # values for icmpv6-type range between 0 and 255
@@ -2076,7 +2117,18 @@ sub ipt_rule_to_cmds {
 		}
 	    };
 
+	    my $add_icmp_type = sub {
+		return if !defined($rule->{'icmp-type'}) || $rule->{'icmp-type'} eq '';
+
+		die "'icmp-type' can only be set if 'icmp', 'icmpv6' or 'ipv6-icmp' is specified\n"
+		    if ($proto ne 'icmp') && ($proto ne 'icmpv6') && ($proto ne 'ipv6-icmp');
+		my $type = $proto eq 'icmp' ? 'icmp-type' : 'icmpv6-type';
+
+		push @match, "-m $proto --$type $rule->{'icmp-type'}";
+	    };
+
 	    # order matters - single port before multiport!
+	    $add_icmp_type->();
 	    $add_dport->() if $multisport;
 	    $add_sport->();
 	    $add_dport->() if !$multisport;
@@ -2726,6 +2778,10 @@ sub parse_fw_rule {
 	    $rule->{log} = $1;
 	    next;
 	}
+	if ($line =~ s/^-icmp-type (\S+)\s*//) {
+	    $rule->{'icmp-type'} = $1;
+	    next;
+	}
 
 	last;
     }
@@ -3154,6 +3210,7 @@ my $format_rules = sub {
 		$raw .= " -dport $rule->{dport}" if $rule->{dport};
 		$raw .= " -sport $rule->{sport}" if $rule->{sport};
 		$raw .= " -log $rule->{log}" if $rule->{log};
+		$raw .= " -icmp-type $rule->{'icmp-type'}" if defined($rule->{'icmp-type'}) && $rule->{'icmp-type'} ne '';
 	    }
 
 	    $raw .= " # " . encode('utf8', $rule->{comment})
-- 
2.20.1





More information about the pve-devel mailing list