From 3d7ec2dabb164cbc2dce5aa8aa37ae156ebad275 Mon Sep 17 00:00:00 2001 From: Eric Garver Date: Tue, 23 Feb 2021 09:18:33 -0500 Subject: [PATCH 36/36] fix(ipset): disallow overlapping entries These are already being blocked by the ipset backend, but we should catch them higher up to avoid differences in the backends. (cherry picked from commit 5b4e8918715a1d2e4abf77ed4eb3252486a19109) --- src/firewall/client.py | 4 +++- src/firewall/core/fw_ipset.py | 4 +++- src/firewall/core/ipset.py | 13 +++++++++++++ src/firewall/server/config_ipset.py | 5 ++++- src/tests/regression/ipset_netmask_allowed.at | 14 ++++++++------ 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/firewall/client.py b/src/firewall/client.py index aa6bd7cd282b..3715ffd29316 100644 --- a/src/firewall/client.py +++ b/src/firewall/client.py @@ -34,7 +34,7 @@ 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 +from firewall.core.ipset import normalize_ipset_entry, check_entry_overlaps_existing from firewall import errors from firewall.errors import FirewallError @@ -1619,6 +1619,7 @@ class FirewallClientIPSetSettings(object): 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) @handle_exceptions @@ -1628,6 +1629,7 @@ class FirewallClientIPSetSettings(object): raise FirewallError(errors.IPSET_WITH_TIMEOUT) entry = normalize_ipset_entry(entry) if entry not in self.settings[5]: + check_entry_overlaps_existing(entry, self.settings[5]) self.settings[5].append(entry) else: raise FirewallError(errors.ALREADY_ENABLED, entry) diff --git a/src/firewall/core/fw_ipset.py b/src/firewall/core/fw_ipset.py index e5348949413c..a285fd4a4aab 100644 --- a/src/firewall/core/fw_ipset.py +++ b/src/firewall/core/fw_ipset.py @@ -25,7 +25,7 @@ __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 + normalize_ipset_entry, check_entry_overlaps_existing from firewall.core.io.ipset import IPSet from firewall import errors from firewall.errors import FirewallError @@ -196,6 +196,7 @@ class FirewallIPSet(object): if entry in obj.entries: raise FirewallError(errors.ALREADY_ENABLED, "'%s' already is in '%s'" % (entry, name)) + check_entry_overlaps_existing(entry, obj.entries) try: for backend in self.backends(): @@ -245,6 +246,7 @@ class FirewallIPSet(object): _entries = set() for _entry in entries: + check_entry_overlaps_existing(_entry, _entries) _entries.add(normalize_ipset_entry(_entry)) entries = list(_entries) diff --git a/src/firewall/core/ipset.py b/src/firewall/core/ipset.py index 5bb21856f648..d6defa395241 100644 --- a/src/firewall/core/ipset.py +++ b/src/firewall/core/ipset.py @@ -302,3 +302,16 @@ def normalize_ipset_entry(entry): _entry.append(_part) return ",".join(_entry) + +def check_entry_overlaps_existing(entry, entries): + """ Check if entry overlaps any entry in the list of entries """ + # Only check simple types + if len(entry.split(",")) > 1: + 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 diff --git a/src/firewall/server/config_ipset.py b/src/firewall/server/config_ipset.py index 18ef5783de62..f33c2a02926f 100644 --- a/src/firewall/server/config_ipset.py +++ b/src/firewall/server/config_ipset.py @@ -33,7 +33,8 @@ from firewall.dbus_utils import dbus_to_python, \ dbus_introspection_prepare_properties, \ dbus_introspection_add_properties from firewall.core.io.ipset import IPSet -from firewall.core.ipset import IPSET_TYPES, normalize_ipset_entry +from firewall.core.ipset import IPSET_TYPES, normalize_ipset_entry, \ + check_entry_overlaps_existing from firewall.core.logger import log from firewall.server.decorators import handle_exceptions, \ dbus_handle_exceptions, dbus_service_method @@ -408,6 +409,7 @@ class FirewallDConfigIPSet(slip.dbus.service.Object): 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) log.debug1("%s.setEntries('[%s]')", self._log_prefix, @@ -432,6 +434,7 @@ class FirewallDConfigIPSet(slip.dbus.service.Object): raise FirewallError(errors.IPSET_WITH_TIMEOUT) if entry in settings[5]: raise FirewallError(errors.ALREADY_ENABLED, entry) + check_entry_overlaps_existing(entry, settings[5]) settings[5].append(entry) self.update(settings) diff --git a/src/tests/regression/ipset_netmask_allowed.at b/src/tests/regression/ipset_netmask_allowed.at index b5165d94b220..fd08afd3b57c 100644 --- a/src/tests/regression/ipset_netmask_allowed.at +++ b/src/tests/regression/ipset_netmask_allowed.at @@ -9,15 +9,17 @@ dnl an add for the whole range. i.e. 1.2.3.4/24 --> 1.2.3.[0.255] (256 dnl entries). dnl dnl In nftables, we allow this by using actual intervals. -FWD_CHECK([--permanent --ipset foobar --add-entry 1.2.3.0/24], 0, [ignore]) -FWD_CHECK([ --ipset foobar --add-entry 1.2.3.0/24], 0, [ignore]) +FWD_CHECK([--permanent --ipset foobar --add-entry 1.2.3.4/24], 0, [ignore]) +FWD_CHECK([ --ipset foobar --add-entry 1.2.3.4/24], 0, [ignore]) dnl check the edge case FWD_CHECK([--permanent --ipset foobar --add-entry 4.3.2.1/32], 0, [ignore]) FWD_CHECK([ --ipset foobar --add-entry 4.3.2.1/32], 0, [ignore]) -dnl overlaps should be denied by ipset -FWD_CHECK([ --ipset foobar --add-entry 1.2.3.0/22], 13, [ignore], [ignore]) -FWD_CHECK([ --ipset foobar --add-entry 1.2.3.0/30], 13, [ignore], [ignore]) +dnl overlaps should be denied +FWD_CHECK([--permanent --ipset foobar --add-entry 1.2.3.0/22], 136, [ignore], [ignore]) +FWD_CHECK([ --ipset foobar --add-entry 1.2.3.0/22], 136, [ignore], [ignore]) +FWD_CHECK([--permanent --ipset foobar --add-entry 1.2.3.4/30], 136, [ignore], [ignore]) +FWD_CHECK([ --ipset foobar --add-entry 1.2.3.4/30], 136, [ignore], [ignore]) -FWD_END_TEST([-e '/ERROR: COMMAND_FAILED:/d']) +FWD_END_TEST([-e '/ERROR: INVALID_ENTRY:/d']) -- 2.27.0