151 lines
5.7 KiB
Diff
151 lines
5.7 KiB
Diff
From d8d6d313acd50aa1c87c42fb7a7334b01c516227 Mon Sep 17 00:00:00 2001
|
|
From: Eric Garver <eric@garver.life>
|
|
Date: Tue, 25 Jan 2022 09:29:32 -0500
|
|
Subject: [PATCH 10/10] v1.1.0: 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.39.1
|
|
|