Blob Blame History Raw
From 44dff592c200f81d74b64ba1c729ec8ec3b8612e Mon Sep 17 00:00:00 2001
From: Eric Garver <eric@garver.life>
Date: Tue, 13 Apr 2021 14:35:31 -0400
Subject: [PATCH 23/30] fix(direct): rule order with multiple address with
 -s/-d

Fixes: rhbz 1940928
Fixes: rhbz 1949552
(cherry picked from commit 2be50d366b9ba073e5f86edcd0b412ff48c3fed1)
(cherry picked from commit a545183d6916169cd16648707b9f876ea0833955)
---
 src/firewall/core/fw_direct.py | 53 +++++++++++++++++++++++++++++-----
 src/firewall/core/ipXtables.py | 32 --------------------
 2 files changed, 46 insertions(+), 39 deletions(-)

diff --git a/src/firewall/core/fw_direct.py b/src/firewall/core/fw_direct.py
index e53a72e3326a..76aeda9f19cb 100644
--- a/src/firewall/core/fw_direct.py
+++ b/src/firewall/core/fw_direct.py
@@ -298,7 +298,7 @@ class FirewallDirect(object):
                 r.append((ipv, table, chain, priority, list(args)))
         return r
 
-    def _register_rule(self, rule_id, chain_id, priority, enable):
+    def _register_rule(self, rule_id, chain_id, priority, enable, count):
         if enable:
             if chain_id not in self._rules:
                 self._rules[chain_id] = LastUpdatedOrderedDict()
@@ -307,14 +307,14 @@ class FirewallDirect(object):
                 self._rule_priority_positions[chain_id] = { }
 
             if priority in self._rule_priority_positions[chain_id]:
-                self._rule_priority_positions[chain_id][priority] += 1
+                self._rule_priority_positions[chain_id][priority] += count
             else:
-                self._rule_priority_positions[chain_id][priority] = 1
+                self._rule_priority_positions[chain_id][priority] = count
         else:
             del self._rules[chain_id][rule_id]
             if len(self._rules[chain_id]) == 0:
                 del self._rules[chain_id]
-            self._rule_priority_positions[chain_id][priority] -= 1
+            self._rule_priority_positions[chain_id][priority] -= count
 
     # DIRECT PASSTHROUGH (untracked)
 
@@ -376,6 +376,34 @@ class FirewallDirect(object):
                 r.append(list(args))
         return r
 
+    def split_value(self, rules, opts):
+        """Split values combined with commas for options in opts"""
+
+        out_rules = [ ]
+        for rule in rules:
+            processed = False
+            for opt in opts:
+                try:
+                    i = rule.index(opt)
+                except ValueError:
+                    pass
+                else:
+                    if len(rule) > i and "," in rule[i+1]:
+                        # For all items in the comma separated list in index
+                        # i of the rule, a new rule is created with a single
+                        # item from this list
+                        processed = True
+                        items = rule[i+1].split(",")
+                        for item in items:
+                            _rule = rule[:]
+                            _rule[i+1] = item
+                            out_rules.append(_rule)
+            if not processed:
+                out_rules.append(rule)
+
+        return out_rules
+
+
     def _rule(self, enable, ipv, table, chain, priority, args, transaction):
         self._check_ipv_table(ipv, table)
         # Do not create zone chains if we're using nftables. Only allow direct
@@ -458,6 +486,7 @@ class FirewallDirect(object):
         # has index 1.
 
         index = 1
+        count = 0
         if chain_id in self._rule_priority_positions:
             positions = sorted(self._rule_priority_positions[chain_id].keys())
             j = 0
@@ -465,11 +494,21 @@ class FirewallDirect(object):
                 index += self._rule_priority_positions[chain_id][positions[j]]
                 j += 1
 
-        transaction.add_rule(backend, backend.build_rule(enable, table, _chain, index, args))
+        # split the direct rule in some cases as iptables-restore can't handle
+        # compound args.
+        #
+        args_list = [list(args)]
+        args_list = self.split_value(args_list, [ "-s", "--source" ])
+        args_list = self.split_value(args_list, [ "-d", "--destination" ])
+
+        for _args in args_list:
+            transaction.add_rule(backend, backend.build_rule(enable, table, _chain, index, tuple(_args)))
+            index += 1
+            count += 1
 
-        self._register_rule(rule_id, chain_id, priority, enable)
+        self._register_rule(rule_id, chain_id, priority, enable, count)
         transaction.add_fail(self._register_rule,
-                             rule_id, chain_id, priority, not enable)
+                             rule_id, chain_id, priority, not enable, count)
 
     def _chain(self, add, ipv, table, chain, transaction):
         self._check_ipv_table(ipv, table)
diff --git a/src/firewall/core/ipXtables.py b/src/firewall/core/ipXtables.py
index 968b75867849..818ce3f153d0 100644
--- a/src/firewall/core/ipXtables.py
+++ b/src/firewall/core/ipXtables.py
@@ -200,36 +200,6 @@ class ip4tables(object):
                                                      " ".join(_args), ret))
         return ret
 
-    def split_value(self, rules, opts=None):
-        """Split values combined with commas for options in opts"""
-
-        if opts is None:
-            return rules
-
-        out_rules = [ ]
-        for rule in rules:
-            processed = False
-            for opt in opts:
-                try:
-                    i = rule.index(opt)
-                except ValueError:
-                    pass
-                else:
-                    if len(rule) > i and "," in rule[i+1]:
-                        # For all items in the comma separated list in index
-                        # i of the rule, a new rule is created with a single
-                        # item from this list
-                        processed = True
-                        items = rule[i+1].split(",")
-                        for item in items:
-                            _rule = rule[:]
-                            _rule[i+1] = item
-                            out_rules.append(_rule)
-            if not processed:
-                out_rules.append(rule)
-
-        return out_rules
-
     def _rule_replace(self, rule, pattern, replacement):
         try:
             i = rule.index(pattern)
@@ -472,8 +442,6 @@ class ip4tables(object):
 
         for table in table_rules:
             rules = table_rules[table]
-            rules = self.split_value(rules, [ "-s", "--source" ])
-            rules = self.split_value(rules, [ "-d", "--destination" ])
 
             temp_file.write("*%s\n" % table)
             for rule in rules:
-- 
2.27.0