Blob Blame History Raw
From 3fbf366505d866c042e9dbc29a3fb6f30aff5459 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/35] 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 cb71601436854404b59e53fbdf3eaea1dec9bd80)
---
 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       |  2 ++
 8 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/config/firewalld.conf b/config/firewalld.conf
index 423b7ea0733a..ebf8021226b7 100644
--- a/config/firewalld.conf
+++ b/config/firewalld.conf
@@ -71,3 +71,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 1e229ed1d8b6..8108066e88bf 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 4a81e8e61858..f72bad526d65 100644
--- a/doc/xml/firewalld.dbus.xml
+++ b/doc/xml/firewalld.dbus.xml
@@ -2577,6 +2577,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 5bb318c5b269..c009d93e4164 100644
--- a/src/firewall/config/__init__.py.in
+++ b/src/firewall/config/__init__.py.in
@@ -132,3 +132,4 @@ FALLBACK_AUTOMATIC_HELPERS = "system"
 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 a09c022a2baf..07bc9f375771 100644
--- a/src/firewall/core/fw.py
+++ b/src/firewall/core/fw.py
@@ -127,6 +127,7 @@ class Firewall(object):
         self.nf_conntrack_helper_setting = 0
         self.nf_conntrack_helpers = { }
         self.nf_nat_helpers = { }
+        self._allow_zone_drifting = config.FALLBACK_ALLOW_ZONE_DRIFTING
 
     def individual_calls(self):
         return self._individual_calls
@@ -324,6 +325,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 c7a7ba283e0e..aec62e3a753c 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 b1b839da82ea..4315c6ac1589 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 45559311eabb..65ac702f4713 100644
--- a/src/tests/dbus/firewalld.conf.at
+++ b/src/tests/dbus/firewalld.conf.at
@@ -3,6 +3,7 @@ AT_KEYWORDS(dbus)
 
 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 "system"
 string "CleanupOnExit" : variant string "no"
 string "DefaultZone" : variant string "public"
@@ -36,6 +37,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