From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 0AB7C1FF142 for ; Tue, 07 Apr 2026 09:37:05 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6B315F133; Tue, 7 Apr 2026 09:37:35 +0200 (CEST) From: Arthur Bied-Charreton To: pve-devel@lists.proxmox.com Subject: [PATCH firewall/manager 0/5] Allow updating references to firewall objects when editing them Date: Tue, 7 Apr 2026 09:36:53 +0200 Message-ID: <20260407073658.90818-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-SPAM-LEVEL: Spam detection results: 0 AWL -0.096 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Message-ID-Hash: 2OCH74QH6JXYKZN2D3RIL4D2MZMKY56S X-Message-ID-Hash: 2OCH74QH6JXYKZN2D3RIL4D2MZMKY56S X-MailFrom: abied-charreton@jett.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/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