docs(xml): Typo

Reviewed-on: #2

README.md

@@ -1,32 +1,89 @@
 # update-firewall-source
 
-Update a firewall rule that relies on dynamic DNS names
+A `firewalld` direct rules generator
 
-# What
+## What
 
-* This script assumes exclusive ownership of the `firewalld` direct rules file `/etc/firewalld/direct.xml` or whereever configured 
+Script `update-firewall-source.py`, UFS for short, assists `firewalld` in writing `iptables`-compatible rules, the so-called _direct_ rules.
 
-* List of address can be empty, direct file will then be removed
+UFS focuses on environments where the following is true:
-* After every execution script will trigger systemd firewalld service restart
+
-* No subnet, will simply not be validated
+1. You're on a Red Hat Enterprise Linux or a derivative operating system
-* Include example systemd unit file and install instructions 
+2. You want to keep using its default firewall management tool `firewalld`
-* firewall-cmd --check-config
+3. You want to use Docker
-* default location for config file and default name for config file
+4. You want published Docker ports to not be accessible from everywhere
-* we should deduplicate list
+
-* is intended to help with just docker-user
+## Why
-* man page https://ipset.netfilter.org/iptables.man.html we do iptables
+
-* added in order
+By installing a moderately modern version of Docker Engine it will very kindly take control of some aspects of firewall rules. If you don't do anything with what Docker gives you the end result is that all ports you publish via Docker (and by extension `docker compose`) are quite literally published to the entire Internet. All source addresses can access published ports on your machine which may not necessarily be desired.
-* related, established? needed?
+
-* section names as comments?
+On the one hand Docker expects you to add custom rules for container access to an `iptables` chain called `DOCKER-USER`. On the other hand Docker does not care for how to limit access to host ports.
-* Comment max 256 chars
+
+UFS handles both container ports and host ports. It largely follows suggestions outlined by [John Michael Carr's August 2017 unrouted.io blog post "Docker meet firewall - finally an answer"](https://unrouted.io/2017/08/15/docker-firewall/).
+
+## How
+
+`update-firewall-source.py` uses a `config.ini` file that may in its simplest form look somewhat like this:
+
+```
+[My home]
+addr = some.dyndns.host.net
+ports = 22, 80, 443
+
+[deny-all]
+target = DROP
+addr =
+ports =
+proto =
+state =
+```
+
+Over in the ['examples' directory](examples) you will find systemd `.service` and `.timer` example files to regularly execute UFS.
+
+Its systemd journal output will look somewhat like this:
+
+```
+systemd[1]:     Starting firewalld direct rules generator...
+python[961809]: Generating rules from section '[My home]' ...
+python[961809]: Verifying address ['some.dyndns.host.net'] ...
+python[961809]: For 'some.dyndns.host.net' found records: ['1.2.3.4', '2606:4700:20::681a:804']
+python[961809]: Adding IPv4 address '1.2.3.4' ...
+python[961809]: For section '[My home]' option 'do_ipv6' equals false. Skipping IPv6 handling of
+python[961809]: 2606:4700:20::681a:804' ...
+python[961809]: Writing new firewalld direct config ...
+python[961809]: Restarting systemd firewalld.service unit ...
+python[961809]: Done
+systemd[1]:     update-firewall-source.service: Succeeded.
+systemd[1]:     Started firewalld direct rules generator.
+```
+
+## Tying it together
+
+A Docker Engine installation nowadays adds the `iptables` chain `DOCKER-USER` which is all well and good. Adding rules to it makes sure that Docker's published ports can only be accessed from where you want.
+
+If you want to cover both Docker containers and the host OS, however, that doesn't fly. UFS adds a chain named `FILTERS`. This chain is called from both `DOCKER-USER` (anything accessing a Docker published port goes this route) **_and_** from the `INPUT` chain (anything headed for the host operating system goes that way).
+
+You only maintain the `FILTERS` chain and don't have to worry about whether an application is unknowingly accessible via public Internet - no matter if that app is a container or a `dnf` package. Even better: UFS does management for you, you just give it a `config.ini` file.
+
+Find more in-depth info on how `ip(6)tables` evolves with UFS down in the ["iptables behind the scenes" section](#iptables-behind-the-scenes).
 
 # Prep
 
-Python dependencies aside make sure that your OS has headers and static libraries for D-Bus GLib bindings installed as well as generic D-Bus development files. On a Rocky Linux 8 installation for example these come via:
+Aside from Python dependencies make sure that your OS has headers and static libraries for D-Bus GLib bindings installed as well as generic D-Bus development files. On a Rocky Linux 8 installation for example these come via:
 ```
 dnf -y install dbus-glib-devel dbus-devel
 ```
 
+This script assumes write access to `firewalld` direct rules file `/etc/firewalld/direct.xml` or whereever else you've configured this file to live. Typically that means you're going to want to run UFS as `root`.
+
+UFS understands the environment variable `UFS_LOGLEVEL` to set its log verbosity. `UFS_LOGLEVEL` defaults to `INFO`, for more verbosity change it to `DEBUG`, for less change it to either `WARNING` or even just `ERROR`. The example systemd `.service` file in ['examples' directory](examples) makes use of the following decleration:
+
+```
+Environment='...' 'UFS_LOGLEVEL=INFO'
+```
+
+Since `UFS_LOGLEVEL=INFO` is default anyway this particular example is redundant and serves as starting point for you.
+
 # Config structure
 
 Package configuration happens via a `config.ini` file that follows INI-style syntax. Copy [examples/config.ini.example](examples/config.ini.example) to `config.ini` to get started:
@@ -53,10 +110,18 @@ addr =
 ports = 80, 443
 proto = tcp
 state = NEW
+hitcount =
 do_ipv6 = false
 firewalld_direct_file_abs = /etc/firewalld/direct.xml
 restart_firewalld_after_change = true
 
+[anyone-may-icmp-with-limit]
+addr =
+ports =
+proto = icmp
+state = NEW,UNTRACKED
+hitcount = 120/60
+
 [anyone-can-access-website]
 
 # Unsetting 'proto' while having a 'ports' value results in an invalid section
@@ -71,8 +136,9 @@ addr = 2606:4700:20::681a:804, lowendtalk.com
 ports = 80, 443
 do_ipv6 = true
 
-[allow-anyone-to-access-mail-services]
+[anyone-may-access-mail-services]
 ports = 143, 993, 110, 995, 25, 465, 587
+hitcount = 120/60
 
 [deny-all]
 target = DROP
@@ -106,7 +172,7 @@ proto =
 state =
 do_ipv6 = true
 ```
-If a packet has traversed rules this far without being accepted it will be dropped. Note that if any of your custom `[sections]` use `do_ipv6 = true` your final `DROP` rule should do the same. Otherwise you'll just get `DROP` rule in `iptables` but not in `ip6tables`.
+If a packet has traversed rules this far without being accepted it will be dropped. Note that if any of your custom `[sections]` use `do_ipv6 = true` your final `DROP` rule should do the same. Otherwise you'll just get a `DROP` rule in `iptables` but not in `ip6tables`.
 
 # Options
 
@@ -122,12 +188,25 @@ In `[DEFAULT]` section the following settings are called globals. They're only v
 
 A custom `[section]` has the following options. We're calling them locals most of which are optional.
 
-* `target`, __*mandatory*__, defaults to `ACCEPT`, can be any valid `iptables` target. Must not be empty nor unset. A string specifying the fate of a packet that matched this rule. See "TARGETS" section in [iptables man page](https://ipset.netfilter.org/iptables.man.html). You're most likely going to want to stick to either `ACCEPT` or `DROP`. By default matching packets are accepted. We do not do our own validation of what you write here. By default (see [Globals](#globals)) `do_config_check` equals to true in which case we let `firewalld` do a config check to catch nonsense rules.
+* `target`, __*mandatory*__, defaults to `ACCEPT`, can be any valid `iptables` target. Must not be empty nor unset. A string specifying the fate of a packet that matched this rule. See "TARGETS" section in [iptables man page](https://ipset.netfilter.org/iptables.man.html). You're most likely going to want to stick to either `ACCEPT` or `DROP`. By default matching packets are accepted. We do not do our own validation of what you write here. `firewalld` will try its best to get your files loaded into `ip(6)tables`. It will complain via its systemd journal if that fails for example because of a bogus target.
 
-* `addr`, __*optional*__, defaults to an empty string: A comma-separated list of any combination of IPv4 addresses, IPv6 addresses and domain names. When `update-firewall-source.py` constructs `firewalld` rules these addresses are allowed to access the server. If left undefined `addr` defaults to an empty list meaning rules apply to any and all source address.
+  ```
+  # Valid example:
+  target = DROP 
+  ```
+
+* `addr`, __*optional*__, defaults to an empty string: A comma-separated list of any combination of IPv4 addresses, IPv6 addresses and domain names. When `update-firewall-source.py` constructs `firewalld` rules these addresses are allowed to access the server. If left undefined `addr` defaults to an empty list meaning rules apply to any and all source addresses.
 
   Subnets are unsupported, both as subnet masks (`142.251.36.195/255.255.255.248`) and in [CIDR](https://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing) notation (`142.251.36.195/29`). Do not single- nor double-quote list entries. Do feel free to separate entries with comma-space instead of just a comma.
 
+  ```
+  # Valid example:
+  addr = 2606:4700:20::681a:804, lowendtalk.com
+  
+  # Also valid (this is the default):
+  addr = 
+  ```
+
 * `ports`, __*optional*__, defaults to `80, 443`: A comma-separated list of ports that should be accessible from `addr`. If empty `addr` may access all ports. See [iptables-extensions man page, section "multiport"](https://ipset.netfilter.org/iptables-extensions.man.html#lbBM) for syntax reference. All port-based rules use `iptables ... --match multiport` even if you're only allowing access to a single port. In essence construct your ports list with any combination of single ports (`80, 443, 8080`) and port ranges (`6660:7000, 61000:65535`).
 
   ```
@@ -186,13 +265,141 @@ A custom `[section]` has the following options. We're calling them locals most o
     ports = 
     proto =
     ```
+  * Protocol strings `icmpv6` and `icmp` are treated specially. You can use either one as your `proto =`, UFS will internally automatically use `icmpv6` for `ip6tables` and will use `icmp` for `iptables` rules.
 
 * `state`, __*optional*__, defaults to `NEW`: Comma-separated list of connection tracking states against which a packet is matched. Most of the time your rules will want to use the default `NEW`. The final `DROP` rule present in the example `config.ini` file at [examples/config.ini.example](examples/config.ini.example) is one occasion where you'll want to deviate and unset `state` to an empty value. See ["state" extension man page in iptables docs](https://ipset.netfilter.org/iptables-extensions.man.html#lbCC) for reference.
 
-* `do_ipv6`, __*optional*__, defaults to `false`: Decide if you want `firewalld` to generate `ip6tables` rules in addition to `iptables` rules. A default install of Docker Engine will have its IPv6 support disabled in `/etc/docker/daemon.json` in which case `ip6tables` will not have a `DOCKER-USER` or similar Docker-related chains. In this default setup having `update-firewall-source.py` generate an otherwise unused `DOCKER-USER` chain and adding rules to it clutters your rule set. Consider setting this to `true` if and when your Docker install uses IPv6.
+  ```
+  # Valid example:
+  state =
+  ```
+
+* `hitcount`, **_optional_**, defaults to an empty value: A rate-limiting feature. Set this to `hits/seconds` to limit the amount of matched packets to `hits` over the course of `seconds`, e.g. `10/60` sets the maximum packet rate to 10 packets over the course of 60 seconds. Any packet exceeding the rate will be dropped.
+
+  Adding a `hitcount` will automatically add 2 `ip(6)tables` rules right before the actual rules. Rules follow the [iptables "recent" extension](https://ipset.netfilter.org/iptables-extensions.man.html#lbBW). The first rule does `--update`, the second one does `--set` followed by the rule you specified.
+
+  Given config section:
+  ```
+  [anyone-may-access-mail-services]
+  ports = 143, 993, 110, 995, 25, 465, 587
+  hitcount = 120/60
+  ```
+  UFS generates rules:
+  ```
+  target
+  DROP   ... multiport dports 143,993,110,995,25,465,587 recent: UPDATE seconds: 60 hit_count: 120 ...
+         ... multiport dports 143,993,110,995,25,465,587 recent: SET ...
+  ACCEPT ... state NEW multiport dports 143,993,110,995,25,465,587 ...
+  ```
+  Where the first `DROP` target will drop packets that have exceeded their hit count, the second `recent: SET` simply marks all matching packets to be added into the hitcount bucket and the third one is the actual `ACCEPT` rule permitting access **_if_** a source's hitcount permits it.
+
+* `do_ipv6`, __*optional*__, defaults to `false`: Decide if you want `firewalld` to generate `ip6tables` rules in addition to `iptables` rules. A default install of Docker Engine will have its IPv6 support disabled in `/etc/docker/daemon.json`. You may still want your machine to handle incoming IPv6 traffic. If your machine truly doesn't use IPv6 feel free to leave this at `false`. Otherwise `update-firewall-source.py` generates unused rules that clutter your rule set.
 
   If this is `true` IPv6 addresses found or resolved in `addr` in a `[section]` will be discarded.
 
+  ```
+  # Valid example:
+  do_ipv6 = true
+  ```
+
+## iptables behind the scenes
+
+In an `iptables` rule set in the `filter` table by default you'll see something like this. We're only focusing on `INPUT` and `FORWARD` chains here as these are the ones relevant to Docker.
+
+```
+Chain INPUT (policy ACCEPT 140 packets, 10115 bytes)
+num   pkts bytes target     prot opt in     out     source               destination
+
+Chain FORWARD (policy ACCEPT 0 packets, 0 bytes)
+num   pkts bytes target     prot opt in     out     source               destination
+
+...
+```
+
+Once you install a moderately modern Docker Engine it changes things up to this:
+
+```
+Chain INPUT (policy ACCEPT 1027 packets, 78370 bytes)
+num   pkts bytes target     prot opt in     out     source               destination
+
+Chain FORWARD (policy ACCEPT 0 packets, 0 bytes)
+num   pkts bytes target     prot opt in     out     source               destination
+1        0     0 DOCKER-USER  all  --  *      *       0.0.0.0/0            0.0.0.0/0
+2        0     0 DOCKER-ISOLATION-STAGE-1  all  --  *      *       0.0.0.0/0            0.0.0.0/0
+3        0     0 ACCEPT     all  --  *      docker0  0.0.0.0/0            0.0.0.0/0            ctstate RELATED,ESTABLISHED
+4        0     0 DOCKER     all  --  *      docker0  0.0.0.0/0            0.0.0.0/0
+5        0     0 ACCEPT     all  --  docker0 !docker0  0.0.0.0/0            0.0.0.0/0
+6        0     0 ACCEPT     all  --  docker0 docker0  0.0.0.0/0            0.0.0.0/0
+
+...
+
+Chain DOCKER-USER (1 references)
+num   pkts bytes target     prot opt in     out     source               destination
+1        0     0 RETURN     all  --  *      *       0.0.0.0/0            0.0.0.0/0
+```
+
+With UFS you're extending this into a configuration where UFS' `FILTERS` chain is called from both Docker's `DOCKER-USER` chain **_and_** the host OS `INPUT` chain. Ideally `FILTERS` ends with a `DROP` target to make sure nothing accesses your services that's not supposed to access them.
+
+```
+Chain INPUT (policy ACCEPT 56 packets, 4596 bytes)
+num   pkts bytes target     prot opt in     out     source               destination
+1     2910  207K ACCEPT     all  --  lo     *       0.0.0.0/0            0.0.0.0/0
+2     1309  111K FILTERS    all  --  *      *       0.0.0.0/0            0.0.0.0/0
+
+Chain FORWARD (policy ACCEPT 0 packets, 0 bytes)
+num   pkts bytes target     prot opt in     out     source               destination
+1        0     0 DOCKER-ISOLATION-STAGE-1  all  --  *      *       0.0.0.0/0            0.0.0.0/0
+2        0     0 DOCKER-USER  all  --  *      *       0.0.0.0/0            0.0.0.0/0
+3        0     0 ACCEPT     all  --  *      docker0  0.0.0.0/0            0.0.0.0/0            ctstate RELATED,ESTABLISHED
+4        0     0 DOCKER     all  --  *      docker0  0.0.0.0/0            0.0.0.0/0
+5        0     0 ACCEPT     all  --  docker0 !docker0  0.0.0.0/0            0.0.0.0/0
+6        0     0 ACCEPT     all  --  docker0 docker0  0.0.0.0/0            0.0.0.0/0
+
+...
+
+Chain FILTERS (2 references)
+num   pkts bytes target     prot opt in     out     source               destination
+1     1267  107K ACCEPT     all  --  *      *       0.0.0.0/0            0.0.0.0/0            state RELATED,ESTABLISHED
+2        0     0 ACCEPT     tcp  --  *      *       1.2.3.4              0.0.0.0/0            state NEW multiport dports 22,80,443 /* My home */
+3        1   162 DROP       all  --  *      *       0.0.0.0/0            0.0.0.0/0            /* deny-all */
+
+Chain DOCKER-USER (1 references)
+num   pkts bytes target     prot opt in     out     source               destination
+1        0     0 FILTERS    all  --  ens3   *       0.0.0.0/0            0.0.0.0/0
+2        0     0 RETURN     all  --  *      *       0.0.0.0/0            0.0.0.0/0
+
+...
+```
+
+## Rule comments
+
+The `[section]` name is used as `ip(6)tables` rule comment. The `[section]` name is truncated to the first 256 characters to fit into an `ip(6)tables` comment if needed.
+
+Example:
+
+```
+[My home]
+addr = some.dyndns.host.net
+ports = 22, 80, 443
+
+[deny-all]
+target = DROP
+addr =
+ports =
+proto =
+state =
+```
+
+Results in `ip(6)tables` rules:
+
+```
+Chain FILTERS (2 references)
+num   pkts bytes target     prot opt in     out     source               destination
+1     1267  107K ACCEPT     all  --  *      *       0.0.0.0/0            0.0.0.0/0            state RELATED,ESTABLISHED
+2        0     0 ACCEPT     tcp  --  *      *       1.2.3.4              0.0.0.0/0            state NEW multiport dports 22,80,443 /* My home */
+3        0     0 DROP       all  --  *      *       0.0.0.0/0            0.0.0.0/0            /* deny-all */
+```
+
 # Development
 
 ## Conventional Commits

examples/config.ini.example

@@ -4,10 +4,18 @@ addr =
 ports = 80, 443
 proto = tcp
 state = NEW
+hitcount =
 do_ipv6 = false
 firewalld_direct_file_abs = /etc/firewalld/direct.xml
 restart_firewalld_after_change = true
 
+[anyone-may-icmp-with-limit]
+addr =
+ports =
+proto = icmp
+state = NEW,UNTRACKED
+hitcount = 120/60
+
 [anyone-can-access-website]
 
 # Unsetting 'proto' while having a 'ports' value results in an invalid section
@@ -22,8 +30,9 @@ addr = 2606:4700:20::681a:804, lowendtalk.com
 ports = 80, 443
 do_ipv6 = true
 
-[allow-anyone-to-access-mail-services]
+[anyone-may-access-mail-services]
 ports = 143, 993, 110, 995, 25, 465, 587
+hitcount = 120/60
 
 [deny-all]
 target = DROP

update-firewall-source.py

@@ -1,6 +1,6 @@
 # Exit with various exit codes
 import sys
-# Path manipulation
+# Path and env manipulation
 import os
 # Manipulate style and content of logs
 import logging
@@ -22,23 +22,21 @@ import inflect
 import dbus
 # Find physical network interface via 'find' command
 import subprocess
+# Diff new and existing firewalld direct rules XML structure
+import difflib
 
 
 # Exit codes
 # 1 : Config file invalid, it has no sections
 # 2 : Config file invalid, sections must define at least CONST.CFG_MANDATORY
-# 3 : Performing a firewalld rules check failed
+# 3 : No physical network device found at "/sys/class/net"
-# 4 : Performing a firewalld rules encountered a FileNotFoundError
+# 4 : Linux find command exited non-zero trying to find a physical network device at "/sys/class/net"
-# 5 : Unable to open firewalld direct rules file
+# 5 : Unable to open firewalld direct rules file for reading
-# 6 : Source and destination are identical when attempting to back up firewalld direct rules file
+# 6 : Kernel sysfs export for network devices at "/sys/class/net" doesn't exist
 # 7 : An option that must have a non-null value is either unset or null
 # 8 : Exception while adding a chain XML element to firewalld direct rules
 # 9 : Unable to open firewalld direct rules file for updating
 # 10: Unable to restart systemd firewalld.service unit
-# 11: Unable to add a <rule/> tag to firewalld
-# 12: Kernel sysfs export for network devices at "/sys/class/net" doesn't exist
-# 13: Linux find command exited non-zero trying to find a physical network device at "/sys/class/net"
-# 14: No physical network device found at "/sys/class/net"
 
 
 class CONST(object):
@@ -60,6 +58,7 @@ class CONST(object):
         {"key": "ports", "value": "80, 443", "is_global": False, "empty_ok": True},
         {"key": "proto", "value": "tcp", "is_global": False, "empty_ok": True},
         {"key": "state", "value": "NEW", "is_global": False, "empty_ok": True},
+        {"key": "hitcount", "value": "", "is_global": False, "empty_ok": True},
         {"key": "do_ipv6", "value": "false", "is_global": False, "empty_ok": False},
         {"key": "firewalld_direct_abs", "value": "/etc/firewalld/direct.xml", "is_global": True, "empty_ok": False},
         {"key": "restart_firewalld_after_change", "value": "true", "is_global": True, "empty_ok": False}
@@ -78,18 +77,17 @@ class CONST(object):
     CFG_MANDATORY = [section_cfg["key"] for section_cfg in CFG_KNOWN_SECTION if section_cfg["is_mandatory"]]
 
 
+is_systemd = any([systemd_env_var in os.environ for systemd_env_var in ["SYSTEMD_EXEC_PID", "INVOCATION_ID"]])
 logging.basicConfig(
     # Default for all modules is NOTSET so log everything
     level="NOTSET",
     format=CONST.LOG_FORMAT,
     datefmt="[%X]",
     handlers=[RichHandler(
-        show_time=False if any([systemd_env_var in os.environ for systemd_env_var in [
+        show_time=False if is_systemd else True,
-            "SYSTEMD_EXEC_PID",
+        show_path=False if is_systemd else True,
-            "INVOCATION_ID"]]) else True,
+        show_level=False if is_systemd else True,
-        rich_tracebacks=True,
+        rich_tracebacks=True
-        show_path=False,
-        show_level=False
     )]
 )
 log = logging.getLogger("rich")
@@ -125,6 +123,10 @@ internal_empty_ok = [default["key"] for default in CONST.CFG_KNOWN_DEFAULTS if d
 config = ConfigParser(defaults=internal_defaults,
                       converters={'list': lambda x: [i.strip() for i in x.split(',') if len(x) > 0]})
 config.read(CONST.CFG_DEFAULT_ABS_PATH)
+exit_code_desc = {
+    5: "Unable to open firewalld direct rules file for reading",
+    9: "Unable to open firewalld direct rules file for updating"
+}
 
 
 def print_section_header(
@@ -285,16 +287,58 @@ def add_rule_elem(
         prio: int,
         target: str,
         /, *,
-        arg_section_name: str = None,
+        arg_section: str = None,
         arg_proto: str = None,
         arg_state: str = None,
         arg_ports: list = None,
+        arg_hitcount: str = None,
         arg_address: str = None,
         arg_chain: str = "FILTERS",
         arg_in_interface: str = None) -> bool:
 
     global arg_fw_rule_data
 
+    if arg_proto == "icmpv6" and address_family == "ipv4":
+        arg_proto = "icmp"
+    if arg_proto == "icmp" and address_family == "ipv6":
+        arg_proto = "icmpv6"
+
+    if arg_hitcount:
+        try:
+            lxml.etree.SubElement(arg_fw_rule_data, "rule",
+                                  ipv=f"{address_family}",
+                                  table=f"filter",
+                                  chain=arg_chain,
+                                  priority=f"""{prio}""").text = \
+                f"""{"--in-interface " + arg_in_interface + " " if arg_in_interface else ""}""" \
+                f"""{"--protocol " + arg_proto + " " if arg_proto else ""}""" \
+                f"""{"--match multiport --destination-ports " + ",".join(arg_ports) + " " if arg_ports else ""}""" \
+                f"""{"--source " + arg_address + " " if arg_address else ""}""" \
+                f"""{"--match recent --name " + chr(34) + arg_section[:256] + chr(34) +
+                " --update --hitcount " + arg_hitcount.split("/")[0] + " --seconds " + arg_hitcount.split("/")[1] + " " 
+                if arg_section else ""}""" \
+                f"""--jump DROP"""
+            prio += 1
+
+            lxml.etree.SubElement(arg_fw_rule_data, "rule",
+                                  ipv=f"{address_family}",
+                                  table=f"filter",
+                                  chain=arg_chain,
+                                  priority=f"""{prio}""").text = \
+                f"""{"--in-interface " + arg_in_interface + " " if arg_in_interface else ""}""" \
+                f"""{"--protocol " + arg_proto + " " if arg_proto else ""}""" \
+                f"""{"--match multiport --destination-ports " + ",".join(arg_ports) + " " if arg_ports else ""}""" \
+                f"""{"--source " + arg_address + " " if arg_address else ""}""" \
+                f"""{"--match recent --name " + chr(34) + arg_section[:256] + chr(34) +
+                " --set" if arg_section else ""}"""
+            prio += 1
+        except lxml.etree.LxmlError as le:
+            log.error(f"""Failed to add XML '<rule ipv=f"{address_family}" .../>'\n"""
+                      f"Verbatim exception was:\n"
+                      f"f{le}\n"
+                      f"Exiting 8 ...")
+            sys.exit(8)
+
     try:
         lxml.etree.SubElement(arg_fw_rule_data, "rule",
                               ipv=f"{address_family}",
@@ -307,8 +351,7 @@ def add_rule_elem(
             f"""{"--match multiport --destination-ports " + ",".join(arg_ports) + " " if arg_ports else ""}""" \
             f"""{"--source " + arg_address + " " if arg_address else ""}""" \
             f"""--jump {target}""" \
-            f"""
+            f"""{" --match comment --comment " + chr(34) + arg_section[:256] + chr(34) if arg_section else ""}"""
-            {" --match comment --comment " + chr(34) + arg_section_name[:256] + chr(34) if arg_section_name else ""}"""
     except lxml.etree.LxmlError as le:
         log.error(f"""Failed to add XML '<rule ipv=f"{address_family}" .../>'\n"""
                   f"Verbatim exception was:\n"
@@ -338,23 +381,23 @@ def get_phy_nics() -> list:
                       f"{cpe.cmd}\n"
                       f"Verbatim command output was:\n"
                       f"{cpe.output.rstrip()}\n"
-                      f"Exiting 13 ...")
+                      f"Exiting 4 ...")
-            sys.exit(13)
+            sys.exit(4)
         else:
             if not phy_nics_find.stdout:
                 log.error(f"No physical network device found at {linux_sysfs_nics_abs!r}.\n"
                           f"Command was:\n"
                           f"{phy_nics_find.args}\n"
-                          f"Exiting 14 ...")
+                          f"Exiting 3 ...")
-                sys.exit(14)
+                sys.exit(3)
             for line in phy_nics_find.stdout.rstrip().split("\n"):
                 log.debug(f"Found physical network device {(phy_nic := os.path.basename(line))!r}")
                 phy_nics.append(phy_nic)
     else:
         log.error(f"Path {linux_sysfs_nics_abs!r} does not exist. This might not be a Linux-y operating system. "
                   f"Without that location we'll not be able to separate physical network interfaces from virtual ones. "
-                  f"Exiting 12 ...")
+                  f"Exiting 6 ...")
-        sys.exit(12)
+        sys.exit(6)
 
     log.debug(f"List of identified physical network interfaces: {phy_nics}")
     return phy_nics
@@ -365,7 +408,8 @@ def add_fw_rule_to_xml(
         section_name: str,
         target: str,
         ports: list,
-        proto: str) -> bool:
+        proto: str,
+        hitcount: str) -> bool:
     global arg_fw_rule_data
     global arg_allow_sources
     addr = arg_allow_sources
@@ -382,12 +426,16 @@ def add_fw_rule_to_xml(
                     address_family,
                     rules_already_added[address_family],
                     target,
-                    arg_section_name=section_name,
+                    arg_section=section_name,
                     arg_proto=proto,
                     arg_state=config_obj.get(section_name, "state"),
                     arg_ports=ports,
+                    arg_hitcount=hitcount,
                     arg_address=address)
-                rules_already_added[address_family] += 1
+                if hitcount:
+                    rules_already_added[address_family] += 3
+                else:
+                    rules_already_added[address_family] += 1
         if not len(addr["ipv4"]) and not len(addr["ipv6"]):
             if address_family == "ipv4" or (address_family == "ipv6"
                                             and
@@ -398,11 +446,15 @@ def add_fw_rule_to_xml(
                     address_family,
                     rules_already_added[address_family],
                     target,
-                    arg_section_name=section_name,
+                    arg_section=section_name,
                     arg_proto=proto,
                     arg_state=config_obj.get(section_name, "state"),
-                    arg_ports=ports)
+                    arg_ports=ports,
-                rules_already_added[address_family] += 1
+                    arg_hitcount=hitcount)
+                if hitcount:
+                    rules_already_added[address_family] += 3
+                else:
+                    rules_already_added[address_family] += 1
 
     return True
 
@@ -473,14 +525,34 @@ def gen_fwd_direct_scaffolding() -> lxml.builder.ElementMaker:
     return fw_rule_data
 
 
-def write_new_fwd_direct_xml(
+def ose_handler(
-        config_obj: configparser.ConfigParser()) -> bool:
+        os_error: OSError,
+        human_text: str = None,
+        exit_code: int = None) -> None:
+    nl = "\n"
+    log.error(f"{human_text if human_text else exit_code_desc.get(exit_code)}"
+              f"{nl}Verbatim exception was:\n"
+              f"{os_error}"
+              f"""{nl + "Exiting " + str(exit_code) + " ..." if exit_code else ""}""")
+
+
+def get_xml_str_repr() -> str:
     global arg_fw_rule_data
 
     fwd_direct_xml_str = lxml.etree.tostring(arg_fw_rule_data,
                                              pretty_print=True,
                                              encoding="UTF-8",
                                              xml_declaration=True).decode()
+
+    return fwd_direct_xml_str
+
+
+def write_new_fwd_direct_xml(
+        config_obj: configparser.ConfigParser()) -> bool:
+    global arg_fw_rule_data
+
+    fwd_direct_xml_str = get_xml_str_repr()
+
     try:
         with open(config_obj.get(configparser.DEFAULTSECT, "firewalld_direct_abs"), "r+") as fwd_file_handle:
             log.info(f"Writing new firewalld direct config ...")
@@ -490,10 +562,7 @@ def write_new_fwd_direct_xml(
             fwd_file_handle.write(fwd_direct_xml_str)
             fwd_file_handle.truncate()
     except OSError as ose:
-        log.error(f"Unable to open firewalld direct rules file for updating.\n"
+        ose_handler(os_error=ose, exit_code=9)
-                  f"Verbatim exception was:\n"
-                  f"f{ose}\n"
-                  f"Exiting 9 ...")
         sys.exit(9)
     else:
         return True
@@ -501,14 +570,14 @@ def write_new_fwd_direct_xml(
 
 def restart_systemd_firewalld() -> bool:
     sysbus = dbus.SystemBus()
-    systemd1 = sysbus.get_object('org.freedesktop.systemd1', '/org/freedesktop/systemd1')
+    systemd1 = sysbus.get_object("org.freedesktop.systemd1", "/org/freedesktop/systemd1")
-    manager = dbus.Interface(systemd1, 'org.freedesktop.systemd1.Manager')
+    manager = dbus.Interface(systemd1, "org.freedesktop.systemd1.Manager")
 
-    firewalld_unit = manager.LoadUnit('firewalld.service')
+    firewalld_unit = manager.LoadUnit("firewalld.service")
-    firewalld_proxy = sysbus.get_object('org.freedesktop.systemd1', str(firewalld_unit))
+    firewalld_proxy = sysbus.get_object("org.freedesktop.systemd1", str(firewalld_unit))
-    firewalld_active_state = firewalld_proxy.Get('org.freedesktop.systemd1.Unit',
+    firewalld_active_state = firewalld_proxy.Get("org.freedesktop.systemd1.Unit",
-                                                 'ActiveState',
+                                                 "ActiveState",
-                                                 dbus_interface='org.freedesktop.DBus.Properties')
+                                                 dbus_interface="org.freedesktop.DBus.Properties")
 
     if firewalld_active_state == "inactive":
         log.info(f"systemd firewalld.service unit is inactive, ignoring restart instruction, leaving as-is ...")
@@ -553,7 +622,31 @@ def add_firewall_shim(arg_phy_nics: list) -> None:
                     )
 
 
-if __name__ == '__main__':
+def has_xml_changed(
+        config_obj: configparser.ConfigParser()) -> bool:
+    arg_fwd_file_abs = os.path.abspath(config_obj.get(configparser.DEFAULTSECT, "firewalld_direct_file_abs"))
+
+    try:
+        with open(arg_fwd_file_abs, "r") as fwd_file_abs_handle:
+            fwd_file_abs_content = fwd_file_abs_handle.read()
+            fwd_direct_xml_str = get_xml_str_repr()
+            diff_result = difflib.Differ().compare(fwd_file_abs_content.splitlines(), fwd_direct_xml_str.splitlines())
+            s = difflib.SequenceMatcher(isjunk=None, a=fwd_file_abs_content, b=fwd_direct_xml_str, autojunk=False)
+    except OSError as ose:
+        ose_handler(os_error=ose, exit_code=5)
+        sys.exit(5)
+    else:
+        if s.ratio() < 1:
+            nl = "\n"
+            log.info(f"Changing firewalld rules. Diff as follows:\n"
+                     f"""{nl.join(diff_result)}""")
+            return True
+        else:
+            log.info(f"No diff in firewalld XML config, no need to write new file.")
+            return False
+
+
+if __name__ == "__main__":
     validate_default_section(config)
     if config_has_valid_section(config):
         validate_config_sections(config)
@@ -579,7 +672,8 @@ if __name__ == '__main__':
                            section,
                            target=config.get(section, "target"),
                            ports=config.getlist(section, "ports"),
-                           proto=config.get(section, "proto"))
+                           proto=config.get(section, "proto"),
+                           hitcount=config.get(section, "hitcount"))
     for arg_address_family in ["ipv4", "ipv6"]:
         if rules_count(arg_address_family):
             add_rule_elem(
@@ -589,6 +683,7 @@ if __name__ == '__main__':
                 arg_state="ESTABLISHED,RELATED")
     add_firewall_shim(get_phy_nics())
 
-    write_new_fwd_direct_xml(config)
+    if has_xml_changed(config):
-    if config.getboolean(configparser.DEFAULTSECT, "restart_firewalld_after_change"):
+        write_new_fwd_direct_xml(config)
-        restart_systemd_firewalld()
+        if config.getboolean(configparser.DEFAULTSECT, "restart_firewalld_after_change"):
+            restart_systemd_firewalld()