From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 954061FF0C2 for ; Thu, 22 Oct 2026 14:48:34 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id BCFC52172D; Thu, 22 Oct 2026 14:47:37 +0200 (CEST) From: Arthur Bied-Charreton To: pve-devel@lists.proxmox.com Subject: [PATCH container/docs/firewall/manager/network/qemu-server v4 00/27] handle dangling references when firewall objects go away Date: Thu, 22 Oct 2026 14:46:41 +0200 Message-ID: <20261022124708.429069-1-a.bied-charreton@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1792673236578 X-SPAM-LEVEL: Spam detection results: 1 AWL -1.774 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_DBL_SPAM 5 Contains a spam URL listed in the Spamhaus DBL blocklist [sdn.pm] Message-ID-Hash: XL5Q6CGOKX7OEADJJSA4N3RMH4IEOCRJ X-Message-ID-Hash: XL5Q6CGOKX7OEADJJSA4N3RMH4IEOCRJ X-MailFrom: a.bied-charreton@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Renaming or deleting a firewall object (an IPSet, alias or security group) that rules still reference leaves those references dangling. The firewall fails to parse the affected rules and drops them from the generated ruleset. For security groups, the reference reaches the backend, which creates empty chains on iptables, and causes the ruleset to fail to load on nftables. This is especially bad in the rename case, where one might reasonably expect the references to follow the new object name and walk away thinking the firewall rules are active. This series makes the affected operations offer to deal with the references instead of leaving them behind. pve-firewall gains a shared helper, update_refs(), whcih finds references in rules, security groups and IPset members and applies one of three actions: 'rename' rewrites the reference to point at the new name, 'disable' disables the referencing rules, and 'drop' removes them. An IPSet member has no disabled state, so 'disable' removes members as well. These options are added to the API as follows: POST .../firewall/ipset dangling-references keep|disable|drop|rename|force-rename PUT .../firewall/aliases/{name} dangling-references keep|disable|drop|rename|force-rename POST /cluster/firewall/groups dangling-references keep|disable|drop|rename|force-rename DELETE .../ipset/{name} dangling-references keep|disable|drop DELETE .../aliases/{name} dangling-references keep|disable|drop DELETE /cluster/firewall/groups/{name} dangling-references keep|disable|drop PUT /cluster/sdn dangling-ipset-references keep|disable|drop DELETE /nodes/{node}/qemu/{vmid} dangling-ipset-references keep|disable|drop DELETE /nodes/{node}/lxc/{vmid} dangling-ipset-references keep|disable|drop For a cluster object, the helper also walks every downstream config in the cluster (guest, host and vnet), locking and saving each one. On a rename, the new object is persisted before its references are rewritten, so a concurrent compilation does not observe a reference to an object that does not exist yet. If a cluster-wide rewrite is interrupted part way, passing 'force-rename' resumes it (without this, the next attempt would fail due to the target name already existing in the config). While these options allow to trigger changes to the firewall configurations from other endpoints requiring different permissions, like guest destroy and SDN apply, they do not require any additional permissions, see full explanation here [0]. The API keeps backwards-compatible defaults: 'keep' everywhere, with the exception of POST /cluster/firewall/groups, which was already renaming references by default and keeps doing so. The UI however defaults towards renaming/dropping dangling references, as this is expected to be the normal case. Pre-built packages on sani under: abied-charreton/firewall-object-references/v4/ Build dependencies: pve-container, qemu-server <- pve-firewall pve-manager <- pve-docs Runtime dependencies: pve-network, pve-manager <- pve-firewall Changes since [v3] - Rename the option from 'update-references' (no|yes|force) to 'dangling-references' (keep|disable|drop|rename|force-rename) so the parameter name stays the same everywhere. - Add 'drop' and 'disable' actions to the rename paths - Handle security groups as well (the rename path was missing vnet configs and the delete path did nothing, fixes #8176 [1]). - Move all helpers related to firewall object references out of PVE::Firewall::Helpers into PVE::Firewall::References. - update_refs() takes a callback for saving the cluster config so it can decide when the object has to be persisted instead of relying on the caller for correctness. - Run every rename and delete as a task and log every change made to the configs to the task log for better observability. - Drop 'force-rename' from the GUI and document it as an escape hatch for partially applied renames - Trim the API option descriptions and move the reasoning into a pve-docs patch. - Merge the two ExtJS widgets into one ComboGrid taking the list of actions from the caller and showing each descriptoin directly in a column instead of using a tooltip. - Preselect 'rename' and 'drop' in the UI for edits and removals, respectively. API defaults keep doing the same as they were before. - Move the pve-firewall tests to src/test - Move selector to advanced section in ipset/alias edit/remove dialogs [v3] https://lore.proxmox.com/pve-devel/20260925094230.844917-1-a.bied-charreton@proxmox.com/ [v2] https://lore.proxmox.com/pve-devel/20260818133413.450776-1-a.bied-charreton@proxmox.com/ [v1] https://lore.proxmox.com/pve-devel/20260407073658.90818-1-a.bied-charreton@proxmox.com/ [0] https://lore.proxmox.com/pve-devel/awaoshjzbr7adisjngsrcts4zs3hxgoxw2lgcoq66gtiap3al2@mpxrbmtyfcwj/ [1] https://bugzilla.proxmox.com/show_bug.cgi?id=8176 pve-firewall: Arthur Bied-Charreton (14): firewall: move object reference helpers to their own module firewall: references: add helpers to update object references firewall: references: log the references an object update touched firewall: references: add wrappers for the SDN-generated IPSets firewall: parser: do not log errors for disabled rules firewall: ipset: add option to handle dangling references on rename firewall: ipset: add option to handle dangling references on delete firewall: aliases: add option to handle dangling references on rename firewall: aliases: add option to handle dangling references on delete firewall: api: run reference updates as tasks firewall: tests: add tests for object reference updates firewall: references: handle security groups firewall: groups: add option to handle dangling references on rename fix #8176: firewall: groups: handle dangling references on delete src/PVE/API2/Firewall/Aliases.pm | 218 +++++++++--- src/PVE/API2/Firewall/Cluster.pm | 6 +- src/PVE/API2/Firewall/Groups.pm | 199 ++++++----- src/PVE/API2/Firewall/IPSet.pm | 215 +++++++++--- src/PVE/API2/Firewall/Rules.pm | 53 ++- src/PVE/API2/Firewall/VM.pm | 7 +- src/PVE/Firewall.pm | 2 +- src/PVE/Firewall/Helpers.pm | 38 --- src/PVE/Firewall/Makefile | 2 +- src/PVE/Firewall/References.pm | 570 +++++++++++++++++++++++++++++++ src/test/Makefile | 2 +- src/test/test_update_refs.pl | 413 ++++++++++++++++++++++ 12 files changed, 1473 insertions(+), 252 deletions(-) create mode 100644 src/PVE/Firewall/References.pm create mode 100755 src/test/test_update_refs.pl pve-network: Arthur Bied-Charreton (1): apply: add option to handle dangling references on VNet deletion src/PVE/API2/Network/SDN.pm | 38 +++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) qemu-server: Arthur Bied-Charreton (1): api: destroy_vm: add option to handle dangling IPSet references src/PVE/API2/Qemu.pm | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) pve-container: Arthur Bied-Charreton (1): api: destroy_vm: add option to handle dangling IPSet references src/PVE/API2/LXC.pm | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) pve-docs: Arthur Bied-Charreton (2): firewall: remove trailing spaces firewall: document renaming and removing objects pve-firewall.adoc | 76 +++++++++++++++++++++++++++++++++++++++++++---- pvesdn.adoc | 9 ++++-- 2 files changed, 76 insertions(+), 9 deletions(-) pve-manager: Arthur Bied-Charreton (8): ui: firewall: add a common widget for handling dangling references ui: firewall: ipset: add controls for dangling references ui: firewall: aliases: add controls for dangling references ui: sdn: apply: add control for dangling IPSet references ui: guest destroy: use let for non-constant variable bindings ui: guest destroy: add control for dangling IPSet references ui: firewall: show the task for reference updates ui: firewall: groups: add controls for dangling references www/manager6/Makefile | 1 + www/manager6/Utils.js | 6 + www/manager6/dc/SecurityGroups.js | 81 ++++++++++++- www/manager6/grid/FirewallAliases.js | 125 ++++++++++++++++---- www/manager6/grid/FirewallObjectCommon.js | 136 ++++++++++++++++++++++ www/manager6/panel/IPSet.js | 84 +++++++++++-- www/manager6/sdn/StatusView.js | 118 +++++++++++++++---- www/manager6/window/SafeDestroyGuest.js | 69 ++++++++++- 8 files changed, 563 insertions(+), 57 deletions(-) create mode 100644 www/manager6/grid/FirewallObjectCommon.js Summary over all repositories: 25 files changed, 2188 insertions(+), 318 deletions(-) -- Generated by murpp 0.12.1