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 AB8FF96C86 for ; Thu, 26 Jan 2023 15:30:53 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8ED7824A00 for ; Thu, 26 Jan 2023 15:30:53 +0100 (CET) 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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Thu, 26 Jan 2023 15:30:48 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 1BF37465AB for ; Thu, 26 Jan 2023 15:30:48 +0100 (CET) From: Leo Nunner To: pve-devel@lists.proxmox.com Date: Thu, 26 Jan 2023 15:30:18 +0100 Message-Id: <20230126143020.150338-4-l.nunner@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230126143020.150338-1-l.nunner@proxmox.com> References: <20230126143020.150338-1-l.nunner@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.080 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [ipset.pm, firewall.pm, groups.pm, aliases.pm] Subject: [pve-devel] [PATCH firewall 3/4] config: make groups, IPSets and aliases case-insensitive 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: Thu, 26 Jan 2023 14:30:53 -0000 So far, the names of groups and IPSets were automatically converted into lower-case. Aliases were parsed in a *kind of* case-insensitive way, where both the original name AND the lower-case one was stored. This patch removes all instances where names are converted to lower-case. We need some legacy handling for the aliases here, since rules/… will contain the lower-case name for them, while the definition itself has the original name. As such, internally, the names still get lower-cased while resolving aliases; this has no effect on the user. Signed-off-by: Leo Nunner --- src/PVE/API2/Firewall/Aliases.pm | 28 +++++++++++------ src/PVE/API2/Firewall/Groups.pm | 4 +-- src/PVE/API2/Firewall/IPSet.pm | 4 +-- src/PVE/Firewall.pm | 54 ++++++++++++++++++-------------- 4 files changed, 52 insertions(+), 38 deletions(-) diff --git a/src/PVE/API2/Firewall/Aliases.pm b/src/PVE/API2/Firewall/Aliases.pm index c1af408..9f8e71e 100644 --- a/src/PVE/API2/Firewall/Aliases.pm +++ b/src/PVE/API2/Firewall/Aliases.pm @@ -72,7 +72,16 @@ my $aliases_to_list = sub { my $list = []; foreach my $k (sort keys %$aliases) { - push @$list, $aliases->{$k}; + my $alias = $aliases->{$k}; + my $data = { + name => $k, + cidr => $alias->{cidr}, + ipversion => $alias->{ipversion}, + }; + if(my $comment = $alias->{comment}) { + $data->{comment} = $comment; + } + push @$list, $data; } return $list; }; @@ -148,10 +157,10 @@ sub register_create_alias { my ($fw_conf, $aliases) = $class->load_config($param); - my $name = lc($param->{name}); + my $name = $param->{name}; raise_param_exc({ name => "alias '$param->{name}' already exists" }) - if defined($aliases->{$name}); + if grep { lc($_) eq lc($name) } (keys $aliases->%*); my $data = { name => $param->{name}, cidr => $param->{cidr} }; $data->{comment} = $param->{comment} if $param->{comment}; @@ -188,7 +197,7 @@ sub register_read_alias { my ($fw_conf, $aliases) = $class->load_config($param); - my $name = lc($param->{name}); + my $name = $param->{name}; raise_param_exc({ name => "no such alias" }) if !defined($aliases->{$name}); @@ -234,20 +243,19 @@ sub register_update_alias { PVE::Tools::assert_if_modified($digest, $param->{digest}); - my $rename_to = lc($param->{rename}) if defined($param->{rename}); - my $rename_from = lc($param->{name}); + my $rename_to = $param->{rename} if defined($param->{rename}); + my $rename_from = $param->{name}; raise_param_exc({ name => "no such alias" }) if !$aliases->{$rename_from}; - my $data = { name => $param->{name}, cidr => $param->{cidr} }; + my $data = { name => $rename_from, cidr => $param->{cidr} }; $data->{comment} = $param->{comment} if $param->{comment}; $aliases->{$rename_from} = $data; if ($rename_to && ($rename_from ne $rename_to)) { raise_param_exc({ name => "alias '$param->{rename}' already exists" }) - if defined($aliases->{$rename_to}); - $aliases->{$rename_from}->{name} = $param->{rename}; + if grep { lc($_) eq lc($rename_to) } (keys $aliases->%*); $aliases->{$rename_to} = $aliases->{$rename_from}; delete $aliases->{$rename_from}; } @@ -291,7 +299,7 @@ sub register_delete_alias { my (undef, $digest) = PVE::Firewall::copy_list_with_digest($list); PVE::Tools::assert_if_modified($digest, $param->{digest}); - my $name = lc($param->{name}); + my $name = $param->{name}; delete $aliases->{$name}; $class->save_aliases($param, $fw_conf, $aliases); diff --git a/src/PVE/API2/Firewall/Groups.pm b/src/PVE/API2/Firewall/Groups.pm index cf9dc06..6d30c53 100644 --- a/src/PVE/API2/Firewall/Groups.pm +++ b/src/PVE/API2/Firewall/Groups.pm @@ -116,7 +116,7 @@ __PACKAGE__->register_method({ # prevent overwriting an existing group raise_param_exc({ group => "Security group '$rename_to' does already exist" }) - if $cluster_conf->{groups}->{$rename_to} && + if (grep { lc($_) eq lc($rename_to) } (keys $cluster_conf->{groups}->%*)) && $rename_to ne $rename_from; if ($rename_from eq $rename_to) { @@ -179,7 +179,7 @@ __PACKAGE__->register_method({ } else { # In this context, $rename_to is the name for the new group raise_param_exc({ group => "Security group '$rename_to' already exists" }) - if $cluster_conf->{groups}->{$rename_to}; + if grep { lc($_) eq lc($rename_to) } (keys $cluster_conf->{groups}->%*); $cluster_conf->{groups}->{$rename_to} = []; $cluster_conf->{group_comments}->{$rename_to} = $comment if defined($comment); diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm index fee9046..bb7f098 100644 --- a/src/PVE/API2/Firewall/IPSet.pm +++ b/src/PVE/API2/Firewall/IPSet.pm @@ -660,7 +660,7 @@ sub register_create { # prevent overwriting existing ipset raise_param_exc({ name => "IPSet '$rename_to' does already exist"}) - if $fw_conf->{ipset}->{$rename_to} && + if (grep { lc($_) eq lc($rename_to) } (keys $fw_conf->{ipset}->%*)) && $rename_to ne $rename_from; my $data = delete $fw_conf->{ipset}->{$rename_from}; @@ -672,7 +672,7 @@ sub register_create { } else { # In this context, $rename_to is the name for the new IPSet raise_param_exc({ name => "IPSet '$rename_to' already exists" }) - if $fw_conf->{ipset}->{$rename_to}; + if grep { lc($_) eq lc($rename_to) } (keys $fw_conf->{ipset}->%*); $fw_conf->{ipset}->{$rename_to} = []; $fw_conf->{ipset_comments}->{$rename_to} = $param->{comment} if defined($param->{comment}); diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm index 4924d51..cbb72f5 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -1646,18 +1646,17 @@ sub verify_rule { if ($value =~ m/^\+(${ipset_name_pattern})$/) { &$add_error($name, "no such ipset '$1'") if !($cluster_conf->{ipset}->{$1} || ($fw_conf && $fw_conf->{ipset}->{$1})); - } else { &$add_error($name, "invalid ipset name '$value'"); } } elsif ($value =~ m/^${ip_alias_pattern}$/){ - my $alias = lc($value); - &$add_error($name, "no such alias '$value'") - if !($cluster_conf->{aliases}->{$alias} || ($fw_conf && $fw_conf->{aliases}->{$alias})); - my $e = $fw_conf ? $fw_conf->{aliases}->{$alias} : undef; - $e = $cluster_conf->{aliases}->{$alias} if !$e && $cluster_conf; + my $ipversion; + + eval { (undef, $ipversion) = resolve_alias($cluster_conf, $fw_conf, $value) }; + &$add_error($name, "no such alias '$value' $@") + if $@; - &$set_ip_version($e->{ipversion}); + &$set_ip_version($ipversion); } } }; @@ -2069,11 +2068,8 @@ sub ipt_gen_src_or_dst_match { die "invalid security group name '$adr'\n"; } } elsif ($adr =~ m/^${ip_alias_pattern}$/){ - my $alias = lc($adr); - my $e = $fw_conf ? $fw_conf->{aliases}->{$alias} : undef; - $e = $cluster_conf->{aliases}->{$alias} if !$e && $cluster_conf; - die "no such alias '$adr'\n" if !$e; - $match = "-${dir} $e->{cidr}"; + my ($cidr) = resolve_alias($cluster_conf, $fw_conf, $adr); + $match = "-${dir} $cidr"; } elsif ($adr =~ m/\-/){ $match = "-m iprange --${srcdst}-range $adr"; } else { @@ -2919,11 +2915,22 @@ sub parse_clusterfw_option { sub resolve_alias { my ($clusterfw_conf, $fw_conf, $cidr) = @_; - my $alias = lc($cidr); - my $e = $fw_conf ? $fw_conf->{aliases}->{$alias} : undef; - $e = $clusterfw_conf->{aliases}->{$alias} if !$e && $clusterfw_conf; + my $alias = $cidr; + my $e; + + # This is a special legacy case: aliases were the only thing that used to be case-insensitive. Due to this, old + # configs can contain upper-case aliases in the definition, but rules contain the lower-case name. + if ($fw_conf) { + my ($name) = grep { lc($_) eq lc($alias) } keys $fw_conf->{aliases}->%*; + $e = $fw_conf->{aliases}->{$name} if $name; + } + + if (!$e && $clusterfw_conf) { + my ($name) = grep { lc($_) eq lc($alias) } keys $clusterfw_conf->{aliases}->%*; + $e = $clusterfw_conf->{aliases}->{$name} if $name; + } - die "no such alias '$cidr'\n" if !$e;; + die "no such alias '$cidr'\n" if !$e; return wantarray ? ($e->{cidr}, $e->{ipversion}) : $e->{cidr}; } @@ -2959,12 +2966,11 @@ sub parse_alias { ($cidr, $ipversion) = parse_ip_or_cidr($cidr); my $data = { - name => $name, cidr => $cidr, ipversion => $ipversion, }; - $data->{comment} = $comment if $comment; - return $data; + $data->{comment} = $comment if $comment; + return ($name, $data); } return undef; @@ -3010,7 +3016,7 @@ sub generic_fw_config_parser { if ($empty_conf->{groups} && ($line =~ m/^\[group\s+(\S+)\]\s*(?:#\s*(.*?)\s*)?$/i)) { $section = 'groups'; - $group = lc($1); + $group = $1; my $comment = $2; eval { die "security group name too long\n" if length($group) > $max_group_name_length; @@ -3035,7 +3041,7 @@ sub generic_fw_config_parser { if ($empty_conf->{ipset} && ($line =~ m/^\[ipset\s+(\S+)\]\s*(?:#\s*(.*?)\s*)?$/i)) { $section = 'ipset'; - $group = lc($1); + $group = $1; my $comment = $2; eval { die "ipset name too long\n" if length($group) > $max_ipset_name_length; @@ -3075,8 +3081,8 @@ sub generic_fw_config_parser { warn "$prefix: $@" if $@; } elsif ($section eq 'aliases') { eval { - my $data = parse_alias($line); - $res->{aliases}->{lc($data->{name})} = $data; + my ($name, $data) = parse_alias($line); + $res->{aliases}->{$name} = $data; }; warn "$prefix: $@" if $@; } elsif ($section eq 'rules') { @@ -3285,7 +3291,7 @@ my $format_aliases = sub { $raw .= "[ALIASES]\n\n"; foreach my $k (keys %$aliases) { my $e = $aliases->{$k}; - $raw .= "$e->{name} $e->{cidr}"; + $raw .= "$k $e->{cidr}"; $raw .= " # " . encode('utf8', $e->{comment}) if $e->{comment} && $e->{comment} !~ m/^\s*$/; $raw .= "\n"; -- 2.30.2