From bb3b827f0ecf207274f6608154449ca2160aac37 Mon Sep 17 00:00:00 2001 From: Eric Garver Date: Fri, 3 Nov 2023 12:58:33 -0400 Subject: [PATCH] fix(nftables): always flush main table on start Resolves: RHEL-5818 --- ...ore-nftables-add-delete-table-helper.patch | 45 ++++++++++ ...les-always-flush-main-table-on-start.patch | 38 +++++++++ ...nUpOnExit-verify-restart-does-not-du.patch | 82 +++++++++++++++++++ ...ables-policy-use-delete-table-helper.patch | 32 ++++++++ firewalld.spec | 9 +- 5 files changed, 205 insertions(+), 1 deletion(-) create mode 100644 0014-v1.2.0-chore-nftables-add-delete-table-helper.patch create mode 100644 0015-v1.2.0-fix-nftables-always-flush-main-table-on-start.patch create mode 100644 0016-v1.2.0-test-CleanUpOnExit-verify-restart-does-not-du.patch create mode 100644 0017-v1.2.0-chore-nftables-policy-use-delete-table-helper.patch diff --git a/0014-v1.2.0-chore-nftables-add-delete-table-helper.patch b/0014-v1.2.0-chore-nftables-add-delete-table-helper.patch new file mode 100644 index 0000000..5f34719 --- /dev/null +++ b/0014-v1.2.0-chore-nftables-add-delete-table-helper.patch @@ -0,0 +1,45 @@ +From 08f76e2aa6d7ca35cfb626f20ace1f9036cda3a0 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Mon, 14 Aug 2023 09:13:29 -0400 +Subject: [PATCH 14/17] v1.2.0: chore(nftables): add delete table helper + +This is to workaround an nftables issue where using the "delete" verb on +a table that does not exist will throw ENOENT. We can't use the newer +"destroy" verb because it's too new to rely upon. + +A simple hack is to always add the table before deleting it. The "add" +is ignored if the table already exists. + +(cherry picked from commit 8be561d26931832f000526cc41293700faa6c877) +--- + src/firewall/core/nftables.py | 14 ++++++++++++++ + 1 file changed, 14 insertions(+) + +diff --git a/src/firewall/core/nftables.py b/src/firewall/core/nftables.py +index 2764bcf93645..1959bdce73be 100644 +--- a/src/firewall/core/nftables.py ++++ b/src/firewall/core/nftables.py +@@ -396,6 +396,20 @@ class nftables(object): + # Tables always exist in nftables + return [table] if table else IPTABLES_TO_NFT_HOOK.keys() + ++ 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. ++ rules = [] ++ for family in ["inet", "ip", "ip6"]: ++ rules.append({"add": {"table": {"family": family, ++ "name": table}}}) ++ rules.append({"delete": {"table": {"family": family, ++ "name": table}}}) ++ return rules ++ + def build_flush_rules(self): + # Policy is stashed in a separate table that we're _not_ going to + # flush. As such, we retain the policy rule handles and ref counts. +-- +2.39.3 + diff --git a/0015-v1.2.0-fix-nftables-always-flush-main-table-on-start.patch b/0015-v1.2.0-fix-nftables-always-flush-main-table-on-start.patch new file mode 100644 index 0000000..9ac7b6f --- /dev/null +++ b/0015-v1.2.0-fix-nftables-always-flush-main-table-on-start.patch @@ -0,0 +1,38 @@ +From 0704ea3fef79cc1532f913ac1598e297016e1905 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Thu, 10 Aug 2023 08:43:03 -0400 +Subject: [PATCH 15/17] v1.2.0: fix(nftables): always flush main table on start + +On start created_tables will not contain the main "firewalld" table so a +flush command is not issued. We should always attempt to flush. If +CleanupOnExit=no, then not flushing causes duplicate rules on restart. + +Fixes: rhbz2222044 +(cherry picked from commit 6a155ea7195f2c720625e2452afa41544b4b4227) +--- + src/firewall/core/nftables.py | 6 ++---- + 1 file changed, 2 insertions(+), 4 deletions(-) + +diff --git a/src/firewall/core/nftables.py b/src/firewall/core/nftables.py +index 1959bdce73be..e3e06d75f663 100644 +--- a/src/firewall/core/nftables.py ++++ b/src/firewall/core/nftables.py +@@ -427,13 +427,11 @@ class nftables(object): + self.policy_priority_counts = {} + self.zone_source_index_cache = {} + +- rules = [] + for family in ["inet", "ip", "ip6"]: + if TABLE_NAME in self.created_tables[family]: +- rules.append({"delete": {"table": {"family": family, +- "name": TABLE_NAME}}}) + self.created_tables[family].remove(TABLE_NAME) +- return rules ++ ++ return self._build_delete_table_rules(TABLE_NAME) + + def _build_set_policy_rules_ct_rules(self, enable): + add_del = { True: "add", False: "delete" }[enable] +-- +2.39.3 + diff --git a/0016-v1.2.0-test-CleanUpOnExit-verify-restart-does-not-du.patch b/0016-v1.2.0-test-CleanUpOnExit-verify-restart-does-not-du.patch new file mode 100644 index 0000000..5c18204 --- /dev/null +++ b/0016-v1.2.0-test-CleanUpOnExit-verify-restart-does-not-du.patch @@ -0,0 +1,82 @@ +From 8c79246dbc5b8945c22b313ad51be698f2b61316 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Wed, 9 Aug 2023 14:39:08 -0400 +Subject: [PATCH 16/17] v1.2.0: test(CleanUpOnExit): verify restart does not + duplicate rules + +Coverage: rhbz2222044 +(cherry picked from commit c66e752a00c05a5afa58904850d244f50528059e) +--- + src/tests/regression/regression.at | 1 + + src/tests/regression/rhbz2222044.at | 50 +++++++++++++++++++++++++++++ + 2 files changed, 51 insertions(+) + create mode 100644 src/tests/regression/rhbz2222044.at + +diff --git a/src/tests/regression/regression.at b/src/tests/regression/regression.at +index 889c66dd175d..bc9aeb1a8624 100644 +--- a/src/tests/regression/regression.at ++++ b/src/tests/regression/regression.at +@@ -52,3 +52,4 @@ m4_include([regression/rhbz2181406.at]) + m4_include([regression/ipset_scale.at]) + m4_include([regression/gh881.at]) + m4_include([regression/gh1011.at]) ++m4_include([regression/rhbz2222044.at]) +diff --git a/src/tests/regression/rhbz2222044.at b/src/tests/regression/rhbz2222044.at +new file mode 100644 +index 000000000000..9f3b1615b2f9 +--- /dev/null ++++ b/src/tests/regression/rhbz2222044.at +@@ -0,0 +1,50 @@ ++FWD_START_TEST([duplicate rules after restart]) ++AT_KEYWORDS(rhbz2222044) ++AT_SKIP_IF([! NS_CMD([command -v wc >/dev/null 2>&1])]) ++ ++dnl rules have not changed so rule count should not change ++m4_define([check_rule_count], [ ++m4_if(nftables, FIREWALL_BACKEND, [ ++NS_CHECK([nft list table inet firewalld | wc -l], 0, [dnl ++237 ++]) ++NS_CHECK([nft list table ip firewalld | wc -l], 0, [dnl ++105 ++]) ++NS_CHECK([nft list table ip6 firewalld | wc -l], 0, [dnl ++105 ++]) ++], [ dnl iptables ++NS_CHECK([iptables-save | wc -l], 0, [dnl ++256 ++]) ++])]) ++ ++dnl -------------------------- ++dnl -------------------------- ++ ++AT_CHECK([sed -i 's/^CleanupOnExit.*/CleanupOnExit=yes/' ./firewalld.conf]) ++FWD_RELOAD() ++ ++check_rule_count() ++FWD_RESTART() ++check_rule_count() ++ ++check_rule_count() ++FWD_RELOAD() ++check_rule_count() ++ ++dnl Now do it again, but with CleanupOnExit=no ++AT_CHECK([sed -i 's/^CleanupOnExit.*/CleanupOnExit=no/' ./firewalld.conf]) ++FWD_RELOAD() ++ ++check_rule_count() ++FWD_RESTART() ++check_rule_count() ++ ++check_rule_count() ++FWD_RELOAD() ++check_rule_count() ++ ++m4_undefine([check_rule_count]) ++FWD_END_TEST() +-- +2.39.3 + diff --git a/0017-v1.2.0-chore-nftables-policy-use-delete-table-helper.patch b/0017-v1.2.0-chore-nftables-policy-use-delete-table-helper.patch new file mode 100644 index 0000000..8bb305d --- /dev/null +++ b/0017-v1.2.0-chore-nftables-policy-use-delete-table-helper.patch @@ -0,0 +1,32 @@ +From 2ca79f8ebbadcf39f9b378b7fd296fcef13a4c54 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Mon, 14 Aug 2023 09:21:17 -0400 +Subject: [PATCH 17/17] v1.2.0: chore(nftables): policy: use delete table + helper + +Use the new table delete helper when deleting the policy table. + +(cherry picked from commit a291a5d2f03711c2c6b0079128626204229ad79e) +--- + src/firewall/core/nftables.py | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/src/firewall/core/nftables.py b/src/firewall/core/nftables.py +index e3e06d75f663..2a13b2678a94 100644 +--- a/src/firewall/core/nftables.py ++++ b/src/firewall/core/nftables.py +@@ -489,9 +489,9 @@ class nftables(object): + if policy_key in self.rule_to_handle: + rules.append(rule) + ++ rules += self._build_delete_table_rules(TABLE_NAME_POLICY) ++ + if TABLE_NAME_POLICY in self.created_tables["inet"]: +- rules.append({"delete": {"table": {"family": "inet", +- "name": TABLE_NAME_POLICY}}}) + self.created_tables["inet"].remove(TABLE_NAME_POLICY) + else: + FirewallError(UNKNOWN_ERROR, "not implemented") +-- +2.39.3 + diff --git a/firewalld.spec b/firewalld.spec index cb4f0b2..c49fce7 100644 --- a/firewalld.spec +++ b/firewalld.spec @@ -1,7 +1,7 @@ Summary: A firewall daemon with D-Bus interface providing a dynamic firewall Name: firewalld Version: 0.9.11 -Release: 3%{?dist} +Release: 4%{?dist} URL: http://www.firewalld.org License: GPLv2+ Source0: https://github.com/firewalld/firewalld/releases/download/v%{version}/firewalld-%{version}.tar.gz @@ -18,6 +18,10 @@ Patch10: 0010-v1.1.0-fix-ipset-further-reduce-cost-of-entry-overla.patch Patch11: 0011-v1.1.0-fix-ipset-exception-on-overlap-checking-empty.patch Patch12: 0012-v1.1.0-test-ipset-verify-remove-entries-from-file.patch Patch13: 0013-v1.2.0-fix-ipset-fix-configuring-IP-range-for-ipsets.patch +Patch14: 0014-v1.2.0-chore-nftables-add-delete-table-helper.patch +Patch15: 0015-v1.2.0-fix-nftables-always-flush-main-table-on-start.patch +Patch16: 0016-v1.2.0-test-CleanUpOnExit-verify-restart-does-not-du.patch +Patch18: 0017-v1.2.0-chore-nftables-policy-use-delete-table-helper.patch BuildArch: noarch BuildRequires: autoconf @@ -219,6 +223,9 @@ desktop-file-install --delete-original \ %{_mandir}/man1/firewall-config*.1* %changelog +* Fri Nov 03 2023 Eric Garver - 0.9.11-4 +- fix(nftables): always flush main table on start + * Fri Nov 03 2023 Eric Garver - 0.9.11-3 - fix(ipset): fix configuring IP range for ipsets with nftables