public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH proxmox-firewall 1/4] cargo: bump dependencies
@ 2025-01-23 10:12 Stefan Hanreich
  2025-01-23 10:12 ` [pve-devel] [PATCH proxmox-firewall 2/4] debian: remove dependency on proxmox-schema Stefan Hanreich
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Stefan Hanreich @ 2025-01-23 10:12 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 Cargo.toml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Cargo.toml b/Cargo.toml
index 31d15be..079fb79 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -6,4 +6,4 @@ members = [
 resolver = "2"
 
 [workspace.dependencies]
-proxmox-ve-config = { version = "0.2.0" }
+proxmox-ve-config = { version = "0.2.2" }
-- 
2.39.5


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


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

* [pve-devel] [PATCH proxmox-firewall 2/4] debian: remove dependency on proxmox-schema
  2025-01-23 10:12 [pve-devel] [PATCH proxmox-firewall 1/4] cargo: bump dependencies Stefan Hanreich
@ 2025-01-23 10:12 ` Stefan Hanreich
  2025-01-24 14:06   ` Hannes Dürr
  2025-01-23 10:12 ` [pve-devel] [PATCH proxmox-firewall 3/4] security groups: skip in forward chain when interface is specified Stefan Hanreich
  2025-01-23 10:13 ` [pve-devel] [PATCH proxmox-firewall 4/4] tests: add test for security groups in cluster config Stefan Hanreich
  2 siblings, 1 reply; 7+ messages in thread
From: Stefan Hanreich @ 2025-01-23 10:12 UTC (permalink / raw)
  To: pve-devel

Nothing in this workspace directly depends on it, so any library using
this would have to pull it in themselves.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 debian/control | 1 -
 1 file changed, 1 deletion(-)

diff --git a/debian/control b/debian/control
index ea2401b..7353ed8 100644
--- a/debian/control
+++ b/debian/control
@@ -10,7 +10,6 @@ Build-Depends: cargo:native,
                librust-libc-0.2+default-dev,
                librust-log-0.4+default-dev (>= 0.4.17-~~),
                librust-nix-0.26+default-dev (>= 0.26.1-~~),
-               librust-proxmox-schema-3+default-dev (>= 3.1.2-~~),
                librust-proxmox-sortable-macro-dev,
                librust-proxmox-sys-dev (>= 0.6~),
                librust-proxmox-ve-config-dev (>= 0.2~),
-- 
2.39.5


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


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

* [pve-devel] [PATCH proxmox-firewall 3/4] security groups: skip in forward chain when interface is specified
  2025-01-23 10:12 [pve-devel] [PATCH proxmox-firewall 1/4] cargo: bump dependencies Stefan Hanreich
  2025-01-23 10:12 ` [pve-devel] [PATCH proxmox-firewall 2/4] debian: remove dependency on proxmox-schema Stefan Hanreich
@ 2025-01-23 10:12 ` Stefan Hanreich
  2025-01-24 14:35   ` Hannes Dürr
  2025-01-23 10:13 ` [pve-devel] [PATCH proxmox-firewall 4/4] tests: add test for security groups in cluster config Stefan Hanreich
  2 siblings, 1 reply; 7+ messages in thread
From: Stefan Hanreich @ 2025-01-23 10:12 UTC (permalink / raw)
  To: pve-devel

Security groups can be bound to a specific interface. The notion of
this breaks down when considering the forward direction, since there
are two interfaces involved: incoming and outgoing, which can be
different depending on the kind of traffic.

With the current implementation, the firewall refuses to generate
rulesets with security groups that are bound to specific interfaces.
Check for this case explicitly and skip creating rules in the forward
chain when a security group bound to a specific interface is
encountered.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-firewall/src/rule.rs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/proxmox-firewall/src/rule.rs b/proxmox-firewall/src/rule.rs
index b20a9c5..14ee544 100644
--- a/proxmox-firewall/src/rule.rs
+++ b/proxmox-firewall/src/rule.rs
@@ -201,6 +201,10 @@ fn handle_iface(rules: &mut [NftRule], env: &NftRuleEnv, name: &str) -> Result<(
 
 impl ToNftRules for RuleGroup {
     fn to_nft_rules(&self, rules: &mut Vec<NftRule>, env: &NftRuleEnv) -> Result<(), Error> {
+        if env.direction == Direction::Forward && self.iface().is_some() {
+            return Ok(());
+        }
+
         let chain_name = format!("group-{}-{}", self.group(), env.direction);
 
         rules.push(NftRule::new(Statement::jump(chain_name)));
-- 
2.39.5


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


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

* [pve-devel] [PATCH proxmox-firewall 4/4] tests: add test for security groups in cluster config
  2025-01-23 10:12 [pve-devel] [PATCH proxmox-firewall 1/4] cargo: bump dependencies Stefan Hanreich
  2025-01-23 10:12 ` [pve-devel] [PATCH proxmox-firewall 2/4] debian: remove dependency on proxmox-schema Stefan Hanreich
  2025-01-23 10:12 ` [pve-devel] [PATCH proxmox-firewall 3/4] security groups: skip in forward chain when interface is specified Stefan Hanreich
@ 2025-01-23 10:13 ` Stefan Hanreich
  2 siblings, 0 replies; 7+ messages in thread
From: Stefan Hanreich @ 2025-01-23 10:13 UTC (permalink / raw)
  To: pve-devel

There was a bug where rulesets with security groups bound to a
specific interface would cause the firewall to fail to create a new
ruleset. Catch this by adding a security group bound to an interface
to the ruleset.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-firewall/tests/input/cluster.fw       |  1 +
 .../integration_tests__firewall.snap          | 55 ++++++++++++++++++-
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/proxmox-firewall/tests/input/cluster.fw b/proxmox-firewall/tests/input/cluster.fw
index 23168ae..3be7a72 100644
--- a/proxmox-firewall/tests/input/cluster.fw
+++ b/proxmox-firewall/tests/input/cluster.fw
@@ -18,6 +18,7 @@ dc/network1
 
 [RULES]
 
+GROUP network1 -i eth0
 IN ACCEPT -log nolog
 
 [group network1]
diff --git a/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap b/proxmox-firewall/tests/snapshots/integration_tests__firewall.snap
index 9194fc6..4a0398d 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": [
@@ -1848,6 +1847,33 @@ snapshot_kind: text
         }
       }
     },
+    {
+      "add": {
+        "rule": {
+          "family": "inet",
+          "table": "proxmox-firewall",
+          "chain": "cluster-in",
+          "expr": [
+            {
+              "match": {
+                "op": "==",
+                "left": {
+                  "meta": {
+                    "key": "iifname"
+                  }
+                },
+                "right": "eth0"
+              }
+            },
+            {
+              "jump": {
+                "target": "group-network1-in"
+              }
+            }
+          ]
+        }
+      }
+    },
     {
       "add": {
         "rule": {
@@ -1900,6 +1926,33 @@ snapshot_kind: text
         }
       }
     },
+    {
+      "add": {
+        "rule": {
+          "family": "inet",
+          "table": "proxmox-firewall",
+          "chain": "cluster-out",
+          "expr": [
+            {
+              "match": {
+                "op": "==",
+                "left": {
+                  "meta": {
+                    "key": "oifname"
+                  }
+                },
+                "right": "eth0"
+              }
+            },
+            {
+              "jump": {
+                "target": "group-network1-out"
+              }
+            }
+          ]
+        }
+      }
+    },
     {
       "add": {
         "rule": {
-- 
2.39.5


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


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

* Re: [pve-devel] [PATCH proxmox-firewall 2/4] debian: remove dependency on proxmox-schema
  2025-01-23 10:12 ` [pve-devel] [PATCH proxmox-firewall 2/4] debian: remove dependency on proxmox-schema Stefan Hanreich
@ 2025-01-24 14:06   ` Hannes Dürr
  2025-01-24 14:23     ` Stefan Hanreich
  0 siblings, 1 reply; 7+ messages in thread
From: Hannes Dürr @ 2025-01-24 14:06 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hanreich

this doesn't apply anymore on master

On 1/23/25 11:12, Stefan Hanreich wrote:
> Nothing in this workspace directly depends on it, so any library using
> this would have to pull it in themselves.
>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>   debian/control | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/debian/control b/debian/control
> index ea2401b..7353ed8 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -10,7 +10,6 @@ Build-Depends: cargo:native,
>                  librust-libc-0.2+default-dev,
>                  librust-log-0.4+default-dev (>= 0.4.17-~~),
>                  librust-nix-0.26+default-dev (>= 0.26.1-~~),
> -               librust-proxmox-schema-3+default-dev (>= 3.1.2-~~),
>                  librust-proxmox-sortable-macro-dev,
>                  librust-proxmox-sys-dev (>= 0.6~),
>                  librust-proxmox-ve-config-dev (>= 0.2~),


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


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

* Re: [pve-devel] [PATCH proxmox-firewall 2/4] debian: remove dependency on proxmox-schema
  2025-01-24 14:06   ` Hannes Dürr
@ 2025-01-24 14:23     ` Stefan Hanreich
  0 siblings, 0 replies; 7+ messages in thread
From: Stefan Hanreich @ 2025-01-24 14:23 UTC (permalink / raw)
  To: Hannes Dürr, Proxmox VE development discussion

On 1/24/25 15:06, Hannes Dürr wrote:
> this doesn't apply anymore on master

You can skip this commit, there is another from Christoph which removed
even more unused dependencies


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

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

* Re: [pve-devel] [PATCH proxmox-firewall 3/4] security groups: skip in forward chain when interface is specified
  2025-01-23 10:12 ` [pve-devel] [PATCH proxmox-firewall 3/4] security groups: skip in forward chain when interface is specified Stefan Hanreich
@ 2025-01-24 14:35   ` Hannes Dürr
  0 siblings, 0 replies; 7+ messages in thread
From: Hannes Dürr @ 2025-01-24 14:35 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hanreich

Tested this:
* enable nftables firewall
* create a security group
* insert the security group to host firewall with interface vmbr0
* enable vm firewall
* insert the security group to vm firewall with interface net0
* check for errors with journalctl -f

no more errors occur, please consider this

Tested-by: Hannes Duerr <h.duerr@proxmox.com>

On 1/23/25 11:12, Stefan Hanreich wrote:
> Security groups can be bound to a specific interface. The notion of
> this breaks down when considering the forward direction, since there
> are two interfaces involved: incoming and outgoing, which can be
> different depending on the kind of traffic.
>
> With the current implementation, the firewall refuses to generate
> rulesets with security groups that are bound to specific interfaces.
> Check for this case explicitly and skip creating rules in the forward
> chain when a security group bound to a specific interface is
> encountered.
>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>   proxmox-firewall/src/rule.rs | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/proxmox-firewall/src/rule.rs b/proxmox-firewall/src/rule.rs
> index b20a9c5..14ee544 100644
> --- a/proxmox-firewall/src/rule.rs
> +++ b/proxmox-firewall/src/rule.rs
> @@ -201,6 +201,10 @@ fn handle_iface(rules: &mut [NftRule], env: &NftRuleEnv, name: &str) -> Result<(
>   
>   impl ToNftRules for RuleGroup {
>       fn to_nft_rules(&self, rules: &mut Vec<NftRule>, env: &NftRuleEnv) -> Result<(), Error> {
> +        if env.direction == Direction::Forward && self.iface().is_some() {
> +            return Ok(());
> +        }
> +
>           let chain_name = format!("group-{}-{}", self.group(), env.direction);
>   
>           rules.push(NftRule::new(Statement::jump(chain_name)));


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


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

end of thread, other threads:[~2025-01-24 14:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-23 10:12 [pve-devel] [PATCH proxmox-firewall 1/4] cargo: bump dependencies Stefan Hanreich
2025-01-23 10:12 ` [pve-devel] [PATCH proxmox-firewall 2/4] debian: remove dependency on proxmox-schema Stefan Hanreich
2025-01-24 14:06   ` Hannes Dürr
2025-01-24 14:23     ` Stefan Hanreich
2025-01-23 10:12 ` [pve-devel] [PATCH proxmox-firewall 3/4] security groups: skip in forward chain when interface is specified Stefan Hanreich
2025-01-24 14:35   ` Hannes Dürr
2025-01-23 10:13 ` [pve-devel] [PATCH proxmox-firewall 4/4] tests: add test for security groups in cluster config Stefan Hanreich

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