[pve-devel] [PATCH proxmox-firewall/ve-rs v2 0/3] Fix ICMP types in nftables

[pve-devel] [PATCH proxmox-firewall/ve-rs v2 0/3] Fix ICMP types in nftables

[Gabriel Goller]

Currently when setting ICMP types on the old firewall (iptables) then
switching to the new one (nftables) a few types will fail because they have
been renamed in nftables or do not exist in nftables. This affects both icmp
and icmpv6 where we now map options that do not exist in nftables to their
respective type/code combinations. This allows us to have exactly the same
behavior in nftables as we have in iptables.

Changelog:
v1, thanks Stefan and Thomas:
 * no hard error when mapping not possible
 * map all the types by falling back to custom type/code
 * also map icmp types in addition to icmpv6

ve-rs:

Gabriel Goller (1):
  fix: firewall: introduce iptables to nftables mapping for icmpv6-types

Wolfgang Bumiller (1):
  fix: firewall: introduce iptables to nftables mapping for icmp-types

 proxmox-ve-config/src/firewall/cluster.rs     |   9 +-
 proxmox-ve-config/src/firewall/types/rule.rs  |   9 +-
 .../src/firewall/types/rule_match.rs          | 139 +++++++++++++++---
 3 files changed, 129 insertions(+), 28 deletions(-)


proxmox-firewall:

Gabriel Goller (1):
  tests: add icmpv6 type mapping test

 proxmox-firewall/tests/input/host.fw          |  1 +
 .../integration_tests__firewall.snap          | 63 +++++++++++++++++++
 2 files changed, 64 insertions(+)


Summary over all repositories:
  5 files changed, 193 insertions(+), 28 deletions(-)

-- 
Generated by git-murpp 0.8.0


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

[pve-devel] [PATCH ve-rs v2 1/2] fix: firewall: introduce iptables to nftables mapping for icmp-types

[Gabriel Goller]

From: Wolfgang Bumiller <w.bumiller@proxmox.com>

Some of the icmp-type options that we expose in the UI don't exist as
standalone nftables options, so we need to create them using a custom
type and code id.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-ve-config/src/firewall/types/rule.rs  |  9 +--
 .../src/firewall/types/rule_match.rs          | 67 ++++++++++++++++---
 2 files changed, 63 insertions(+), 13 deletions(-)

diff --git a/proxmox-ve-config/src/firewall/types/rule.rs b/proxmox-ve-config/src/firewall/types/rule.rs
index 3ad8cf059f09..d20145cb902e 100644
--- a/proxmox-ve-config/src/firewall/types/rule.rs
+++ b/proxmox-ve-config/src/firewall/types/rule.rs
@@ -254,7 +254,7 @@ mod tests {
         alias::{AliasName, AliasScope},
         ipset::{IpsetName, IpsetScope},
         log::LogLevel,
-        rule_match::{Icmp, IcmpCode, IpAddrMatch, IpMatch, Ports, Protocol, Udp},
+        rule_match::{Icmp, IcmpCode, IcmpType, IpAddrMatch, IpMatch, Ports, Protocol, Udp},
     };
 
     use super::*;
@@ -338,9 +338,10 @@ mod tests {
                         ),
                     )
                     .ok(),
-                    proto: Some(Protocol::Icmp(Icmp::new_code(IcmpCode::Named(
-                        "port-unreachable"
-                    )))),
+                    proto: Some(Protocol::Icmp(Icmp::new_ty_and_code(
+                        IcmpType::Numeric(3),
+                        IcmpCode::Numeric(3)
+                    ))),
                     log: Some(LogLevel::Nolog),
                     ..Default::default()
                 }),
diff --git a/proxmox-ve-config/src/firewall/types/rule_match.rs b/proxmox-ve-config/src/firewall/types/rule_match.rs
index 7fcd35c80d86..f99a936b454a 100644
--- a/proxmox-ve-config/src/firewall/types/rule_match.rs
+++ b/proxmox-ve-config/src/firewall/types/rule_match.rs
@@ -463,10 +463,10 @@ impl Icmp {
         }
     }
 
-    pub fn new_code(code: IcmpCode) -> Self {
+    pub fn new_ty_and_code(ty: IcmpType, code: IcmpCode) -> Self {
         Self {
+            ty: Some(ty),
             code: Some(code),
-            ..Default::default()
         }
     }
 
@@ -487,19 +487,61 @@ impl Icmp {
     }
 }
 
+/// Some icmp_types are not supported by nftables. See:
+/// https://wiki.nftables.org/wiki-nftables/index.php/Supported_features_compared_to_xtables#icmp
+/// Some have an exact equivalent in nftables and for some others we need to set a custom type and
+/// code combination.
+#[sortable]
+const IPTABLES_ICMP_TYPES_MAPPING: [(&str, IcmpTypeMap); 23] = sorted!([
+    ("network-unreachable", IcmpTypeMap::Custom((3, 0))),
+    ("host-unreachable", IcmpTypeMap::Custom((3, 1))),
+    ("protocol-unreachable", IcmpTypeMap::Custom((3, 2))),
+    ("port-unreachable", IcmpTypeMap::Custom((3, 3))),
+    ("fragmentation-needed", IcmpTypeMap::Custom((3, 4))),
+    ("source-route-failed", IcmpTypeMap::Custom((3, 5))),
+    ("network-unknown", IcmpTypeMap::Custom((3, 6))),
+    ("host-unknown", IcmpTypeMap::Custom((3, 7))),
+    ("network-prohibited", IcmpTypeMap::Custom((3, 9))),
+    ("host-prohibited", IcmpTypeMap::Custom((3, 10))),
+    ("TOS-network-unreachable", IcmpTypeMap::Custom((3, 11))),
+    ("TOS-host-unreachable", IcmpTypeMap::Custom((3, 12))),
+    ("communication-prohibited", IcmpTypeMap::Custom((3, 13))),
+    ("host-precedence-violation", IcmpTypeMap::Custom((3, 14))),
+    ("precedence-cutoff", IcmpTypeMap::Custom((3, 15))),
+    ("network-redirect", IcmpTypeMap::Custom((5, 0))),
+    ("host-redirect", IcmpTypeMap::Custom((5, 1))),
+    ("TOS-network-redirect", IcmpTypeMap::Custom((5, 2))),
+    ("TOS-host-redirect", IcmpTypeMap::Custom((5, 3))),
+    ("ttl-zero-during-transit", IcmpTypeMap::Custom((11, 0))),
+    ("ttl-zero-during-reassembly", IcmpTypeMap::Custom((11, 1))),
+    ("ip-header-bad", IcmpTypeMap::Custom((12, 0))),
+    ("required-option-missing", IcmpTypeMap::Custom((12, 1))),
+]);
+
 impl FromStr for Icmp {
     type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let mut this = Self::default();
 
-        if let Ok(ty) = s.parse() {
-            this.ty = Some(ty);
-            return Ok(this);
+        // Some icmp types exist in iptables, but do not in nftables. Some of these map exactly
+        // onto other types, for others we need to use a custom type/code combination.
+        if let Ok(index) = IPTABLES_ICMP_TYPES_MAPPING.binary_search_by(|v| v.0.cmp(s)) {
+            match IPTABLES_ICMP_TYPES_MAPPING[index].1 {
+                IcmpTypeMap::Map(mapped_nftables_type) => {
+                    this.ty = Some(IcmpType::Named(mapped_nftables_type));
+                    return Ok(this);
+                }
+                IcmpTypeMap::Custom((ty, code)) => {
+                    this.ty = Some(IcmpType::Numeric(ty));
+                    this.code = Some(IcmpCode::Numeric(code));
+                    return Ok(this);
+                }
+            }
         }
 
-        if let Ok(code) = s.parse() {
-            this.code = Some(code);
+        if let Ok(ty) = s.parse() {
+            this.ty = Some(ty);
             return Ok(this);
         }
 
@@ -646,6 +688,13 @@ impl Icmpv6 {
     }
 }
 
+enum IcmpTypeMap {
+    /// This icmp type can be mapped exactly to an equivalent nftables type
+    Map(&'static str),
+    /// This icmp type needs to be represented using a custom type and code combination
+    Custom((u8, u8)),
+}
+
 impl FromStr for Icmpv6 {
     type Err = Error;
 
@@ -938,8 +987,8 @@ mod tests {
         assert_eq!(
             icmp,
             Icmp {
-                ty: None,
-                code: Some(IcmpCode::Named("port-unreachable"))
+                ty: Some(IcmpType::Numeric(3)),
+                code: Some(IcmpCode::Numeric(3))
             }
         );
     }
-- 
2.47.3



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

[pve-devel] [PATCH ve-rs v2 2/2] fix: firewall: introduce iptables to nftables mapping for icmpv6-types

[Gabriel Goller]

Some of the iptables icmpv6-type options we expose in the UI don't exist
in nftables, so we need to make a mapping with a custom type and code to
support them. There are also some options that have been renamed in
nftables, which can be mapped to the new name:
https://wiki.nftables.org/wiki-nftables/index.php/Supported_features_compared_to_xtables#icmp6

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-ve-config/src/firewall/cluster.rs     |  9 +--
 .../src/firewall/types/rule_match.rs          | 72 ++++++++++++++++---
 2 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/proxmox-ve-config/src/firewall/cluster.rs b/proxmox-ve-config/src/firewall/cluster.rs
index d588302b0783..5e1143fb2617 100644
--- a/proxmox-ve-config/src/firewall/cluster.rs
+++ b/proxmox-ve-config/src/firewall/cluster.rs
@@ -143,7 +143,7 @@ mod tests {
         log::{LogLevel, LogRateLimitTimescale},
         rule::{Kind, RuleGroup},
         rule_match::{
-            Icmpv6, Icmpv6Code, IpAddrMatch, IpMatch, Ports, Protocol, RuleMatch, Tcp, Udp,
+            Icmpv6, Icmpv6Code, Icmpv6Type, IpAddrMatch, IpMatch, Ports, Protocol, RuleMatch, Tcp, Udp
         },
     };
 
@@ -331,9 +331,10 @@ IN BGP(REJECT) -log crit -source 1.2.3.4
                             .unwrap()
                         ))),
                     }),
-                    proto: Some(Protocol::Icmpv6(Icmpv6::new_code(Icmpv6Code::Named(
-                        "port-unreachable"
-                    )))),
+                    proto: Some(Protocol::Icmpv6(Icmpv6::new_ty_and_code(
+                        Icmpv6Type::Numeric(1),
+                        Icmpv6Code::Numeric(4)
+                    ))),
                     log: Some(LogLevel::Nolog),
                     ..Default::default()
                 }),
diff --git a/proxmox-ve-config/src/firewall/types/rule_match.rs b/proxmox-ve-config/src/firewall/types/rule_match.rs
index f99a936b454a..99625d0e31a5 100644
--- a/proxmox-ve-config/src/firewall/types/rule_match.rs
+++ b/proxmox-ve-config/src/firewall/types/rule_match.rs
@@ -664,10 +664,10 @@ impl Icmpv6 {
         }
     }
 
-    pub fn new_code(code: Icmpv6Code) -> Self {
+    pub fn new_ty_and_code(ty: Icmpv6Type, code: Icmpv6Code) -> Self {
         Self {
+            ty: Some(ty),
             code: Some(code),
-            ..Default::default()
         }
     }
 
@@ -695,23 +695,73 @@ enum IcmpTypeMap {
     Custom((u8, u8)),
 }
 
+/// Some icmp_types are not supported by nftables. See:
+/// https://wiki.nftables.org/wiki-nftables/index.php/Supported_features_compared_to_xtables#icmp6
+/// Some have an exact equivalent in nftables and for some others we need to set a custom type and
+/// code combination.
+#[sortable]
+const IPTABLES_ICMPV6_TYPES_MAPPING: [(&str, IcmpTypeMap); 19] = sorted!([
+    ("no-route", IcmpTypeMap::Custom((1, 0))),
+    ("communication-prohibited", IcmpTypeMap::Custom((1, 1))),
+    ("beyond-scope", IcmpTypeMap::Custom((1, 2))),
+    ("address-unreachable", IcmpTypeMap::Custom((1, 3))),
+    ("port-unreachable", IcmpTypeMap::Custom((1, 4))),
+    ("failed-policy", IcmpTypeMap::Custom((1, 5))),
+    ("reject-route'", IcmpTypeMap::Custom((1, 6))),
+    ("ttl-zero-during-transit", IcmpTypeMap::Custom((3, 0))),
+    ("ttl-zero-during-reassembly", IcmpTypeMap::Custom((3, 1))),
+    ("bad-header", IcmpTypeMap::Custom((4, 0))),
+    ("unknown-header-type", IcmpTypeMap::Custom((4, 1))),
+    ("unknown-option", IcmpTypeMap::Custom((4, 2))),
+    ("router-solicitation", IcmpTypeMap::Map("nd-router-solicit")),
+    ("router-advertisement", IcmpTypeMap::Map("nd-router-advert")),
+    (
+        "neighbor-solicitation",
+        IcmpTypeMap::Map("nd-neighbor-solicit")
+    ),
+    (
+        "neighbour-solicitation",
+        IcmpTypeMap::Map("nd-neighbor-solicit")
+    ),
+    (
+        "neighbor-advertisement",
+        IcmpTypeMap::Map("nd-neighbor-advert")
+    ),
+    (
+        "neighbour-advertisement",
+        IcmpTypeMap::Map("nd-neighbor-advert")
+    ),
+    ("redirect", IcmpTypeMap::Map("nd-redirect")),
+]);
+
 impl FromStr for Icmpv6 {
     type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let mut this = Self::default();
 
-        if let Ok(ty) = s.parse() {
-            this.ty = Some(ty);
-            return Ok(this);
+        // Some icmpv6 types exist in iptables, but do not in nftables. Some of these map exactly
+        // onto other types, for others we need to use a custom type/code combination.
+        if let Ok(index) = IPTABLES_ICMPV6_TYPES_MAPPING.binary_search_by(|v| v.0.cmp(s)) {
+            match IPTABLES_ICMPV6_TYPES_MAPPING[index].1 {
+                IcmpTypeMap::Map(mapped_nftables_type) => {
+                    this.ty = Some(Icmpv6Type::Named(mapped_nftables_type));
+                    return Ok(this);
+                }
+                IcmpTypeMap::Custom((ty, code)) => {
+                    this.ty = Some(Icmpv6Type::Numeric(ty));
+                    this.code = Some(Icmpv6Code::Numeric(code));
+                    return Ok(this);
+                }
+            }
         }
 
-        if let Ok(code) = s.parse() {
-            this.code = Some(code);
+        if let Ok(ty) = s.parse() {
+            this.ty = Some(ty);
             return Ok(this);
         }
 
-        bail!("supplied string is neither a valid icmpv6 type nor code");
+        bail!("supplied string is not a valid icmpv6 type");
     }
 }
 
@@ -1015,13 +1065,13 @@ mod tests {
             }
         );
 
-        icmp = "admin-prohibited".parse().expect("valid icmpv6 code");
+        icmp = "unknown-header-type".parse().expect("valid icmpv6 type");
 
         assert_eq!(
             icmp,
             Icmpv6 {
-                ty: None,
-                code: Some(Icmpv6Code::Named("admin-prohibited"))
+                ty: Some(Icmpv6Type::Numeric(4)),
+                code: Some(Icmpv6Code::Numeric(1))
             }
         );
     }
-- 
2.47.3



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

[pve-devel] [PATCH proxmox-firewall v2 1/1] tests: add icmpv6 type mapping test

[Gabriel Goller]

We now map the iptables icmpv6-types to the nftables icmpv6-types which
have slightly different names. Add a simple test that shows the mapping
between "neighbor-solicitation" and "nd-neighbor-solicit".

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-firewall/tests/input/host.fw          |  1 +
 .../integration_tests__firewall.snap          | 63 +++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/proxmox-firewall/tests/input/host.fw b/proxmox-firewall/tests/input/host.fw
index ddfcb1c4d2c8..7b89aad86317 100644
--- a/proxmox-firewall/tests/input/host.fw
+++ b/proxmox-firewall/tests/input/host.fw
@@ -20,6 +20,7 @@ nf_conntrack_helpers: amanda,ftp,irc,netbios-ns,pptp,sane,sip,snmp,tftp
 IN DNS(ACCEPT) -source dc/network1 -log nolog
 IN DHCPv6(ACCEPT) -log nolog
 IN DHCPfwd(ACCEPT) -log nolog
+IN ACCEPT --icmp-type neighbor-solicitation --proto ipv6-icmp --log info
 IN Ping(REJECT)
 IN REJECT -p udp --dport 443
 OUT REJECT -p udp --dport 443
diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
index e3db8ae2db10..e6ba681d8095 100644
--- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
+++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
@@ -1,6 +1,7 @@
 ---
 source: proxmox-firewall/tests/integration_tests.rs
 expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
+snapshot_kind: text
 ---
 {
   "nftables": [
@@ -3593,6 +3594,68 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
         }
       }
     },
+    {
+      "add": {
+        "rule": {
+          "family": "inet",
+          "table": "proxmox-firewall",
+          "chain": "host-in",
+          "expr": [
+            {
+              "match": {
+                "op": "==",
+                "left": {
+                  "payload": {
+                    "protocol": "icmpv6",
+                    "field": "type"
+                  }
+                },
+                "right": "nd-neighbor-solicit"
+              }
+            },
+            {
+              "limit": {
+                "rate": 2,
+                "per": "second",
+                "burst": 12
+              }
+            },
+            {
+              "log": {
+                "prefix": ":0:6:host-in: ACCEPT: ",
+                "group": 0
+              }
+            }
+          ]
+        }
+      }
+    },
+    {
+      "add": {
+        "rule": {
+          "family": "inet",
+          "table": "proxmox-firewall",
+          "chain": "host-in",
+          "expr": [
+            {
+              "match": {
+                "op": "==",
+                "left": {
+                  "payload": {
+                    "protocol": "icmpv6",
+                    "field": "type"
+                  }
+                },
+                "right": "nd-neighbor-solicit"
+              }
+            },
+            {
+              "accept": null
+            }
+          ]
+        }
+      }
+    },
     {
       "add": {
         "rule": {
-- 
2.47.3



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

Re: [pve-devel] [PATCH ve-rs v2 1/2] fix: firewall: introduce iptables to nftables mapping for icmp-types

[Gabriel Goller]

On 06.10.2025 12:19, Gabriel Goller wrote:
>From: Wolfgang Bumiller <w.bumiller@proxmox.com>

Oops, this was a --ruse-message accident, please ignore this, it's my
commit.


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

Re: [pve-devel] [PATCH proxmox-firewall/ve-rs v2 0/3] Fix ICMP types in nftables

[Stefan Hanreich]

tested this series and found another pre-existing issue with rule
generation, which I sent a patch for [1].

Together with the patch series I sent applied, this patch series solves
the issues on my test instance. Code LGTM as well - so consider this:

Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>
Reviewed-by: Stefan Hanreich <s.hanreich@proxmox.com>


[1]
https://lore.proxmox.com/pve-devel/20251008131457.178090-1-s.hanreich@proxmox.com/T/#t

On 10/6/25 12:19 PM, Gabriel Goller wrote:
> Currently when setting ICMP types on the old firewall (iptables) then
> switching to the new one (nftables) a few types will fail because they have
> been renamed in nftables or do not exist in nftables. This affects both icmp
> and icmpv6 where we now map options that do not exist in nftables to their
> respective type/code combinations. This allows us to have exactly the same
> behavior in nftables as we have in iptables.
> 
> Changelog:
> v1, thanks Stefan and Thomas:
>  * no hard error when mapping not possible
>  * map all the types by falling back to custom type/code
>  * also map icmp types in addition to icmpv6
> 
> ve-rs:
> 
> Gabriel Goller (1):
>   fix: firewall: introduce iptables to nftables mapping for icmpv6-types
> 
> Wolfgang Bumiller (1):
>   fix: firewall: introduce iptables to nftables mapping for icmp-types
> 
>  proxmox-ve-config/src/firewall/cluster.rs     |   9 +-
>  proxmox-ve-config/src/firewall/types/rule.rs  |   9 +-
>  .../src/firewall/types/rule_match.rs          | 139 +++++++++++++++---
>  3 files changed, 129 insertions(+), 28 deletions(-)
> 
> 
> proxmox-firewall:
> 
> Gabriel Goller (1):
>   tests: add icmpv6 type mapping test
> 
>  proxmox-firewall/tests/input/host.fw          |  1 +
>  .../integration_tests__firewall.snap          | 63 +++++++++++++++++++
>  2 files changed, 64 insertions(+)
> 
> 
> Summary over all repositories:
>   5 files changed, 193 insertions(+), 28 deletions(-)
> 



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