From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id B21F61FF191 for ; Tue, 7 Oct 2025 15:11:51 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7CCA018FFC; Tue, 7 Oct 2025 15:11:54 +0200 (CEST) From: Thomas Lamprecht To: pve-devel@lists.proxmox.com Date: Tue, 7 Oct 2025 15:09:54 +0200 Message-ID: <20251007131113.2785229-1-t.lamprecht@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1759842650957 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.027 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 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. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [cluster.pm, proxmox.com, firewall.pm] Subject: [pve-devel] [PATCH firewall] cluster config: soft-deprecate auto-disable timestamp support X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" The firewall got some crude auto-disable support added in commit 72d055f ("firewall autodisable"), but integration into our UI never made it from the mailing list into the repo, as the last reviewed version [0] was never followed up on. As it's unused by us, undocumented in general and not supported in the newer rust based NFT alternative implementation of our firewall it's probably best to soft-deprecate it now and remove it completely with the next major release. The reason for doing this now is that Hannes is working on firewall integration in PDM, and that uses pve-api-types which is mostly generated from the API schema, and the integer for 'enabled' made him question this. If we ever want an auto-disable we probably should encode it as dedicated variable and also rethink the checks/process needed for triggering it, because the original idea might have been susceptible to temporary false positives due to connection tracking allowing the connection that would not be allowed once the conntrack info expired. And it might be better to spawn a worker task handling the enable + wait + disable-if-no-ack cycle such that it's actually logged. Anyway, as of now switch the 'enabled' property to a boolean type, but override the input schema to still accept a integer for backward compat but warn if we parse anything other than a integer boolean; if this causes issue I think it would be even fine to change the schema for both, input and ouput schemas. [0]: https://lists.proxmox.com/pipermail/pve-devel/2015-July/015863.html Reported-by: Hannes Laimer Signed-off-by: Thomas Lamprecht --- @Hannes: please test if the rust serialization code works with the input still being marked as integer here, as said, otherwise we can change it to boolean and see if anybody notices (which for this I'd risk) or maybe try using oneOf. src/PVE/API2/Firewall/Cluster.pm | 10 +++++++++- src/PVE/Firewall.pm | 12 ++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/PVE/API2/Firewall/Cluster.pm b/src/PVE/API2/Firewall/Cluster.pm index 51d0e43..b8d1d23 100644 --- a/src/PVE/API2/Firewall/Cluster.pm +++ b/src/PVE/API2/Firewall/Cluster.pm @@ -76,7 +76,7 @@ my $add_option_properties = sub { my ($properties) = @_; foreach my $k (keys %$option_properties) { - $properties->{$k} = $option_properties->{$k}; + $properties->{$k} = $option_properties->{$k} if !defined($properties->{$k}); } return $properties; @@ -119,6 +119,14 @@ __PACKAGE__->register_method({ parameters => { additionalProperties => 0, properties => &$add_option_properties({ + # TODO: remove override for enable with PVE 10. This was only done for backward compat, + # but the timestamp is nowhere used and our UI only sets booleans anyway. + enable => { + description => "Enable or disable the firewall cluster wide.", + type => 'integer', + minimum => 0, + optional => 1, + }, delete => { type => 'string', format => 'pve-configid-list', diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm index ec9c9ae..999d206 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -1283,8 +1283,7 @@ sub copy_list_with_digest { our $cluster_option_properties = { enable => { description => "Enable or disable the firewall cluster wide.", - type => 'integer', - minimum => 0, + type => 'boolean', default => 0, optional => 1, }, @@ -3321,9 +3320,14 @@ sub parse_clusterfw_option { if ($line =~ m/^(enable):\s*(\d+)\s*$/i) { $opt = lc($1); $value = int($2); - if (($value > 1) && ((time() - $value) > 60)) { - $value = 0; + if ($value > 1) { + # TODO: remove this with PVE 10 + warn "deprecated auto-disable timestamp format used for 'enable' cluster firewall" + . " config property; support for this will be removed in PVE 10\n"; + + $value = 0 if (time() - $value) > 60; } + $value = $value ? 1 : 0; } elsif ($line =~ m/^(ebtables):\s*(0|1)\s*$/i) { $opt = lc($1); $value = int($2); -- 2.47.3 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel