From 44dff592c200f81d74b64ba1c729ec8ec3b8612e Mon Sep 17 00:00:00 2001 From: Eric Garver 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