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

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

[Gabriel Goller]

Currently when setting ICMPv6 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. The most prominent are
neighbor-solicitation/advertisement but there are a few more. There are also
some that are not supported in nftables and need to be handled accordingly.
Add a mapping which maps old types to new types and converts them when parsing
the config. This way we are transparent and can switch to using the new
nftables names in the future.

ve-rs:

Gabriel Goller (2):
  fix: firewall: introduce iptables to nftables mapping for icmpv6-types
  firewall: correctly return errors when parsing icmpv6 types and codes.

 .../src/firewall/types/rule_match.rs          | 89 ++++++++++++++-----
 1 file changed, 69 insertions(+), 20 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:
  3 files changed, 133 insertions(+), 20 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 1/2] fix: firewall: introduce iptables to nftables mapping for icmpv6-types

[Gabriel Goller]

nftables changed the names of the icmpv6-types and they don't overlap
completely with the old iptables names. Introduce a mapping that
converts old names into the new ones. A few of these are not supported,
see here for more info:
https://wiki.nftables.org/wiki-nftables/index.php/Supported_features_compared_to_xtables#icmp6

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 .../src/firewall/types/rule_match.rs          | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/proxmox-ve-config/src/firewall/types/rule_match.rs b/proxmox-ve-config/src/firewall/types/rule_match.rs
index 7fcd35c80d86..8202cda57895 100644
--- a/proxmox-ve-config/src/firewall/types/rule_match.rs
+++ b/proxmox-ve-config/src/firewall/types/rule_match.rs
@@ -697,6 +697,31 @@ const ICMPV6_TYPES: [(&str, u8); 19] = sorted!([
     ("time-exceeded", 3),
 ]);
 
+/// Some icmp_types are not supported by nftables. See:
+/// https://wiki.nftables.org/wiki-nftables/index.php/Supported_features_compared_to_xtables#icmp6
+#[sortable]
+const IPTABLES_ICMP_TYPES_MAPPING: [(&str, Option<&str>); 19] = sorted!([
+    ("no-route", None),
+    ("communication-prohibited", None),
+    ("beyond-scope", None),
+    ("address-unreachable", None),
+    ("port-unreachable", None),
+    ("failed-policy", None),
+    ("reject-route'", None),
+    ("ttl-zero-during-transit", None),
+    ("ttl-zero-during-reassembly", None),
+    ("bad-header", None),
+    ("unknown-header-type", None),
+    ("unknown-option", None),
+    ("router-solicitation", Some("nd-router-solicit")),
+    ("router-advertisement", Some("nd-router-advert")),
+    ("neighbor-solicitation", Some("nd-neighbor-solicit")),
+    ("neighbour-solicitation", Some("nd-neighbor-solicit")),
+    ("neighbor-advertisement", Some("nd-neighbor-advert")),
+    ("neighbour-advertisement", Some("nd-neighbor-advert")),
+    ("redirect", Some("nd-redirect")),
+]);
+
 impl std::str::FromStr for Icmpv6Type {
     type Err = Error;
 
@@ -713,6 +738,14 @@ impl std::str::FromStr for Icmpv6Type {
             return Ok(Self::Named(ICMPV6_TYPES[index].0));
         }
 
+        if let Ok(index) = IPTABLES_ICMP_TYPES_MAPPING.binary_search_by(|v| v.0.cmp(s)) {
+            if let Some(mapped_nftables_type) = IPTABLES_ICMP_TYPES_MAPPING[index].1 {
+                return Ok(Self::Named(mapped_nftables_type));
+            } else {
+                bail!("icmp_type {s:?} is unsupported in nftables");
+            }
+        }
+
         bail!("{s:?} is not a valid icmpv6 type");
     }
 }
-- 
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 1/2] fix: firewall: introduce iptables to nftables mapping for icmpv6-types

[Stefan Hanreich]

On 9/16/25 11:32 AM, Gabriel Goller wrote:
> nftables changed the names of the icmpv6-types and they don't overlap
> completely with the old iptables names. Introduce a mapping that
> converts old names into the new ones. A few of these are not supported,
> see here for more info:
> https://wiki.nftables.org/wiki-nftables/index.php/Supported_features_compared_to_xtables#icmp6

Did you find a reasoning for that? Are they not in use anymore /
deprecated? Then I guess we should not make that a hard error, but
possibly a warning and soft failure? In the other case (still in use), I
think we should still try to generate rules for them.

Since those are configurations that users can have pre-existing, we
should handle them gracefully instead of just erroring out on
encountering them.

There are even other possible values that are still not considered here
like 'TOS-network-unreachable'. Since they are all mappable to a numeric
type/code combo - we should take all possible values for the field [1]
[2] to preserve compatibility with existing configurations?

Not sure if they're accurate, but pve-manager seems to have the
respective information on type / code combinations [3]. Can take a
closer look at it and send a follow-up.

Not sure if this is a blocker, it might be a bit too obscure / niche to
prevent this series from getting merged... - can always just do a follow-up.

[1]
https://git.proxmox.com/?p=pve-firewall.git;a=blob;f=src/PVE/Firewall.pm;h=49430b174bb2fdd56ce586f90bf929c5648f9060;hb=HEAD#l785
[2]
https://git.proxmox.com/?p=pve-firewall.git;a=blob;f=src/PVE/Firewall.pm;h=49430b174bb2fdd56ce586f90bf929c5648f9060;hb=HEAD#l826
[3]
https://git.proxmox.com/?p=pve-manager.git;a=blob;f=www/manager6/grid/FirewallRules.js;h=0db817ebce0e9254d18f172a6e02a7a12e7a481c;hb=HEAD#l83

> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  .../src/firewall/types/rule_match.rs          | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/proxmox-ve-config/src/firewall/types/rule_match.rs b/proxmox-ve-config/src/firewall/types/rule_match.rs
> index 7fcd35c80d86..8202cda57895 100644
> --- a/proxmox-ve-config/src/firewall/types/rule_match.rs
> +++ b/proxmox-ve-config/src/firewall/types/rule_match.rs
> @@ -697,6 +697,31 @@ const ICMPV6_TYPES: [(&str, u8); 19] = sorted!([
>      ("time-exceeded", 3),
>  ]);
>  
> +/// Some icmp_types are not supported by nftables. See:
> +/// https://wiki.nftables.org/wiki-nftables/index.php/Supported_features_compared_to_xtables#icmp6
> +#[sortable]
> +const IPTABLES_ICMP_TYPES_MAPPING: [(&str, Option<&str>); 19] = sorted!([
> +    ("no-route", None),
> +    ("communication-prohibited", None),
> +    ("beyond-scope", None),
> +    ("address-unreachable", None),
> +    ("port-unreachable", None),
> +    ("failed-policy", None),
> +    ("reject-route'", None),
> +    ("ttl-zero-during-transit", None),
> +    ("ttl-zero-during-reassembly", None),
> +    ("bad-header", None),
> +    ("unknown-header-type", None),
> +    ("unknown-option", None),
> +    ("router-solicitation", Some("nd-router-solicit")),
> +    ("router-advertisement", Some("nd-router-advert")),
> +    ("neighbor-solicitation", Some("nd-neighbor-solicit")),
> +    ("neighbour-solicitation", Some("nd-neighbor-solicit")),
> +    ("neighbor-advertisement", Some("nd-neighbor-advert")),
> +    ("neighbour-advertisement", Some("nd-neighbor-advert")),
> +    ("redirect", Some("nd-redirect")),
> +]);
> +
>  impl std::str::FromStr for Icmpv6Type {
>      type Err = Error;
>  
> @@ -713,6 +738,14 @@ impl std::str::FromStr for Icmpv6Type {
>              return Ok(Self::Named(ICMPV6_TYPES[index].0));
>          }
>  
> +        if let Ok(index) = IPTABLES_ICMP_TYPES_MAPPING.binary_search_by(|v| v.0.cmp(s)) {
> +            if let Some(mapped_nftables_type) = IPTABLES_ICMP_TYPES_MAPPING[index].1 {
> +                return Ok(Self::Named(mapped_nftables_type));
> +            } else {
> +                bail!("icmp_type {s:?} is unsupported in nftables");
> +            }
> +        }
> +
>          bail!("{s:?} is not a valid icmpv6 type");
>      }
>  }



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

[pve-devel] [PATCH ve-rs 2/2] firewall: correctly return errors when parsing icmpv6 types and codes.

[Gabriel Goller]

Correctly capture and output the error when parsing a icmpv6 type or
code. This makes it easier to debug icmpv6 parsing errors.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 .../src/firewall/types/rule_match.rs          | 56 ++++++++++++-------
 1 file changed, 36 insertions(+), 20 deletions(-)

diff --git a/proxmox-ve-config/src/firewall/types/rule_match.rs b/proxmox-ve-config/src/firewall/types/rule_match.rs
index 8202cda57895..28d3e16b27ba 100644
--- a/proxmox-ve-config/src/firewall/types/rule_match.rs
+++ b/proxmox-ve-config/src/firewall/types/rule_match.rs
@@ -493,17 +493,25 @@ impl FromStr for Icmp {
     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);
-        }
-
-        if let Ok(code) = s.parse() {
-            this.code = Some(code);
-            return Ok(this);
+        match s.parse::<IcmpType>() {
+            Ok(ty) => {
+                this.ty = Some(ty);
+                Ok(this)
+            }
+            Err(type_error) => match s.parse::<IcmpCode>() {
+                Ok(code) => {
+                    this.code = Some(code);
+                    Ok(this)
+                }
+                Err(code_error) => {
+                    bail!(
+                        "supplied string is neither a valid icmp type nor code: {:?} - {:?}",
+                        type_error,
+                        code_error,
+                    );
+                }
+            },
         }
-
-        bail!("supplied string is neither a valid icmp type nor code");
     }
 }
 
@@ -652,17 +660,25 @@ impl FromStr for Icmpv6 {
     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);
-        }
-
-        if let Ok(code) = s.parse() {
-            this.code = Some(code);
-            return Ok(this);
+        match s.parse::<Icmpv6Type>() {
+            Ok(ty) => {
+                this.ty = Some(ty);
+                Ok(this)
+            }
+            Err(type_error) => match s.parse::<Icmpv6Code>() {
+                Ok(code) => {
+                    this.code = Some(code);
+                    Ok(this)
+                }
+                Err(code_error) => {
+                    bail!(
+                        "supplied string is neither a valid icmpv6 type nor code: {:?} - {:?}",
+                        type_error,
+                        code_error,
+                    );
+                }
+            },
         }
-
-        bail!("supplied string is neither a valid icmpv6 type nor code");
     }
 }
 
-- 
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 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