[pve-devel] [PATCH proxmox-ve-rs 1/3] firewall macros: fix macros using icmp protocol

[pve-devel] [PATCH proxmox-ve-rs 1/3] firewall macros: fix macros using icmp protocol

[Stefan Hanreich]

Macros containing rules for the ICMP protocol used dport instead of
icmp-type for specifying the type of ICMP messages. This is how
pve-firewall used to specify them, but the nftables firewall uses a
separate key for this in the macros. This caused all ICMP types to be
allowed instead of restricting them to the types specified in the
macro.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-ve-config/resources/macros.json | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/proxmox-ve-config/resources/macros.json b/proxmox-ve-config/resources/macros.json
index 2fcc0fb..37032c0 100644
--- a/proxmox-ve-config/resources/macros.json
+++ b/proxmox-ve-config/resources/macros.json
@@ -478,19 +478,19 @@
   "NeighborDiscovery": {
     "code": [
       {
-        "dport": "nd-router-solicit",
+        "icmp-type": "nd-router-solicit",
         "proto": "icmpv6"
       },
       {
-        "dport": "nd-router-advert",
+        "icmp-type": "nd-router-advert",
         "proto": "icmpv6"
       },
       {
-        "dport": "nd-neighbor-solicit",
+        "icmp-type": "nd-neighbor-solicit",
         "proto": "icmpv6"
       },
       {
-        "dport": "nd-neighbor-advert",
+        "icmp-type": "nd-neighbor-advert",
         "proto": "icmpv6"
       }
     ],
@@ -577,7 +577,7 @@
   "Ping": {
     "code": [
       {
-        "dport": "echo-request",
+        "icmp-type": "echo-request",
         "proto": "icmp"
       }
     ],
@@ -856,7 +856,7 @@
         "proto": "udp"
       },
       {
-        "dport": "echo-request",
+        "icmp-type": "echo-request",
         "proto": "icmp"
       }
     ],
-- 
2.39.5


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

[pve-devel] [PATCH proxmox-ve-rs 2/3] fix #6108: firewall macros: Add missing ICMPv6 statements

[Stefan Hanreich]

Some macros only contained rules for ICMP echo requests, but not their
ICMPv6 counterparts. Add them, so they work properly with IPv6 setups.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-ve-config/resources/macros.json | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/proxmox-ve-config/resources/macros.json b/proxmox-ve-config/resources/macros.json
index 37032c0..f90fb38 100644
--- a/proxmox-ve-config/resources/macros.json
+++ b/proxmox-ve-config/resources/macros.json
@@ -579,6 +579,10 @@
       {
         "icmp-type": "echo-request",
         "proto": "icmp"
+      },
+      {
+        "icmp-type": "echo-request",
+        "proto": "icmpv6"
       }
     ],
     "desc": "ICMP echo request"
@@ -858,6 +862,10 @@
       {
         "icmp-type": "echo-request",
         "proto": "icmp"
+      },
+      {
+        "icmp-type": "echo-request",
+        "proto": "icmpv6"
       }
     ],
     "desc": "Traceroute (for up to 30 hops) traffic"
-- 
2.39.5


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

[pve-devel] [PATCH proxmox-firewall 3/3] tests: add Ping macro to tests

[Stefan Hanreich]

Rules using the Ping macro were wrongly generated due to the ICMP
macros using the wrong format for specifying ICMP type. The test cases
did not include any macros utilizing the ICMP protocol. Add them to
catch any errors related to ICMP parsing in the future.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
Depends on bumped proxmox-ve-config to work.

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

diff --git a/proxmox-firewall/tests/input/host.fw b/proxmox-firewall/tests/input/host.fw
index a61b0bd..ddfcb1c 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 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 9194fc6..d25ece8 100644
--- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
+++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
@@ -1,7 +1,6 @@
 ---
 source: proxmox-firewall/tests/integration_tests.rs
 expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
-snapshot_kind: text
 ---
 {
   "nftables": [
@@ -3533,6 +3532,62 @@ snapshot_kind: text
         }
       }
     },
+    {
+      "add": {
+        "rule": {
+          "family": "inet",
+          "table": "proxmox-firewall",
+          "chain": "host-in",
+          "expr": [
+            {
+              "match": {
+                "op": "==",
+                "left": {
+                  "payload": {
+                    "protocol": "icmp",
+                    "field": "type"
+                  }
+                },
+                "right": "echo-request"
+              }
+            },
+            {
+              "jump": {
+                "target": "do-reject"
+              }
+            }
+          ]
+        }
+      }
+    },
+    {
+      "add": {
+        "rule": {
+          "family": "inet",
+          "table": "proxmox-firewall",
+          "chain": "host-in",
+          "expr": [
+            {
+              "match": {
+                "op": "==",
+                "left": {
+                  "payload": {
+                    "protocol": "icmpv6",
+                    "field": "type"
+                  }
+                },
+                "right": "echo-request"
+              }
+            },
+            {
+              "jump": {
+                "target": "do-reject"
+              }
+            }
+          ]
+        }
+      }
+    },
     {
       "add": {
         "rule": {
-- 
2.39.5


_______________________________________________
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 3/3] tests: add Ping macro to tests

[Hannes Laimer]

Tested these changes, I could reproduce the described problem, and
after applying the patches the macros only matches the correct ICMP
packets, not all.

so consider this:
Tested-by: Hannes Laimer <h.laimer@proxmox.com>

On 04.02.25 10:57, Stefan Hanreich wrote:
> Rules using the Ping macro were wrongly generated due to the ICMP
> macros using the wrong format for specifying ICMP type. The test cases
> did not include any macros utilizing the ICMP protocol. Add them to
> catch any errors related to ICMP parsing in the future.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> Depends on bumped proxmox-ve-config to work.
> 
>   proxmox-firewall/tests/input/host.fw          |  1 +
>   .../integration_tests__firewall.snap          | 57 ++++++++++++++++++-
>   2 files changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/proxmox-firewall/tests/input/host.fw b/proxmox-firewall/tests/input/host.fw
> index a61b0bd..ddfcb1c 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 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 9194fc6..d25ece8 100644
> --- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
> +++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
> @@ -1,7 +1,6 @@
>   ---
>   source: proxmox-firewall/tests/integration_tests.rs
>   expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
> -snapshot_kind: text
>   ---
>   {
>     "nftables": [
> @@ -3533,6 +3532,62 @@ snapshot_kind: text
>           }
>         }
>       },
> +    {
> +      "add": {
> +        "rule": {
> +          "family": "inet",
> +          "table": "proxmox-firewall",
> +          "chain": "host-in",
> +          "expr": [
> +            {
> +              "match": {
> +                "op": "==",
> +                "left": {
> +                  "payload": {
> +                    "protocol": "icmp",
> +                    "field": "type"
> +                  }
> +                },
> +                "right": "echo-request"
> +              }
> +            },
> +            {
> +              "jump": {
> +                "target": "do-reject"
> +              }
> +            }
> +          ]
> +        }
> +      }
> +    },
> +    {
> +      "add": {
> +        "rule": {
> +          "family": "inet",
> +          "table": "proxmox-firewall",
> +          "chain": "host-in",
> +          "expr": [
> +            {
> +              "match": {
> +                "op": "==",
> +                "left": {
> +                  "payload": {
> +                    "protocol": "icmpv6",
> +                    "field": "type"
> +                  }
> +                },
> +                "right": "echo-request"
> +              }
> +            },
> +            {
> +              "jump": {
> +                "target": "do-reject"
> +              }
> +            }
> +          ]
> +        }
> +      }
> +    },
>       {
>         "add": {
>           "rule": {



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

Re: [pve-devel] [PATCH proxmox-ve-rs 1/3] firewall macros: fix macros using icmp protocol

[Thomas Lamprecht]

On Tue, 04 Feb 2025 10:57:31 +0100, Stefan Hanreich wrote:
> Macros containing rules for the ICMP protocol used dport instead of
> icmp-type for specifying the type of ICMP messages. This is how
> pve-firewall used to specify them, but the nftables firewall uses a
> separate key for this in the macros. This caused all ICMP types to be
> allowed instead of restricting them to the types specified in the
> macro.
> 
> [...]

Applied, thanks!

[1/3] firewall macros: fix macros using icmp protocol
      commit: 2a32eee42fa3fa90616666477cdbf3da5cbfae8f
[2/3] fix #6108: firewall macros: Add missing ICMPv6 statements
      commit: 8f2f3c62954d4052cd08a56993cf006773e760af
[3/3] tests: add Ping macro to tests
      commit: 75d2c9793c49a5e030d68237a1734d6c922e80cf


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