Blob Blame History Raw
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