From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 99C281FF0AE for ; Tue, 27 Oct 2026 09:57:25 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 02F1721741; Tue, 27 Oct 2026 09:56:30 +0100 (CET) From: Hannes Laimer To: pve-devel@lists.proxmox.com Subject: [PATCH proxmox-ve-rs v3 09/45] ve-config: sdn: microseg: narrow rules by protocol and ports Date: Tue, 27 Oct 2026 09:54:51 +0100 Message-ID: <20261027085527.702738-10-h.laimer@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261027085527.702738-1-h.laimer@proxmox.com> References: <20261027085527.702738-1-h.laimer@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793091343029 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.336 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) 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 Message-ID-Hash: DZ5KGPHLSRJXMFEWKSUOBNZPXGHHAGG3 X-Message-ID-Hash: DZ5KGPHLSRJXMFEWKSUOBNZPXGHHAGG3 X-MailFrom: h.laimer@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: A rule can be narrowed to a protocol, optionally with ports or an ICMP type, or to a firewall macro standing in for them. It then decides only the traffic it matches. For a pair, the highest priority with a rule for the whole pair gives the verdict. Narrowed rules down to that priority are checked first, in priority order. So a narrowed rule makes an exception to a whole-pair rule of the same priority. The model also reports each rule's place in the evaluation order, so a caller that lists the rules does not have to work the order out again. What a rule narrows to is part of its id, so two rules on the same predicates can coexist. Like the predicates, it cannot be changed after creation. A both-ways rule matches the reverse direction with the ports swapped. It cannot be narrowed to an ICMP type, since the reverse type cannot be derived in general. Signed-off-by: Hannes Laimer --- proxmox-ve-config/src/firewall/fw_macros.rs | 7 + proxmox-ve-config/src/firewall/ports.rs | 65 +++++ proxmox-ve-config/src/sdn/microseg/api.rs | 64 ++++- proxmox-ve-config/src/sdn/microseg/mod.rs | 266 ++++++++++++++++++-- proxmox-ve-config/src/sdn/microseg/types.rs | 116 ++++++++- 5 files changed, 496 insertions(+), 22 deletions(-) diff --git a/proxmox-ve-config/src/firewall/fw_macros.rs b/proxmox-ve-config/src/firewall/fw_macros.rs index 5fa8dab1ca26..e0bdf365635b 100644 --- a/proxmox-ve-config/src/firewall/fw_macros.rs +++ b/proxmox-ve-config/src/firewall/fw_macros.rs @@ -67,3 +67,10 @@ fn macros() -> &'static HashMap { pub fn get_macro(name: &str) -> Option<&'static FwMacro> { macros().get(name) } + +pub fn macro_name(name: &str) -> Option<&'static str> { + macros() + .keys() + .find(|known| known.eq_ignore_ascii_case(name)) + .map(String::as_str) +} diff --git a/proxmox-ve-config/src/firewall/ports.rs b/proxmox-ve-config/src/firewall/ports.rs index f88a442adab4..62d3bacda228 100644 --- a/proxmox-ve-config/src/firewall/ports.rs +++ b/proxmox-ve-config/src/firewall/ports.rs @@ -78,3 +78,68 @@ pub fn parse_named_port(name: &str) -> Result { .find(name) .ok_or_else(|| format_err!("unknown port name {name:?}")) } + +#[derive(Default)] +struct NamedProtocols { + protocols: std::collections::HashMap, +} + +impl NamedProtocols { + fn new() -> Self { + use std::io::BufRead; + + log::trace!("loading /etc/protocols"); + + let mut this = Self::default(); + + let file = match std::fs::File::open("/etc/protocols") { + Ok(file) => file, + Err(_) => return this, + }; + + for line in std::io::BufReader::new(file).lines() { + let line = match line { + Ok(line) => line, + Err(_) => break, + }; + + let line = line.split_once('#').map_or(line.as_str(), |(text, _)| text); + + let mut parts = line.split_ascii_whitespace(); + + let (name, number) = match (parts.next(), parts.next()) { + (Some(name), Some(number)) => (name, number), + _ => continue, + }; + + let number: u8 = match number.parse() { + Ok(number) => number, + Err(_) => continue, + }; + + // the first line naming a protocol wins, as for getprotobyname + for name in std::iter::once(name).chain(parts) { + this.protocols.entry(name.to_string()).or_insert(number); + } + } + + this + } + + fn find(&self, name: &str) -> Option { + self.protocols.get(name).copied() + } +} + +fn named_protocols() -> &'static NamedProtocols { + static NAMED_PROTOCOLS: OnceLock = OnceLock::new(); + + NAMED_PROTOCOLS.get_or_init(NamedProtocols::new) +} + +/// Parse a named protocol with the help of `/etc/protocols`. +pub fn parse_named_protocol(name: &str) -> Result { + named_protocols() + .find(name) + .ok_or_else(|| format_err!("unknown protocol name {name:?}")) +} diff --git a/proxmox-ve-config/src/sdn/microseg/api.rs b/proxmox-ve-config/src/sdn/microseg/api.rs index 08b5ee1f4653..fe125c4d4c1b 100644 --- a/proxmox-ve-config/src/sdn/microseg/api.rs +++ b/proxmox-ve-config/src/sdn/microseg/api.rs @@ -8,6 +8,8 @@ use anyhow::{Error, bail}; use proxmox_section_config::typed::ApiSectionDataEntry; use serde::Deserialize; +use crate::firewall::fw_macros; + use super::types::{ Binding, GroupSection, GuestAssignmentSection, GuestInfo, MicrosegEntry, MicrosegId, MicrosegRunningConfig, RegexAssignmentSection, RuleSection, Tag, TagAssignmentSection, @@ -52,6 +54,16 @@ pub struct RuleCreate { #[serde(deserialize_with = "proxmox_serde::perl::deserialize_bool")] pub allow: bool, #[serde(default)] + pub proto: Option, + #[serde(default)] + pub sport: Option, + #[serde(default)] + pub dport: Option, + #[serde(default, rename = "icmp-type")] + pub icmp_type: Option, + #[serde(default, rename = "macro")] + pub fw_macro: Option, + #[serde(default)] pub comment: Option, } @@ -195,6 +207,7 @@ fn rule_id( src_groups: &[&str], dst_match: PredicateMatch, dst_groups: &[&str], + narrowing: &[Option<&str>], bidirectional: bool, ) -> Result { let mut hash = Fnv1a::new(); @@ -202,6 +215,9 @@ fn rule_id( hash.groups(src_groups); hash.match_kind(dst_match); hash.groups(dst_groups); + for field in narrowing { + hash.text(field.unwrap_or_default()); + } hash.flag(bidirectional); format!("r{:016x}", hash.0).parse() } @@ -289,11 +305,30 @@ fn build_entry( MicrosegCreate::Rule(rule) => { let src_groups = sorted_groups(&rule.src, entries)?; let dst_groups = sorted_groups(&rule.dst, entries)?; + let trimmed = |field: Option| field.map(|value| value.trim().to_string()); + let proto = trimmed(rule.proto); + let sport = trimmed(rule.sport); + let dport = trimmed(rule.dport); + let icmp_type = trimmed(rule.icmp_type); + // the expansion looks a macro up by its exact name, so the table's spelling is + // stored and an unknown name stays as given for validation to refuse + let fw_macro = trimmed(rule.fw_macro).map(|name| match fw_macros::macro_name(&name) { + Some(known) => known.to_string(), + None => name, + }); + let narrowing = [ + proto.as_deref(), + sport.as_deref(), + dport.as_deref(), + icmp_type.as_deref(), + fw_macro.as_deref(), + ]; let id = rule_id( rule.src_match, &src_groups, rule.dst_match, &dst_groups, + &narrowing, rule.bidirectional, )?; MicrosegEntry::Rule(RuleSection { @@ -305,6 +340,11 @@ fn build_entry( bidirectional: rule.bidirectional, prio: rule.prio, allow: rule.allow, + proto, + sport, + dport, + icmp_type, + fw_macro, comment: rule.comment, }) } @@ -540,13 +580,33 @@ fn predicate_str(match_kind: PredicateMatch, groups: &[impl AsRef]) -> Stri format!("{kind}{{{list}}}") } +/// Rendered from the stored fields, since expanding a macro can fail and its name is what a user +/// recognizes. +fn narrowing_str(rule: &RuleSection) -> Option { + if let Some(name) = &rule.fw_macro { + return Some(format!("macro {name}")); + } + let parts = [ + rule.proto.clone(), + rule.sport.as_ref().map(|ports| format!("sport {ports}")), + rule.dport.as_ref().map(|ports| format!("dport {ports}")), + rule.icmp_type.as_ref().map(|kind| format!("type {kind}")), + ]; + let narrowing = parts.into_iter().flatten().collect::>().join(" "); + (!narrowing.is_empty()).then_some(narrowing) +} + fn describe_rule(rule: &RuleSection) -> String { let arrow = if rule.bidirectional { "<->" } else { "->" }; - format!( + let base = format!( "rule {} {arrow} {}", predicate_str(rule.src_match, &rule.src), predicate_str(rule.dst_match, &rule.dst), - ) + ); + match narrowing_str(rule) { + Some(narrowing) => format!("{base} ({narrowing})"), + None => base, + } } /// A human-readable description of an entry for diagnostics, by what a user would recognize rather diff --git a/proxmox-ve-config/src/sdn/microseg/mod.rs b/proxmox-ve-config/src/sdn/microseg/mod.rs index 620f342ad2b0..3aaf299f8169 100644 --- a/proxmox-ve-config/src/sdn/microseg/mod.rs +++ b/proxmox-ve-config/src/sdn/microseg/mod.rs @@ -1,12 +1,12 @@ //! Microseg subsystem of the SDN config. //! //! Microseg is an identity-based stateless firewall. Each managed guest NIC gets the *set* of -//! *groups* of the first *assignment* in admin order that covers it. The render allocates a wire +//! *groups* of the first *assignment* in match order that covers it. The render allocates a wire //! identity for the set of every assignment (see [`identity`]), and the running config carries it //! to every host, so a NIC can only land in a class that already exists. Each host looks the -//! identities up and compiles the policy *rules* into a `(src_id, dst_id) -> allow|deny` map. A -//! rule matches traffic by a `src` and a `dst` *predicate*, each a match kind over a list of -//! groups. +//! identities up and folds the policy *rules* per `(src_id, dst_id)` pair. The fold yields the +//! narrowed rules to check and the verdict for the traffic none of them matches. A rule matches +//! traffic by a `src` and a `dst` *predicate*, each a match kind over a list of groups. pub mod api; pub mod identity; @@ -18,6 +18,7 @@ pub use types::{ MicrosegRunningConfig, RegexAssignmentSection, RuleSection, Tag, TagAssignmentSection, }; +use std::cmp::Ordering; use std::collections::{BTreeMap, BTreeSet, HashMap}; use anyhow::{Context, Error, bail}; @@ -25,6 +26,8 @@ use identity::UNTAGGED_GROUP; use regex::Regex; use serde::Serialize; +use crate::firewall::types::rule_match::{IcmpType, Icmpv6Type, Ports, Protocol, Sctp, Tcp, Udp}; + /// Validate a microseg config before anything renders from it, so resolving group names and /// expanding rules cannot fail further down. pub fn validate(entries: &BTreeMap) -> Result<(), Error> { @@ -86,6 +89,14 @@ fn validate_entry( Ok(()) } +fn has_icmp_type(protocol: &Protocol) -> bool { + match protocol { + Protocol::Icmp(icmp) => !matches!(icmp.ty(), None | Some(IcmpType::Any)), + Protocol::Icmpv6(icmp) => !matches!(icmp.ty(), None | Some(Icmpv6Type::Any)), + _ => false, + } +} + fn validate_rule( entries: &BTreeMap, subject: &str, @@ -133,6 +144,18 @@ fn validate_rule( for group in rule.src.iter().chain(&rule.dst) { known_group(entries, group.as_ref()).with_context(|| subject.to_string())?; } + let protocols = match rule.protocols() { + Ok(protocols) => protocols, + Err(err) => bail!("{subject}: {err}"), + }; + // a macro can expand to an entry with an ICMP type, so the check reads the expanded + // matches and not the rule's own field + if rule.bidirectional && protocols.iter().any(has_icmp_type) { + bail!( + "{subject} is bidirectional and names an ICMP type, the mirror does not \ + derive a reverse type" + ); + } Ok(()) } @@ -405,6 +428,31 @@ pub fn render( }) } +fn evaluation_order(a: &ResolvedRule, b: &ResolvedRule) -> Ordering { + b.prio + .cmp(&a.prio) + .then(b.is_narrowed().cmp(&a.is_narrowed())) + .then(a.allow.cmp(&b.allow)) + // the id keeps the order stable, and the orientation after it, since both orientations of + // a bidirectional rule carry the same id + .then(a.id.cmp(&b.id)) + .then(a.reverse.cmp(&b.reverse)) +} + +/// Each rule's place in the evaluation order, by rule id. The first has the lowest number. +pub fn rule_order( + entries: &BTreeMap, +) -> Result, Error> { + let mut rules = resolved_rules(entries)?; + rules.sort_by(evaluation_order); + let mut order = HashMap::new(); + for rule in rules { + let place = order.len(); + order.entry(rule.id).or_insert(place); + } + Ok(order) +} + /// Whether a guest renamed from `old` to `new` enters or leaves the names a regex assignment /// matches, decided by the same expressions that resolve the NICs. pub fn name_match_changed( @@ -439,10 +487,35 @@ pub fn assignment_order(entries: &BTreeMap) -> HashMap, + /// Set on the mirrored orientation of a bidirectional rule. + reverse: bool, +} + +impl ResolvedRule { + fn is_narrowed(&self) -> bool { + !self.protocols.is_empty() + } +} + +fn reversed_protocol(protocol: &Protocol) -> Protocol { + let swapped = |ports: &Ports| Ports::new(ports.dport().cloned(), ports.sport().cloned()); + match protocol { + Protocol::Dccp(ports) => Protocol::Dccp(swapped(ports)), + Protocol::Sctp(sctp) => Protocol::Sctp(Sctp::new(swapped(sctp.ports()))), + Protocol::Tcp(tcp) => Protocol::Tcp(Tcp::new(swapped(tcp.ports()))), + Protocol::Udp(udp) => Protocol::Udp(Udp::new(swapped(udp.ports()))), + Protocol::UdpLite(ports) => Protocol::UdpLite(swapped(ports)), + Protocol::Icmp(_) | Protocol::Icmpv6(_) | Protocol::Named(_) | Protocol::Numeric(_) => { + protocol.clone() + } + } } pub fn resolved_rules( @@ -452,16 +525,22 @@ pub fn resolved_rules( for rule in rule_entries(entries) { let context = || format!("rule '{}'", rule.id); let forward = ResolvedRule { + id: rule.id.clone(), src: predicate(entries, rule.src_match, &rule.src).with_context(context)?, dst: predicate(entries, rule.dst_match, &rule.dst).with_context(context)?, prio: rule.prio, allow: rule.allow, + protocols: rule.protocols().with_context(context)?, + reverse: false, }; let mirrored = rule.bidirectional.then(|| ResolvedRule { + id: forward.id.clone(), src: forward.dst.clone(), dst: forward.src.clone(), prio: forward.prio, allow: forward.allow, + protocols: forward.protocols.iter().map(reversed_protocol).collect(), + reverse: true, }); resolved.push(forward); resolved.extend(mirrored); @@ -469,26 +548,39 @@ pub fn resolved_rules( Ok(resolved) } -/// Fold the rules that fire on one `(src_class, dst_class)` pair into its verdict. The highest -/// priority wins, and a single deny among the rules tied there overrides the allows. Without a -/// firing rule the pair denies. -pub fn fold<'a>(firing: impl IntoIterator) -> bool { - let mut best: Option<(u16, bool)> = None; - for rule in firing { - best = Some(match best { - Some((prio, _)) if rule.prio > prio => (rule.prio, rule.allow), - Some((prio, allow)) if rule.prio == prio => (prio, allow && rule.allow), - Some(current) => current, - None => (rule.prio, rule.allow), - }); +#[derive(Debug)] +pub struct Folded<'a> { + /// Narrowed rules in evaluation order. The first one whose protocol match fires decides. + pub checks: Vec<&'a ResolvedRule>, + pub allow: bool, +} + +/// Fold the rules that fire on one `(src_class, dst_class)` pair into its verdict and checks. The +/// verdict comes from the highest priority with a rule that decides the whole pair, and a single +/// deny among those rules wins. Without such a rule the pair denies. The checks are the narrowed +/// rules from the top priority down to that one, denies ahead of allows within a priority. +pub fn fold<'a>(firing: impl IntoIterator) -> Folded<'a> { + let mut firing: Vec<&ResolvedRule> = firing.into_iter().collect(); + firing.sort_by(|a, b| evaluation_order(a, b)); + + let mut checks = Vec::new(); + let mut allow = false; + for level in firing.chunk_by(|a, b| a.prio == b.prio) { + checks.extend(level.iter().filter(|rule| rule.is_narrowed())); + let mut deciding = level.iter().filter(|rule| !rule.is_narrowed()).peekable(); + if deciding.peek().is_some() { + allow = deciding.all(|rule| rule.allow); + break; + } } - best.is_some_and(|(_, allow)| allow) + Folded { checks, allow } } #[cfg(test)] mod tests { use super::*; + use crate::firewall::types::rule_match::RuleOptions; pub(super) fn firing_rules<'a>( rules: &'a [ResolvedRule], @@ -679,7 +771,9 @@ mod tests { } fn verdict(rules: &[ResolvedRule], src: &identity::GroupSet, dst: &identity::GroupSet) -> bool { - fold(firing_rules(rules, src, dst)) + let folded = fold(firing_rules(rules, src, dst)); + assert!(folded.checks.is_empty(), "no narrowed rules expected"); + folded.allow } fn rule_full( @@ -702,6 +796,11 @@ mod tests { bidirectional: false, prio, allow, + proto: None, + sport: None, + dport: None, + icmp_type: None, + fw_macro: None, comment: None, }), ) @@ -742,4 +841,135 @@ mod tests { // from an interface in web alone only the allow fires assert!(verdict(&tied_rules, &set(&["web"]), &set(&["db"]))); } + + fn narrowed( + name: &str, + src: &[&str], + dst: &[&str], + prio: u16, + allow: bool, + proto: Option<&str>, + dport: Option<&str>, + ) -> (String, MicrosegEntry) { + let entry = rule_full( + name, + src, + PredicateMatch::Any, + dst, + PredicateMatch::Any, + prio, + allow, + ); + with_rule(entry, |rule| { + rule.proto = proto.map(str::to_string); + rule.dport = dport.map(str::to_string); + }) + } + + #[test] + fn cell_orders_checks_and_decides_the_rest() { + let entries: BTreeMap = BTreeMap::from([ + group("web"), + group("db"), + rule_full( + "whole", + &["web"], + PredicateMatch::Any, + &["db"], + PredicateMatch::Any, + 0, + true, + ), + narrowed("ssh", &["web"], &["db"], 10, false, Some("tcp"), Some("22")), + narrowed("dns", &["web"], &["db"], 10, true, Some("udp"), Some("53")), + narrowed("http", &["web"], &["db"], 20, true, Some("tcp"), Some("80")), + ]); + let rules = resolved(&entries); + let folded = fold(firing_rules(&rules, &set(&["web"]), &set(&["db"]))); + let order: Vec<&str> = folded.checks.iter().map(|r| r.id.as_ref()).collect(); + assert_eq!( + order, + ["http", "ssh", "dns"], + "prio first, then deny before allow" + ); + assert!(folded.allow, "the whole-pair allow decides the rest"); + + // A whole-pair deny at prio 15 ends the walk before the narrowed rules below it. + let mut cut = entries.clone(); + cut.extend([rule_full( + "cut", + &["web"], + PredicateMatch::Any, + &["db"], + PredicateMatch::Any, + 15, + false, + )]); + let rules = resolved(&cut); + let folded = fold(firing_rules(&rules, &set(&["web"]), &set(&["db"]))); + let order: Vec<&str> = folded.checks.iter().map(|r| r.id.as_ref()).collect(); + assert_eq!(order, ["http"]); + assert!(!folded.allow); + + // Only narrowed rules, so the rest is denied. + let mut only = entries.clone(); + only.remove("whole"); + let rules = resolved(&only); + let folded = fold(firing_rules(&rules, &set(&["web"]), &set(&["db"]))); + assert_eq!(folded.checks.len(), 3); + assert!(!folded.allow); + } + + #[test] + fn bidirectional_narrowing_swaps_the_ports() { + let tcp = |sport: Option<&str>, dport: Option<&str>| { + Protocol::from_options(&RuleOptions { + proto: Some("tcp".to_string()), + sport: sport.map(str::to_string), + dport: dport.map(str::to_string), + ..Default::default() + }) + .expect("valid options") + .expect("a protocol") + }; + let check = |rules: &[ResolvedRule], src: &str, dst: &str| { + let folded = fold(firing_rules(rules, &set(&[src]), &set(&[dst]))); + assert_eq!(folded.checks.len(), 1, "one check per direction"); + folded.checks[0].protocols.clone() + }; + + let explicit = BTreeMap::from([ + group("web"), + group("db"), + bidirectional(narrowed( + "bsql", + &["web"], + &["db"], + 0, + true, + Some("tcp"), + Some("5432"), + )), + ]); + validate(&explicit).expect("valid"); + let rules = resolved(&explicit); + assert_eq!(check(&rules, "web", "db"), [tcp(None, Some("5432"))]); + assert_eq!( + check(&rules, "db", "web"), + [tcp(Some("5432"), None)], + "the reverse direction matches the port as a source port" + ); + + let from_macro = BTreeMap::from([ + group("web"), + group("db"), + bidirectional(with_rule(rule("bhttps", &["web"], &["db"], true), |rule| { + rule.fw_macro = Some("HTTPS".into()) + })), + ]); + validate(&from_macro).expect("valid"); + let rules = resolved(&from_macro); + assert_eq!(check(&rules, "web", "db"), [tcp(None, Some("443"))]); + assert_eq!(check(&rules, "db", "web"), [tcp(Some("443"), None)]); + } } diff --git a/proxmox-ve-config/src/sdn/microseg/types.rs b/proxmox-ve-config/src/sdn/microseg/types.rs index 18367490e076..ee0a03beec51 100644 --- a/proxmox-ve-config/src/sdn/microseg/types.rs +++ b/proxmox-ve-config/src/sdn/microseg/types.rs @@ -2,12 +2,16 @@ use std::collections::BTreeMap; -use anyhow::{Error, bail}; +use anyhow::{Error, anyhow, bail}; use const_format::concatcp; use serde::{Deserialize, Serialize}; use proxmox_schema::{ApiStringFormat, ApiType, api, api_string_type, const_regex}; +use crate::firewall::fw_macros; +use crate::firewall::ports::parse_named_protocol; +use crate::firewall::types::rule_match::{Protocol, RuleOptions}; + use super::PredicateMatch; use super::identity; use super::identity::UNTAGGED_GROUP; @@ -119,6 +123,36 @@ pub struct GroupSection { minimum: 0, maximum: 65535, }, + proto: { + type: String, + optional: true, + max_length: 32, + description: "IP protocol, a name from /etc/protocols or a number.", + }, + sport: { + type: String, + optional: true, + max_length: 128, + description: "Source port list, only with a port-carrying protocol.", + }, + dport: { + type: String, + optional: true, + max_length: 128, + description: "Destination port list, only with a port-carrying protocol.", + }, + "icmp-type": { + type: String, + optional: true, + max_length: 64, + description: "ICMP type, only with an ICMP protocol.", + }, + "macro": { + type: String, + optional: true, + max_length: 128, + description: "Firewall macro standing in for the protocol and ports.", + }, comment: { type: String, optional: true, @@ -131,6 +165,9 @@ pub struct GroupSection { /// A policy rule. It fires on traffic from an interface whose group set matches `(src_match, src)` /// to one whose set matches `(dst_match, dst)`, and decides it with `allow`. A rule ahead of it in /// the evaluation order decides first. Traffic no rule decides is denied. +/// +/// A rule may be narrowed to a protocol with ports or an ICMP type, or to a firewall macro that +/// stands in for them. pub struct RuleSection { pub(crate) id: MicrosegId, /// Groups the source predicate lists. @@ -145,7 +182,8 @@ pub struct RuleSection { /// How the destination predicate matches its groups. #[serde(default, rename = "dst-match")] pub(crate) dst_match: PredicateMatch, - /// Whether the rule also decides the reverse direction, with the predicates swapped. + /// Whether the rule also decides the reverse direction, with the predicates and the ports + /// swapped. #[serde( default, deserialize_with = "proxmox_serde::perl::deserialize_bool", @@ -159,9 +197,83 @@ pub struct RuleSection { #[serde(deserialize_with = "proxmox_serde::perl::deserialize_bool")] pub(crate) allow: bool, #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) proto: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) sport: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) dport: Option, + #[serde(default, rename = "icmp-type", skip_serializing_if = "Option::is_none")] + pub(crate) icmp_type: Option, + #[serde(default, rename = "macro", skip_serializing_if = "Option::is_none")] + pub(crate) fw_macro: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] pub(crate) comment: Option, } +impl RuleSection { + /// The protocol matches narrowing this rule, empty for a rule that decides the whole pair. + /// Ports are only accepted with a port-carrying protocol and an ICMP type only with an ICMP + /// protocol, since either would otherwise be silently ignored. + pub(super) fn protocols(&self) -> Result, Error> { + let explicit = [&self.proto, &self.sport, &self.dport, &self.icmp_type] + .iter() + .any(|field| field.is_some()); + if let Some(name) = &self.fw_macro { + if explicit { + bail!("a macro cannot be combined with a protocol, ports or an ICMP type"); + } + return match fw_macros::get_macro(name) { + Some(fw_macro) => Ok(fw_macro.code.clone()), + None => bail!("unknown macro '{name}'"), + }; + } + let Some(proto) = &self.proto else { + if explicit { + bail!("ports and an ICMP type need a protocol"); + } + return Ok(Vec::new()); + }; + let proto = proto.trim(); + let options = RuleOptions { + proto: (!proto.is_empty()).then(|| proto.to_string()), + sport: self.sport.clone(), + dport: self.dport.clone(), + icmp_type: self.icmp_type.clone(), + ..Default::default() + }; + let mut protocol = Protocol::from_options(&options)? + .ok_or_else(|| anyhow!("empty protocol specification"))?; + // the data plane matches the number, so an unknown name fails validation rather than the + // apply + if let Protocol::Named(name) = &protocol { + let number = parse_named_protocol(name)?.to_string(); + let options = RuleOptions { + proto: Some(number), + ..options + }; + protocol = Protocol::from_options(&options)? + .ok_or_else(|| anyhow!("empty protocol specification"))?; + } + let ports = self.sport.is_some() || self.dport.is_some(); + let carries_ports = matches!( + protocol, + Protocol::Tcp(_) + | Protocol::Udp(_) + | Protocol::Sctp(_) + | Protocol::Dccp(_) + | Protocol::UdpLite(_) + ); + if ports && !carries_ports { + bail!("protocol '{proto}' carries no ports"); + } + let icmp = matches!(protocol, Protocol::Icmp(_) | Protocol::Icmpv6(_)); + if self.icmp_type.is_some() && !icmp { + bail!("an ICMP type needs an ICMP protocol, not '{proto}'"); + } + Ok(vec![protocol]) + } +} + fn is_false(value: &bool) -> bool { !*value } -- 2.47.3