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 8A1D01FF15C for ; Wed, 30 Oct 2024 15:12:53 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 57B771ACFD; Wed, 30 Oct 2024 15:12:54 +0100 (CET) From: Gabriel Goller To: pve-devel@lists.proxmox.com Date: Wed, 30 Oct 2024 15:12:48 +0100 Message-Id: <20241030141248.284898-2-g.goller@proxmox.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20241030141248.284898-1-g.goller@proxmox.com> References: <20241030141248.284898-1-g.goller@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.041 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH proxmox-firewall 2/2] firewall: delete unused sets 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" Delete unused sets in nftables. We check if there are existing sets which will not be recreated on the next iteration and delete them. Signed-off-by: Gabriel Goller --- proxmox-firewall/src/bin/proxmox-firewall.rs | 60 +++++++++++++++++++- proxmox-nftables/src/client.rs | 9 ++- proxmox-nftables/src/types.rs | 33 +++++++++-- 3 files changed, 95 insertions(+), 7 deletions(-) diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs index 4732e51984b5..5871da677067 100644 --- a/proxmox-firewall/src/bin/proxmox-firewall.rs +++ b/proxmox-firewall/src/bin/proxmox-firewall.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::io::Write; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; @@ -7,6 +8,9 @@ use anyhow::{Context, Error}; use proxmox_firewall::config::{FirewallConfig, PveFirewallConfigLoader, PveNftConfigLoader}; use proxmox_firewall::firewall::Firewall; +use proxmox_nftables::command::{Add, Commands, Delete, List, ListOutput}; +use proxmox_nftables::types::SetName; +use proxmox_nftables::Command; use proxmox_nftables::{client::NftError, NftClient}; const RULE_BASE: &str = include_str!("../../resources/proxmox-firewall.nft"); @@ -26,6 +30,55 @@ fn remove_firewall() -> Result<(), std::io::Error> { Ok(()) } +/// Fetch all the sets that currently exist, then get all the sets that will be added in the next +/// interation. Make the difference between them and remove the unused ones. +fn delete_unused_sets(commands: &Commands) -> Result { + let mut delete_commands = Commands::new(vec![]); + + // get sets that will be added + let new_sets = commands + .iter() + .filter_map(|w| match w { + Command::Add(Add::Set(x)) => Some(x.config.name()), + _ => None, + }) + .collect::>(); + + // get existing sets + let list_commands = Commands::new(vec![List::sets()]); + let existing_sets = NftClient::run_json_commands(&list_commands); + match existing_sets { + Ok(Some(existing_sets)) => { + let existing_sets = existing_sets + .nftables + .iter() + .filter_map(|item| { + if let ListOutput::Set(e) = item { + Some(e.name()) + } else { + None + } + }) + .collect::>(); + let to_delete = existing_sets.difference(&new_sets); + + log::debug!("existing sets: {:#?}", existing_sets); + log::debug!("new sets: {:#?}", new_sets); + log::debug!("these sets are going to be deleted: {:#?}", to_delete); + + for set in to_delete.into_iter().cloned().cloned() { + delete_commands.push(Command::Delete(Delete::Set(set))); + } + Ok(delete_commands) + } + Ok(None) => { + log::debug!("no sets exist"); + Ok(delete_commands) + } + Err(err) => Err(err), + } +} + fn handle_firewall() -> Result<(), Error> { let config = FirewallConfig::new(&PveFirewallConfigLoader::new(), &PveNftConfigLoader::new())?; @@ -38,13 +91,18 @@ fn handle_firewall() -> Result<(), Error> { log::info!("creating the firewall skeleton"); NftClient::run_commands(RULE_BASE)?; - let commands = firewall.full_host_fw()?; + let mut commands = firewall.full_host_fw()?; log::info!("Running proxmox-firewall commands"); for (idx, c) in commands.iter().enumerate() { log::debug!("cmd #{idx} {}", serde_json::to_string(&c)?); } + match delete_unused_sets(&commands) { + Ok(mut delete_commands) => commands.append(&mut delete_commands), + Err(err) => log::error!("error deleting unused sets {err:?}"), + } + let response = NftClient::run_json_commands(&commands)?; if let Some(output) = response { diff --git a/proxmox-nftables/src/client.rs b/proxmox-nftables/src/client.rs index eaa3dd2136ee..0fd950cdc1e1 100644 --- a/proxmox-nftables/src/client.rs +++ b/proxmox-nftables/src/client.rs @@ -52,8 +52,13 @@ impl NftClient { let output = Self::execute_nft_commands(true, &json)?; if !output.is_empty() { - let parsed_output: Option = serde_json::from_str(&output).ok(); - return Ok(parsed_output); + return match serde_json::from_str::(&output) { + Ok(output) => Ok(Some(output)), + Err(err) => { + log::error!("Error deserializing output: {err:#?}"); + Ok(None) + } + }; } Ok(None) diff --git a/proxmox-nftables/src/types.rs b/proxmox-nftables/src/types.rs index 320c757c7cba..258bfc899734 100644 --- a/proxmox-nftables/src/types.rs +++ b/proxmox-nftables/src/types.rs @@ -19,7 +19,7 @@ use proxmox_ve_config::guest::types::Vmid; #[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] pub struct Handle(i32); -#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize, Hash)] #[serde(rename_all = "lowercase")] pub enum TableFamily { Ip, @@ -210,7 +210,7 @@ impl TableName { } } -#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct TablePart { family: TableFamily, table: String, @@ -564,7 +564,7 @@ impl DerefMut for AddMap { #[derive(Clone, Debug, Deserialize, Serialize)] pub struct AddSet { #[serde(flatten)] - config: SetConfig, + pub config: SetConfig, #[serde(default, skip_serializing_if = "Vec::is_empty")] elem: NfVec, @@ -602,7 +602,7 @@ impl AddSet { } } -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, Hash)] pub struct SetName { #[serde(flatten)] table: TablePart, @@ -909,9 +909,34 @@ impl ListChain { } #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(rename_all = "kebab-case")] pub struct ListSet { #[serde(flatten)] name: SetName, + + #[serde(rename = "type", default, skip_serializing_if = "Vec::is_empty")] + ty: NfVec, + + #[serde(skip_serializing_if = "Option::is_none")] + policy: Option, + + #[serde(skip_serializing_if = "Vec::is_empty", default)] + flags: Vec, + + #[serde(skip_serializing_if = "Option::is_none")] + handle: Option, + + #[serde(skip_serializing_if = "Option::is_none")] + elem: Option>, + + #[serde(skip_serializing_if = "Option::is_none")] + timeout: Option, + + #[serde(skip_serializing_if = "Option::is_none")] + gc_interval: Option, + + #[serde(skip_serializing_if = "Option::is_none")] + size: Option, } impl ListSet { -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel