[pve-devel] [PATCH proxmox-firewall 1/2] firewall: improve handling of ARP traffic for guests

[pve-devel] [PATCH proxmox-firewall 1/2] firewall: improve handling of ARP traffic for guests

[Stefan Hanreich]

In order to be able to send outgoing ARP packets when the default
policy is set to drop or reject, we need to explicitly allow ARP
traffic in the outgoing chain of guests. We need to do this in the
guest chain itself in order to be able to filter spoofed packets via
the MAC filter.

Contrary to the out direction we can simply accept all incoming ARP
traffic, since we do not do any MAC filtering for incoming traffic.
Since we create fdb entries for every NIC, guests should only see ARP
traffic for their MAC addresses anyway.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Originally-by: Laurent Guerby <laurent@guerby.net>
---
 proxmox-firewall/resources/proxmox-firewall.nft           | 1 +
 proxmox-firewall/src/firewall.rs                          | 8 ++++----
 .../tests/snapshots/integration_tests__firewall.snap      | 4 ++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/proxmox-firewall/resources/proxmox-firewall.nft b/proxmox-firewall/resources/proxmox-firewall.nft
index f36bf3b..90b5d5a 100644
--- a/proxmox-firewall/resources/proxmox-firewall.nft
+++ b/proxmox-firewall/resources/proxmox-firewall.nft
@@ -305,6 +305,7 @@ table bridge proxmox-firewall-guests {
 
     chain vm-in {
         type filter hook postrouting priority 0; policy accept;
+        ether type arp accept
         oifname vmap @vm-map-in
     }
 }
diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs
index 41b1df2..0da3ab7 100644
--- a/proxmox-firewall/src/firewall.rs
+++ b/proxmox-firewall/src/firewall.rs
@@ -516,7 +516,7 @@ impl Firewall {
 
         commands.append(&mut vec![
             Add::rule(AddRule::from_statement(
-                chain_in.clone(),
+                chain_in,
                 Statement::jump(ndp_chains.0),
             )),
             Add::rule(AddRule::from_statement(
@@ -532,17 +532,17 @@ impl Firewall {
         };
 
         commands.push(Add::rule(AddRule::from_statement(
-            chain_out,
+            chain_out.clone(),
             Statement::jump(ra_chain_out),
         )));
 
-        // we allow incoming ARP by default, except if blocked by any option above
+        // we allow outgoing ARP, except if blocked by the MAC filter above
         let arp_rule = vec![
             Match::new_eq(Payload::field("ether", "type"), Expression::from("arp")).into(),
             Statement::make_accept(),
         ];
 
-        commands.push(Add::rule(AddRule::from_statements(chain_in, arp_rule)));
+        commands.push(Add::rule(AddRule::from_statements(chain_out, arp_rule)));
 
         Ok(())
     }
diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
index 092ccef..2ca151f 100644
--- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
+++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
@@ -2923,7 +2923,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
         "rule": {
           "family": "bridge",
           "table": "proxmox-firewall-guests",
-          "chain": "guest-100-in",
+          "chain": "guest-100-out",
           "expr": [
             {
               "match": {
@@ -3569,7 +3569,7 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
         "rule": {
           "family": "bridge",
           "table": "proxmox-firewall-guests",
-          "chain": "guest-101-in",
+          "chain": "guest-101-out",
           "expr": [
             {
               "match": {
-- 
2.39.2


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

[pve-devel] [PATCH proxmox-firewall 2/2] firewall: improve conntrack handling

[Stefan Hanreich]

The output chain did not have any conntrack rules, which lead to
issues when the default output policy is not accept. Also, move the
conntrack rules to the beginning of all chains.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Originally-by: Laurent Guerby <laurent@guerby.net>
---
Based this on the earlier patch in order to avoid conflicts when
applying both patches.

 .../resources/proxmox-firewall.nft            |  9 ++----
 proxmox-firewall/src/firewall.rs              |  7 ----
 .../integration_tests__firewall.snap          | 32 -------------------
 3 files changed, 2 insertions(+), 46 deletions(-)

diff --git a/proxmox-firewall/resources/proxmox-firewall.nft b/proxmox-firewall/resources/proxmox-firewall.nft
index 90b5d5a..411e143 100644
--- a/proxmox-firewall/resources/proxmox-firewall.nft
+++ b/proxmox-firewall/resources/proxmox-firewall.nft
@@ -32,7 +32,6 @@ add chain bridge proxmox-firewall-guests allow-ndp-out
 add chain bridge proxmox-firewall-guests block-ndp-out
 add chain bridge proxmox-firewall-guests allow-ra-out
 add chain bridge proxmox-firewall-guests block-ra-out
-add chain bridge proxmox-firewall-guests after-vm-in
 add chain bridge proxmox-firewall-guests do-reject
 add chain bridge proxmox-firewall-guests vm-out {type filter hook prerouting priority 0; policy accept;}
 add chain bridge proxmox-firewall-guests vm-in {type filter hook postrouting priority 0; policy accept;}
@@ -64,7 +63,6 @@ flush chain bridge proxmox-firewall-guests allow-ndp-out
 flush chain bridge proxmox-firewall-guests block-ndp-out
 flush chain bridge proxmox-firewall-guests allow-ra-out
 flush chain bridge proxmox-firewall-guests block-ra-out
-flush chain bridge proxmox-firewall-guests after-vm-in
 flush chain bridge proxmox-firewall-guests do-reject
 flush chain bridge proxmox-firewall-guests vm-out
 flush chain bridge proxmox-firewall-guests vm-in
@@ -293,18 +291,15 @@ table bridge proxmox-firewall-guests {
         reject with icmp type host-prohibited
     }
 
-    chain after-vm-in {
-        ct state established,related accept
-        ether type != arp ct state invalid drop
-    }
-
     chain vm-out {
         type filter hook prerouting priority 0; policy accept;
+        ether type != arp ct state vmap { established : accept, related : accept, invalid : drop }
         iifname vmap @vm-map-out
     }
 
     chain vm-in {
         type filter hook postrouting priority 0; policy accept;
+        ether type != arp ct state vmap { established : accept, related : accept, invalid : drop }
         ether type arp accept
         oifname vmap @vm-map-in
     }
diff --git a/proxmox-firewall/src/firewall.rs b/proxmox-firewall/src/firewall.rs
index 0da3ab7..4c85ea2 100644
--- a/proxmox-firewall/src/firewall.rs
+++ b/proxmox-firewall/src/firewall.rs
@@ -810,13 +810,6 @@ impl Firewall {
             )));
         }
 
-        if direction == Direction::In {
-            commands.push(Add::rule(AddRule::from_statement(
-                chain.clone(),
-                Statement::jump("after-vm-in"),
-            )));
-        }
-
         self.create_log_rule(
             commands,
             config.log_level(direction),
diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
index 2ca151f..669bad9 100644
--- a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
+++ b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
@@ -3181,22 +3181,6 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
         }
       }
     },
-    {
-      "add": {
-        "rule": {
-          "family": "bridge",
-          "table": "proxmox-firewall-guests",
-          "chain": "guest-100-in",
-          "expr": [
-            {
-              "jump": {
-                "target": "after-vm-in"
-              }
-            }
-          ]
-        }
-      }
-    },
     {
       "add": {
         "rule": {
@@ -3638,22 +3622,6 @@ expression: "firewall.full_host_fw().expect(\"firewall can be generated\")"
         }
       }
     },
-    {
-      "add": {
-        "rule": {
-          "family": "bridge",
-          "table": "proxmox-firewall-guests",
-          "chain": "guest-101-in",
-          "expr": [
-            {
-              "jump": {
-                "target": "after-vm-in"
-              }
-            }
-          ]
-        }
-      }
-    },
     {
       "add": {
         "rule": {
-- 
2.39.2


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

[pve-devel] applied-series: [PATCH proxmox-firewall 1/2] firewall: improve handling of ARP traffic for guests

[Thomas Lamprecht]

Am 15/05/2024 um 15:37 schrieb Stefan Hanreich:
> In order to be able to send outgoing ARP packets when the default
> policy is set to drop or reject, we need to explicitly allow ARP
> traffic in the outgoing chain of guests. We need to do this in the
> guest chain itself in order to be able to filter spoofed packets via
> the MAC filter.
> 
> Contrary to the out direction we can simply accept all incoming ARP
> traffic, since we do not do any MAC filtering for incoming traffic.
> Since we create fdb entries for every NIC, guests should only see ARP
> traffic for their MAC addresses anyway.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> Originally-by: Laurent Guerby <laurent@guerby.net>
> ---
>  proxmox-firewall/resources/proxmox-firewall.nft           | 1 +
>  proxmox-firewall/src/firewall.rs                          | 8 ++++----
>  .../tests/snapshots/integration_tests__firewall.snap      | 4 ++--
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
>

applied both patches, thanks!

I reworded the subject here too and re-ordered the git trailers, as they
should have a causal order where possible. I.e., if someone else made a
patch, or helped you to do so, their co-authored-by or originally-by is
normally before your signed-off-by, as your "signature" shows that all
above it is (to your best knowledge) correct w.r.t patch ownership and
description, and like on "real" documents that signature goes at the
bottom.


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