[PATCH proxmox-ve-rs v3 14/45] microseg: add a rule option that matches non-first fragments

[Hannes Laimer <h.laimer@proxmox.com>]

A narrowed rule matches only frames with a transport header. Only the
first fragment of a datagram carries one. So a pair allowed only
through narrowed rules dropped every later fragment.

A rule can now be narrowed to non-first fragments, of one protocol or
of every protocol. Like any narrowed rule it decides only what it
matches. The only other narrowing it accepts is a protocol, since a
fragment carries no ports and no ICMP type.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 proxmox-microseg/src/ebpf.rs                | 12 +++++
 proxmox-ve-config/src/sdn/microseg/api.rs   |  7 +++
 proxmox-ve-config/src/sdn/microseg/mod.rs   | 50 ++++++++++++++++-----
 proxmox-ve-config/src/sdn/microseg/state.rs |  4 ++
 proxmox-ve-config/src/sdn/microseg/types.rs | 27 +++++++++--
 5 files changed, 85 insertions(+), 15 deletions(-)

diff --git a/proxmox-microseg/src/ebpf.rs b/proxmox-microseg/src/ebpf.rs
index 02ea16d5a2ef..717e02fe3ff6 100644
--- a/proxmox-microseg/src/ebpf.rs
+++ b/proxmox-microseg/src/ebpf.rs
@@ -88,6 +88,17 @@ fn check_entries(state: &DesiredState) -> anyhow::Result<Vec<Vec<CheckEntry>>> {
         for protocol in &check.protocols {
             entries.extend(protocol_entries(protocol, check.allow)?);
         }
+        if check.fragments {
+            if entries.is_empty() {
+                entries.push(CheckEntry {
+                    allow: check.allow,
+                    ..Default::default()
+                });
+            }
+            for fragment_entry in &mut entries {
+                fragment_entry.fragment = true;
+            }
+        }
         checks.push(entries);
     }
     Ok(checks)
@@ -210,6 +221,7 @@ mod tests {
             }],
             checks: vec![Check {
                 protocols,
+                fragments: false,
                 allow: true,
             }],
             nics: Vec::new(),
diff --git a/proxmox-ve-config/src/sdn/microseg/api.rs b/proxmox-ve-config/src/sdn/microseg/api.rs
index d9cd22344cb5..650658fe0f13 100644
--- a/proxmox-ve-config/src/sdn/microseg/api.rs
+++ b/proxmox-ve-config/src/sdn/microseg/api.rs
@@ -63,6 +63,8 @@ pub struct RuleCreate {
     pub icmp_type: Option<String>,
     #[serde(default, rename = "macro")]
     pub fw_macro: Option<String>,
+    #[serde(default, deserialize_with = "proxmox_serde::perl::deserialize_bool")]
+    pub fragments: bool,
     #[serde(default)]
     pub comment: Option<String>,
 }
@@ -211,6 +213,7 @@ fn rule_id(
     dst_match: PredicateMatch,
     dst_groups: &[&str],
     narrowing: &[Option<&str>],
+    fragments: bool,
     bidirectional: bool,
 ) -> Result<MicrosegId, Error> {
     let mut hash = Fnv1a::new();
@@ -221,6 +224,7 @@ fn rule_id(
     for field in narrowing {
         hash.text(field.unwrap_or_default());
     }
+    hash.flag(fragments);
     hash.flag(bidirectional);
     format!("r{:016x}", hash.0).parse()
 }
@@ -332,6 +336,7 @@ fn build_entry(
                 rule.dst_match,
                 &dst_groups,
                 &narrowing,
+                rule.fragments,
                 rule.bidirectional,
             )?;
             MicrosegEntry::Rule(RuleSection {
@@ -348,6 +353,7 @@ fn build_entry(
                 dport,
                 icmp_type,
                 fw_macro,
+                fragments: rule.fragments,
                 comment: rule.comment,
             })
         }
@@ -598,6 +604,7 @@ fn narrowing_str(rule: &RuleSection) -> Option<String> {
         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}")),
+        rule.fragments.then(|| "fragments".to_string()),
     ];
     let narrowing = parts.into_iter().flatten().collect::<Vec<_>>().join(" ");
     (!narrowing.is_empty()).then_some(narrowing)
diff --git a/proxmox-ve-config/src/sdn/microseg/mod.rs b/proxmox-ve-config/src/sdn/microseg/mod.rs
index be48b890206f..c3f776d1226a 100644
--- a/proxmox-ve-config/src/sdn/microseg/mod.rs
+++ b/proxmox-ve-config/src/sdn/microseg/mod.rs
@@ -588,28 +588,26 @@ struct ResolvedRule {
     dst: identity::Predicate,
     prio: u16,
     allow: bool,
-    /// The protocol matches narrowing the rule. Empty for a rule deciding the whole pair.
     protocols: Vec<Protocol>,
-    /// Set on the mirrored orientation of a bidirectional rule.
+    fragments: bool,
     reverse: bool,
 }
 
 impl ResolvedRule {
     fn is_narrowed(&self) -> bool {
-        !self.protocols.is_empty()
+        self.fragments || !self.protocols.is_empty()
     }
 
-    /// A narrowed deny that can match neighbor discovery.
+    /// A narrowed deny that can match neighbor discovery. A non-first fragment carries no ICMPv6
+    /// header, so a fragments rule cannot.
     fn can_deny_nd(&self) -> bool {
         !self.allow
+            && !self.fragments
             && self.protocols.iter().any(|protocol| match protocol {
-                Protocol::Icmpv6(icmp) => matches!(
-                    icmp.ty(),
-                    None | Some(Icmpv6Type::Any | Icmpv6Type::Numeric(135 | 136))
-                        | Some(Icmpv6Type::Named(
-                            "nd-neighbor-solicit" | "nd-neighbor-advert"
-                        ))
-                ),
+                Protocol::Icmpv6(icmp) => icmp
+                    .ty()
+                    .and_then(Icmpv6Type::number)
+                    .is_none_or(|ty| ty == 135 || ty == 136),
                 _ => false,
             })
     }
@@ -644,6 +642,7 @@ fn resolved_rules(entries: &BTreeMap<String, MicrosegEntry>) -> Result<Vec<Resol
             prio: rule.prio,
             allow: rule.allow,
             protocols: rule.protocols().with_context(context)?,
+            fragments: rule.fragments,
             reverse: false,
         };
         let mirrored = rule.bidirectional.then(|| ResolvedRule {
@@ -653,6 +652,7 @@ fn resolved_rules(entries: &BTreeMap<String, MicrosegEntry>) -> Result<Vec<Resol
             prio: forward.prio,
             allow: forward.allow,
             protocols: forward.protocols.iter().map(reversed_protocol).collect(),
+            fragments: forward.fragments,
             reverse: true,
         });
         resolved.push(forward);
@@ -921,6 +921,7 @@ mod tests {
                 dport: None,
                 icmp_type: None,
                 fw_macro: None,
+                fragments: false,
                 comment: None,
             }),
         )
@@ -1040,6 +1041,33 @@ mod tests {
         assert!(!folded.allow);
     }
 
+    #[test]
+    fn a_fragments_rule_decides_the_fragments_only() {
+        let with = |modify: fn(&mut RuleSection)| {
+            BTreeMap::from([
+                group("web"),
+                group("db"),
+                with_rule(rule("frag", &["web"], &["db"], true), modify),
+            ])
+        };
+        let entries = with(|rule| rule.fragments = true);
+        validate(&entries).expect("valid");
+        let rules = resolved(&entries);
+        let folded = fold(firing_rules(&rules, &set(&["web"]), &set(&["db"])));
+        assert!(folded.checks[0].fragments);
+        assert!(
+            !folded.allow,
+            "the allow is for the fragments, not the pair"
+        );
+
+        let with_ports = with(|rule| {
+            rule.fragments = true;
+            rule.proto = Some("tcp".to_string());
+            rule.dport = Some("22".to_string());
+        });
+        validate(&with_ports).expect_err("a fragment carries no ports");
+    }
+
     #[test]
     fn bidirectional_narrowing_swaps_the_ports() {
         let tcp = |sport: Option<&str>, dport: Option<&str>| {
diff --git a/proxmox-ve-config/src/sdn/microseg/state.rs b/proxmox-ve-config/src/sdn/microseg/state.rs
index 1392860f5062..ce733de9b37d 100644
--- a/proxmox-ve-config/src/sdn/microseg/state.rs
+++ b/proxmox-ve-config/src/sdn/microseg/state.rs
@@ -43,6 +43,9 @@ pub struct Pair {
 #[derive(Debug)]
 pub struct Check {
     pub protocols: Vec<Protocol>,
+    /// The check matches non-first fragments, those of `protocols` or of every protocol when there
+    /// is none.
+    pub fragments: bool,
     pub allow: bool,
 }
 
@@ -273,6 +276,7 @@ fn fold_pairs(firing: &[(u16, u16, &ResolvedRule)]) -> (Vec<Pair>, Vec<Check>) {
                     .or_insert_with(|| {
                         checks.push(Check {
                             protocols: rule.protocols.clone(),
+                            fragments: rule.fragments,
                             allow: rule.allow,
                         });
                         checks.len() - 1
diff --git a/proxmox-ve-config/src/sdn/microseg/types.rs b/proxmox-ve-config/src/sdn/microseg/types.rs
index 39f80764e5e0..a8c3fcb0557b 100644
--- a/proxmox-ve-config/src/sdn/microseg/types.rs
+++ b/proxmox-ve-config/src/sdn/microseg/types.rs
@@ -165,6 +165,10 @@ pub struct GroupSection {
             max_length: 128,
             description: "Firewall macro standing in for the protocol and ports.",
         },
+        fragments: {
+            type: Boolean,
+            optional: true,
+        },
         comment: {
             type: String,
             optional: true,
@@ -179,7 +183,8 @@ pub struct GroupSection {
 /// 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.
+/// stands in for them. A non-first fragment carries no transport header, so among the
+/// narrowed rules only one narrowed to fragments matches it.
 pub struct RuleSection {
     pub(crate) id: MicrosegId,
     /// Groups the source predicate lists.
@@ -218,18 +223,32 @@ pub struct RuleSection {
     pub(crate) icmp_type: Option<String>,
     #[serde(default, rename = "macro", skip_serializing_if = "Option::is_none")]
     pub(crate) fw_macro: Option<String>,
+    /// Whether the rule matches non-first fragments, those of `proto` or of every protocol.
+    #[serde(
+        default,
+        deserialize_with = "proxmox_serde::perl::deserialize_bool",
+        skip_serializing_if = "is_false"
+    )]
+    pub(crate) fragments: bool,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub(crate) comment: Option<String>,
 }
 
 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.
+    /// The protocol matches narrowing this rule, empty for a rule that decides the whole pair or
+    /// matches the fragments of every protocol. Ports without a port-carrying protocol and an
+    /// ICMP type without an ICMP protocol return Err, since the protocol match would ignore
+    /// them.
     pub(super) fn protocols(&self) -> Result<Vec<Protocol>, Error> {
         let explicit = [&self.proto, &self.sport, &self.dport, &self.icmp_type]
             .iter()
             .any(|field| field.is_some());
+        let beyond_proto = [&self.sport, &self.dport, &self.icmp_type, &self.fw_macro]
+            .iter()
+            .any(|field| field.is_some());
+        if self.fragments && beyond_proto {
+            bail!("a fragment carries no ports and no ICMP type, so only a protocol can narrow it");
+        }
         if let Some(name) = &self.fw_macro {
             if explicit {
                 bail!("a macro cannot be combined with a protocol, ports or an ICMP type");
-- 
2.47.3