[PATCH firewall/manager 0/5] Allow updating references to firewall objects when editing them

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

Renaming/deleting ipsets or aliases that are referenced by other rules,
security groups, or ipsets currently leaves dangling references behind.
This creates broken firewall configurations where the rules with those
dangling references fail to parse, meaning they are ignored by the
firewall.

This is especially dangerous in the renaming case, where a user might
reasonably expect references to update automatically, meaning they may
go about their day after renaming an object without realizing a whole
set of firewall rules is now inactive.

This patch series addresses this by adding optional parameters to the
related firewall endpoints that allow updating/deleting all references
to an object when editing/removing it. The UI patches add checkboxes to
the edit/remove windows to optionally pass this parameter.

The casing in the firewall API is inconsistent: ipset names are stored
as-is in the SectionConfig (e.g. 'FOO'), but lowercased when the config
is loaded. This means a rule referencing '+dc/FOO' will fail to match
its own definition after a load. In practice this is self-correcting:
lowercasing is applied in-place, so definitions are written back in
lowercase on the next read-write cycle, and the API does not allow
creating rules with references that do not match anything. There is
however a window between object creation and that first cycle where
references can be broken. To avoid false negatives in that window,
this series uses case-insensitive matching when scanning for
references, and renamed references are always written back lowercased.

Stefan and I talked about this off-list and came to the conclusion that
enforcing either casing may break existing configs that have been
manually edited, so a more general fix would be better folded into a
major release.

The UI patches introduce some code duplication. I tried to generalize
the remove button with an optional extra URL parameter, but I did not
find a good name for it, nor did I find a nice common ground that did
not involve making the call sites even more verbose with configuration
options, which is why I ended up leaving it like this. I am of course
open to suggestions.

The original plan was to have an upfront check for would-be-dangling
references before allowing any edit to an object, however this can be a
very expensive operation, since an object defined at the cluster level
may be referenced by guest and host configurations in the whole cluster.
A check like this would trigger an unconditional scan of all those
configs when clicking the remove buttons, which is why it is kept
opt-in in the UI.


pve-firewall:

Arthur Bied-Charreton (3):
  Add helpers for updating alias and ipset references
  ipset: Add option to update references on rename/delete
  aliases: Add option to update references on rename/delete

 src/PVE/API2/Firewall/Aliases.pm | 115 +++++++++++++++++++++++++++++++
 src/PVE/API2/Firewall/Helpers.pm |  66 ++++++++++++++++++
 src/PVE/API2/Firewall/IPSet.pm   | 108 ++++++++++++++++++++++++++++-
 3 files changed, 288 insertions(+), 1 deletion(-)


pve-manager:

Arthur Bied-Charreton (2):
  ipset: Add option to update references on rename/delete
  aliases: Add option to update references on rename/delete

 www/manager6/grid/FirewallAliases.js | 101 +++++++++++++++++++++------
 www/manager6/panel/IPSet.js          |  60 +++++++++++++++-
 2 files changed, 136 insertions(+), 25 deletions(-)


Summary over all repositories:
  5 files changed, 424 insertions(+), 26 deletions(-)

-- 
Generated by murpp 0.11.0