public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH proxmox-firewall v3 1/1] service: flush firewall rules on force disable
@ 2024-07-17 13:16 Stefan Hanreich
  2024-07-22 15:43 ` [pve-devel] applied: " Thomas Lamprecht
  0 siblings, 1 reply; 2+ messages in thread
From: Stefan Hanreich @ 2024-07-17 13:16 UTC (permalink / raw)
  To: pve-devel

When disabling the nftables firewall again, there is a race condition
where the nftables ruleset never gets flushed and persists after
disabling.

The nftables firewall update loop does a noop when the force disable
file exists. It only flushes the ruleset when nftables is disabled in
the configuration file but the force disable file does not yet exist.

This can lead to the following situation:

* nftables is activated and created its ruleset
* user switches from nftables firewall back to iptables firewall
* pve-firewall runs and creates the force disable file
* proxmox-firewall sees that the file exists and does nothing

Reported-by: Hannes Laimer <h.laimer@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
Changes from v2 to v3:
* Use proper debug output formatter

Changes from v1 to v2:
* Removed misleading/wrong section about the probability of this
  happening
* Added a detailed description of the scenario this commit prevents

 proxmox-firewall/src/bin/proxmox-firewall.rs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/proxmox-firewall/src/bin/proxmox-firewall.rs b/proxmox-firewall/src/bin/proxmox-firewall.rs
index f7e816e..4732e51 100644
--- a/proxmox-firewall/src/bin/proxmox-firewall.rs
+++ b/proxmox-firewall/src/bin/proxmox-firewall.rs
@@ -91,6 +91,10 @@ fn main() -> Result<(), std::io::Error> {
 
     while !term.load(Ordering::Relaxed) {
         if force_disable_flag.exists() {
+            if let Err(error) = remove_firewall() {
+                log::error!("unable to disable firewall: {error:?}");
+            }
+
             std::thread::sleep(Duration::from_secs(5));
             continue;
         }
-- 
2.39.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] 2+ messages in thread

* [pve-devel] applied: [PATCH proxmox-firewall v3 1/1] service: flush firewall rules on force disable
  2024-07-17 13:16 [pve-devel] [PATCH proxmox-firewall v3 1/1] service: flush firewall rules on force disable Stefan Hanreich
@ 2024-07-22 15:43 ` Thomas Lamprecht
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Lamprecht @ 2024-07-22 15:43 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hanreich

Am 17/07/2024 um 15:16 schrieb Stefan Hanreich:
> When disabling the nftables firewall again, there is a race condition
> where the nftables ruleset never gets flushed and persists after
> disabling.
> 
> The nftables firewall update loop does a noop when the force disable
> file exists. It only flushes the ruleset when nftables is disabled in
> the configuration file but the force disable file does not yet exist.
> 
> This can lead to the following situation:
> 
> * nftables is activated and created its ruleset
> * user switches from nftables firewall back to iptables firewall
> * pve-firewall runs and creates the force disable file
> * proxmox-firewall sees that the file exists and does nothing
> 
> Reported-by: Hannes Laimer <h.laimer@proxmox.com>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> Changes from v2 to v3:
> * Use proper debug output formatter
> 
> Changes from v1 to v2:
> * Removed misleading/wrong section about the probability of this
>   happening
> * Added a detailed description of the scenario this commit prevents
> 
>  proxmox-firewall/src/bin/proxmox-firewall.rs | 4 ++++
>  1 file changed, 4 insertions(+)
> 
>

applied, thanks!


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


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

end of thread, other threads:[~2024-07-22 15:43 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-17 13:16 [pve-devel] [PATCH proxmox-firewall v3 1/1] service: flush firewall rules on force disable Stefan Hanreich
2024-07-22 15:43 ` [pve-devel] applied: " Thomas Lamprecht

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