public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-firewall 1/2] fix: firewall: apply `nt_conntrack_allow_invalid` to all chains
@ 2025-02-20 15:12 Hannes Laimer
  2025-02-20 15:12 ` [pbs-devel] [PATCH proxmox-firewall 2/2] firewall: apply `nt_conntrack_allow_invalid` option to host table Hannes Laimer
  2025-02-20 15:13 ` [pbs-devel] [PATCH proxmox-firewall 1/2] fix: firewall: apply `nt_conntrack_allow_invalid` to all chains Hannes Laimer
  0 siblings, 2 replies; 3+ messages in thread
From: Hannes Laimer @ 2025-02-20 15:12 UTC (permalink / raw)
  To: pbs-devel

... on the guest table. There is no reason to not repect that option
on those two chains. These two were missed in the referenced commit.

Fixes: 64dc344b ("firewall: apply `nt_conntrack_allow_invalid` option to guest table")
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 proxmox-firewall/resources/proxmox-firewall.nft | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/proxmox-firewall/resources/proxmox-firewall.nft b/proxmox-firewall/resources/proxmox-firewall.nft
index 2dd7c48..30f7b4f 100644
--- a/proxmox-firewall/resources/proxmox-firewall.nft
+++ b/proxmox-firewall/resources/proxmox-firewall.nft
@@ -356,7 +356,7 @@ table bridge proxmox-firewall-guests {
     }
 
     chain pre-vm-out {
-        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop }
+        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : jump invalid-conntrack }
     }
 
     chain vm-out {
@@ -384,7 +384,7 @@ table bridge proxmox-firewall-guests {
 
     chain before-bridge {
         meta protocol arp accept
-        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop }
+        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : jump invalid-conntrack }
     }
 
     chain forward {
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [pbs-devel] [PATCH proxmox-firewall 2/2] firewall: apply `nt_conntrack_allow_invalid` option to host table
  2025-02-20 15:12 [pbs-devel] [PATCH proxmox-firewall 1/2] fix: firewall: apply `nt_conntrack_allow_invalid` to all chains Hannes Laimer
@ 2025-02-20 15:12 ` Hannes Laimer
  2025-02-20 15:13 ` [pbs-devel] [PATCH proxmox-firewall 1/2] fix: firewall: apply `nt_conntrack_allow_invalid` to all chains Hannes Laimer
  1 sibling, 0 replies; 3+ messages in thread
From: Hannes Laimer @ 2025-02-20 15:12 UTC (permalink / raw)
  To: pbs-devel

... on all chains that check for ct state. Since we support this option,
we should also use it in our firewall rule generation.

This is a follow-up to
  64dc344b ("firewall: apply `nt_conntrack_allow_invalid` option to guest table")

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 .../resources/proxmox-firewall.nft            | 15 +++++-------
 proxmox-firewall/src/firewall.rs              | 11 ++++++---
 .../integration_tests__firewall.snap          | 23 ++++++++++++-------
 3 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/proxmox-firewall/resources/proxmox-firewall.nft b/proxmox-firewall/resources/proxmox-firewall.nft
index 30f7b4f..455d1c3 100644
--- a/proxmox-firewall/resources/proxmox-firewall.nft
+++ b/proxmox-firewall/resources/proxmox-firewall.nft
@@ -14,7 +14,6 @@ add chain inet proxmox-firewall allow-ndp-in
 add chain inet proxmox-firewall block-ndp-in
 add chain inet proxmox-firewall allow-ndp-out
 add chain inet proxmox-firewall block-ndp-out
-add chain inet proxmox-firewall block-conntrack-invalid
 add chain inet proxmox-firewall block-smurfs
 add chain inet proxmox-firewall allow-icmp
 add chain inet proxmox-firewall log-drop-smurfs
@@ -55,7 +54,6 @@ flush chain inet proxmox-firewall allow-ndp-in
 flush chain inet proxmox-firewall block-ndp-in
 flush chain inet proxmox-firewall allow-ndp-out
 flush chain inet proxmox-firewall block-ndp-out
-flush chain inet proxmox-firewall block-conntrack-invalid
 flush chain inet proxmox-firewall block-smurfs
 flush chain inet proxmox-firewall allow-icmp
 flush chain inet proxmox-firewall log-drop-smurfs
@@ -176,10 +174,6 @@ table inet proxmox-firewall {
         icmpv6 type { nd-router-solicit, nd-neighbor-solicit, nd-neighbor-advert } drop
     }
 
-    chain block-conntrack-invalid {
-        ct state invalid drop
-    }
-
     chain block-smurfs {
         ip saddr 0.0.0.0/32 return
         meta pkttype broadcast goto log-drop-smurfs
@@ -229,7 +223,7 @@ table inet proxmox-firewall {
         oifname "lo" accept
 
         jump allow-icmp
-        ct state vmap { invalid : drop, established : accept, related : accept }
+        ct state vmap { invalid : jump invalid-conntrack, established : accept, related : accept }
     }
 
     chain option-in {}
@@ -241,7 +235,7 @@ table inet proxmox-firewall {
 
     chain before-bridge {
         meta protocol arp accept
-        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop }
+        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : jump invalid-conntrack }
     }
 
     chain host-bridge-input {
@@ -284,9 +278,12 @@ table inet proxmox-firewall {
     chain host-out {}
 
     chain cluster-forward {}
-    chain host-forward {}
+    chain host-forward {
+        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : jump invalid-conntrack }
+    }
 
     chain ct-in {}
+    chain invalid-conntrack { }
 }
 
 table bridge proxmox-firewall-guests {
diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs
index 88fb460..607fc75 100644
--- a/proxmox-firewall/src/firewall.rs
+++ b/proxmox-firewall/src/firewall.rs
@@ -99,6 +99,10 @@ impl Firewall {
         ChainPart::new(Self::guest_table(), "invalid-conntrack".to_string())
     }
 
+    fn host_invalid_conntrack_chain() -> ChainPart {
+        ChainPart::new(Self::host_table(), "invalid-conntrack".to_string())
+    }
+
     fn host_conntrack_chain() -> ChainPart {
         ChainPart::new(Self::host_table(), "ct-in".to_string())
     }
@@ -144,6 +148,7 @@ impl Firewall {
             Flush::chain(Self::host_option_chain(Direction::Out)),
             Flush::chain(Self::host_chain(Direction::Forward)),
             Flush::chain(Self::guest_invalid_conntrack_chain()),
+            Flush::chain(Self::host_invalid_conntrack_chain()),
             Flush::map(Self::guest_vmap(Direction::In)),
             Flush::map(Self::guest_vmap(Direction::Out)),
             Flush::map(Self::bridge_vmap(Self::guest_table())),
@@ -533,12 +538,12 @@ impl Firewall {
             log::debug!("set block_invalid_conntrack");
 
             commands.push(Add::rule(AddRule::from_statement(
-                chain_in,
-                Statement::jump("block-conntrack-invalid"),
+                Self::guest_invalid_conntrack_chain(),
+                Statement::make_drop(),
             )));
 
             commands.push(Add::rule(AddRule::from_statement(
-                Self::guest_invalid_conntrack_chain(),
+                Self::host_invalid_conntrack_chain(),
                 Statement::make_drop(),
             )));
         }
diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
index 9194fc6..24f66a5 100644
--- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
+++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
@@ -104,6 +104,15 @@ snapshot_kind: text
         }
       }
     },
+    {
+      "flush": {
+        "chain": {
+          "family": "inet",
+          "table": "proxmox-firewall",
+          "name": "invalid-conntrack"
+        }
+      }
+    },
     {
       "flush": {
         "map": {
@@ -3280,14 +3289,12 @@ snapshot_kind: text
     {
       "add": {
         "rule": {
-          "family": "inet",
-          "table": "proxmox-firewall",
-          "chain": "option-in",
+          "family": "bridge",
+          "table": "proxmox-firewall-guests",
+          "chain": "invalid-conntrack",
           "expr": [
             {
-              "jump": {
-                "target": "block-conntrack-invalid"
-              }
+              "drop": null
             }
           ]
         }
@@ -3296,8 +3303,8 @@ snapshot_kind: text
     {
       "add": {
         "rule": {
-          "family": "bridge",
-          "table": "proxmox-firewall-guests",
+          "family": "inet",
+          "table": "proxmox-firewall",
           "chain": "invalid-conntrack",
           "expr": [
             {
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-firewall 1/2] fix: firewall: apply `nt_conntrack_allow_invalid` to all chains
  2025-02-20 15:12 [pbs-devel] [PATCH proxmox-firewall 1/2] fix: firewall: apply `nt_conntrack_allow_invalid` to all chains Hannes Laimer
  2025-02-20 15:12 ` [pbs-devel] [PATCH proxmox-firewall 2/2] firewall: apply `nt_conntrack_allow_invalid` option to host table Hannes Laimer
@ 2025-02-20 15:13 ` Hannes Laimer
  1 sibling, 0 replies; 3+ messages in thread
From: Hannes Laimer @ 2025-02-20 15:13 UTC (permalink / raw)
  To: pbs-devel

ups, wrong list. sorry for the noise

On 2/20/25 16:12, Hannes Laimer wrote:
> ... on the guest table. There is no reason to not repect that option
> on those two chains. These two were missed in the referenced commit.
> 
> Fixes: 64dc344b ("firewall: apply `nt_conntrack_allow_invalid` option to guest table")
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>   proxmox-firewall/resources/proxmox-firewall.nft | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/proxmox-firewall/resources/proxmox-firewall.nft b/proxmox-firewall/resources/proxmox-firewall.nft
> index 2dd7c48..30f7b4f 100644
> --- a/proxmox-firewall/resources/proxmox-firewall.nft
> +++ b/proxmox-firewall/resources/proxmox-firewall.nft
> @@ -356,7 +356,7 @@ table bridge proxmox-firewall-guests {
>       }
>   
>       chain pre-vm-out {
> -        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop }
> +        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : jump invalid-conntrack }
>       }
>   
>       chain vm-out {
> @@ -384,7 +384,7 @@ table bridge proxmox-firewall-guests {
>   
>       chain before-bridge {
>           meta protocol arp accept
> -        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop }
> +        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : jump invalid-conntrack }
>       }
>   
>       chain forward {



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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-02-20 15:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-20 15:12 [pbs-devel] [PATCH proxmox-firewall 1/2] fix: firewall: apply `nt_conntrack_allow_invalid` to all chains Hannes Laimer
2025-02-20 15:12 ` [pbs-devel] [PATCH proxmox-firewall 2/2] firewall: apply `nt_conntrack_allow_invalid` option to host table Hannes Laimer
2025-02-20 15:13 ` [pbs-devel] [PATCH proxmox-firewall 1/2] fix: firewall: apply `nt_conntrack_allow_invalid` to all chains Hannes Laimer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal