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

[RFE]: policy/zone: Keep service sorted to make it easier to spot changes (diffs) #745

Open
mbiebl opened this issue Jan 12, 2021 · 2 comments
Labels
feature New feature or enhancement.

Comments

@mbiebl
Copy link
Contributor

mbiebl commented Jan 12, 2021

Originally filed as downstream bug report https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=833527

I noticed this on the FreedomBox, where etckeeper store every change in
/etc/ in git. After configuring Tor on the test machine, 'git diff'
show this set of changes to the /etc/firewalld/zones/external.xml file:

diff --git a/firewalld/zones/external.xml b/firewalld/zones/external.xml
index 73c852a..99911e8 100644
--- a/firewalld/zones/external.xml
+++ b/firewalld/zones/external.xml
@@ -2,11 +2,14 @@
 <zone>
   <short>External</short>
   <description>For use on external networks. You do not trust the other comput
ers on networks to not harm your computer. Only selected incoming connections a
re accepted.</description>
-  <service name="xmpp-server"/>
-  <service name="https"/>
   <service name="http"/>
+  <service name="tor-obfs3"/>
+  <service name="tor-obfs4"/>
+  <service name="https"/>
   <service name="xmpp-bosh"/>
   <service name="xmpp-client"/>
+  <service name="tor-orport"/>
+  <service name="xmpp-server"/>
   <service name="ssh"/>
   <masquerade/>
 </zone>

It is harder than it had to be to notice what was added and figuring out
which services were only moved in the list. Please change firewalld to
keep the content of its files sorted, to make it possible for etckeeper
to only report changes to the files.

@erig0
Copy link
Collaborator

erig0 commented Jan 12, 2021

I think that's fair. It should also be a very simple change. Below is probably sufficient.

diff --git a/src/firewall/core/io/policy.py b/src/firewall/core/io/policy.py
index 8de7604a0fb2..eb7961363b4c 100644
--- a/src/firewall/core/io/policy.py
+++ b/src/firewall/core/io/policy.py
@@ -395,7 +395,7 @@ def common_writer(obj, handler):
         handler.ignorableWhitespace("\n")
 
     # services
-    for service in uniqify(obj.services):
+    for service in sorted(uniqify(obj.services)):
         handler.ignorableWhitespace("  ")
         handler.simpleElement("service", { "name": service })
         handler.ignorableWhitespace("\n")

@erig0
Copy link
Collaborator

erig0 commented Jan 12, 2021

There are a lot of these quality of life enhancements that could be done. Just off the top of my head:

  • sort rich rules by priority
  • sort ports by port number
  • sort protocols by name
  • sort elements by element name

@erig0 erig0 changed the title Keep service lists sorted to make it easier to spot changes? [RFE]: policy/zone: Keep service sorted to make it easier to spot changes (diffs) Jan 12, 2021
@erig0 erig0 added the feature New feature or enhancement. label Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or enhancement.
Projects
None yet
Development

No branches or pull requests

2 participants