From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 0D56F9A040 for ; Tue, 16 May 2023 11:10:01 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id DCDBE1BC7E for ; Tue, 16 May 2023 11:09:30 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 16 May 2023 11:09:29 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 6659A44BDE for ; Tue, 16 May 2023 11:09:29 +0200 (CEST) From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= To: pve-devel@lists.proxmox.com Date: Tue, 16 May 2023 11:09:23 +0200 Message-Id: <20230516090924.1944193-1-f.gruenbichler@proxmox.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.074 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [firewall.pm] Subject: [pve-devel] [PATCH firewall 1/2] icmp: factor out check for relevant protocols X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 May 2023 09:10:01 -0000 this were not entirely consistent and sometimes the checks were repeated. Signed-off-by: Fabian Grünbichler --- src/PVE/Firewall.pm | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm index a16c035..5fa264a 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -856,6 +856,11 @@ my $is_valid_icmp_type = sub { } }; +my $proto_is_icmp = sub { + my $proto = shift; + return $proto eq 'icmp' || $proto eq 'icmpv6' || $proto eq 'ipv6-icmp'; +}; + sub init_firewall_macros { $pve_fw_parsed_macros = {}; @@ -1540,7 +1545,7 @@ my $rule_properties = { optional => 1, }, 'icmp-type' => { - description => "Specify icmp-type. Only valid if proto equals 'icmp'.", + description => "Specify icmp-type. Only valid if proto equals 'icmp' or 'icmpv6'/'ipv6-icmp'.", type => 'string', format => 'pve-fw-icmp-type-spec', optional => 1, }, @@ -1733,12 +1738,14 @@ sub verify_rule { } } + my $is_icmp = 0; if ($rule->{proto}) { 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 'ipv6-icmp'; + $is_icmp = $proto_is_icmp->($rule->{proto}); } if ($rule->{dport}) { @@ -1748,14 +1755,13 @@ sub verify_rule { &$add_error('proto', "missing property - 'dport' requires this property") if !$proto; &$add_error('dport', "protocol '$proto' does not support ports") - if !$PROTOCOLS_WITH_PORTS->{$proto} && - $proto ne 'icmp' && $proto ne 'icmpv6'; # special cases + if !$PROTOCOLS_WITH_PORTS->{$proto} && !$is_icmp; #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'; + if !$is_icmp; &$add_error('icmp-type', "'icmp-type' cannot be specified together with 'dport'") if $rule->{dport}; if ($proto eq 'icmp' && !$icmp_type_names->{$icmp_type}) { @@ -2138,6 +2144,7 @@ sub ipt_rule_to_cmds { if (my $proto = $rule->{proto}) { push @match, "-p $proto"; + my $is_icmp = $proto_is_icmp->($proto); 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); @@ -2178,7 +2185,7 @@ sub ipt_rule_to_cmds { 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'); + if !$is_icmp; my $type = $proto eq 'icmp' ? 'icmp-type' : 'icmpv6-type'; push @match, "-m $proto --$type $rule->{'icmp-type'}"; -- 2.30.2