[PATCH pve-firewall v2 1/9] api: helpers: add helper to update firewall object references

[Arthur Bied-Charreton <a.bied-charreton@proxmox.com>]

Renaming or deleting a firewall object (ipset or alias) that is still
referenced by rules or ipset members leaves dangling references. The
firewall fails to parse the affected rules and drops them, so an edit
can effectively disable a whole set of rules.

Add a shared helper that finds and rewrites (on rename) or removes (on
delete) all such references. When operating on the cluster config it
also does so for every downstream config (guest, host and vnet) across
the cluster. Matching is case-insensitive and rewritten references are
normalized to lowercase.

Downstream configs are locked and saved individually as they are
visited, so on a cluster rename the caller must persist the config with
the new object already present before calling update_refs (keep both the
old and new object until all references are migrated). Otherwise a
concurrent firewall compilation could encounter a reference to an object
that does not exist yet and drop the rule.

Object references are not guaranteed to be scoped (dc/, guest/). This is
not an issue for cluster, host and vnet configs, as in those cases the
reference can only point to an object defined in the cluster config.
Guest configs can however define their own objects. Unscoped references
in guest rules are therefore resolved by first checking for a definition
in the relevant guest config, and only then in the cluster config, to
prevent overwriting the wrong reference.

The ipset and alias endpoints build on this in the following commits.

Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 src/PVE/Firewall/Helpers.pm | 195 ++++++++++++++++++++++++++++++++++++
 1 file changed, 195 insertions(+)

diff --git a/src/PVE/Firewall/Helpers.pm b/src/PVE/Firewall/Helpers.pm
index fa3646c..3bdb7dc 100644
--- a/src/PVE/Firewall/Helpers.pm
+++ b/src/PVE/Firewall/Helpers.pm
@@ -18,8 +18,12 @@ our @EXPORT_OK = qw(
     clone_vmfw_conf
     collect_refs
     flush_fw_ct_entries_by_mark
+    update_refs
+    get_object_spec
 );
 
+require PVE::Firewall;
+
 my $pvefw_conf_dir = "/etc/pve/firewall";
 
 sub lock_vmfw_conf {
@@ -234,4 +238,195 @@ sub flush_fw_ct_entries_by_mark {
     );
 }
 
+=head3 map_items($items, $action, $matches)
+
+Apply C<$action> to each item C<$item> in C<$items> for which C<$matches->($item)> is true. Remove
+C<$item> from C<$items> if C<$action->($item)> returns C<undef>.
+
+Return the updated items arrayref and a boolean indicating whether any item was matched.
+
+=cut
+
+sub map_items {
+    my ($items, $action, $matches) = @_;
+    my @result;
+    my $modified = 0;
+    for my $item (($items // [])->@*) {
+        if ($matches->($item)) {
+            $modified = 1;
+            my $new = $action->($item);
+            push @result, $new if defined $new;
+        } else {
+            push @result, $item;
+        }
+    }
+    return (\@result, $modified);
+}
+
+=head3 foreach_conf_in_env($conf, $rule_env, $rewrite)
+
+Apply C<$rewrite> to the main firewall configs and, if C<$rule_env> is 'cluster', to all guest, host
+and vnet firewall configs across the cluster. Configs where C<$rewrite> returns true are saved. The
+caller is responsible for locking and saving the cluster config (C<$conf>).
+
+=cut
+
+sub foreach_conf_in_env {
+    my ($conf, $rule_env, $rewrite) = @_;
+
+    $rewrite->($conf, $rule_env, 0);
+
+    return if $rule_env ne 'cluster';
+
+    my $vmlist = PVE::Cluster::get_vmlist();
+    my $vmids = ($vmlist // {})->{ids} // {};
+    for my $vmid (keys $vmids->%*) {
+        PVE::Firewall::lock_vmfw_conf(
+            $vmid,
+            10,
+            sub {
+                my $env = $vmlist->{ids}->{$vmid}->{type} eq 'lxc' ? 'ct' : 'vm';
+                my $guest_conf = PVE::Firewall::load_vmfw_conf($conf, $env, $vmid);
+                if ($rewrite->($guest_conf, 'cluster', 1)) {
+                    PVE::Firewall::save_vmfw_conf($vmid, $guest_conf);
+                }
+            },
+        );
+    }
+
+    for my $node (PVE::Cluster::get_nodelist()->@*) {
+        my $host_conf_path = "/etc/pve/nodes/$node/host.fw";
+        PVE::Firewall::lock_hostfw_conf(
+            $node,
+            10,
+            sub {
+                my $host_conf = PVE::Firewall::load_hostfw_conf($conf, $host_conf_path);
+                if ($rewrite->($host_conf, 'cluster', 0)) {
+                    PVE::Firewall::save_hostfw_conf($host_conf, $host_conf_path);
+                }
+            },
+        );
+    }
+
+    my $vnets = (PVE::Network::SDN::Vnets::config(1) // {})->{ids} // {};
+    for my $vnet (keys $vnets->%*) {
+        PVE::Firewall::lock_vnetfw_conf(
+            $vnet,
+            10,
+            sub {
+                my $vnet_conf = PVE::Firewall::load_vnetfw_conf($conf, 'vnet', $vnet);
+                if ($rewrite->($vnet_conf, 'cluster', 0)) {
+                    PVE::Firewall::save_vnetfw_conf($vnet, $vnet_conf);
+                }
+            },
+        );
+    }
+}
+
+my $object_ref_specs = {
+    ipset => { prefix => '+', self => 'ipset' },
+    aliases => { prefix => '', self => 'aliases' },
+};
+
+=head3 get_object_spec($kind)
+
+Get the spec hash for C<$kind>. Refer to the C<update_refs> POD for details.
+
+=cut
+
+sub get_object_spec {
+    my ($kind) = @_;
+    return $object_ref_specs->{$kind};
+}
+
+=head3 rewrite_refs_in_conf($conf, $spec, $old, $new, $env, $is_guest)
+
+Rename all references (or with C<$new> undef, delete referencing rules) to C<$old> across C<$conf>.
+
+Only exposed for testing, see POD for C<update_refs> for details.
+
+=cut
+
+sub rewrite_refs_in_conf {
+    my ($conf, $spec, $old, $new, $env, $is_guest) = @_;
+
+    my $ref_fields = ['source', 'dest', 'cidr'];
+    my $shadowed = $is_guest && $conf->{ $spec->{self} }->{$old};
+
+    my $scopes = [];
+    if ($env eq 'cluster') {
+        push $scopes->@*, 'dc/';
+        push $scopes->@*, '' if !$shadowed;
+    } else {
+        push $scopes->@*, '';
+        push $scopes->@*, 'guest/';
+    }
+
+    my $prefix = $spec->{prefix};
+    my $repl = { map { ("$prefix$_$old" => defined($new) ? "$prefix$_$new" : undef) } $scopes->@* };
+
+    my $matches = sub {
+        my ($obj) = @_;
+        grep { exists($repl->{ lc($obj->{$_} // '') }) } $ref_fields->@*;
+    };
+
+    my $rewrite = sub {
+        my ($obj) = @_;
+        return undef if !defined($new);
+        for my $f ($ref_fields->@*) {
+            my $r = lc($obj->{$f} // '');
+            $obj->{$f} = $repl->{$r} if exists($repl->{$r});
+        }
+        return $obj;
+    };
+
+    my $modified = 0;
+    my ($rules, $ch) = map_items($conf->{rules}, $rewrite, $matches);
+    $conf->{rules} = $rules;
+    $modified ||= $ch;
+
+    for my $section (qw(groups ipset)) {
+        my $map = $conf->{$section} // {};
+        for my $key (keys $map->%*) {
+            ($map->{$key}, my $c) = map_items($map->{$key}, $rewrite, $matches);
+            $modified ||= $c;
+        }
+    }
+
+    return $modified;
+}
+
+=head3 update_refs($conf, $spec, $old, $new, $rule_env)
+
+Rename (or, with C<$new> undef, delete) all references to a firewall object across the environment.
+C<$spec> describes the object kind:
+
+    { prefix => '+' | '', self => 'ipset' | 'aliases' }
+
+C<prefix> is the prefix a reference carries, C<self> is the section a downstream config may use to
+shadow a same-named cluster object. References are matched in rules, security groups and IPSet
+members. Matching is case-insensitive and renames are written back lowercased.
+
+The caller is responsible for locking and saving C<$conf>.
+
+If C<$conf> is the cluster config, i.e. C<$rule_env eq 'cluster'>, guest, host and vnet configs will
+be sequentially locked, updated and saved. Therefore, if this function is called for the cluster
+environment, a I<renaming> caller must first persist C<$conf> with the new (renamed) object present,
+so references rewritten in those downstream configs do not point at a not-yet-saved object during
+concurrent compilations.
+
+=cut
+
+sub update_refs {
+    my ($conf, $spec, $old, $new, $rule_env) = @_;
+
+    my ($lc_old, $lc_new) = (lc($old), defined($new) ? lc($new) : undef);
+
+    my $code = sub {
+        my ($fw_conf, $env, $is_guest) = @_;
+        return rewrite_refs_in_conf($fw_conf, $spec, $lc_old, $lc_new, $env, $is_guest);
+    };
+
+    return foreach_conf_in_env($conf, $rule_env, $code);
+}
 1;
-- 
2.47.3