firewalld/SOURCES/0010-v1.1.0-fix-ipset-further-reduce-cost-of-entry-overla.patch

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