all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Stefan Hanreich <s.hanreich@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Gabriel Goller <g.goller@proxmox.com>
Subject: Re: [pve-devel] [PATCH ve-rs 1/2] fix: firewall: introduce iptables to nftables mapping for icmpv6-types
Date: Wed, 1 Oct 2025 18:46:36 +0200	[thread overview]
Message-ID: <0d1a852c-876e-4484-bf19-35d616c7542a@proxmox.com> (raw)
In-Reply-To: <20250916093116.114942-2-g.goller@proxmox.com>

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


  reply	other threads:[~2025-10-01 16:46 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-16  9:31 [pve-devel] [PATCH proxmox-firewall/ve-rs 0/3] Fix ICMPv6 types in nftables Gabriel Goller
2025-09-16  9:31 ` [pve-devel] [PATCH ve-rs 1/2] fix: firewall: introduce iptables to nftables mapping for icmpv6-types Gabriel Goller
2025-10-01 16:46   ` Stefan Hanreich [this message]
2025-10-06 10:14     ` Gabriel Goller
2025-10-04 13:17   ` Thomas Lamprecht
2025-10-06 10:13     ` Gabriel Goller
2025-09-16  9:31 ` [pve-devel] [PATCH ve-rs 2/2] firewall: correctly return errors when parsing icmpv6 types and codes Gabriel Goller
2025-09-16  9:31 ` [pve-devel] [PATCH proxmox-firewall 1/1] tests: add icmpv6 type mapping test Gabriel Goller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0d1a852c-876e-4484-bf19-35d616c7542a@proxmox.com \
    --to=s.hanreich@proxmox.com \
    --cc=g.goller@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal