From fb11903b8efd287f72e634fb8a4b4ff2034151fe Mon Sep 17 00:00:00 2001

From: Paul Laufer <50234787+refual@users.noreply.github.com>

Date: Fri, 27 Nov 2020 12:23:11 +0100

Subject: [PATCH 47/48] feat(config): add CleanupModulesOnExit configuration

 option

Fixes: rhbz 1520532

Fixes: #533

Closes: #721

(cherry picked from commit 152a51537a7840afd0879ab4b60178bef4ec16a2)

---

 config/firewalld.conf                  |  9 +++++++-

 doc/xml/firewalld.conf.xml             | 11 ++++++++++

 doc/xml/firewalld.dbus.xml             |  9 ++++++++

 src/firewall/config/__init__.py.in     |  1 +

 src/firewall/core/fw.py                | 29 +++++++++++++++++++-------

 src/firewall/core/io/firewalld_conf.py | 19 +++++++++++++----

 src/firewall/server/config.py          | 23 +++++++++++++-------

 src/tests/dbus/firewalld.conf.at       |  2 ++

 8 files changed, 82 insertions(+), 21 deletions(-)

diff --git a/config/firewalld.conf b/config/firewalld.conf

index a0556c0bbf5b..3abbc9c998c1 100644

--- a/config/firewalld.conf

+++ b/config/firewalld.conf

@@ -7,10 +7,17 @@ DefaultZone=public

 # Clean up on exit

 # If set to no or false the firewall configuration will not get cleaned up

-# on exit or stop of firewalld

+# on exit or stop of firewalld.

 # Default: yes

 CleanupOnExit=yes

+# Clean up kernel modules on exit

+# If set to yes or true the firewall related kernel modules will be

+# unloaded on exit or stop of firewalld. This might attempt to unload

+# modules not originally loaded by firewalld.

+# Default: no

+CleanupModulesOnExit=no

+

 # Lockdown

 # If set to enabled, firewall changes with the D-Bus interface will be limited

 # to applications that are listed in the lockdown whitelist.

diff --git a/doc/xml/firewalld.conf.xml b/doc/xml/firewalld.conf.xml

index 0bf4c2d4d011..dd6ffb214eb3 100644

--- a/doc/xml/firewalld.conf.xml

+++ b/doc/xml/firewalld.conf.xml

@@ -88,6 +88,17 @@

 	</listitem>

       </varlistentry>

+      <varlistentry>

+        <term><option>CleanupModulesOnExit</option></term>

+        <listitem>

+          <para>

+            Setting this option to yes or true unloads all firewall-related

+            kernel modules when firewalld is stopped. The default value is no

+            or false.

+          </para>

+        </listitem>

+      </varlistentry>

+

       <varlistentry>

 	<term><option>CleanupOnExit</option></term>

         <listitem>

diff --git a/doc/xml/firewalld.dbus.xml b/doc/xml/firewalld.dbus.xml

index d17cb8b6c1ec..466220b40b21 100644

--- a/doc/xml/firewalld.dbus.xml

+++ b/doc/xml/firewalld.dbus.xml

@@ -2798,6 +2798,15 @@

               </para>

             </listitem>

           </varlistentry>

+          <varlistentry id="FirewallD1.config.Properties.CleanupModulesOnExit">

+            <term>CleanupModulesOnExit - s - (rw)</term>

+            <listitem>

+              <para>

+                Setting this option to yes or true unloads all firewall-related

+                kernel modules when firewalld is stopped.

+              </para>

+            </listitem>

+          </varlistentry>

           <varlistentry id="FirewallD1.config.Properties.CleanupOnExit">

             <term>CleanupOnExit - s - (rw)</term>

             <listitem>

diff --git a/src/firewall/config/__init__.py.in b/src/firewall/config/__init__.py.in

index 0dec7913f694..5d6d769fbf15 100644

--- a/src/firewall/config/__init__.py.in

+++ b/src/firewall/config/__init__.py.in

@@ -125,6 +125,7 @@ FIREWALL_BACKEND_VALUES = [ "nftables", "iptables" ]

 FALLBACK_ZONE = "public"

 FALLBACK_MINIMAL_MARK = 100

 FALLBACK_CLEANUP_ON_EXIT = True

+FALLBACK_CLEANUP_MODULES_ON_EXIT = False

 FALLBACK_LOCKDOWN = False

 FALLBACK_IPV6_RPFILTER = True

 FALLBACK_INDIVIDUAL_CALLS = False

diff --git a/src/firewall/core/fw.py b/src/firewall/core/fw.py

index 3eb54e37ab5c..4171697bdb94 100644

--- a/src/firewall/core/fw.py

+++ b/src/firewall/core/fw.py

@@ -105,12 +105,13 @@ class Firewall(object):

         self.__init_vars()

     def __repr__(self):

-        return '%s(%r, %r, %r, %r, %r, %r, %r, %r, %r, %r, %r, %r, %r)' % \

+        return '%s(%r, %r, %r, %r, %r, %r, %r, %r, %r, %r, %r, %r, %r, %r)' % \

             (self.__class__, self.ip4tables_enabled, self.ip6tables_enabled,

              self.ebtables_enabled, self._state, self._panic,

              self._default_zone, self._module_refcount, self._marks,

-             self.cleanup_on_exit, self.ipv6_rpfilter_enabled,

-             self.ipset_enabled, self._individual_calls, self._log_denied)

+             self.cleanup_on_exit, self.cleanup_modules_on_exit,

+             self.ipv6_rpfilter_enabled, self.ipset_enabled,

+             self._individual_calls, self._log_denied)

     def __init_vars(self):

         self._state = "INIT"

@@ -120,6 +121,7 @@ class Firewall(object):

         self._marks = [ ]

         # fallback settings will be overloaded by firewalld.conf

         self.cleanup_on_exit = config.FALLBACK_CLEANUP_ON_EXIT

+        self.cleanup_modules_on_exit = config.FALLBACK_CLEANUP_MODULES_ON_EXIT

         self.ipv6_rpfilter_enabled = config.FALLBACK_IPV6_RPFILTER

         self._individual_calls = config.FALLBACK_INDIVIDUAL_CALLS

         self._log_denied = config.FALLBACK_LOG_DENIED

@@ -232,6 +234,13 @@ class Firewall(object):

                 log.debug1("CleanupOnExit is set to '%s'",

                            self.cleanup_on_exit)

+            if self._firewalld_conf.get("CleanupModulesOnExit"):

+                value = self._firewalld_conf.get("CleanupModulesOnExit")

+                if value is not None and value.lower() in [ "yes", "true" ]:

+                    self.cleanup_modules_on_exit = True

+                log.debug1("CleanupModulesOnExit is set to '%s'",

+                           self.cleanup_modules_on_exit)

+

             if self._firewalld_conf.get("Lockdown"):

                 value = self._firewalld_conf.get("Lockdown")

                 if value is not None and value.lower() in [ "yes", "true" ]:

@@ -667,11 +676,15 @@ class Firewall(object):

         self.__init_vars()

     def stop(self):

-        if self.cleanup_on_exit and not self._offline:

-            self.flush()

-            self.ipset.flush()

-            self.set_policy("ACCEPT")

-            self.modules_backend.unload_firewall_modules()

+        if not self._offline:

+            if self.cleanup_on_exit:

+                self.flush()

+                self.ipset.flush()

+                self.set_policy("ACCEPT")

+

+            if self.cleanup_modules_on_exit:

+                log.debug1('Unloading firewall kernel modules')

+                self.modules_backend.unload_firewall_modules()

         self.cleanup()

diff --git a/src/firewall/core/io/firewalld_conf.py b/src/firewall/core/io/firewalld_conf.py

index 7c7092120676..70258400ef06 100644

--- a/src/firewall/core/io/firewalld_conf.py

+++ b/src/firewall/core/io/firewalld_conf.py

@@ -28,10 +28,11 @@ from firewall import config

 from firewall.core.logger import log

 from firewall.functions import b2u, u2b, PY2

-valid_keys = [ "DefaultZone", "MinimalMark", "CleanupOnExit", "Lockdown",

-               "IPv6_rpfilter", "IndividualCalls", "LogDenied",

-               "AutomaticHelpers", "FirewallBackend", "FlushAllOnReload",

-               "RFC3964_IPv4", "AllowZoneDrifting" ]

+valid_keys = [ "DefaultZone", "MinimalMark", "CleanupOnExit",

+               "CleanupModulesOnExit", "Lockdown", "IPv6_rpfilter",

+               "IndividualCalls", "LogDenied", "AutomaticHelpers",

+               "FirewallBackend", "FlushAllOnReload", "RFC3964_IPv4",

+               "AllowZoneDrifting" ]

 class firewalld_conf(object):

     def __init__(self, filename):

@@ -75,6 +76,7 @@ class firewalld_conf(object):

             self.set("DefaultZone", config.FALLBACK_ZONE)

             self.set("MinimalMark", str(config.FALLBACK_MINIMAL_MARK))

             self.set("CleanupOnExit", "yes" if config.FALLBACK_CLEANUP_ON_EXIT else "no")

+            self.set("CleanupModulesOnExit", "yes" if config.FALLBACK_CLEANUP_MODULES_ON_EXIT else "no")

             self.set("Lockdown", "yes" if config.FALLBACK_LOCKDOWN else "no")

             self.set("IPv6_rpfilter","yes" if config.FALLBACK_IPV6_RPFILTER else "no")

             self.set("IndividualCalls", "yes" if config.FALLBACK_INDIVIDUAL_CALLS else "no")

@@ -135,6 +137,15 @@ class firewalld_conf(object):

                             config.FALLBACK_CLEANUP_ON_EXIT)

             self.set("CleanupOnExit", "yes" if config.FALLBACK_CLEANUP_ON_EXIT else "no")

+        # check module cleanup on exit

+        value = self.get("CleanupModulesOnExit")

+        if not value or value.lower() not in [ "no", "false", "yes", "true" ]:

+            if value is not None:

+                log.warning("CleanupModulesOnExit '%s' is not valid, using default "

+                            "value %s", value if value else '',

+                            config.FALLBACK_CLEANUP_MODULES_ON_EXIT)

+            self.set("CleanupModulesOnExit", "yes" if config.FALLBACK_CLEANUP_MODULES_ON_EXIT else "no")

+

         # check lockdown

         value = self.get("Lockdown")

         if not value or value.lower() not in [ "yes", "true", "no", "false" ]:

diff --git a/src/firewall/server/config.py b/src/firewall/server/config.py

index 031ef5d1afaa..8815920c6893 100644

--- a/src/firewall/server/config.py

+++ b/src/firewall/server/config.py

@@ -100,6 +100,7 @@ class FirewallDConfig(slip.dbus.service.Object):

         dbus_introspection_prepare_properties(self,

                                               config.dbus.DBUS_INTERFACE_CONFIG,

                                               { "CleanupOnExit": "readwrite",

+                                                "CleanupModulesOnExit": "readwrite",

                                                 "IPv6_rpfilter": "readwrite",

                                                 "Lockdown": "readwrite",

                                                 "MinimalMark": "readwrite",

@@ -554,9 +555,9 @@ class FirewallDConfig(slip.dbus.service.Object):

     @dbus_handle_exceptions

     def _get_property(self, prop):

         if prop not in [ "DefaultZone", "MinimalMark", "CleanupOnExit",

-                         "Lockdown", "IPv6_rpfilter", "IndividualCalls",

-                         "LogDenied", "AutomaticHelpers", "FirewallBackend",

-                         "FlushAllOnReload", "RFC3964_IPv4",

+                         "CleanupModulesOnExit", "Lockdown", "IPv6_rpfilter",

+                         "IndividualCalls", "LogDenied", "AutomaticHelpers",

+                         "FirewallBackend", "FlushAllOnReload", "RFC3964_IPv4",

                          "AllowZoneDrifting" ]:

             raise dbus.exceptions.DBusException(

                 "org.freedesktop.DBus.Error.InvalidArgs: "

@@ -578,6 +579,10 @@ class FirewallDConfig(slip.dbus.service.Object):

             if value is None:

                 value = "yes" if config.FALLBACK_CLEANUP_ON_EXIT else "no"

             return dbus.String(value)

+        elif prop == "CleanupModulesOnExit":

+            if value is None:

+                value = "yes" if config.FALLBACK_CLEANUP_MODULES_ON_EXIT else "no"

+            return dbus.String(value)

         elif prop == "Lockdown":

             if value is None:

                 value = "yes" if config.FALLBACK_LOCKDOWN else "no"

@@ -623,6 +628,8 @@ class FirewallDConfig(slip.dbus.service.Object):

             return dbus.Int32(self._get_property(prop))

         elif prop == "CleanupOnExit":

             return dbus.String(self._get_property(prop))

+        elif prop == "CleanupModulesOnExit":

+            return dbus.String(self._get_property(prop))

         elif prop == "Lockdown":

             return dbus.String(self._get_property(prop))

         elif prop == "IPv6_rpfilter":

@@ -679,9 +686,9 @@ class FirewallDConfig(slip.dbus.service.Object):

         ret = { }

         if interface_name == config.dbus.DBUS_INTERFACE_CONFIG:

             for x in [ "DefaultZone", "MinimalMark", "CleanupOnExit",

-                       "Lockdown", "IPv6_rpfilter", "IndividualCalls",

-                       "LogDenied", "AutomaticHelpers", "FirewallBackend",

-                       "FlushAllOnReload", "RFC3964_IPv4",

+                       "CleanupModulesOnExit", "Lockdown", "IPv6_rpfilter",

+                       "IndividualCalls", "LogDenied", "AutomaticHelpers",

+                       "FirewallBackend", "FlushAllOnReload", "RFC3964_IPv4",

                        "AllowZoneDrifting" ]:

                 ret[x] = self._get_property(x)

         elif interface_name in [ config.dbus.DBUS_INTERFACE_CONFIG_DIRECT,

@@ -706,12 +713,12 @@ class FirewallDConfig(slip.dbus.service.Object):

         self.accessCheck(sender)

         if interface_name == config.dbus.DBUS_INTERFACE_CONFIG:

-            if property_name in [ "CleanupOnExit", "Lockdown",

+            if property_name in [ "CleanupOnExit", "Lockdown", "CleanupModulesOnExit",

                                   "IPv6_rpfilter", "IndividualCalls",

                                   "LogDenied",

                                   "FirewallBackend", "FlushAllOnReload",

                                   "RFC3964_IPv4", "AllowZoneDrifting" ]:

-                if property_name in [ "CleanupOnExit", "Lockdown",

+                if property_name in [ "CleanupOnExit", "Lockdown", "CleanupModulesOnExit",

                                       "IPv6_rpfilter", "IndividualCalls" ]:

                     if new_value.lower() not in [ "yes", "no",

                                                   "true", "false" ]:

diff --git a/src/tests/dbus/firewalld.conf.at b/src/tests/dbus/firewalld.conf.at

index 9fc5502a8d0b..9a04a3bd491c 100644

--- a/src/tests/dbus/firewalld.conf.at

+++ b/src/tests/dbus/firewalld.conf.at

@@ -17,6 +17,7 @@ dnl Verify defaults over dbus. Should be inline with default firewalld.conf.

 DBUS_GETALL([config], [config], 0, [dnl

 string "AllowZoneDrifting" : variant string "no"

 string "AutomaticHelpers" : variant string "no"

+string "CleanupModulesOnExit" : variant string "no"

 string "CleanupOnExit" : variant string "no"

 string "DefaultZone" : variant string "public"

 string "FirewallBackend" : variant string "nftables"

@@ -45,6 +46,7 @@ _helper([IPv6_rpfilter], [string:"yes"], [variant string "yes"])

 _helper([IndividualCalls], [string:"yes"], [variant string "yes"])

 _helper([FirewallBackend], [string:"iptables"], [variant string "iptables"])

 _helper([FlushAllOnReload], [string:"no"], [variant string "no"])

+_helper([CleanupModulesOnExit], [string:"yes"], [variant string "yes"])

 _helper([CleanupOnExit], [string:"yes"], [variant string "yes"])

 _helper([RFC3964_IPv4], [string:"no"], [variant string "no"])

 _helper([AllowZoneDrifting], [string:"yes"], [variant string "yes"])

-- 

2.31.1