[pve-devel] [PATCH proxmox-firewall 1/2] fix #7068: show rule comments in nftables output

[Robert Obkircher <r.obkircher@proxmox.com>]

Include rule comments from the UI in the generated nftables rules if
the preserve_comments option is enabled. Truncate them to at most 128
bytes to match the limit in libnftnl.

Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
---
 proxmox-firewall/src/rule.rs | 56 +++++++++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/proxmox-firewall/src/rule.rs b/proxmox-firewall/src/rule.rs
index b79f91c..6be6720 100644
--- a/proxmox-firewall/src/rule.rs
+++ b/proxmox-firewall/src/rule.rs
@@ -36,14 +36,19 @@ pub(crate) struct NftRule {
     family: Option<Family>,
     statements: Vec<Statement>,
     terminal_statements: Vec<Statement>,
+    comment: Option<String>,
 }
 
 impl NftRule {
+    /// from NFTNL_UDATA_COMMENT_MAXLEN
+    pub const MAX_COMMENT_LEN: usize = 128;
+
     pub fn from_terminal_statements(terminal_statements: Vec<Statement>) -> Self {
         Self {
             family: None,
             statements: Vec::new(),
             terminal_statements,
+            comment: None,
         }
     }
 
@@ -52,6 +57,7 @@ impl NftRule {
             family: None,
             statements: Vec::new(),
             terminal_statements: vec![terminal_statement],
+            comment: None,
         }
     }
 
@@ -81,6 +87,41 @@ impl NftRule {
         ipfilter.to_nft_rules(&mut rules, env)?;
         Ok(rules)
     }
+
+    pub fn set_comment(&mut self, comment: &str) {
+        self.comment = Some(Self::truncate_comment(comment).to_string())
+    }
+
+    fn truncate_comment(comment: &str) -> &str {
+        &comment[..my_floor_char_boundary(comment, Self::MAX_COMMENT_LEN)]
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::NftRule;
+
+    #[test]
+    fn test_truncate_129() {
+        let comment = "Mid character trucation of 129 byte comment: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa🦀🦀";
+        assert_eq!(comment.len(), 129);
+        let truncated = NftRule::truncate_comment(comment);
+        assert_eq!(truncated.len(), 125);
+        assert!(comment.starts_with(truncated));
+    }
+}
+
+// TODO: replace with str::floor_char_boundary once rustc 1.91.0 is available
+fn my_floor_char_boundary(s: &str, index: usize) -> usize {
+    if index >= s.len() {
+        s.len()
+    } else {
+        s.char_indices()
+            .map(|(i, _)| i)
+            .take_while(|i| *i <= index)
+            .last()
+            .unwrap_or(0)
+    }
 }
 
 impl Deref for NftRule {
@@ -101,7 +142,12 @@ impl NftRule {
     pub fn into_add_rule(self, chain: ChainPart) -> AddRule {
         let statements = self.statements.into_iter().chain(self.terminal_statements);
 
-        AddRule::from_statements(chain, statements)
+        let result = AddRule::from_statements(chain, statements);
+        if let Some(comment) = self.comment {
+            result.with_comment(comment)
+        } else {
+            result
+        }
     }
 
     pub fn family(&self) -> Option<Family> {
@@ -175,11 +221,19 @@ impl ToNftRules for Rule {
     fn to_nft_rules(&self, rules: &mut Vec<NftRule>, env: &NftRuleEnv) -> Result<(), Error> {
         log::trace!("generating nft rules for config rule {self:?}");
 
+        let before = rules.len();
         match self.kind() {
             Kind::Match(rule) => rule.to_nft_rules(rules, env)?,
             Kind::Group(group) => group.to_nft_rules(rules, env)?,
         };
 
+        if env.firewall_config.host().preserve_comments() {
+            if let Some(comment) = self.comment() {
+                for nft_rule in &mut rules[before..] {
+                    nft_rule.set_comment(comment);
+                }
+            }
+        }
         Ok(())
     }
 }
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel