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 F073A1FF14F for ; Fri, 08 May 2026 18:33:41 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B1CDE1EF60; Fri, 8 May 2026 18:32:33 +0200 (CEST) From: Stefan Hanreich To: pve-devel@lists.proxmox.com Subject: [PATCH proxmox-ve-rs v6 02/24] prefix lists: implement validation for prefix lists Date: Fri, 8 May 2026 18:31:11 +0200 Message-ID: <20260508163134.481912-3-s.hanreich@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260508163134.481912-1-s.hanreich@proxmox.com> References: <20260508163134.481912-1-s.hanreich@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1778257794403 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.635 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 Message-ID-Hash: QP7Y6GX3X7OATTVYZDCV2G2KVUN3D5TF X-Message-ID-Hash: QP7Y6GX3X7OATTVYZDCV2G2KVUN3D5TF X-MailFrom: s.hanreich@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: Implement validation for prefix list entries, that enforces the invariants that are required for entries to be valid. Since FRR rejects invalid prefix lists and refuses to start, it is important that invalid prefix lists are caught early by the stack to prevent potentially crashing the FRR daemon. Signed-off-by: Stefan Hanreich --- proxmox-ve-config/src/sdn/prefix_list.rs | 246 +++++++++++++++++++++++ 1 file changed, 246 insertions(+) diff --git a/proxmox-ve-config/src/sdn/prefix_list.rs b/proxmox-ve-config/src/sdn/prefix_list.rs index 0efe81d..672ee88 100644 --- a/proxmox-ve-config/src/sdn/prefix_list.rs +++ b/proxmox-ve-config/src/sdn/prefix_list.rs @@ -31,6 +31,8 @@ use proxmox_schema::{ UpdaterType, }; +use crate::common::valid::Validatable; + pub const PREFIX_LIST_ID_REGEX_STR: &str = r"(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-_]){0,30}(?:[a-zA-Z0-9]){0,1})"; @@ -82,7 +84,26 @@ pub struct PrefixListSection { pub(crate) entries: Vec>, } +impl Validatable for PrefixListSection { + type Error = anyhow::Error; + + fn validate(&self) -> Result<(), Self::Error> { + for entry in &self.entries { + entry.validate()? + } + + Ok(()) + } +} + impl PrefixListSection { + pub fn new(id: PrefixListId) -> Self { + Self { + id, + entries: Vec::new(), + } + } + /// Return the ID of the Prefix List. pub fn id(&self) -> &PrefixListId { &self.id @@ -202,6 +223,8 @@ impl PrefixListSection { anyhow::bail!("entry with sequence number {} already exists", entry.seq); } + entry.validate()?; + self.entries.push(entry.into()); Ok(()) } @@ -300,6 +323,48 @@ impl PrefixListEntry { } } +impl Validatable for PrefixListEntry { + type Error = anyhow::Error; + + fn validate(&self) -> Result<(), Self::Error> { + // Ensure that: + // prefixmask <= ge <= le + + let (max_mask, current_mask) = match self.prefix { + Cidr::Ipv4(ipv4_cidr) => (32, ipv4_cidr.mask() as u32), + Cidr::Ipv6(ipv6_cidr) => (128, ipv6_cidr.mask() as u32), + }; + + if let Some(le) = self.le { + if le > max_mask { + anyhow::bail!("Prefix <= must not be greater than {max_mask}"); + } + + if current_mask > le { + anyhow::bail!("Prefix <= must not be greater than {current_mask}"); + } + + if let Some(ge) = self.ge { + if ge > le { + anyhow::bail!("Prefix >= must not be greater than Prefix <= ({ge})"); + } + } + } + + if let Some(ge) = self.ge { + if ge > max_mask { + anyhow::bail!("Prefix >= must not be greater than {max_mask}"); + } + + if current_mask > ge { + anyhow::bail!("Prefix >= must be greater than {current_mask}"); + } + } + + Ok(()) + } +} + #[api( "id-property": "id", "id-schema": { @@ -317,6 +382,15 @@ pub enum PrefixList { PrefixList(PrefixListSection), } +impl Validatable for PrefixList { + type Error = anyhow::Error; + + fn validate(&self) -> Result<(), Self::Error> { + let PrefixList::PrefixList(prefix_list_section) = self; + prefix_list_section.validate() + } +} + #[cfg(feature = "frr")] pub mod frr { use super::*; @@ -492,4 +566,176 @@ prefix-list: somelist PrefixList::parse_section_config("prefix-lists.cfg", section_config)?; Ok(()) } + + #[test] + fn test_prefix_list_seq_nr() -> Result<(), anyhow::Error> { + let mut prefix_list = PrefixListSection::new( + PrefixListId::from_string("test".to_string()).expect("valid prefix list id"), + ); + + assert_eq!(prefix_list.next_seq_number(), 5); + + prefix_list + .try_insert_entry(PrefixListEntry { + action: PrefixListAction::Permit, + prefix: Cidr::new_v4([192, 0, 2, 0], 24).expect("valid cidr"), + le: None, + ge: None, + seq: 100, + }) + .expect("valid entry"); + + assert_eq!(prefix_list.next_seq_number(), 105); + + prefix_list.remove_entry(100).expect("could be removed"); + assert_eq!(prefix_list.next_seq_number(), 5); + + Ok(()) + } + + #[test] + fn test_prefix_list_entry_update() -> Result<(), anyhow::Error> { + let mut prefix_list = PrefixListSection::new( + PrefixListId::from_string("test".to_string()).expect("valid prefix list id"), + ); + + assert_eq!(prefix_list.next_seq_number(), 5); + + prefix_list + .try_insert_entry(PrefixListEntry { + action: PrefixListAction::Permit, + prefix: Cidr::new_v4([192, 0, 2, 0], 24).expect("valid cidr"), + le: None, + ge: None, + seq: 100, + }) + .expect("valid entry"); + + prefix_list + .try_insert_entry(PrefixListEntry { + action: PrefixListAction::Permit, + prefix: Cidr::new_v4([192, 0, 2, 0], 24).expect("valid cidr"), + le: None, + ge: None, + seq: 200, + }) + .expect("valid entry"); + + prefix_list + .try_update_entry( + 100, + api::PrefixListEntryUpdater { + action: None, + prefix: None, + le: None, + ge: None, + seq: Some(200), + }, + Vec::new(), + ) + .expect_err("seq nr already exists"); + + prefix_list + .try_update_entry( + 150, + api::PrefixListEntryUpdater { + action: None, + prefix: None, + le: None, + ge: None, + seq: Some(100), + }, + Vec::new(), + ) + .expect_err("old seq nr doesn't exist"); + + prefix_list + .try_update_entry( + 100, + api::PrefixListEntryUpdater { + action: None, + prefix: None, + le: None, + ge: None, + seq: Some(10), + }, + Vec::new(), + ) + .expect("changing sequence number from 100 to 10 works"); + + prefix_list + .entry(10) + .expect("entry has been successfully updated"); + + Ok(()) + } + + #[test] + fn test_invalid_prefix_list_entry() -> Result<(), anyhow::Error> { + let mut prefix_list = PrefixListSection::new( + PrefixListId::from_string("test".to_string()).expect("valid prefix list id"), + ); + + prefix_list + .try_insert_entry(PrefixListEntry { + action: PrefixListAction::Permit, + prefix: Cidr::new_v4([192, 0, 2, 0], 24).expect("valid cidr"), + le: Some(23), + ge: None, + seq: 100, + }) + .expect_err("le is larger than prefix size"); + + prefix_list + .try_insert_entry(PrefixListEntry { + action: PrefixListAction::Permit, + prefix: Cidr::new_v4([192, 0, 2, 0], 24).expect("valid cidr"), + le: Some(23), + ge: None, + seq: 100, + }) + .expect_err("le is larger than prefix size"); + + prefix_list + .try_insert_entry(PrefixListEntry { + action: PrefixListAction::Permit, + prefix: Cidr::new_v4([192, 0, 2, 0], 24).expect("valid cidr"), + le: None, + ge: Some(23), + seq: 100, + }) + .expect_err("ge is larger than prefix size"); + + prefix_list + .try_insert_entry(PrefixListEntry { + action: PrefixListAction::Permit, + prefix: Cidr::new_v4([192, 0, 2, 0], 24).expect("valid cidr"), + le: Some(25), + ge: Some(27), + seq: 100, + }) + .expect_err("le is larger than ge"); + + prefix_list + .try_insert_entry(PrefixListEntry { + action: PrefixListAction::Permit, + prefix: Cidr::new_v4([192, 0, 2, 0], 24).expect("valid cidr"), + le: None, + ge: None, + seq: 100, + }) + .expect("valid entry"); + + prefix_list + .try_insert_entry(PrefixListEntry { + action: PrefixListAction::Permit, + prefix: Cidr::new_v4([192, 0, 2, 0], 24).expect("valid cidr"), + le: None, + ge: None, + seq: 100, + }) + .expect_err("entry with seq already exists"); + + Ok(()) + } } -- 2.47.3