Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Status toggle button wipes out gufw rules from the profile, and reloads them from ufw #54

Closed
dominikborkowski opened this issue Sep 28, 2022 · 3 comments

Comments

@dominikborkowski
Copy link
Contributor

This is a baffling behavior: turning the firewall off with the toggle button next to Status wipes out all gufw rules from its profile file. Toggling it back on reads them from UFW.

This wouldn't be a problem if not for the fact that Gufw cannot modify rules that originate from UFW, only remove them. So by a mere act of disabling/renabling firewall, user is left with immutable rules. Unless I'm missing something obvious, this is hardly a desirable behavior.

This was tested on latest Ubuntu 22. I believe there is Ubuntu bug that was filed over a year ago, but there seems to be no traction: https://bugs.launchpad.net/gui-ufw/+bug/1956171

@dominikborkowski
Copy link
Contributor Author

Found the culprit of this behavior, and proposed a fix via this PR: #55

@deadwood2
Copy link

Here is my take at fixing this bug. As far as I can see this is related to wrong state management. Writing profile at status change requires reading rules. Reading ufw rules requires issuing /usr/sbin/ufw status numbered. This will only return rules if ufw is enabled. However in current code, we first disable ufw and only then try to read the rules which results with empty list of rules and no matching to profiles rules. Then we save empty list, wiping out all profile rules.

--- gufw/gufw/model/firewall.py	2023-04-04 17:54:30.882633595 +0200
+++ firewall.py	2023-04-04 17:58:03.000000000 +0200
@@ -131,8 +131,18 @@
     
     def set_status(self, status):
         self.status = status
+
+        if status == True:
+            self.backend.set_status(status)
+
+        # get_rules() need to be called on enabled firewall, otherwise they will
+        # wipe out profile rules, because the colleciton of ufw_rules will be
+        # empty
         self.backend.set_profile_values(self.profile, status, self.incoming, self.outgoing, self.routed, self.get_rules(False))
-        self.backend.set_status(status)
+
+        if status == False:
+            self.backend.set_status(status)
+
     
     def get_policy(self, policy):
         if policy == 'incoming':

@costales
Copy link
Owner

Fixed in next release. Thank you for the PR!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants