From eeb2b58b84e6a61d5c212e78e2cf1a44846d5301 Mon Sep 17 00:00:00 2001

From: Eric Garver <eric@garver.life>

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

Subject: [PATCH 139/146] 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 3bbd15a5317b59e175e2a060d1a6ecf4c2129b32)

---

 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 63df409bf567..02be07b9b892 100644

--- a/config/firewalld.conf

+++ b/config/firewalld.conf

@@ -55,3 +55,15 @@ LogDenied=off

 # will be used. Possible values are: yes, no and system.

 # Default: system

 AutomaticHelpers=system

+

+# 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 afb94b90937f..9d8017df3112 100644

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

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

@@ -144,6 +144,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 ec82d4cad077..ea0be9cefd1c 100644

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

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

@@ -2558,6 +2558,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 1b2168bde44d..3926c8fdb3a3 100644

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

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

@@ -128,3 +128,4 @@ FALLBACK_INDIVIDUAL_CALLS = False

 FALLBACK_LOG_DENIED = "off"

 FALLBACK_AUTOMATIC_HELPERS = "system"

 FALLBACK_FIREWALL_BACKEND = "iptables"

+FALLBACK_ALLOW_ZONE_DRIFTING = False

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

index b1643a1ebff4..5d3cf6e6ce44 100644

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

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

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

         self._automatic_helpers = config.FALLBACK_AUTOMATIC_HELPERS

         self._firewall_backend = config.FALLBACK_FIREWALL_BACKEND

         self.nf_conntrack_helper_setting = 0

+        self._allow_zone_drifting = config.FALLBACK_ALLOW_ZONE_DRIFTING

     def individual_calls(self):

         return self._individual_calls

@@ -269,6 +270,19 @@ class Firewall(object):

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

                                self._automatic_helpers)

+            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 9aee2dc6f9b7..a640d8e2f201 100644

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

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

@@ -28,9 +28,9 @@ 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" ]

+               "AutomaticHelpers", "AllowZoneDrifting" ]

 class firewalld_conf(object):

     def __init__(self, filename):

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

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

             self.set("LogDenied", config.FALLBACK_LOG_DENIED)

             self.set("AutomaticHelpers", config.FALLBACK_AUTOMATIC_HELPERS)

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

             raise

         for line in f:

@@ -174,6 +175,14 @@ class firewalld_conf(object):

                             config.FALLBACK_AUTOMATIC_HELPERS)

             self.set("AutomaticHelpers", str(config.FALLBACK_AUTOMATIC_HELPERS))

+        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 cd640ba881ca..86b4e4428748 100644

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

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

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

                                                 "IndividualCalls": "readwrite",

                                                 "LogDenied": "readwrite",

                                                 "AutomaticHelpers": "readwrite",

+                                                "AllowZoneDrifting": "readwrite",

                                               })

     @handle_exceptions

@@ -484,7 +485,7 @@ class FirewallDConfig(slip.dbus.service.Object):

     def _get_property(self, prop):

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

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

-                         "LogDenied", "AutomaticHelpers" ]:

+                         "LogDenied", "AutomaticHelpers", "AllowZoneDrifting"]:

             raise dbus.exceptions.DBusException(

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

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

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

             if value is None:

                 value = config.FALLBACK_AUTOMATIC_HELPERS

             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):

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

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

         elif prop == "AutomaticHelpers":

             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: "

@@ -583,7 +590,7 @@ class FirewallDConfig(slip.dbus.service.Object):

         if interface_name == config.dbus.DBUS_INTERFACE_CONFIG:

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

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

-                       "LogDenied", "AutomaticHelpers" ]:

+                       "LogDenied", "AutomaticHelpers", "AllowZoneDrifting" ]:

                 ret[x] = self._get_property(x)

         elif interface_name in [ config.dbus.DBUS_INTERFACE_CONFIG_DIRECT,

                                  config.dbus.DBUS_INTERFACE_CONFIG_POLICIES ]:

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

         if interface_name == config.dbus.DBUS_INTERFACE_CONFIG:

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

                                   "IPv6_rpfilter", "IndividualCalls",

-                                  "LogDenied", "AutomaticHelpers" ]:

+                                  "LogDenied", "AutomaticHelpers",

+                                  "AllowZoneDrifting" ]:

                 if property_name == "MinimalMark":

                     try:

                         int(new_value)

@@ -638,6 +646,12 @@ 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()

                 self.PropertiesChanged(interface_name,

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

index 05eb3dd5f650..0884e21b6368 100644

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

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

@@ -3,6 +3,7 @@ FWD_START_TEST([firewalld.conf])

 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 "system"

 string "CleanupOnExit" : variant string "no"

 string "DefaultZone" : variant string "public"

@@ -13,6 +14,7 @@ string "LogDenied" : variant string "off"

 string "MinimalMark" : variant int32 100

 ])], [

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

+string "AllowZoneDrifting" : variant string "no"

 string "AutomaticHelpers" : variant string "system"

 string "CleanupOnExit" : variant string "no"

 string "DefaultZone" : variant string "public"

@@ -39,6 +41,7 @@ _helper([LogDenied], [string:"all"], [variant string "all"])

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

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

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

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

 dnl Note: DefaultZone is RO

 m4_undefine([_helper])

-- 

2.23.0