[pve-devel] [PATCH firewall 3/4] config: make groups, IPSets and aliases case-insensitive
Leo Nunner
l.nunner at proxmox.com
Thu Jan 26 15:30:18 CET 2023
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 <l.nunner at proxmox.com>
---
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
More information about the pve-devel
mailing list