diff --git a/SOURCES/0037-docs-firewall-cmd-client-conntrack-helpers-must-use-.patch b/SOURCES/0037-docs-firewall-cmd-client-conntrack-helpers-must-use-.patch new file mode 100644 index 0000000..6b2607d --- /dev/null +++ b/SOURCES/0037-docs-firewall-cmd-client-conntrack-helpers-must-use-.patch @@ -0,0 +1,74 @@ +From 78060c945be591b4fe8a1b0d3f206585d3948676 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Fri, 2 Jul 2021 11:19:18 -0400 +Subject: [PATCH 37/50] docs(firewall-*cmd): client conntrack helpers must use + a policy + +Fixes: rhbz 1899933 +Fixes: rhbz 1975484 +(cherry picked from commit adb4ccd88e6c1fd460c9c674d89fdf89299c3970) +(cherry picked from commit 8cd0da7032080ada6b80b7f97faec6a30a8d45f5) +--- + doc/xml/firewall-cmd.xml.in | 17 +++++++++++++++++ + doc/xml/firewall-offline-cmd.xml | 17 +++++++++++++++++ + 2 files changed, 34 insertions(+) + +diff --git a/doc/xml/firewall-cmd.xml.in b/doc/xml/firewall-cmd.xml.in +index 691117f3dbff..8cd67e388ef5 100644 +--- a/doc/xml/firewall-cmd.xml.in ++++ b/doc/xml/firewall-cmd.xml.in +@@ -634,6 +634,23 @@ + + The option is not combinable with the option. + ++ ++ Note: Some services define connection tracking helpers. ++ Helpers that may operate in client mode (e.g. tftp) must be added to an ++ outbound policy instead of a zone to take effect for clients. Otherwise ++ the helper will not be applied to the outbound traffic. The related ++ traffic, as defined by the connection tracking helper, on the return ++ path (ingress) will be allowed by the stateful firewall rules. ++ ++ ++ An example of an outbound policy for connection tracking helpers: ++ ++# firewall-cmd --permanent --new-policy clientConntrack ++# firewall-cmd --permanent --policy clientConntrack --add-ingress-zone HOST ++# firewall-cmd --permanent --policy clientConntrack --add-egress-zone ANY ++# firewall-cmd --permanent --policy clientConntrack --add-service tftp ++ ++ + + + +diff --git a/doc/xml/firewall-offline-cmd.xml b/doc/xml/firewall-offline-cmd.xml +index 92ec55be4623..8e2dd7989956 100644 +--- a/doc/xml/firewall-offline-cmd.xml ++++ b/doc/xml/firewall-offline-cmd.xml +@@ -722,6 +722,23 @@ + + The service is one of the firewalld provided services. To get a list of the supported services, use firewall-cmd --get-services. + ++ ++ Note: Some services define connection tracking helpers. ++ Helpers that may operate in client mode (e.g. tftp) must be added to an ++ outbound policy instead of a zone to take effect for clients. Otherwise ++ the helper will not be applied to the outbound traffic. The related ++ traffic, as defined by the connection tracking helper, on the return ++ path (ingress) will be allowed by the stateful firewall rules. ++ ++ ++ An example of an outbound policy for connection tracking helpers: ++ ++# firewall-cmd --new-policy clientConntrack ++# firewall-cmd --policy clientConntrack --add-ingress-zone HOST ++# firewall-cmd --policy clientConntrack --add-egress-zone ANY ++# firewall-cmd --policy clientConntrack --add-service tftp ++ ++ + + + +-- +2.27.0 + diff --git a/SOURCES/0038-fix-nftables-do-not-log-icmp-block-if-inversion.patch b/SOURCES/0038-fix-nftables-do-not-log-icmp-block-if-inversion.patch new file mode 100644 index 0000000..f6c5b0b --- /dev/null +++ b/SOURCES/0038-fix-nftables-do-not-log-icmp-block-if-inversion.patch @@ -0,0 +1,29 @@ +From de28755c4e14224f6303c864327fffe7d2639268 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Mon, 13 Sep 2021 15:45:53 -0400 +Subject: [PATCH 38/50] fix(nftables): do not log icmp block if inversion + +Fixes: #696 +Fixes: rhbz1945833 +(cherry picked from commit 50a5ed2d0fa6169c6780488dae931a3b4fce47ab) +(cherry picked from commit a451b033200b289c6fac823f7dce23c37a38a3d1) +--- + src/firewall/core/nftables.py | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/firewall/core/nftables.py b/src/firewall/core/nftables.py +index e3ae988bbdab..29a9a2492032 100644 +--- a/src/firewall/core/nftables.py ++++ b/src/firewall/core/nftables.py +@@ -1601,7 +1601,7 @@ class nftables(object): + rule.update(self._rich_rule_priority_fragment(rich_rule)) + rules.append({add_del: {"rule": rule}}) + else: +- if self._fw.get_log_denied() != "off" and self._fw.policy.query_icmp_block_inversion(policy): ++ if self._fw.get_log_denied() != "off" and not self._fw.policy.query_icmp_block_inversion(policy): + rules.append({add_del: {"rule": {"family": "inet", + "table": TABLE_NAME, + "chain": final_chain, +-- +2.27.0 + diff --git a/SOURCES/0039-test-icmp-don-t-log-blocked-if-ICMP-inversion.patch b/SOURCES/0039-test-icmp-don-t-log-blocked-if-ICMP-inversion.patch new file mode 100644 index 0000000..bcc8fe3 --- /dev/null +++ b/SOURCES/0039-test-icmp-don-t-log-blocked-if-ICMP-inversion.patch @@ -0,0 +1,135 @@ +From b2075eb46f1798ba897ca443ea14872b17267d69 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Mon, 13 Sep 2021 14:54:42 -0400 +Subject: [PATCH 39/50] test(icmp): don't log blocked if ICMP inversion + +Coverage: #696 +Coverage: rhbz1945833 +(cherry picked from commit b7d9a74731f9c5a1a6faf0f2959adcc315d2ca16) +(cherry picked from commit d45c614967cec6a608c93c1e8531089d7b1150bb) +--- + src/tests/regression/gh696.at | 102 +++++++++++++++++++++++++++++ + src/tests/regression/regression.at | 1 + + 2 files changed, 103 insertions(+) + create mode 100644 src/tests/regression/gh696.at + +diff --git a/src/tests/regression/gh696.at b/src/tests/regression/gh696.at +new file mode 100644 +index 000000000000..19b8d485a0a5 +--- /dev/null ++++ b/src/tests/regression/gh696.at +@@ -0,0 +1,102 @@ ++FWD_START_TEST([icmp-block-inversion no log blocked]) ++AT_KEYWORDS(icmp gh696 rhbz1945833) ++ ++FWD_CHECK([--permanent --zone public --remove-icmp-block-inversion], 0, [ignore], [ignore]) ++FWD_CHECK([--permanent --zone public --add-icmp-block echo-request], 0, [ignore]) ++FWD_RELOAD() ++ ++NFT_LIST_RULES([inet], [filter_IN_public_deny], 0, [dnl ++ table inet firewalld { ++ chain filter_IN_public_deny { ++ icmp type echo-request reject with icmpx type admin-prohibited ++ icmpv6 type echo-request reject with icmpx type admin-prohibited ++ } ++ } ++]) ++ ++IPTABLES_LIST_RULES([filter], [IN_public_deny], 0, [dnl ++ REJECT icmp -- 0.0.0.0/0 0.0.0.0/0 icmptype 8 reject-with icmp-host-prohibited ++]) ++IP6TABLES_LIST_RULES([filter], [IN_public_deny], 0, [dnl ++ REJECT icmpv6 ::/0 ::/0 ipv6-icmptype 128 reject-with icmp6-adm-prohibited ++]) ++ ++dnl since inversion is disabled we should get logs when the ICMP is blocked. ++FWD_CHECK([--set-log-denied all], 0, [ignore]) ++ ++NFT_LIST_RULES([inet], [filter_IN_public_deny], 0, [dnl ++ table inet firewalld { ++ chain filter_IN_public_deny { ++ icmp type echo-request log prefix ""filter_zone_public_HOST_ICMP_BLOCK: "" ++ icmp type echo-request reject with icmpx type admin-prohibited ++ icmpv6 type echo-request log prefix ""filter_zone_public_HOST_ICMP_BLOCK: "" ++ icmpv6 type echo-request reject with icmpx type admin-prohibited ++ } ++ } ++]) ++ ++IPTABLES_LIST_RULES([filter], [IN_public_deny], 0, [dnl ++ LOG icmp -- 0.0.0.0/0 0.0.0.0/0 icmptype 8 LOG flags 0 level 4 prefix "zone_public_HOST_ICMP_BLOCK: " ++ REJECT icmp -- 0.0.0.0/0 0.0.0.0/0 icmptype 8 reject-with icmp-host-prohibited ++]) ++IP6TABLES_LIST_RULES([filter], [IN_public_deny], 0, [dnl ++ LOG icmpv6 ::/0 ::/0 ipv6-icmptype 128 LOG flags 0 level 4 prefix "zone_public_HOST_ICMP_BLOCK: " ++ REJECT icmpv6 ::/0 ::/0 ipv6-icmptype 128 reject-with icmp6-adm-prohibited ++]) ++ ++dnl ######################################## ++dnl ######################################## ++dnl Same as above, but with icmp block inversion. ++dnl ######################################## ++dnl ######################################## ++ ++FWD_CHECK([--permanent --zone public --add-icmp-block-inversion], 0, [ignore]) ++FWD_CHECK([--set-log-denied off], 0, [ignore]) ++ ++NFT_LIST_RULES([inet], [filter_IN_public_allow], 0, [dnl ++ table inet firewalld { ++ chain filter_IN_public_allow { ++ tcp dport 22 ct state new,untracked accept ++ ip6 daddr fe80::/64 udp dport 546 ct state new,untracked accept ++ icmp type echo-request accept ++ icmpv6 type echo-request accept ++ } ++ } ++]) ++ ++IPTABLES_LIST_RULES([filter], [IN_public_allow], 0, [dnl ++ ACCEPT tcp -- 0.0.0.0/0 0.0.0.0/0 tcp dpt:22 ctstate NEW,UNTRACKED ++ ACCEPT icmp -- 0.0.0.0/0 0.0.0.0/0 icmptype 8 ++]) ++IP6TABLES_LIST_RULES([filter], [IN_public_allow], 0, [dnl ++ ACCEPT tcp ::/0 ::/0 tcp dpt:22 ctstate NEW,UNTRACKED ++ ACCEPT udp ::/0 fe80::/64 udp dpt:546 ctstate NEW,UNTRACKED ++ ACCEPT icmpv6 ::/0 ::/0 ipv6-icmptype 128 ++]) ++ ++dnl since inversion is enabled, it should be the same whether set-log-denied is ++dnl enabled or not. ++FWD_CHECK([--set-log-denied all], 0, [ignore]) ++ ++NFT_LIST_RULES([inet], [filter_IN_public_allow], 0, [dnl ++ table inet firewalld { ++ chain filter_IN_public_allow { ++ tcp dport 22 ct state new,untracked accept ++ ip6 daddr fe80::/64 udp dport 546 ct state new,untracked accept ++ icmp type echo-request accept ++ icmpv6 type echo-request accept ++ } ++ } ++]) ++ ++IPTABLES_LIST_RULES([filter], [IN_public_allow], 0, [dnl ++ ACCEPT tcp -- 0.0.0.0/0 0.0.0.0/0 tcp dpt:22 ctstate NEW,UNTRACKED ++ ACCEPT icmp -- 0.0.0.0/0 0.0.0.0/0 icmptype 8 ++]) ++IP6TABLES_LIST_RULES([filter], [IN_public_allow], 0, [dnl ++ ACCEPT tcp ::/0 ::/0 tcp dpt:22 ctstate NEW,UNTRACKED ++ ACCEPT udp ::/0 fe80::/64 udp dpt:546 ctstate NEW,UNTRACKED ++ ACCEPT icmpv6 ::/0 ::/0 ipv6-icmptype 128 ++]) ++ ++FWD_END_TEST([-d '/WARNING: NOT_ENABLED: icmp-block-inversion/d']) +diff --git a/src/tests/regression/regression.at b/src/tests/regression/regression.at +index aadd948a459f..ba41a56b29b5 100644 +--- a/src/tests/regression/regression.at ++++ b/src/tests/regression/regression.at +@@ -42,3 +42,4 @@ m4_include([regression/ipset_netmask_allowed.at]) + m4_include([regression/rhbz1940928.at]) + m4_include([regression/rhbz1936896.at]) + m4_include([regression/rhbz1914935.at]) ++m4_include([regression/gh696.at]) +-- +2.27.0 + diff --git a/SOURCES/0040-fix-nftables-rich-source-address-with-netmask.patch b/SOURCES/0040-fix-nftables-rich-source-address-with-netmask.patch new file mode 100644 index 0000000..740dcf0 --- /dev/null +++ b/SOURCES/0040-fix-nftables-rich-source-address-with-netmask.patch @@ -0,0 +1,38 @@ +From 12fd98893d190df9581d04155fa9207d2adb5573 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Wed, 15 Sep 2021 14:12:37 -0400 +Subject: [PATCH 40/50] fix(nftables): rich: source address with netmask + +Fixes: rhbz1917766 +(cherry picked from commit 3809fef17dc779052a3f050041fe90e3599f35be) +(cherry picked from commit 32d5eb8d94a2b39a4dda10fec351ad6fbab7d486) +--- + src/firewall/core/nftables.py | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/src/firewall/core/nftables.py b/src/firewall/core/nftables.py +index 29a9a2492032..059cd8869dbb 100644 +--- a/src/firewall/core/nftables.py ++++ b/src/firewall/core/nftables.py +@@ -22,6 +22,7 @@ from __future__ import absolute_import + + import copy + import json ++import ipaddress + + from firewall.core.logger import log + from firewall.functions import check_mac, getPortRange, normalizeIP6, \ +@@ -1213,8 +1214,8 @@ class nftables(object): + family = "ip" + elif check_address("ipv4", address): + family = "ip" +- addr_len = address.split("/") +- address = {"prefix": {"addr": addr_len[0], "len": int(addr_len[1])}} ++ normalized_address = ipaddress.IPv4Network(address, strict=False) ++ address = {"prefix": {"addr": normalized_address.network_address.compressed, "len": normalized_address.prefixlen}} + elif check_single_address("ipv6", address): + family = "ip6" + address = normalizeIP6(address) +-- +2.27.0 + diff --git a/SOURCES/0041-test-rich-source-address-with-netmask.patch b/SOURCES/0041-test-rich-source-address-with-netmask.patch new file mode 100644 index 0000000..51f4366 --- /dev/null +++ b/SOURCES/0041-test-rich-source-address-with-netmask.patch @@ -0,0 +1,56 @@ +From 0be3d6ba5d6a1cb17c965a5454cc156fbb2ac867 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Wed, 15 Sep 2021 13:47:01 -0400 +Subject: [PATCH 41/50] test(rich): source address with netmask + +Coverage: rhbz1917766 +(cherry picked from commit 9e9f94061b129e22e8c6fc2f8985d782bfe09689) +(cherry picked from commit 498c6b221ebbca09401ae5f98498c6a148ae602f) +--- + src/tests/regression/regression.at | 1 + + src/tests/regression/rhbz1917766.at | 24 ++++++++++++++++++++++++ + 2 files changed, 25 insertions(+) + create mode 100644 src/tests/regression/rhbz1917766.at + +diff --git a/src/tests/regression/regression.at b/src/tests/regression/regression.at +index ba41a56b29b5..f9d42d6e2765 100644 +--- a/src/tests/regression/regression.at ++++ b/src/tests/regression/regression.at +@@ -43,3 +43,4 @@ m4_include([regression/rhbz1940928.at]) + m4_include([regression/rhbz1936896.at]) + m4_include([regression/rhbz1914935.at]) + m4_include([regression/gh696.at]) ++m4_include([regression/rhbz1917766.at]) +diff --git a/src/tests/regression/rhbz1917766.at b/src/tests/regression/rhbz1917766.at +new file mode 100644 +index 000000000000..b25d0a2f9740 +--- /dev/null ++++ b/src/tests/regression/rhbz1917766.at +@@ -0,0 +1,24 @@ ++FWD_START_TEST([rich rule source with netmask]) ++AT_KEYWORDS(rich rhbz1917766) ++ ++dnl Note: IPv6 only supports CIDR notation. It does not support address/netmask ++dnl notation. ++ ++FWD_CHECK([ --zone public --add-rich-rule='rule family=ipv4 source address="192.168.1.0/255.255.255.0" accept'], 0, [ignore]) ++FWD_CHECK([--permanent --zone public --add-rich-rule='rule family=ipv4 source address="192.168.1.0/255.255.255.0" accept'], 0, [ignore]) ++ ++AT_DATA([./zones/foobar.xml], [dnl ++ ++ ++ foobar ++ foobar ++ ++ ++ ++ ++ ++]) ++FWD_RELOAD() ++FWD_CHECK([--zone foobar --add-interface foobar0], 0, [ignore]) ++ ++FWD_END_TEST() +-- +2.27.0 + diff --git a/SOURCES/0042-test-zone-source-with-netmask.patch b/SOURCES/0042-test-zone-source-with-netmask.patch new file mode 100644 index 0000000..b63d66e --- /dev/null +++ b/SOURCES/0042-test-zone-source-with-netmask.patch @@ -0,0 +1,26 @@ +From 8ef0683614704039f1dc7bfe22ace159f9961f15 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Wed, 15 Sep 2021 14:38:28 -0400 +Subject: [PATCH 42/50] test(zone): source with netmask + +(cherry picked from commit e635bdffa630c827ff0ed2fc2bb201d560631be0) +(cherry picked from commit 818f39d4c3b029b12e744505cfe35b0b47bed7db) +--- + src/tests/cli/firewall-cmd.at | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/src/tests/cli/firewall-cmd.at b/src/tests/cli/firewall-cmd.at +index 450737776a9f..f36f634853fa 100644 +--- a/src/tests/cli/firewall-cmd.at ++++ b/src/tests/cli/firewall-cmd.at +@@ -214,6 +214,7 @@ sources: $1 + + check_zone_source([1.2.3.4]) + check_zone_source([192.168.1.0/24]) ++ check_zone_source([192.168.1.1/255.255.255.0]) + IF_HOST_SUPPORTS_IPV6_RULES([ + check_zone_source([3ffe:501:ffff::/64]) + check_zone_source([dead:beef::babe]) +-- +2.27.0 + diff --git a/SOURCES/0043-fix-fw_config-zone-on-rename-remove-then-add.patch b/SOURCES/0043-fix-fw_config-zone-on-rename-remove-then-add.patch new file mode 100644 index 0000000..2c597fd --- /dev/null +++ b/SOURCES/0043-fix-fw_config-zone-on-rename-remove-then-add.patch @@ -0,0 +1,43 @@ +From b2c9302e8a4ad1ab7535a557b2f9c9aa49b49629 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Wed, 27 Oct 2021 11:09:39 -0400 +Subject: [PATCH 43/50] fix(fw_config): zone: on rename remove then add + +Remove the old object before creating the new one. This avoids issues +such as conflicting configuration in the objects that check_config() may +trip over. + +(cherry picked from commit 3aec1dfe449d0bcb52884341770e4def0de27f56) +(cherry picked from commit a58b45d8ee3221309ec0c6f919c266b5cfc6f89a) +--- + src/firewall/core/fw_config.py | 11 +++++++---- + 1 file changed, 7 insertions(+), 4 deletions(-) + +diff --git a/src/firewall/core/fw_config.py b/src/firewall/core/fw_config.py +index 148ce1b4e32c..a97d0b23a6ac 100644 +--- a/src/firewall/core/fw_config.py ++++ b/src/firewall/core/fw_config.py +@@ -984,13 +984,16 @@ class FirewallConfig(object): + + def rename_zone(self, obj, name): + self.check_builtin_zone(obj) +- new_zone = self._copy_zone(obj, name) ++ obj_conf = obj.export_config_dict() + self._remove_zone(obj) ++ try: ++ new_zone = self.new_zone_dict(name, obj_conf) ++ except: ++ # re-add original if rename failed ++ self.new_zone_dict(obj.name, obj_conf) ++ raise + return new_zone + +- def _copy_zone(self, obj, name): +- return self.new_zone_dict(name, obj.export_config_dict()) +- + # policy objects + + def get_policy_objects(self): +-- +2.27.0 + diff --git a/SOURCES/0044-fix-io-functions-check_config-against-on-disk-conf.patch b/SOURCES/0044-fix-io-functions-check_config-against-on-disk-conf.patch new file mode 100644 index 0000000..39bce67 --- /dev/null +++ b/SOURCES/0044-fix-io-functions-check_config-against-on-disk-conf.patch @@ -0,0 +1,98 @@ +From 0e28840f5c3362d032f2f805cbbe6fbbaa217437 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Wed, 27 Oct 2021 13:58:27 -0400 +Subject: [PATCH 44/50] fix(io/functions): check_config against on disk conf + +Before this change the runtime FirewallConfig() instance was used. This +caused some permanent configuration issues to not be caught due to +comparing against the runtime instances of all objects. + +For example, two zones in permanent configuration may use the same +interface (which is not valid), but if the runtime configuration does +not have have these interface assignments then check_config() won't +catch the issue since it compares against the runtime configuration. + +Fix is to build a temporary FirewallConfig() instance for all the +on-disk/permanent configuration. + +(cherry picked from commit 35d4facc8962cd1b66bc245fe03f658d491e1061) +(cherry picked from commit 55a799e872dc88b1341a6bc38af33e77dfedb72f) +--- + src/firewall/core/io/functions.py | 47 ++++++++++++++++++++++--------- + 1 file changed, 34 insertions(+), 13 deletions(-) + +diff --git a/src/firewall/core/io/functions.py b/src/firewall/core/io/functions.py +index 0c7b1886426c..35a7eaf8dec8 100644 +--- a/src/firewall/core/io/functions.py ++++ b/src/firewall/core/io/functions.py +@@ -24,6 +24,7 @@ import os + from firewall import config + from firewall.errors import FirewallError + ++from firewall.core.fw_config import FirewallConfig + from firewall.core.io.zone import zone_reader + from firewall.core.io.service import service_reader + from firewall.core.io.ipset import ipset_reader +@@ -34,26 +35,46 @@ from firewall.core.io.direct import Direct + from firewall.core.io.lockdown_whitelist import LockdownWhitelist + from firewall.core.io.firewalld_conf import firewalld_conf + +-def check_config(fw=None): ++def check_config(fw): ++ fw_config = FirewallConfig(fw) + readers = { +- "ipset" : (ipset_reader, [config.FIREWALLD_IPSETS, config.ETC_FIREWALLD_IPSETS]), +- "helper" : (helper_reader, [config.FIREWALLD_HELPERS, config.ETC_FIREWALLD_HELPERS]), +- "icmptype" : (icmptype_reader, [config.FIREWALLD_ICMPTYPES, config.ETC_FIREWALLD_ICMPTYPES]), +- "service" : (service_reader, [config.FIREWALLD_SERVICES, config.ETC_FIREWALLD_SERVICES]), +- "zone" : (zone_reader, [config.FIREWALLD_ZONES, config.ETC_FIREWALLD_ZONES]), +- "policy" : (policy_reader, [config.FIREWALLD_POLICIES, config.ETC_FIREWALLD_POLICIES]), ++ "ipset": {"reader": ipset_reader, ++ "add": fw_config.add_ipset, ++ "dirs": [config.FIREWALLD_IPSETS, config.ETC_FIREWALLD_IPSETS], ++ }, ++ "helper": {"reader": helper_reader, ++ "add": fw_config.add_helper, ++ "dirs": [config.FIREWALLD_HELPERS, config.ETC_FIREWALLD_HELPERS], ++ }, ++ "icmptype": {"reader": icmptype_reader, ++ "add": fw_config.add_icmptype, ++ "dirs": [config.FIREWALLD_ICMPTYPES, config.ETC_FIREWALLD_ICMPTYPES], ++ }, ++ "service": {"reader": service_reader, ++ "add": fw_config.add_service, ++ "dirs": [config.FIREWALLD_SERVICES, config.ETC_FIREWALLD_SERVICES], ++ }, ++ "zone": {"reader": zone_reader, ++ "add": fw_config.add_zone, ++ "dirs": [config.FIREWALLD_ZONES, config.ETC_FIREWALLD_ZONES], ++ }, ++ "policy": {"reader": policy_reader, ++ "add": fw_config.add_policy_object, ++ "dirs": [config.FIREWALLD_POLICIES, config.ETC_FIREWALLD_POLICIES], ++ }, + } + for reader in readers.keys(): +- for dir in readers[reader][1]: +- if not os.path.isdir(dir): ++ for _dir in readers[reader]["dirs"]: ++ if not os.path.isdir(_dir): + continue +- for file in sorted(os.listdir(dir)): ++ for file in sorted(os.listdir(_dir)): + if file.endswith(".xml"): + try: +- obj = readers[reader][0](file, dir) +- if fw and reader in ["zone", "policy"]: +- obj.fw_config = fw.config ++ obj = readers[reader]["reader"](file, _dir) ++ if reader in ["zone", "policy"]: ++ obj.fw_config = fw_config + obj.check_config(obj.export_config()) ++ readers[reader]["add"](obj) + except FirewallError as error: + raise FirewallError(error.code, "'%s': %s" % (file, error.msg)) + except Exception as msg: +-- +2.27.0 + diff --git a/SOURCES/0045-fix-zone-detect-same-source-interface-in-zones.patch b/SOURCES/0045-fix-zone-detect-same-source-interface-in-zones.patch new file mode 100644 index 0000000..a5542f3 --- /dev/null +++ b/SOURCES/0045-fix-zone-detect-same-source-interface-in-zones.patch @@ -0,0 +1,46 @@ +From 8311259a6e2a6ac475c3d8c9a2df099469bf8277 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Wed, 27 Oct 2021 10:13:59 -0400 +Subject: [PATCH 45/50] fix(zone): detect same source/interface in zones + +Fixes: rhbz2014383 +(cherry picked from commit 4b721abb087a529596722a045a63a65af2e0566a) +(cherry picked from commit 081fcfe7b255b2e0f91c4a3dc55539e4cfd4b7d1) +--- + src/firewall/core/io/zone.py | 15 +++++++++++++++ + 1 file changed, 15 insertions(+) + +diff --git a/src/firewall/core/io/zone.py b/src/firewall/core/io/zone.py +index 3aea94a13155..4291ec9cba00 100644 +--- a/src/firewall/core/io/zone.py ++++ b/src/firewall/core/io/zone.py +@@ -193,11 +193,26 @@ class Zone(IO_Object): + for interface in config: + if not checkInterface(interface): + raise FirewallError(errors.INVALID_INTERFACE, interface) ++ if self.fw_config: ++ for zone in self.fw_config.get_zones(): ++ if zone == self.name: ++ continue ++ if interface in self.fw_config.get_zone(zone).interfaces: ++ raise FirewallError(errors.INVALID_INTERFACE, ++ "interface '{}' already bound to zone '{}'".format(interface, zone)) + elif item == "sources": + for source in config: + if not checkIPnMask(source) and not checkIP6nMask(source) and \ + not check_mac(source) and not source.startswith("ipset:"): + raise FirewallError(errors.INVALID_ADDR, source) ++ if self.fw_config: ++ for zone in self.fw_config.get_zones(): ++ if zone == self.name: ++ continue ++ if source in self.fw_config.get_zone(zone).sources: ++ raise FirewallError(errors.INVALID_ADDR, ++ "source '{}' already bound to zone '{}'".format(source, zone)) ++ + + def check_name(self, name): + super(Zone, self).check_name(name) +-- +2.27.0 + diff --git a/SOURCES/0046-test-zone-detect-same-source-interface-in-zones.patch b/SOURCES/0046-test-zone-detect-same-source-interface-in-zones.patch new file mode 100644 index 0000000..3ee2c37 --- /dev/null +++ b/SOURCES/0046-test-zone-detect-same-source-interface-in-zones.patch @@ -0,0 +1,88 @@ +From 63754e688baba56c7e625b53d39aa7380a754094 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Mon, 25 Oct 2021 09:35:51 -0400 +Subject: [PATCH 46/50] test(zone): detect same source/interface in zones + +Coverage: rhbz2014383 +(cherry picked from commit 6f68d295ac5edcdb10c062e2fba7b810ce2db58c) +(cherry picked from commit a15069d5542c2af391266f2da5f4137766d11a57) +--- + src/tests/regression/regression.at | 1 + + src/tests/regression/rhbz2014383.at | 56 +++++++++++++++++++++++++++++ + 2 files changed, 57 insertions(+) + create mode 100644 src/tests/regression/rhbz2014383.at + +diff --git a/src/tests/regression/regression.at b/src/tests/regression/regression.at +index f9d42d6e2765..a20b913fbe59 100644 +--- a/src/tests/regression/regression.at ++++ b/src/tests/regression/regression.at +@@ -44,3 +44,4 @@ m4_include([regression/rhbz1936896.at]) + m4_include([regression/rhbz1914935.at]) + m4_include([regression/gh696.at]) + m4_include([regression/rhbz1917766.at]) ++m4_include([regression/rhbz2014383.at]) +diff --git a/src/tests/regression/rhbz2014383.at b/src/tests/regression/rhbz2014383.at +new file mode 100644 +index 000000000000..f2ef766dc1b2 +--- /dev/null ++++ b/src/tests/regression/rhbz2014383.at +@@ -0,0 +1,56 @@ ++FWD_START_TEST([same source in two zone xml]) ++AT_KEYWORDS(zone rhbz2014383) ++ ++AT_CHECK([mkdir -p ./zones]) ++ ++AT_DATA([./zones/foobar.xml], [dnl ++ ++ ++ foobar ++ foobar ++ ++ ++ ++ ++]) ++ ++AT_DATA([./zones/foobar2.xml], [dnl ++ ++ ++ foobar2 ++ foobar2 ++ ++ ++ ++ ++]) ++ ++FWD_CHECK([--check-config], 105, [ignore], [ignore]) ++ ++dnl Do the same thing, but with interfaces ++ ++AT_DATA([./zones/foobar.xml], [dnl ++ ++ ++ foobar ++ foobar ++ ++ ++ ++ ++]) ++ ++AT_DATA([./zones/foobar2.xml], [dnl ++ ++ ++ foobar2 ++ foobar2 ++ ++ ++ ++ ++]) ++ ++FWD_CHECK([--check-config], 104, [ignore], [ignore]) ++ ++FWD_END_TEST([ignore]) +-- +2.27.0 + diff --git a/SOURCES/0047-feat-config-add-CleanupModulesOnExit-configuration-o.patch b/SOURCES/0047-feat-config-add-CleanupModulesOnExit-configuration-o.patch new file mode 100644 index 0000000..fdccfe5 --- /dev/null +++ b/SOURCES/0047-feat-config-add-CleanupModulesOnExit-configuration-o.patch @@ -0,0 +1,302 @@ +From fb11903b8efd287f72e634fb8a4b4ff2034151fe Mon Sep 17 00:00:00 2001 +From: Paul Laufer <50234787+refual@users.noreply.github.com> +Date: Fri, 27 Nov 2020 12:23:11 +0100 +Subject: [PATCH 47/48] feat(config): add CleanupModulesOnExit configuration + option + +Fixes: rhbz 1520532 +Fixes: #533 +Closes: #721 +(cherry picked from commit 152a51537a7840afd0879ab4b60178bef4ec16a2) +--- + config/firewalld.conf | 9 +++++++- + doc/xml/firewalld.conf.xml | 11 ++++++++++ + doc/xml/firewalld.dbus.xml | 9 ++++++++ + src/firewall/config/__init__.py.in | 1 + + src/firewall/core/fw.py | 29 +++++++++++++++++++------- + src/firewall/core/io/firewalld_conf.py | 19 +++++++++++++---- + src/firewall/server/config.py | 23 +++++++++++++------- + src/tests/dbus/firewalld.conf.at | 2 ++ + 8 files changed, 82 insertions(+), 21 deletions(-) + +diff --git a/config/firewalld.conf b/config/firewalld.conf +index a0556c0bbf5b..3abbc9c998c1 100644 +--- a/config/firewalld.conf ++++ b/config/firewalld.conf +@@ -7,10 +7,17 @@ DefaultZone=public + + # Clean up on exit + # If set to no or false the firewall configuration will not get cleaned up +-# on exit or stop of firewalld ++# on exit or stop of firewalld. + # Default: yes + CleanupOnExit=yes + ++# Clean up kernel modules on exit ++# If set to yes or true the firewall related kernel modules will be ++# unloaded on exit or stop of firewalld. This might attempt to unload ++# modules not originally loaded by firewalld. ++# Default: no ++CleanupModulesOnExit=no ++ + # Lockdown + # If set to enabled, firewall changes with the D-Bus interface will be limited + # to applications that are listed in the lockdown whitelist. +diff --git a/doc/xml/firewalld.conf.xml b/doc/xml/firewalld.conf.xml +index 0bf4c2d4d011..dd6ffb214eb3 100644 +--- a/doc/xml/firewalld.conf.xml ++++ b/doc/xml/firewalld.conf.xml +@@ -88,6 +88,17 @@ + + + ++ ++ ++ ++ ++ Setting this option to yes or true unloads all firewall-related ++ kernel modules when firewalld is stopped. The default value is no ++ or false. ++ ++ ++ ++ + + + +diff --git a/doc/xml/firewalld.dbus.xml b/doc/xml/firewalld.dbus.xml +index d17cb8b6c1ec..466220b40b21 100644 +--- a/doc/xml/firewalld.dbus.xml ++++ b/doc/xml/firewalld.dbus.xml +@@ -2798,6 +2798,15 @@ + + + ++ ++ CleanupModulesOnExit - s - (rw) ++ ++ ++ Setting this option to yes or true unloads all firewall-related ++ kernel modules when firewalld is stopped. ++ ++ ++ + + CleanupOnExit - s - (rw) + +diff --git a/src/firewall/config/__init__.py.in b/src/firewall/config/__init__.py.in +index 0dec7913f694..5d6d769fbf15 100644 +--- a/src/firewall/config/__init__.py.in ++++ b/src/firewall/config/__init__.py.in +@@ -125,6 +125,7 @@ FIREWALL_BACKEND_VALUES = [ "nftables", "iptables" ] + FALLBACK_ZONE = "public" + FALLBACK_MINIMAL_MARK = 100 + FALLBACK_CLEANUP_ON_EXIT = True ++FALLBACK_CLEANUP_MODULES_ON_EXIT = False + FALLBACK_LOCKDOWN = False + FALLBACK_IPV6_RPFILTER = True + FALLBACK_INDIVIDUAL_CALLS = False +diff --git a/src/firewall/core/fw.py b/src/firewall/core/fw.py +index 3eb54e37ab5c..4171697bdb94 100644 +--- a/src/firewall/core/fw.py ++++ b/src/firewall/core/fw.py +@@ -105,12 +105,13 @@ class Firewall(object): + self.__init_vars() + + def __repr__(self): +- return '%s(%r, %r, %r, %r, %r, %r, %r, %r, %r, %r, %r, %r, %r)' % \ ++ return '%s(%r, %r, %r, %r, %r, %r, %r, %r, %r, %r, %r, %r, %r, %r)' % \ + (self.__class__, self.ip4tables_enabled, self.ip6tables_enabled, + self.ebtables_enabled, self._state, self._panic, + self._default_zone, self._module_refcount, self._marks, +- self.cleanup_on_exit, self.ipv6_rpfilter_enabled, +- self.ipset_enabled, self._individual_calls, self._log_denied) ++ self.cleanup_on_exit, self.cleanup_modules_on_exit, ++ self.ipv6_rpfilter_enabled, self.ipset_enabled, ++ self._individual_calls, self._log_denied) + + def __init_vars(self): + self._state = "INIT" +@@ -120,6 +121,7 @@ class Firewall(object): + self._marks = [ ] + # fallback settings will be overloaded by firewalld.conf + self.cleanup_on_exit = config.FALLBACK_CLEANUP_ON_EXIT ++ self.cleanup_modules_on_exit = config.FALLBACK_CLEANUP_MODULES_ON_EXIT + self.ipv6_rpfilter_enabled = config.FALLBACK_IPV6_RPFILTER + self._individual_calls = config.FALLBACK_INDIVIDUAL_CALLS + self._log_denied = config.FALLBACK_LOG_DENIED +@@ -232,6 +234,13 @@ class Firewall(object): + log.debug1("CleanupOnExit is set to '%s'", + self.cleanup_on_exit) + ++ if self._firewalld_conf.get("CleanupModulesOnExit"): ++ value = self._firewalld_conf.get("CleanupModulesOnExit") ++ if value is not None and value.lower() in [ "yes", "true" ]: ++ self.cleanup_modules_on_exit = True ++ log.debug1("CleanupModulesOnExit is set to '%s'", ++ self.cleanup_modules_on_exit) ++ + if self._firewalld_conf.get("Lockdown"): + value = self._firewalld_conf.get("Lockdown") + if value is not None and value.lower() in [ "yes", "true" ]: +@@ -667,11 +676,15 @@ class Firewall(object): + self.__init_vars() + + def stop(self): +- if self.cleanup_on_exit and not self._offline: +- self.flush() +- self.ipset.flush() +- self.set_policy("ACCEPT") +- self.modules_backend.unload_firewall_modules() ++ if not self._offline: ++ if self.cleanup_on_exit: ++ self.flush() ++ self.ipset.flush() ++ self.set_policy("ACCEPT") ++ ++ if self.cleanup_modules_on_exit: ++ log.debug1('Unloading firewall kernel modules') ++ self.modules_backend.unload_firewall_modules() + + self.cleanup() + +diff --git a/src/firewall/core/io/firewalld_conf.py b/src/firewall/core/io/firewalld_conf.py +index 7c7092120676..70258400ef06 100644 +--- a/src/firewall/core/io/firewalld_conf.py ++++ b/src/firewall/core/io/firewalld_conf.py +@@ -28,10 +28,11 @@ from firewall import config + from firewall.core.logger import log + from firewall.functions import b2u, u2b, PY2 + +-valid_keys = [ "DefaultZone", "MinimalMark", "CleanupOnExit", "Lockdown", +- "IPv6_rpfilter", "IndividualCalls", "LogDenied", +- "AutomaticHelpers", "FirewallBackend", "FlushAllOnReload", +- "RFC3964_IPv4", "AllowZoneDrifting" ] ++valid_keys = [ "DefaultZone", "MinimalMark", "CleanupOnExit", ++ "CleanupModulesOnExit", "Lockdown", "IPv6_rpfilter", ++ "IndividualCalls", "LogDenied", "AutomaticHelpers", ++ "FirewallBackend", "FlushAllOnReload", "RFC3964_IPv4", ++ "AllowZoneDrifting" ] + + class firewalld_conf(object): + def __init__(self, filename): +@@ -75,6 +76,7 @@ class firewalld_conf(object): + self.set("DefaultZone", config.FALLBACK_ZONE) + self.set("MinimalMark", str(config.FALLBACK_MINIMAL_MARK)) + self.set("CleanupOnExit", "yes" if config.FALLBACK_CLEANUP_ON_EXIT else "no") ++ self.set("CleanupModulesOnExit", "yes" if config.FALLBACK_CLEANUP_MODULES_ON_EXIT else "no") + self.set("Lockdown", "yes" if config.FALLBACK_LOCKDOWN else "no") + self.set("IPv6_rpfilter","yes" if config.FALLBACK_IPV6_RPFILTER else "no") + self.set("IndividualCalls", "yes" if config.FALLBACK_INDIVIDUAL_CALLS else "no") +@@ -135,6 +137,15 @@ class firewalld_conf(object): + config.FALLBACK_CLEANUP_ON_EXIT) + self.set("CleanupOnExit", "yes" if config.FALLBACK_CLEANUP_ON_EXIT else "no") + ++ # check module cleanup on exit ++ value = self.get("CleanupModulesOnExit") ++ if not value or value.lower() not in [ "no", "false", "yes", "true" ]: ++ if value is not None: ++ log.warning("CleanupModulesOnExit '%s' is not valid, using default " ++ "value %s", value if value else '', ++ config.FALLBACK_CLEANUP_MODULES_ON_EXIT) ++ self.set("CleanupModulesOnExit", "yes" if config.FALLBACK_CLEANUP_MODULES_ON_EXIT else "no") ++ + # check lockdown + value = self.get("Lockdown") + if not value or value.lower() not in [ "yes", "true", "no", "false" ]: +diff --git a/src/firewall/server/config.py b/src/firewall/server/config.py +index 031ef5d1afaa..8815920c6893 100644 +--- a/src/firewall/server/config.py ++++ b/src/firewall/server/config.py +@@ -100,6 +100,7 @@ class FirewallDConfig(slip.dbus.service.Object): + dbus_introspection_prepare_properties(self, + config.dbus.DBUS_INTERFACE_CONFIG, + { "CleanupOnExit": "readwrite", ++ "CleanupModulesOnExit": "readwrite", + "IPv6_rpfilter": "readwrite", + "Lockdown": "readwrite", + "MinimalMark": "readwrite", +@@ -554,9 +555,9 @@ class FirewallDConfig(slip.dbus.service.Object): + @dbus_handle_exceptions + def _get_property(self, prop): + if prop not in [ "DefaultZone", "MinimalMark", "CleanupOnExit", +- "Lockdown", "IPv6_rpfilter", "IndividualCalls", +- "LogDenied", "AutomaticHelpers", "FirewallBackend", +- "FlushAllOnReload", "RFC3964_IPv4", ++ "CleanupModulesOnExit", "Lockdown", "IPv6_rpfilter", ++ "IndividualCalls", "LogDenied", "AutomaticHelpers", ++ "FirewallBackend", "FlushAllOnReload", "RFC3964_IPv4", + "AllowZoneDrifting" ]: + raise dbus.exceptions.DBusException( + "org.freedesktop.DBus.Error.InvalidArgs: " +@@ -578,6 +579,10 @@ class FirewallDConfig(slip.dbus.service.Object): + if value is None: + value = "yes" if config.FALLBACK_CLEANUP_ON_EXIT else "no" + return dbus.String(value) ++ elif prop == "CleanupModulesOnExit": ++ if value is None: ++ value = "yes" if config.FALLBACK_CLEANUP_MODULES_ON_EXIT else "no" ++ return dbus.String(value) + elif prop == "Lockdown": + if value is None: + value = "yes" if config.FALLBACK_LOCKDOWN else "no" +@@ -623,6 +628,8 @@ class FirewallDConfig(slip.dbus.service.Object): + return dbus.Int32(self._get_property(prop)) + elif prop == "CleanupOnExit": + return dbus.String(self._get_property(prop)) ++ elif prop == "CleanupModulesOnExit": ++ return dbus.String(self._get_property(prop)) + elif prop == "Lockdown": + return dbus.String(self._get_property(prop)) + elif prop == "IPv6_rpfilter": +@@ -679,9 +686,9 @@ class FirewallDConfig(slip.dbus.service.Object): + ret = { } + if interface_name == config.dbus.DBUS_INTERFACE_CONFIG: + for x in [ "DefaultZone", "MinimalMark", "CleanupOnExit", +- "Lockdown", "IPv6_rpfilter", "IndividualCalls", +- "LogDenied", "AutomaticHelpers", "FirewallBackend", +- "FlushAllOnReload", "RFC3964_IPv4", ++ "CleanupModulesOnExit", "Lockdown", "IPv6_rpfilter", ++ "IndividualCalls", "LogDenied", "AutomaticHelpers", ++ "FirewallBackend", "FlushAllOnReload", "RFC3964_IPv4", + "AllowZoneDrifting" ]: + ret[x] = self._get_property(x) + elif interface_name in [ config.dbus.DBUS_INTERFACE_CONFIG_DIRECT, +@@ -706,12 +713,12 @@ class FirewallDConfig(slip.dbus.service.Object): + self.accessCheck(sender) + + if interface_name == config.dbus.DBUS_INTERFACE_CONFIG: +- if property_name in [ "CleanupOnExit", "Lockdown", ++ if property_name in [ "CleanupOnExit", "Lockdown", "CleanupModulesOnExit", + "IPv6_rpfilter", "IndividualCalls", + "LogDenied", + "FirewallBackend", "FlushAllOnReload", + "RFC3964_IPv4", "AllowZoneDrifting" ]: +- if property_name in [ "CleanupOnExit", "Lockdown", ++ if property_name in [ "CleanupOnExit", "Lockdown", "CleanupModulesOnExit", + "IPv6_rpfilter", "IndividualCalls" ]: + if new_value.lower() not in [ "yes", "no", + "true", "false" ]: +diff --git a/src/tests/dbus/firewalld.conf.at b/src/tests/dbus/firewalld.conf.at +index 9fc5502a8d0b..9a04a3bd491c 100644 +--- a/src/tests/dbus/firewalld.conf.at ++++ b/src/tests/dbus/firewalld.conf.at +@@ -17,6 +17,7 @@ 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 "no" ++string "CleanupModulesOnExit" : variant string "no" + string "CleanupOnExit" : variant string "no" + string "DefaultZone" : variant string "public" + string "FirewallBackend" : variant string "nftables" +@@ -45,6 +46,7 @@ _helper([IPv6_rpfilter], [string:"yes"], [variant string "yes"]) + _helper([IndividualCalls], [string:"yes"], [variant string "yes"]) + _helper([FirewallBackend], [string:"iptables"], [variant string "iptables"]) + _helper([FlushAllOnReload], [string:"no"], [variant string "no"]) ++_helper([CleanupModulesOnExit], [string:"yes"], [variant string "yes"]) + _helper([CleanupOnExit], [string:"yes"], [variant string "yes"]) + _helper([RFC3964_IPv4], [string:"no"], [variant string "no"]) + _helper([AllowZoneDrifting], [string:"yes"], [variant string "yes"]) +-- +2.31.1 + diff --git a/SOURCES/0048-RHEL-only-default-to-CleanupModulesOnExit-yes.patch b/SOURCES/0048-RHEL-only-default-to-CleanupModulesOnExit-yes.patch new file mode 100644 index 0000000..1d05ba3 --- /dev/null +++ b/SOURCES/0048-RHEL-only-default-to-CleanupModulesOnExit-yes.patch @@ -0,0 +1,95 @@ +From 1aef58a8ff6d232cefcc6bd19ea63c0f071bfee3 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Mon, 20 Dec 2021 13:56:55 -0500 +Subject: [PATCH 48/48] RHEL only: default to CleanupModulesOnExit=yes + +Resolves: rhbz1980206 +--- + config/firewalld.conf | 4 ++-- + doc/xml/firewalld.conf.xml | 4 ++-- + src/firewall/config/__init__.py.in | 2 +- + src/firewall/core/fw.py | 2 ++ + src/tests/dbus/firewalld.conf.at | 4 ++-- + 5 files changed, 9 insertions(+), 7 deletions(-) + +diff --git a/config/firewalld.conf b/config/firewalld.conf +index 3abbc9c998c1..c387f87c28be 100644 +--- a/config/firewalld.conf ++++ b/config/firewalld.conf +@@ -15,8 +15,8 @@ CleanupOnExit=yes + # If set to yes or true the firewall related kernel modules will be + # unloaded on exit or stop of firewalld. This might attempt to unload + # modules not originally loaded by firewalld. +-# Default: no +-CleanupModulesOnExit=no ++# Default: yes ++CleanupModulesOnExit=yes + + # Lockdown + # If set to enabled, firewall changes with the D-Bus interface will be limited +diff --git a/doc/xml/firewalld.conf.xml b/doc/xml/firewalld.conf.xml +index dd6ffb214eb3..12d9f5fc563e 100644 +--- a/doc/xml/firewalld.conf.xml ++++ b/doc/xml/firewalld.conf.xml +@@ -93,8 +93,8 @@ + + + Setting this option to yes or true unloads all firewall-related +- kernel modules when firewalld is stopped. The default value is no +- or false. ++ kernel modules when firewalld is stopped. The default value is yes ++ or true. + + + +diff --git a/src/firewall/config/__init__.py.in b/src/firewall/config/__init__.py.in +index 5d6d769fbf15..285e2f034b6b 100644 +--- a/src/firewall/config/__init__.py.in ++++ b/src/firewall/config/__init__.py.in +@@ -125,7 +125,7 @@ FIREWALL_BACKEND_VALUES = [ "nftables", "iptables" ] + FALLBACK_ZONE = "public" + FALLBACK_MINIMAL_MARK = 100 + FALLBACK_CLEANUP_ON_EXIT = True +-FALLBACK_CLEANUP_MODULES_ON_EXIT = False ++FALLBACK_CLEANUP_MODULES_ON_EXIT = True + FALLBACK_LOCKDOWN = False + FALLBACK_IPV6_RPFILTER = True + FALLBACK_INDIVIDUAL_CALLS = False +diff --git a/src/firewall/core/fw.py b/src/firewall/core/fw.py +index 4171697bdb94..5cef18b5f889 100644 +--- a/src/firewall/core/fw.py ++++ b/src/firewall/core/fw.py +@@ -238,6 +238,8 @@ class Firewall(object): + value = self._firewalld_conf.get("CleanupModulesOnExit") + if value is not None and value.lower() in [ "yes", "true" ]: + self.cleanup_modules_on_exit = True ++ if value is not None and value.lower() in [ "no", "false" ]: ++ self.cleanup_modules_on_exit = False + log.debug1("CleanupModulesOnExit is set to '%s'", + self.cleanup_modules_on_exit) + +diff --git a/src/tests/dbus/firewalld.conf.at b/src/tests/dbus/firewalld.conf.at +index 9a04a3bd491c..68832bca33bc 100644 +--- a/src/tests/dbus/firewalld.conf.at ++++ b/src/tests/dbus/firewalld.conf.at +@@ -17,7 +17,7 @@ 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 "no" +-string "CleanupModulesOnExit" : variant string "no" ++string "CleanupModulesOnExit" : variant string "yes" + string "CleanupOnExit" : variant string "no" + string "DefaultZone" : variant string "public" + string "FirewallBackend" : variant string "nftables" +@@ -46,7 +46,7 @@ _helper([IPv6_rpfilter], [string:"yes"], [variant string "yes"]) + _helper([IndividualCalls], [string:"yes"], [variant string "yes"]) + _helper([FirewallBackend], [string:"iptables"], [variant string "iptables"]) + _helper([FlushAllOnReload], [string:"no"], [variant string "no"]) +-_helper([CleanupModulesOnExit], [string:"yes"], [variant string "yes"]) ++_helper([CleanupModulesOnExit], [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"]) +-- +2.31.1 + diff --git a/SOURCES/v1.0.0-0037-fix-ipset-reduce-cost-of-entry-overlap-detection.patch b/SOURCES/v1.0.0-0037-fix-ipset-reduce-cost-of-entry-overlap-detection.patch deleted file mode 100644 index 54a26d0..0000000 --- a/SOURCES/v1.0.0-0037-fix-ipset-reduce-cost-of-entry-overlap-detection.patch +++ /dev/null @@ -1,140 +0,0 @@ -From 3eb0f3239b9b35a1c388a91fc2e546b1e87cb020 Mon Sep 17 00:00:00 2001 -From: Eric Garver -Date: Tue, 30 Nov 2021 14:54:20 -0500 -Subject: [PATCH 37/39] fix(ipset): reduce cost of entry overlap detection - -This increases peak memory usage to reduce the duration it takes to -apply the set entries. Building the list of IPv4Network objects up front -means we don't have to build them multiple times inside the for loop. - -Fixes: #881 -(cherry picked from commit 7f5b736378c0133f46470c42e0c1fb3b95087de5) ---- - src/firewall/client.py | 10 ++++------ - src/firewall/core/fw_ipset.py | 9 +++------ - src/firewall/core/ipset.py | 27 ++++++++++++++++++++++----- - src/firewall/server/config_ipset.py | 10 ++++------ - 4 files changed, 33 insertions(+), 23 deletions(-) - -diff --git a/src/firewall/client.py b/src/firewall/client.py -index 3715ffd29316..fdc88ac7946b 100644 ---- a/src/firewall/client.py -+++ b/src/firewall/client.py -@@ -34,7 +34,8 @@ from firewall.core.base import DEFAULT_ZONE_TARGET, DEFAULT_POLICY_TARGET, DEFAU - from firewall.dbus_utils import dbus_to_python - from firewall.functions import b2u - from firewall.core.rich import Rich_Rule --from firewall.core.ipset import normalize_ipset_entry, check_entry_overlaps_existing -+from firewall.core.ipset import normalize_ipset_entry, check_entry_overlaps_existing, \ -+ check_for_overlapping_entries - from firewall import errors - from firewall.errors import FirewallError - -@@ -1617,11 +1618,8 @@ class FirewallClientIPSetSettings(object): - if "timeout" in self.settings[4] and \ - self.settings[4]["timeout"] != "0": - raise FirewallError(errors.IPSET_WITH_TIMEOUT) -- _entries = set() -- for _entry in dbus_to_python(entries, list): -- check_entry_overlaps_existing(_entry, _entries) -- _entries.add(normalize_ipset_entry(_entry)) -- self.settings[5] = list(_entries) -+ check_for_overlapping_entries(entries) -+ self.settings[5] = entries - @handle_exceptions - def addEntry(self, entry): - if "timeout" in self.settings[4] and \ -diff --git a/src/firewall/core/fw_ipset.py b/src/firewall/core/fw_ipset.py -index a285fd4a4aab..d7878c01921e 100644 ---- a/src/firewall/core/fw_ipset.py -+++ b/src/firewall/core/fw_ipset.py -@@ -25,7 +25,8 @@ __all__ = [ "FirewallIPSet" ] - - from firewall.core.logger import log - from firewall.core.ipset import remove_default_create_options as rm_def_cr_opts, \ -- normalize_ipset_entry, check_entry_overlaps_existing -+ normalize_ipset_entry, check_entry_overlaps_existing, \ -+ check_for_overlapping_entries - from firewall.core.io.ipset import IPSet - from firewall import errors - from firewall.errors import FirewallError -@@ -244,11 +245,7 @@ class FirewallIPSet(object): - def set_entries(self, name, entries): - obj = self.get_ipset(name, applied=True) - -- _entries = set() -- for _entry in entries: -- check_entry_overlaps_existing(_entry, _entries) -- _entries.add(normalize_ipset_entry(_entry)) -- entries = list(_entries) -+ check_for_overlapping_entries(entries) - - for entry in entries: - IPSet.check_entry(entry, obj.options, obj.type) -diff --git a/src/firewall/core/ipset.py b/src/firewall/core/ipset.py -index d6defa395241..66ea4335536d 100644 ---- a/src/firewall/core/ipset.py -+++ b/src/firewall/core/ipset.py -@@ -309,9 +309,26 @@ def check_entry_overlaps_existing(entry, entries): - if len(entry.split(",")) > 1: - return - -+ try: -+ entry_network = ipaddress.ip_network(entry, strict=False) -+ except ValueError: -+ # could not parse the new IP address, maybe a MAC -+ return -+ - for itr in entries: -- try: -- if ipaddress.ip_network(itr, strict=False).overlaps(ipaddress.ip_network(entry, strict=False)): -- raise FirewallError(errors.INVALID_ENTRY, "Entry '{}' overlaps with existing entry '{}'".format(itr, entry)) -- except ValueError: -- pass -+ if entry_network.overlaps(ipaddress.ip_network(itr, strict=False)): -+ raise FirewallError(errors.INVALID_ENTRY, "Entry '{}' overlaps with existing entry '{}'".format(entry, itr)) -+ -+def check_for_overlapping_entries(entries): -+ """ Check if any entry overlaps any entry in the list of entries """ -+ try: -+ entries = [ipaddress.ip_network(x, strict=False) for x in entries] -+ except ValueError: -+ # at least one entry can not be parsed -+ return -+ -+ while entries: -+ entry = entries.pop() -+ for itr in entries: -+ if entry.overlaps(itr): -+ raise FirewallError(errors.INVALID_ENTRY, "Entry '{}' overlaps entry '{}'".format(entry, itr)) -diff --git a/src/firewall/server/config_ipset.py b/src/firewall/server/config_ipset.py -index f33c2a02926f..499ffcb9227a 100644 ---- a/src/firewall/server/config_ipset.py -+++ b/src/firewall/server/config_ipset.py -@@ -34,7 +34,8 @@ from firewall.dbus_utils import dbus_to_python, \ - dbus_introspection_add_properties - from firewall.core.io.ipset import IPSet - from firewall.core.ipset import IPSET_TYPES, normalize_ipset_entry, \ -- check_entry_overlaps_existing -+ check_entry_overlaps_existing, \ -+ check_for_overlapping_entries - from firewall.core.logger import log - from firewall.server.decorators import handle_exceptions, \ - dbus_handle_exceptions, dbus_service_method -@@ -407,11 +408,8 @@ class FirewallDConfigIPSet(slip.dbus.service.Object): - in_signature='as') - @dbus_handle_exceptions - def setEntries(self, entries, sender=None): -- _entries = set() -- for _entry in dbus_to_python(entries, list): -- check_entry_overlaps_existing(_entry, _entries) -- _entries.add(normalize_ipset_entry(_entry)) -- entries = list(_entries) -+ entries = dbus_to_python(entries, list) -+ check_for_overlapping_entries(entries) - log.debug1("%s.setEntries('[%s]')", self._log_prefix, - ",".join(entries)) - self.parent.accessCheck(sender) --- -2.31.1 - diff --git a/SOURCES/v1.0.0-0038-test-ipset-huge-set-of-entries-benchmark.patch b/SOURCES/v1.0.0-0038-test-ipset-huge-set-of-entries-benchmark.patch deleted file mode 100644 index 362202b..0000000 --- a/SOURCES/v1.0.0-0038-test-ipset-huge-set-of-entries-benchmark.patch +++ /dev/null @@ -1,56 +0,0 @@ -From 976260a0d74009cea18f3c60e4b03e7f41de8fa9 Mon Sep 17 00:00:00 2001 -From: Eric Garver -Date: Tue, 30 Nov 2021 14:50:17 -0500 -Subject: [PATCH 38/39] test(ipset): huge set of entries benchmark - -Coverage: #881 -(cherry picked from commit 114936c71ab1b12a5598d06805b7e9e13f7ee190) ---- - src/tests/regression/gh881.at | 25 +++++++++++++++++++++++++ - src/tests/regression/regression.at | 1 + - 2 files changed, 26 insertions(+) - create mode 100644 src/tests/regression/gh881.at - -diff --git a/src/tests/regression/gh881.at b/src/tests/regression/gh881.at -new file mode 100644 -index 000000000000..c7326805b555 ---- /dev/null -+++ b/src/tests/regression/gh881.at -@@ -0,0 +1,25 @@ -+FWD_START_TEST([ipset entry overlap detect perf]) -+AT_KEYWORDS(ipset gh881) -+ -+dnl build a large ipset -+dnl -+AT_DATA([./deny_cidr], []) -+NS_CHECK([sh -c ' -+for I in $(seq 10); do -+ for J in $(seq 250); do -+ echo "10.${I}.${J}.0/24" >> ./deny_cidr -+ done -+done -+']) -+ -+dnl verify non-overlapping does not error -+dnl -+FWD_CHECK([--permanent --new-ipset=deny_set --type=hash:net --option=family=inet --option=hashsize=16384 --option=maxelem=20000], 0, [ignore]) -+NS_CHECK([time timeout 300 firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 0, [ignore], [ignore]) -+ -+dnl verify overlap detection actually detects an overlap -+dnl -+NS_CHECK([echo "10.1.0.0/16" >> ./deny_cidr]) -+NS_CHECK([time timeout 300 firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 136, [ignore], [ignore]) -+ -+FWD_END_TEST() -diff --git a/src/tests/regression/regression.at b/src/tests/regression/regression.at -index aadd948a459f..6ef6579434b1 100644 ---- a/src/tests/regression/regression.at -+++ b/src/tests/regression/regression.at -@@ -42,3 +42,4 @@ m4_include([regression/ipset_netmask_allowed.at]) - m4_include([regression/rhbz1940928.at]) - m4_include([regression/rhbz1936896.at]) - m4_include([regression/rhbz1914935.at]) -+m4_include([regression/gh881.at]) --- -2.31.1 - diff --git a/SOURCES/v1.0.0-0039-fix-ipset-further-reduce-cost-of-entry-overlap-detec.patch b/SOURCES/v1.0.0-0039-fix-ipset-further-reduce-cost-of-entry-overlap-detec.patch deleted file mode 100644 index c9db4f7..0000000 --- a/SOURCES/v1.0.0-0039-fix-ipset-further-reduce-cost-of-entry-overlap-detec.patch +++ /dev/null @@ -1,150 +0,0 @@ -From e61807c71c8532cfd23f9946d903beec7f496abc Mon Sep 17 00:00:00 2001 -From: Eric Garver -Date: Tue, 25 Jan 2022 09:29:32 -0500 -Subject: [PATCH 39/39] fix(ipset): further reduce cost of entry overlap - detection - -This makes the complexity linear by sorting the networks ahead of time. - -Fixes: #881 -Fixes: rhbz2043289 -(cherry picked from commit 36c170db265265e838a089858be4b20dbbd582eb) ---- - src/firewall/core/ipset.py | 59 ++++++++++++++++++++++++++++++++--- - src/tests/regression/gh881.at | 42 ++++++++++++++++++++++--- - 2 files changed, 92 insertions(+), 9 deletions(-) - -diff --git a/src/firewall/core/ipset.py b/src/firewall/core/ipset.py -index 66ea4335536d..b160d8345669 100644 ---- a/src/firewall/core/ipset.py -+++ b/src/firewall/core/ipset.py -@@ -327,8 +327,57 @@ def check_for_overlapping_entries(entries): - # at least one entry can not be parsed - return - -- while entries: -- entry = entries.pop() -- for itr in entries: -- if entry.overlaps(itr): -- raise FirewallError(errors.INVALID_ENTRY, "Entry '{}' overlaps entry '{}'".format(entry, itr)) -+ # We can take advantage of some facts of IPv4Network/IPv6Network and -+ # how Python sorts the networks to quickly detect overlaps. -+ # -+ # Facts: -+ # -+ # 1. IPv{4,6}Network are normalized to remove host bits, e.g. -+ # 10.1.1.0/16 will become 10.1.0.0/16. -+ # -+ # 2. IPv{4,6}Network objects are sorted by: -+ # a. IP address (network bits) -+ # then -+ # b. netmask (significant bits count) -+ # -+ # Because of the above we have these properties: -+ # -+ # 1. big networks (netA) are sorted before smaller networks (netB) -+ # that overlap the big network (netA) -+ # - e.g. 10.1.128.0/17 (netA) sorts before 10.1.129.0/24 (netB) -+ # 2. same value addresses (network bits) are grouped together even -+ # if the number of network bits vary. e.g. /16 vs /24 -+ # - recall that address are normalized to remove host bits -+ # - e.g. 10.1.128.0/17 (netA) sorts before 10.1.128.0/24 (netC) -+ # 3. non-overlapping networks (netD, netE) are always sorted before or -+ # after networks that overlap (netB, netC) the current one (netA) -+ # - e.g. 10.1.128.0/17 (netA) sorts before 10.2.128.0/16 (netD) -+ # - e.g. 10.1.128.0/17 (netA) sorts after 9.1.128.0/17 (netE) -+ # - e.g. 9.1.128.0/17 (netE) sorts before 10.1.129.0/24 (netB) -+ # -+ # With this we know the sorted list looks like: -+ # -+ # list: [ netE, netA, netB, netC, netD ] -+ # -+ # netE = non-overlapping network -+ # netA = big network -+ # netB = smaller network that overlaps netA (subnet) -+ # netC = smaller network that overlaps netA (subnet) -+ # netD = non-overlapping network -+ # -+ # If networks netB and netC exist in the list, they overlap and are -+ # adjacent to netA. -+ # -+ # Checking for overlaps on a sorted list is thus: -+ # -+ # 1. compare adjacent elements in the list for overlaps -+ # -+ # Recall that we only need to detect a single overlap. We do not need to -+ # detect them all. -+ # -+ entries.sort() -+ prev_network = entries.pop(0) -+ for current_network in entries: -+ if prev_network.overlaps(current_network): -+ raise FirewallError(errors.INVALID_ENTRY, "Entry '{}' overlaps entry '{}'".format(prev_network, current_network)) -+ prev_network = current_network -diff --git a/src/tests/regression/gh881.at b/src/tests/regression/gh881.at -index c7326805b555..a5cf7e4eb912 100644 ---- a/src/tests/regression/gh881.at -+++ b/src/tests/regression/gh881.at -@@ -5,21 +5,55 @@ dnl build a large ipset - dnl - AT_DATA([./deny_cidr], []) - NS_CHECK([sh -c ' --for I in $(seq 10); do -+for I in $(seq 250); do - for J in $(seq 250); do - echo "10.${I}.${J}.0/24" >> ./deny_cidr - done - done - ']) -+NS_CHECK([echo "10.254.0.0/16" >> ./deny_cidr]) - - dnl verify non-overlapping does not error - dnl - FWD_CHECK([--permanent --new-ipset=deny_set --type=hash:net --option=family=inet --option=hashsize=16384 --option=maxelem=20000], 0, [ignore]) --NS_CHECK([time timeout 300 firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 0, [ignore], [ignore]) -+NS_CHECK([time firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 0, [ignore], [ignore]) -+ -+dnl still no overlap -+dnl -+AT_DATA([./deny_cidr], [ -+9.0.0.0/8 -+11.1.0.0/16 -+]) -+NS_CHECK([time firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 0, [ignore], [ignore]) - - dnl verify overlap detection actually detects an overlap - dnl --NS_CHECK([echo "10.1.0.0/16" >> ./deny_cidr]) --NS_CHECK([time timeout 300 firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 136, [ignore], [ignore]) -+AT_DATA([./deny_cidr], [ -+10.1.0.0/16 -+10.2.0.0/16 -+10.250.0.0/16 -+]) -+NS_CHECK([time firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 136, [ignore], [ignore]) -+ -+AT_DATA([./deny_cidr], [ -+10.253.0.0/16 -+10.253.128.0/17 -+]) -+NS_CHECK([time firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 136, [ignore], [ignore]) -+ -+AT_DATA([./deny_cidr], [ -+10.1.1.1/32 -+]) -+NS_CHECK([time firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 136, [ignore], [ignore]) -+ -+AT_DATA([./deny_cidr], [ -+10.0.0.0/8 -+10.0.0.0/25 -+]) -+NS_CHECK([time firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 136, [ignore], [ignore]) -+ -+dnl empty file, no additions, but previous ones will remain -+AT_DATA([./deny_cidr], []) -+FWD_CHECK([--permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 0, [ignore], [ignore]) - - FWD_END_TEST() --- -2.31.1 - diff --git a/SOURCES/v1.0.0-0049-fix-ipset-reduce-cost-of-entry-overlap-detection.patch b/SOURCES/v1.0.0-0049-fix-ipset-reduce-cost-of-entry-overlap-detection.patch new file mode 100644 index 0000000..ebebd8b --- /dev/null +++ b/SOURCES/v1.0.0-0049-fix-ipset-reduce-cost-of-entry-overlap-detection.patch @@ -0,0 +1,140 @@ +From 34967402eda57d051b239c1551ecc0259881e7d4 Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Tue, 30 Nov 2021 14:54:20 -0500 +Subject: [PATCH 49/51] fix(ipset): reduce cost of entry overlap detection + +This increases peak memory usage to reduce the duration it takes to +apply the set entries. Building the list of IPv4Network objects up front +means we don't have to build them multiple times inside the for loop. + +Fixes: #881 +(cherry picked from commit 7f5b736378c0133f46470c42e0c1fb3b95087de5) +--- + src/firewall/client.py | 10 ++++------ + src/firewall/core/fw_ipset.py | 9 +++------ + src/firewall/core/ipset.py | 27 ++++++++++++++++++++++----- + src/firewall/server/config_ipset.py | 10 ++++------ + 4 files changed, 33 insertions(+), 23 deletions(-) + +diff --git a/src/firewall/client.py b/src/firewall/client.py +index 3715ffd29316..fdc88ac7946b 100644 +--- a/src/firewall/client.py ++++ b/src/firewall/client.py +@@ -34,7 +34,8 @@ from firewall.core.base import DEFAULT_ZONE_TARGET, DEFAULT_POLICY_TARGET, DEFAU + from firewall.dbus_utils import dbus_to_python + from firewall.functions import b2u + from firewall.core.rich import Rich_Rule +-from firewall.core.ipset import normalize_ipset_entry, check_entry_overlaps_existing ++from firewall.core.ipset import normalize_ipset_entry, check_entry_overlaps_existing, \ ++ check_for_overlapping_entries + from firewall import errors + from firewall.errors import FirewallError + +@@ -1617,11 +1618,8 @@ class FirewallClientIPSetSettings(object): + if "timeout" in self.settings[4] and \ + self.settings[4]["timeout"] != "0": + raise FirewallError(errors.IPSET_WITH_TIMEOUT) +- _entries = set() +- for _entry in dbus_to_python(entries, list): +- check_entry_overlaps_existing(_entry, _entries) +- _entries.add(normalize_ipset_entry(_entry)) +- self.settings[5] = list(_entries) ++ check_for_overlapping_entries(entries) ++ self.settings[5] = entries + @handle_exceptions + def addEntry(self, entry): + if "timeout" in self.settings[4] and \ +diff --git a/src/firewall/core/fw_ipset.py b/src/firewall/core/fw_ipset.py +index a285fd4a4aab..d7878c01921e 100644 +--- a/src/firewall/core/fw_ipset.py ++++ b/src/firewall/core/fw_ipset.py +@@ -25,7 +25,8 @@ __all__ = [ "FirewallIPSet" ] + + from firewall.core.logger import log + from firewall.core.ipset import remove_default_create_options as rm_def_cr_opts, \ +- normalize_ipset_entry, check_entry_overlaps_existing ++ normalize_ipset_entry, check_entry_overlaps_existing, \ ++ check_for_overlapping_entries + from firewall.core.io.ipset import IPSet + from firewall import errors + from firewall.errors import FirewallError +@@ -244,11 +245,7 @@ class FirewallIPSet(object): + def set_entries(self, name, entries): + obj = self.get_ipset(name, applied=True) + +- _entries = set() +- for _entry in entries: +- check_entry_overlaps_existing(_entry, _entries) +- _entries.add(normalize_ipset_entry(_entry)) +- entries = list(_entries) ++ check_for_overlapping_entries(entries) + + for entry in entries: + IPSet.check_entry(entry, obj.options, obj.type) +diff --git a/src/firewall/core/ipset.py b/src/firewall/core/ipset.py +index d6defa395241..66ea4335536d 100644 +--- a/src/firewall/core/ipset.py ++++ b/src/firewall/core/ipset.py +@@ -309,9 +309,26 @@ def check_entry_overlaps_existing(entry, entries): + if len(entry.split(",")) > 1: + return + ++ try: ++ entry_network = ipaddress.ip_network(entry, strict=False) ++ except ValueError: ++ # could not parse the new IP address, maybe a MAC ++ return ++ + for itr in entries: +- try: +- if ipaddress.ip_network(itr, strict=False).overlaps(ipaddress.ip_network(entry, strict=False)): +- raise FirewallError(errors.INVALID_ENTRY, "Entry '{}' overlaps with existing entry '{}'".format(itr, entry)) +- except ValueError: +- pass ++ if entry_network.overlaps(ipaddress.ip_network(itr, strict=False)): ++ raise FirewallError(errors.INVALID_ENTRY, "Entry '{}' overlaps with existing entry '{}'".format(entry, itr)) ++ ++def check_for_overlapping_entries(entries): ++ """ Check if any entry overlaps any entry in the list of entries """ ++ try: ++ entries = [ipaddress.ip_network(x, strict=False) for x in entries] ++ except ValueError: ++ # at least one entry can not be parsed ++ return ++ ++ while entries: ++ entry = entries.pop() ++ for itr in entries: ++ if entry.overlaps(itr): ++ raise FirewallError(errors.INVALID_ENTRY, "Entry '{}' overlaps entry '{}'".format(entry, itr)) +diff --git a/src/firewall/server/config_ipset.py b/src/firewall/server/config_ipset.py +index f33c2a02926f..499ffcb9227a 100644 +--- a/src/firewall/server/config_ipset.py ++++ b/src/firewall/server/config_ipset.py +@@ -34,7 +34,8 @@ from firewall.dbus_utils import dbus_to_python, \ + dbus_introspection_add_properties + from firewall.core.io.ipset import IPSet + from firewall.core.ipset import IPSET_TYPES, normalize_ipset_entry, \ +- check_entry_overlaps_existing ++ check_entry_overlaps_existing, \ ++ check_for_overlapping_entries + from firewall.core.logger import log + from firewall.server.decorators import handle_exceptions, \ + dbus_handle_exceptions, dbus_service_method +@@ -407,11 +408,8 @@ class FirewallDConfigIPSet(slip.dbus.service.Object): + in_signature='as') + @dbus_handle_exceptions + def setEntries(self, entries, sender=None): +- _entries = set() +- for _entry in dbus_to_python(entries, list): +- check_entry_overlaps_existing(_entry, _entries) +- _entries.add(normalize_ipset_entry(_entry)) +- entries = list(_entries) ++ entries = dbus_to_python(entries, list) ++ check_for_overlapping_entries(entries) + log.debug1("%s.setEntries('[%s]')", self._log_prefix, + ",".join(entries)) + self.parent.accessCheck(sender) +-- +2.31.1 + diff --git a/SOURCES/v1.0.0-0050-test-ipset-huge-set-of-entries-benchmark.patch b/SOURCES/v1.0.0-0050-test-ipset-huge-set-of-entries-benchmark.patch new file mode 100644 index 0000000..32e8a22 --- /dev/null +++ b/SOURCES/v1.0.0-0050-test-ipset-huge-set-of-entries-benchmark.patch @@ -0,0 +1,56 @@ +From 344753267f6b40d029a3b690cce74720a355cb4d Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Tue, 30 Nov 2021 14:50:17 -0500 +Subject: [PATCH 50/51] test(ipset): huge set of entries benchmark + +Coverage: #881 +(cherry picked from commit 114936c71ab1b12a5598d06805b7e9e13f7ee190) +--- + src/tests/regression/gh881.at | 25 +++++++++++++++++++++++++ + src/tests/regression/regression.at | 1 + + 2 files changed, 26 insertions(+) + create mode 100644 src/tests/regression/gh881.at + +diff --git a/src/tests/regression/gh881.at b/src/tests/regression/gh881.at +new file mode 100644 +index 000000000000..c7326805b555 +--- /dev/null ++++ b/src/tests/regression/gh881.at +@@ -0,0 +1,25 @@ ++FWD_START_TEST([ipset entry overlap detect perf]) ++AT_KEYWORDS(ipset gh881) ++ ++dnl build a large ipset ++dnl ++AT_DATA([./deny_cidr], []) ++NS_CHECK([sh -c ' ++for I in $(seq 10); do ++ for J in $(seq 250); do ++ echo "10.${I}.${J}.0/24" >> ./deny_cidr ++ done ++done ++']) ++ ++dnl verify non-overlapping does not error ++dnl ++FWD_CHECK([--permanent --new-ipset=deny_set --type=hash:net --option=family=inet --option=hashsize=16384 --option=maxelem=20000], 0, [ignore]) ++NS_CHECK([time timeout 300 firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 0, [ignore], [ignore]) ++ ++dnl verify overlap detection actually detects an overlap ++dnl ++NS_CHECK([echo "10.1.0.0/16" >> ./deny_cidr]) ++NS_CHECK([time timeout 300 firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 136, [ignore], [ignore]) ++ ++FWD_END_TEST() +diff --git a/src/tests/regression/regression.at b/src/tests/regression/regression.at +index a20b913fbe59..4045563d0b91 100644 +--- a/src/tests/regression/regression.at ++++ b/src/tests/regression/regression.at +@@ -45,3 +45,4 @@ m4_include([regression/rhbz1914935.at]) + m4_include([regression/gh696.at]) + m4_include([regression/rhbz1917766.at]) + m4_include([regression/rhbz2014383.at]) ++m4_include([regression/gh881.at]) +-- +2.31.1 + diff --git a/SOURCES/v1.0.0-0051-fix-ipset-further-reduce-cost-of-entry-overlap-detec.patch b/SOURCES/v1.0.0-0051-fix-ipset-further-reduce-cost-of-entry-overlap-detec.patch new file mode 100644 index 0000000..7d1e8a1 --- /dev/null +++ b/SOURCES/v1.0.0-0051-fix-ipset-further-reduce-cost-of-entry-overlap-detec.patch @@ -0,0 +1,150 @@ +From 33b10b9112f2f51df049315438204efec7a5434c Mon Sep 17 00:00:00 2001 +From: Eric Garver +Date: Tue, 25 Jan 2022 09:29:32 -0500 +Subject: [PATCH 51/51] fix(ipset): further reduce cost of entry overlap + detection + +This makes the complexity linear by sorting the networks ahead of time. + +Fixes: #881 +Fixes: rhbz2043289 +(cherry picked from commit 36c170db265265e838a089858be4b20dbbd582eb) +--- + src/firewall/core/ipset.py | 59 ++++++++++++++++++++++++++++++++--- + src/tests/regression/gh881.at | 42 ++++++++++++++++++++++--- + 2 files changed, 92 insertions(+), 9 deletions(-) + +diff --git a/src/firewall/core/ipset.py b/src/firewall/core/ipset.py +index 66ea4335536d..b160d8345669 100644 +--- a/src/firewall/core/ipset.py ++++ b/src/firewall/core/ipset.py +@@ -327,8 +327,57 @@ def check_for_overlapping_entries(entries): + # at least one entry can not be parsed + return + +- while entries: +- entry = entries.pop() +- for itr in entries: +- if entry.overlaps(itr): +- raise FirewallError(errors.INVALID_ENTRY, "Entry '{}' overlaps entry '{}'".format(entry, itr)) ++ # We can take advantage of some facts of IPv4Network/IPv6Network and ++ # how Python sorts the networks to quickly detect overlaps. ++ # ++ # Facts: ++ # ++ # 1. IPv{4,6}Network are normalized to remove host bits, e.g. ++ # 10.1.1.0/16 will become 10.1.0.0/16. ++ # ++ # 2. IPv{4,6}Network objects are sorted by: ++ # a. IP address (network bits) ++ # then ++ # b. netmask (significant bits count) ++ # ++ # Because of the above we have these properties: ++ # ++ # 1. big networks (netA) are sorted before smaller networks (netB) ++ # that overlap the big network (netA) ++ # - e.g. 10.1.128.0/17 (netA) sorts before 10.1.129.0/24 (netB) ++ # 2. same value addresses (network bits) are grouped together even ++ # if the number of network bits vary. e.g. /16 vs /24 ++ # - recall that address are normalized to remove host bits ++ # - e.g. 10.1.128.0/17 (netA) sorts before 10.1.128.0/24 (netC) ++ # 3. non-overlapping networks (netD, netE) are always sorted before or ++ # after networks that overlap (netB, netC) the current one (netA) ++ # - e.g. 10.1.128.0/17 (netA) sorts before 10.2.128.0/16 (netD) ++ # - e.g. 10.1.128.0/17 (netA) sorts after 9.1.128.0/17 (netE) ++ # - e.g. 9.1.128.0/17 (netE) sorts before 10.1.129.0/24 (netB) ++ # ++ # With this we know the sorted list looks like: ++ # ++ # list: [ netE, netA, netB, netC, netD ] ++ # ++ # netE = non-overlapping network ++ # netA = big network ++ # netB = smaller network that overlaps netA (subnet) ++ # netC = smaller network that overlaps netA (subnet) ++ # netD = non-overlapping network ++ # ++ # If networks netB and netC exist in the list, they overlap and are ++ # adjacent to netA. ++ # ++ # Checking for overlaps on a sorted list is thus: ++ # ++ # 1. compare adjacent elements in the list for overlaps ++ # ++ # Recall that we only need to detect a single overlap. We do not need to ++ # detect them all. ++ # ++ entries.sort() ++ prev_network = entries.pop(0) ++ for current_network in entries: ++ if prev_network.overlaps(current_network): ++ raise FirewallError(errors.INVALID_ENTRY, "Entry '{}' overlaps entry '{}'".format(prev_network, current_network)) ++ prev_network = current_network +diff --git a/src/tests/regression/gh881.at b/src/tests/regression/gh881.at +index c7326805b555..a5cf7e4eb912 100644 +--- a/src/tests/regression/gh881.at ++++ b/src/tests/regression/gh881.at +@@ -5,21 +5,55 @@ dnl build a large ipset + dnl + AT_DATA([./deny_cidr], []) + NS_CHECK([sh -c ' +-for I in $(seq 10); do ++for I in $(seq 250); do + for J in $(seq 250); do + echo "10.${I}.${J}.0/24" >> ./deny_cidr + done + done + ']) ++NS_CHECK([echo "10.254.0.0/16" >> ./deny_cidr]) + + dnl verify non-overlapping does not error + dnl + FWD_CHECK([--permanent --new-ipset=deny_set --type=hash:net --option=family=inet --option=hashsize=16384 --option=maxelem=20000], 0, [ignore]) +-NS_CHECK([time timeout 300 firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 0, [ignore], [ignore]) ++NS_CHECK([time firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 0, [ignore], [ignore]) ++ ++dnl still no overlap ++dnl ++AT_DATA([./deny_cidr], [ ++9.0.0.0/8 ++11.1.0.0/16 ++]) ++NS_CHECK([time firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 0, [ignore], [ignore]) + + dnl verify overlap detection actually detects an overlap + dnl +-NS_CHECK([echo "10.1.0.0/16" >> ./deny_cidr]) +-NS_CHECK([time timeout 300 firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 136, [ignore], [ignore]) ++AT_DATA([./deny_cidr], [ ++10.1.0.0/16 ++10.2.0.0/16 ++10.250.0.0/16 ++]) ++NS_CHECK([time firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 136, [ignore], [ignore]) ++ ++AT_DATA([./deny_cidr], [ ++10.253.0.0/16 ++10.253.128.0/17 ++]) ++NS_CHECK([time firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 136, [ignore], [ignore]) ++ ++AT_DATA([./deny_cidr], [ ++10.1.1.1/32 ++]) ++NS_CHECK([time firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 136, [ignore], [ignore]) ++ ++AT_DATA([./deny_cidr], [ ++10.0.0.0/8 ++10.0.0.0/25 ++]) ++NS_CHECK([time firewall-cmd --permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 136, [ignore], [ignore]) ++ ++dnl empty file, no additions, but previous ones will remain ++AT_DATA([./deny_cidr], []) ++FWD_CHECK([--permanent --ipset=deny_set --add-entries-from-file=./deny_cidr], 0, [ignore], [ignore]) + + FWD_END_TEST() +-- +2.31.1 + diff --git a/SPECS/firewalld.spec b/SPECS/firewalld.spec index af9eb18..a4ad0e7 100644 --- a/SPECS/firewalld.spec +++ b/SPECS/firewalld.spec @@ -1,7 +1,7 @@ Summary: A firewall daemon with D-Bus interface providing a dynamic firewall Name: firewalld Version: 0.9.3 -Release: 7%{?dist}.1 +Release: 13%{?dist} URL: http://www.firewalld.org License: GPLv2+ Source0: https://github.com/firewalld/firewalld/releases/download/v%{version}/firewalld-%{version}.tar.gz @@ -41,9 +41,21 @@ Patch33: 0033-fix-policy-warn-instead-of-error-for-overlapping-por.patch Patch34: 0034-test-zone-verify-overlapping-ports-don-t-halt-zone-l.patch Patch35: v1.0.0-0035-fix-ipset-normalize-entries-in-CIDR-notation.patch Patch36: v1.0.0-0036-fix-ipset-disallow-overlapping-entries.patch -Patch37: v1.0.0-0037-fix-ipset-reduce-cost-of-entry-overlap-detection.patch -Patch38: v1.0.0-0038-test-ipset-huge-set-of-entries-benchmark.patch -Patch39: v1.0.0-0039-fix-ipset-further-reduce-cost-of-entry-overlap-detec.patch +Patch37: 0037-docs-firewall-cmd-client-conntrack-helpers-must-use-.patch +Patch38: 0038-fix-nftables-do-not-log-icmp-block-if-inversion.patch +Patch39: 0039-test-icmp-don-t-log-blocked-if-ICMP-inversion.patch +Patch40: 0040-fix-nftables-rich-source-address-with-netmask.patch +Patch41: 0041-test-rich-source-address-with-netmask.patch +Patch42: 0042-test-zone-source-with-netmask.patch +Patch43: 0043-fix-fw_config-zone-on-rename-remove-then-add.patch +Patch44: 0044-fix-io-functions-check_config-against-on-disk-conf.patch +Patch45: 0045-fix-zone-detect-same-source-interface-in-zones.patch +Patch46: 0046-test-zone-detect-same-source-interface-in-zones.patch +Patch47: 0047-feat-config-add-CleanupModulesOnExit-configuration-o.patch +Patch48: 0048-RHEL-only-default-to-CleanupModulesOnExit-yes.patch +Patch49: v1.0.0-0049-fix-ipset-reduce-cost-of-entry-overlap-detection.patch +Patch50: v1.0.0-0050-test-ipset-huge-set-of-entries-benchmark.patch +Patch51: v1.0.0-0051-fix-ipset-further-reduce-cost-of-entry-overlap-detec.patch BuildArch: noarch BuildRequires: autoconf @@ -245,8 +257,24 @@ desktop-file-install --delete-original \ %{_mandir}/man1/firewall-config*.1* %changelog -* Thu Feb 03 2022 Eric Garver - 0.9.3-7.el8_5.1 -- fix(ipset): reduce cost of entry overlap detection +* Thu Feb 03 2022 Eric Garver - 0.9.3-13 +- change default CleanupModulesOnExit=yes + +* Mon Dec 20 2021 Eric Garver - 0.9.3-12 +- feat(config): add CleanupModulesOnExit configuration option +- change default CleanupModulesOnExit=yes + +* Tue Nov 16 2021 Eric Garver - 0.9.3-11 +- fix(zone): detect same source/interface in zones + +* Tue Nov 16 2021 Eric Garver - 0.9.3-10 +- fix(nftables): rich: source address with netmask + +* Tue Nov 16 2021 Eric Garver - 0.9.3-9 +- fix(nftables): do not log icmp block if inversion + +* Tue Nov 16 2021 Eric Garver - 0.9.3-8 +- docs(firewall-*cmd): client conntrack helpers must use a policy * Tue Jul 13 2021 Eric Garver - 0.9.3-7 - fix(ipset): disallow overlapping entries