From 982024e6775c9a9c78713be82519c729107ca4e2 Mon Sep 17 00:00:00 2001

From: Eric Garver <eric@garver.life>

Date: Sun, 19 Jan 2020 14:13:36 -0500

Subject: [PATCH 30/37] feat: AllowZoneDrifting config option

Older versions of firewalld had undocumented behavior known as "zone

drifting". This allowed packets to ingress multiple zones - this is a

violation of zone based firewalls. However, some users rely on this

behavior to have a "catch-all" zone, e.g. the default zone. You can

enable this if you desire such behavior. It's disabled by default for

security reasons.

Note: If "yes" packets will only drift from source based zones to

interface based zones (including the default zone). Packets never drift

from interface based zones to other interfaces based zones (including

the default zone).

(cherry picked from commit afadd377b09dc62b340d24bcf891d31f040d1a18)

(cherry picked from commit afbd6c0e82b77ca9b687169d69bf6c2dc17a9317)

---

 config/firewalld.conf                  | 12 ++++++++++++

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

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

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

 src/firewall/core/fw.py                | 14 ++++++++++++++

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

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

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

 8 files changed, 93 insertions(+), 5 deletions(-)

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

index 82ad062b8a66..532f0452212e 100644

--- a/config/firewalld.conf

+++ b/config/firewalld.conf

@@ -61,3 +61,15 @@ FlushAllOnReload=yes

 # internet.

 # Defaults to "yes".

 RFC3964_IPv4=yes

+

+# AllowZoneDrifting

+# Older versions of firewalld had undocumented behavior known as "zone

+# drifting". This allowed packets to ingress multiple zones - this is a

+# violation of zone based firewalls. However, some users rely on this behavior

+# to have a "catch-all" zone, e.g. the default zone. You can enable this if you

+# desire such behavior. It's disabled by default for security reasons.

+# Note: If "yes" packets will only drift from source based zones to interface

+# based zones (including the default zone). Packets never drift from interface

+# based zones to other interfaces based zones (including the default zone).

+# Possible values; "yes", "no". Defaults to "no".

+AllowZoneDrifting=no

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

index 6003a6fae855..fcfbfd2b68c1 100644

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

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

@@ -183,6 +183,25 @@

             </listitem>

         </varlistentry>

+        <varlistentry>

+            <term><option>AllowZoneDrifting</option></term>

+            <listitem>

+                <para>

+                Older versions of firewalld had undocumented behavior known

+                as "zone drifting". This allowed packets to ingress multiple

+                zones - this is a violation of zone based firewalls. However,

+                some users rely on this behavior to have a "catch-all" zone,

+                e.g. the default zone. You can enable this if you desire such

+                behavior. It's disabled by default for security reasons.

+                Note: If "yes" packets will only drift from source based zones

+                to interface based zones (including the default zone). Packets

+                never drift from interface based zones to other interfaces

+                based zones (including the default zone).

+                Valid values; "yes", "no". Defaults to "no".

+                </para>

+            </listitem>

+        </varlistentry>

+

     </variablelist>

   </refsect1>

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

index 66b0475ec0c8..5d77af976443 100644

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

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

@@ -2578,6 +2578,22 @@

       <refsect3 id="FirewallD1.config.Properties">

         <title>Properties</title>

         <variablelist>

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

+            <term><parameter>AllowZoneDrifting</parameter> - s - (rw)</term>

+            <listitem><para>

+                Older versions of firewalld had undocumented behavior known

+                as "zone drifting". This allowed packets to ingress multiple

+                zones - this is a violation of zone based firewalls. However,

+                some users rely on this behavior to have a "catch-all" zone,

+                e.g. the default zone. You can enable this if you desire such

+                behavior. It's disabled by default for security reasons.

+                Note: If "yes" packets will only drift from source based zones

+                to interface based zones (including the default zone). Packets

+                never drift from interface based zones to other interfaces

+                based zones (including the default zone).

+                Valid values; "yes", "no". Defaults to "no".

+            </para></listitem>

+          </varlistentry>

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

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

             <listitem>

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

index 3274dd430e4e..481eb8de758d 100644

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

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

@@ -130,3 +130,4 @@ FALLBACK_AUTOMATIC_HELPERS = "no"

 FALLBACK_FIREWALL_BACKEND = "nftables"

 FALLBACK_FLUSH_ALL_ON_RELOAD = True

 FALLBACK_RFC3964_IPV4 = True

+FALLBACK_ALLOW_ZONE_DRIFTING = False

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

index 050fb9cd976d..6206ed586988 100644

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

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

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

         self._firewall_backend = config.FALLBACK_FIREWALL_BACKEND

         self._flush_all_on_reload = config.FALLBACK_FLUSH_ALL_ON_RELOAD

         self._rfc3964_ipv4 = config.FALLBACK_RFC3964_IPV4

+        self._allow_zone_drifting = config.FALLBACK_ALLOW_ZONE_DRIFTING

     def individual_calls(self):

         return self._individual_calls

@@ -286,6 +287,19 @@ class Firewall(object):

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

                            self._rfc3964_ipv4)

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

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

+                if value.lower() in [ "no", "false" ]:

+                    self._allow_zone_drifting = False

+                else:

+                    self._allow_zone_drifting = True

+                    log.warning("AllowZoneDrifting is enabled. This is considered "

+                                "an insecure configuration option. It will be "

+                                "removed in a future release. Please consider "

+                                "disabling it now.")

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

+                           self._allow_zone_drifting)

+

         self.config.set_firewalld_conf(copy.deepcopy(self._firewalld_conf))

         self._select_firewall_backend(self._firewall_backend)

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

index 9e2205f93d63..7c7092120676 100644

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

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

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

 from firewall.core.logger import log

 from firewall.functions import b2u, u2b, PY2

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

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

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

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

-               "RFC3964_IPv4" ]

+               "RFC3964_IPv4", "AllowZoneDrifting" ]

 class firewalld_conf(object):

     def __init__(self, filename):

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

             self.set("FirewallBackend", config.FALLBACK_FIREWALL_BACKEND)

             self.set("FlushAllOnReload", "yes" if config.FALLBACK_FLUSH_ALL_ON_RELOAD else "no")

             self.set("RFC3964_IPv4", "yes" if config.FALLBACK_RFC3964_IPV4 else "no")

+            self.set("AllowZoneDrifting", "yes" if config.FALLBACK_ALLOW_ZONE_DRIFTING else "no")

             raise

         for line in f:

@@ -202,6 +203,14 @@ class firewalld_conf(object):

                             config.FALLBACK_RFC3964_IPV4)

             self.set("RFC3964_IPv4", str(config.FALLBACK_RFC3964_IPV4))

+        value = self.get("AllowZoneDrifting")

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

+            if value is not None:

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

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

+                            config.FALLBACK_ALLOW_ZONE_DRIFTING)

+            self.set("AllowZoneDrifting", str(config.FALLBACK_ALLOW_ZONE_DRIFTING))

+

     # save to self.filename if there are key/value changes

     def write(self):

         if len(self._config) < 1:

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

index 1c35f5663d29..b3e193d7e468 100644

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

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

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

                                                 "FirewallBackend": "readwrite",

                                                 "FlushAllOnReload": "readwrite",

                                                 "RFC3964_IPv4": "readwrite",

+                                                "AllowZoneDrifting": "readwrite",

                                               })

     @handle_exceptions

@@ -487,7 +488,8 @@ class FirewallDConfig(slip.dbus.service.Object):

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

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

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

-                         "FlushAllOnReload", "RFC3964_IPv4" ]:

+                         "FlushAllOnReload", "RFC3964_IPv4",

+                         "AllowZoneDrifting" ]:

             raise dbus.exceptions.DBusException(

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

                 "Property '%s' does not exist" % prop)

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

             if value is None:

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

             return dbus.String(value)

+        elif prop == "AllowZoneDrifting":

+            if value is None:

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

+            return dbus.String(value)

     @dbus_handle_exceptions

     def _get_dbus_property(self, prop):

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

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

         elif prop == "RFC3964_IPv4":

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

+        elif prop == "AllowZoneDrifting":

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

         else:

             raise dbus.exceptions.DBusException(

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

@@ -605,7 +613,8 @@ class FirewallDConfig(slip.dbus.service.Object):

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

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

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

-                       "FlushAllOnReload", "RFC3964_IPv4" ]:

+                       "FlushAllOnReload", "RFC3964_IPv4",

+                       "AllowZoneDrifting" ]:

                 ret[x] = self._get_property(x)

         elif interface_name in [ config.dbus.DBUS_INTERFACE_CONFIG_DIRECT,

                                  config.dbus.DBUS_INTERFACE_CONFIG_POLICIES ]:

@@ -633,7 +642,7 @@ class FirewallDConfig(slip.dbus.service.Object):

                                   "IPv6_rpfilter", "IndividualCalls",

                                   "LogDenied", "AutomaticHelpers",

                                   "FirewallBackend", "FlushAllOnReload",

-                                  "RFC3964_IPv4" ]:

+                                  "RFC3964_IPv4", "AllowZoneDrifting" ]:

                 if property_name == "MinimalMark":

                     try:

                         int(new_value)

@@ -677,6 +686,11 @@ class FirewallDConfig(slip.dbus.service.Object):

                         raise FirewallError(errors.INVALID_VALUE,

                                             "'%s' for %s" % \

                                             (new_value, property_name))

+                if property_name == "AllowZoneDrifting":

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

+                        raise FirewallError(errors.INVALID_VALUE,

+                                            "'%s' for %s" % \

+                                            (new_value, property_name))

                 self.config.get_firewalld_conf().set(property_name, new_value)

                 self.config.get_firewalld_conf().write()

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

index 06f6df9bdd70..35aead759a9c 100644

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

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

@@ -4,6 +4,7 @@ AT_KEYWORDS(dbus)

 dnl Verify defaults over dbus. Should be inline with default firewalld.conf.

 IF_HOST_SUPPORTS_NFT_FIB([

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

+string "AllowZoneDrifting" : variant string "no"

 string "AutomaticHelpers" : variant string "no"

 string "CleanupOnExit" : variant string "no"

 string "DefaultZone" : variant string "public"

@@ -17,6 +18,7 @@ string "MinimalMark" : variant int32 100

 string "RFC3964_IPv4" : variant string "yes"

 ])], [

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

+string "AllowZoneDrifting" : variant string "no"

 string "AutomaticHelpers" : variant string "no"

 string "CleanupOnExit" : variant string "no"

 string "DefaultZone" : variant string "public"

@@ -49,6 +51,7 @@ _helper([FirewallBackend], [string:"iptables"], [variant string "iptables"])

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

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

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

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

 dnl Note: DefaultZone is RO

 m4_undefine([_helper])

-- 

2.23.0