rpms/firewalld
7
0
Fork 0
Code Issues Pull Requests Releases Activity
c9s
firewalld/0020-v2.2.0-feat-nftables-table-ownership.patch
Eric Garver bbae99e926 feat(nftables): table ownership 
Resolves: RHEL-17002
2024-07-01 11:11:12 -04:00

418 lines
18 KiB
Diff
Raw Permalink Blame History

From ce75ad2920f457d5b2ce578ead5d8c64f0daa490 Mon Sep 17 00:00:00 2001
From: Eric Garver <eric@garver.life>
Date: Fri, 21 Jun 2024 17:38:29 -0400
Subject: [PATCH 20/22] v2.2.0: feat(nftables): table ownership
This allows using the nftables table flags "owner" and "persist". When
these are in use ONLY firewalld will be able to change the firewalld
table/rules. All other processes will be blocked. This enhances
firewalld robustness by guaranteeing that firewalld's rules can not be
modified without it's knowledge, e.g. by other entities.
This also allows firewalld and nftables services to coexist. An "nft
flush ruleset" will NOT flush the "firewalld" table.
Fixes: RHEL-17002
(cherry picked from commit becd083fc2905921651af73cb15ce8c9aba9203b)
---
config/firewalld.conf | 8 +++
doc/xml/firewalld.conf.xml | 13 ++++
doc/xml/firewalld.dbus.xml | 11 ++++
src/firewall/config/__init__.py.in | 1 +
src/firewall/core/fw.py | 22 +++++++
src/firewall/core/io/firewalld_conf.py | 6 +-
src/firewall/core/nftables.py | 83 ++++++++++++++++++++++----
src/firewall/server/config.py | 16 +++--
src/tests/dbus/firewalld.conf.at | 2 +
src/tests/regression/rhbz2222044.at | 5 ++
10 files changed, 150 insertions(+), 17 deletions(-)
diff --git a/config/firewalld.conf b/config/firewalld.conf
index cf95af3eea8e..28345a13d54e 100644
--- a/config/firewalld.conf
+++ b/config/firewalld.conf
@@ -93,3 +93,11 @@ ReloadPolicy=INPUT:DROP,FORWARD:DROP,OUTPUT:DROP
# internet.
# Defaults to "yes".
RFC3964_IPv4=yes
+
+# NftablesTableOwner
+# If set to yes, the generated nftables rule set will be owned exclusively by
+# firewalld. This prevents other entities from mistakenly (or maliciously)
+# modifying firewalld's rule set. If you intentionally modify firewalld's
+# rules, then you will have to set this to "no".
+# Defaults to "yes".
+NftablesTableOwner=yes
diff --git a/doc/xml/firewalld.conf.xml b/doc/xml/firewalld.conf.xml
index 4b9bfdad15be..cf7134c04012 100644
--- a/doc/xml/firewalld.conf.xml
+++ b/doc/xml/firewalld.conf.xml
@@ -247,6 +247,19 @@
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>NftablesTableOwner</option></term>
+ <listitem>
+ <para>
+ If set to yes, the generated nftables rule set will be owned exclusively by
+ firewalld. This prevents other entities from mistakenly (or maliciously)
+ modifying firewalld's rule set. If you intentionally modify firewalld's
+ rules, then you will have to set this to "no".
+ Defaults to "yes".
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</refsect1>
diff --git a/doc/xml/firewalld.dbus.xml b/doc/xml/firewalld.dbus.xml
index f04cf5ae757b..00d7e8fbe5b1 100644
--- a/doc/xml/firewalld.dbus.xml
+++ b/doc/xml/firewalld.dbus.xml
@@ -2905,6 +2905,17 @@
</para>
</listitem>
</varlistentry>
+ <varlistentry id="FirewallD1.config.Properties.NftablesTableOwner">
+ <term>NftablesTableOwner - s - (rw)</term>
+ <listitem>
+ <para>
+ If set to yes, the generated nftables rule set will be owned exclusively by
+ firewalld. This prevents other entities from mistakenly (or maliciously)
+ modifying firewalld's rule set. If you intentionally modify firewalld's
+ rules, then you will have to set this to "no".
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</refsect3>
</refsect2>
diff --git a/src/firewall/config/__init__.py.in b/src/firewall/config/__init__.py.in
index 2eda337e8180..16291fcf795a 100644
--- a/src/firewall/config/__init__.py.in
+++ b/src/firewall/config/__init__.py.in
@@ -138,3 +138,4 @@ FALLBACK_FLUSH_ALL_ON_RELOAD = True
FALLBACK_RELOAD_POLICY = "INPUT:DROP,FORWARD:DROP,OUTPUT:DROP"
FALLBACK_RFC3964_IPV4 = True
FALLBACK_ALLOW_ZONE_DRIFTING = False
+FALLBACK_NFTABLES_TABLE_OWNER = True
diff --git a/src/firewall/core/fw.py b/src/firewall/core/fw.py
index f91ff53fc37a..557b6e527dbd 100644
--- a/src/firewall/core/fw.py
+++ b/src/firewall/core/fw.py
@@ -119,6 +119,7 @@ class Firewall(object):
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
+ self._nftables_table_owner = config.FALLBACK_NFTABLES_TABLE_OWNER
if self._offline:
self.ip4tables_enabled = False
@@ -290,6 +291,17 @@ class Firewall(object):
log.debug1("ebtables-restore is not supporting the --noflush "
"option, will therefore not be used")
+ self.nftables_backend.probe_support()
+
+ if (
+ self._nftables_table_owner
+ and not self.nftables_backend.supports_table_owner
+ ):
+ log.info1(
+ "Configuration has NftablesTableOwner=True, but it's "
+ "not supported by nftables. Table ownership will be disabled."
+ )
+
def _start_load_firewalld_conf(self):
# load firewalld config
log.debug1("Loading firewalld config file '%s'", config.FIREWALLD_CONF)
@@ -378,6 +390,16 @@ class Firewall(object):
log.debug1("RFC3964_IPv4 is set to '%s'",
self._rfc3964_ipv4)
+ if self._firewalld_conf.get("NftablesTableOwner"):
+ value = self._firewalld_conf.get("NftablesTableOwner")
+ if value.lower() in ["no", "false"]:
+ self._nftables_table_owner = False
+ else:
+ self._nftables_table_owner = True
+ log.debug1(
+ "NftablesTableOwner is set to '%s'", self._nftables_table_owner
+ )
+
self.config.set_firewalld_conf(copy.deepcopy(self._firewalld_conf))
def _start_load_lockdown_whitelist(self):
diff --git a/src/firewall/core/io/firewalld_conf.py b/src/firewall/core/io/firewalld_conf.py
index 20072e26cfcd..d5b47666c0f0 100644
--- a/src/firewall/core/io/firewalld_conf.py
+++ b/src/firewall/core/io/firewalld_conf.py
@@ -32,7 +32,7 @@ valid_keys = [ "DefaultZone", "MinimalMark", "CleanupOnExit",
"CleanupModulesOnExit", "Lockdown", "IPv6_rpfilter",
"IndividualCalls", "LogDenied", "AutomaticHelpers",
"FirewallBackend", "FlushAllOnReload", "RFC3964_IPv4",
- "AllowZoneDrifting", "ReloadPolicy" ]
+ "AllowZoneDrifting", "ReloadPolicy", "NftablesTableOwner" ]
class firewalld_conf(object):
def __init__(self, filename):
@@ -81,6 +81,10 @@ class firewalld_conf(object):
self.set("ReloadPolicy", config.FALLBACK_RELOAD_POLICY)
self.set("RFC3964_IPv4", "yes" if config.FALLBACK_RFC3964_IPV4 else "no")
self.set("AllowZoneDrifting", "yes" if config.FALLBACK_ALLOW_ZONE_DRIFTING else "no")
+ self.set(
+ "NftablesTableOwner",
+ "yes" if config.FALLBACK_NFTABLES_TABLE_OWNER else "no",
+ )
def sanity_check(self):
if self.get("FirewallBackend") == "iptables" and self.get("IPv6_rpfilter") in (
diff --git a/src/firewall/core/nftables.py b/src/firewall/core/nftables.py
index 5a49b34e3a4f..8115bcb9d7f4 100644
--- a/src/firewall/core/nftables.py
+++ b/src/firewall/core/nftables.py
@@ -36,6 +36,7 @@ from nftables.nftables import Nftables
TABLE_NAME = "firewalld"
TABLE_NAME_POLICY = TABLE_NAME + "_" + "policy_drop"
+TABLE_NAME_PROBE = TABLE_NAME + "_" + "probe"
POLICY_CHAIN_PREFIX = "policy_"
# Map iptables (table, chain) to hooks and priorities.
@@ -169,6 +170,7 @@ class nftables(object):
def __init__(self, fw):
self._fw = fw
self.restore_command_exists = True
+ self.supports_table_owner = False
self.available_tables = []
self.rule_to_handle = {}
self.rule_ref_count = {}
@@ -180,6 +182,57 @@ class nftables(object):
self.nftables.set_echo_output(True)
self.nftables.set_handle_output(True)
+ def _probe_support_table_owner(self):
+ try:
+ rules = {
+ "nftables": [
+ {"metainfo": {"json_schema_version": 1}},
+ {
+ "add": {
+ "table": {
+ "family": "inet",
+ "name": TABLE_NAME_PROBE,
+ "flags": ["owner", "persist"],
+ }
+ }
+ },
+ ]
+ }
+
+ rc, output, _ = self.nftables.json_cmd(rules)
+ if rc:
+ raise ValueError("nftables probe table owner failed")
+
+ # old nftables versions would ignore table flags in JSON, so we
+ # must parse back and verify the flags are set.
+ rules = {
+ "nftables": [
+ {"metainfo": {"json_schema_version": 1}},
+ {"list": {"table": {"family": "inet", "name": TABLE_NAME_PROBE}}},
+ ]
+ }
+ self.nftables.set_echo_output(False)
+ rc, output, _ = self.nftables.json_cmd(rules)
+ self.nftables.set_echo_output(True)
+ flags = output["nftables"][1]["table"]["flags"]
+
+ self.set_rule(
+ {"delete": {"table": {"family": "inet", "name": TABLE_NAME_PROBE}}},
+ self._fw.get_log_denied(),
+ )
+
+ if "owner" not in flags or "persist" not in flags:
+ raise ValueError("nftables probe table owner failed")
+
+ log.debug2("nftables: probe_support(): owner flag is supported.")
+ self.supports_table_owner = True
+ except:
+ log.debug2("nftables: probe_support(): owner flag is NOT supported.")
+ self.supports_table_owner = False
+
+ def probe_support(self):
+ self._probe_support_table_owner()
+
def _run_replace_zone_source(self, rule, zone_source_index_cache):
for verb in ["add", "insert", "delete"]:
if verb in rule:
@@ -401,16 +454,27 @@ class nftables(object):
# Tables always exist in nftables
return [table] if table else IPTABLES_TO_NFT_HOOK.keys()
+ def _build_add_table_rules(self, table):
+ rule = {"add": {"table": {"family": "inet", "name": table}}}
+
+ if (
+ table == TABLE_NAME
+ and self._fw._nftables_table_owner
+ and self.supports_table_owner
+ ):
+ rule["add"]["table"]["flags"] = ["owner", "persist"]
+
+ return [rule]
+
def _build_delete_table_rules(self, table):
# To avoid nftables returning ENOENT we always add the table before
# deleting to guarantee it will exist.
#
# In the future, this add+delete should be replaced with "destroy", but
# that verb is too new to rely upon.
- return [{"add": {"table": {"family": "inet",
- "name": table}}},
- {"delete": {"table": {"family": "inet",
- "name": table}}}]
+ return self._build_add_table_rules(table) + [
+ {"delete": {"table": {"family": "inet", "name": table}}},
+ ]
def build_flush_rules(self):
self.rule_to_handle = {}
@@ -437,8 +501,7 @@ class nftables(object):
# a higher priority than our base chains is sufficient.
rules = []
if policy == "PANIC":
- rules.append({"add": {"table": {"family": "inet",
- "name": TABLE_NAME_POLICY}}})
+ rules.extend(self._build_add_table_rules(TABLE_NAME_POLICY))
# Use "raw" priority for panic mode. This occurs before
# conntrack, mangle, nat, etc
@@ -451,8 +514,7 @@ class nftables(object):
"prio": -300 + NFT_HOOK_OFFSET - 1,
"policy": "drop"}}})
elif policy == "DROP":
- rules.append({"add": {"table": {"family": "inet",
- "name": TABLE_NAME_POLICY}}})
+ rules.extend(self._build_add_table_rules(TABLE_NAME_POLICY))
# To drop everything except existing connections we use
# "filter" because it occurs _after_ conntrack.
@@ -511,10 +573,7 @@ class nftables(object):
return list(supported)
def build_default_tables(self):
- default_tables = []
- default_tables.append({"add": {"table": {"family": "inet",
- "name": TABLE_NAME}}})
- return default_tables
+ return self._build_add_table_rules(TABLE_NAME)
def build_default_rules(self, log_denied="off"):
default_rules = []
diff --git a/src/firewall/server/config.py b/src/firewall/server/config.py
index b805e497bb05..c07a1e6a2503 100644
--- a/src/firewall/server/config.py
+++ b/src/firewall/server/config.py
@@ -110,6 +110,7 @@ class FirewallDConfig(DbusServiceObject):
"FlushAllOnReload": "readwrite",
"RFC3964_IPv4": "readwrite",
"AllowZoneDrifting": "readwrite",
+ "NftablesTableOwner": "readwrite",
}
)
@@ -565,7 +566,7 @@ class FirewallDConfig(DbusServiceObject):
"CleanupModulesOnExit", "Lockdown", "IPv6_rpfilter",
"IndividualCalls", "LogDenied", "AutomaticHelpers",
"FirewallBackend", "FlushAllOnReload", "RFC3964_IPv4",
- "AllowZoneDrifting", "IPv6_rpfilter2" ]:
+ "AllowZoneDrifting", "IPv6_rpfilter2", "NftablesTableOwner" ]:
raise dbus.exceptions.DBusException(
"org.freedesktop.DBus.Error.InvalidArgs: "
"Property '%s' does not exist" % prop)
@@ -631,6 +632,10 @@ class FirewallDConfig(DbusServiceObject):
if value is None:
value = "yes" if config.FALLBACK_ALLOW_ZONE_DRIFTING else "no"
return dbus.String(value)
+ elif prop == "NftablesTableOwner":
+ if value is None:
+ value = "yes" if config.FALLBACK_NFTABLES_TABLE_OWNER else "no"
+ return dbus.String(value)
@dbus_handle_exceptions
def _get_dbus_property(self, prop):
@@ -662,6 +667,8 @@ class FirewallDConfig(DbusServiceObject):
return dbus.String(self._get_property(prop))
elif prop == "AllowZoneDrifting":
return dbus.String(self._get_property(prop))
+ elif prop == "NftablesTableOwner":
+ return dbus.String(self._get_property(prop))
else:
raise dbus.exceptions.DBusException(
"org.freedesktop.DBus.Error.InvalidArgs: "
@@ -701,7 +708,7 @@ class FirewallDConfig(DbusServiceObject):
"CleanupModulesOnExit", "Lockdown", "IPv6_rpfilter",
"IndividualCalls", "LogDenied", "AutomaticHelpers",
"FirewallBackend", "FlushAllOnReload", "RFC3964_IPv4",
- "AllowZoneDrifting", "IPv6_rpfilter2" ]:
+ "AllowZoneDrifting", "IPv6_rpfilter2", "NftablesTableOwner" ]:
ret[x] = self._get_property(x)
elif interface_name in [ config.dbus.DBUS_INTERFACE_CONFIG_DIRECT,
config.dbus.DBUS_INTERFACE_CONFIG_POLICIES ]:
@@ -730,11 +737,12 @@ class FirewallDConfig(DbusServiceObject):
"IPv6_rpfilter", "IndividualCalls",
"LogDenied",
"FirewallBackend", "FlushAllOnReload",
- "RFC3964_IPv4", "IPv6_rpfilter2"]:
+ "RFC3964_IPv4", "IPv6_rpfilter2",
+ "NftablesTableOwner" ]:
if property_name in [ "CleanupOnExit", "CleanupModulesOnExit",
"Lockdown", "IPv6_rpfilter",
"IndividualCalls", "FlushAllOnReload",
- "RFC3964_IPv4"]:
+ "RFC3964_IPv4", "NftablesTableOwner" ]:
if new_value.lower() not in [ "yes", "no",
"true", "false" ]:
raise FirewallError(errors.INVALID_VALUE,
diff --git a/src/tests/dbus/firewalld.conf.at b/src/tests/dbus/firewalld.conf.at
index 61c220f3e59c..2ead41baa00a 100644
--- a/src/tests/dbus/firewalld.conf.at
+++ b/src/tests/dbus/firewalld.conf.at
@@ -30,6 +30,7 @@ string "IndividualCalls" : variant string m4_escape(["${EXPECTED_INDIVIDUAL_CALL
string "Lockdown" : variant string "no"
string "LogDenied" : variant string "off"
string "MinimalMark" : variant int32 100
+string "NftablesTableOwner" : variant string "yes"
string "RFC3964_IPv4" : variant string "yes"
])
@@ -54,6 +55,7 @@ _helper([CleanupModulesOnExit], [string:"yes"], [variant string "yes"])
_helper([CleanupOnExit], [string:"no"], [variant string "no"])
_helper([RFC3964_IPv4], [string:"no"], [variant string "no"])
_helper([AllowZoneDrifting], [string:"yes"], [variant string "no"])
+_helper([NftablesTableOwner], [string:"no"], [variant string "no"])
dnl Note: DefaultZone is RO
m4_undefine([_helper])
diff --git a/src/tests/regression/rhbz2222044.at b/src/tests/regression/rhbz2222044.at
index 7e3454509188..2d0333865076 100644
--- a/src/tests/regression/rhbz2222044.at
+++ b/src/tests/regression/rhbz2222044.at
@@ -2,6 +2,11 @@ FWD_START_TEST([duplicate rules after restart])
AT_KEYWORDS(rhbz2222044)
AT_SKIP_IF([! NS_CMD([command -v wc >/dev/null 2>&1])])
+dnl Disable for this test because CI do not support table owner. It's very new
+dnl in nftables.
+AT_CHECK([sed -i 's/^NftablesTableOwner=.*/NftablesTableOwner=no/' ./firewalld.conf])
+FWD_RELOAD()
+
dnl rules have not changed so rule count should not change
m4_define([check_rule_count], [
m4_if(nftables, FIREWALL_BACKEND, [
--
2.43.5
Reference in New Issue View Git Blame Copy Permalink