From 74760c43588be65303795397717d4aa5ef5e4236 Mon Sep 17 00:00:00 2001
From: Eric Garver <eric@garver.life>
Date: Wed, 29 May 2019 15:21:34 -0400
Subject: [PATCH 59/73] fix: do not allow zone drifting
Chain zone dispatch together and always use "goto". This guarantees
there is no fall through to other zones. This was especially problematic
in regards to the default zone.
This removes the _ZONES_SOURCE chains, but adds _ZONES_IFACES. At the
end of _ZONES we do a goto to _ZONES_IFACES. This is so sources always
take precedence over interfaces.
Fixes: #258
Fixes: #441
(cherry picked from commit 70993581d79beb40a3d23bd8cbfb776ad5df5dca)
(cherry picked from commit 16c7603b57d5b07389e9c2ba0ca8b4836b2aaf93)
---
src/firewall/core/fw_zone.py | 6 +--
src/firewall/core/ipXtables.py | 70 ++++++++++++++++------------------
src/firewall/core/nftables.py | 67 +++++++++++++++-----------------
src/tests/firewall-cmd.at | 8 ++--
4 files changed, 69 insertions(+), 82 deletions(-)
diff --git a/src/firewall/core/fw_zone.py b/src/firewall/core/fw_zone.py
index ee02a161bcfb..90ae1036f124 100644
--- a/src/firewall/core/fw_zone.py
+++ b/src/firewall/core/fw_zone.py
@@ -1514,8 +1514,7 @@ class FirewallZone(object):
zone_transaction.add_chain(table, chain)
rules = backend.build_zone_source_interface_rules(enable,
- zone, self._zones[zone].target,
- interface, table, chain, append)
+ zone, interface, table, chain, append)
zone_transaction.add_rules(backend, rules)
# IPSETS
@@ -1555,8 +1554,7 @@ class FirewallZone(object):
zone_transaction.add_chain(table, chain)
rules = backend.build_zone_source_address_rules(enable, zone,
- self._zones[zone].target, source, table,
- chain)
+ source, table, chain)
zone_transaction.add_rules(backend, rules)
def _rule_prepare(self, enable, zone, rule, mark_id, zone_transaction):
diff --git a/src/firewall/core/ipXtables.py b/src/firewall/core/ipXtables.py
index 4a9c06242f08..c2339e40539a 100644
--- a/src/firewall/core/ipXtables.py
+++ b/src/firewall/core/ipXtables.py
@@ -526,11 +526,11 @@ class ip4tables(object):
self.our_chains["raw"].add("%s_direct" % chain)
if chain == "PREROUTING":
- default_rules["raw"].append("-N %s_ZONES_SOURCE" % chain)
default_rules["raw"].append("-N %s_ZONES" % chain)
- default_rules["raw"].append("-A %s -j %s_ZONES_SOURCE" % (chain, chain))
+ default_rules["raw"].append("-N %s_ZONES_IFACES" % chain)
default_rules["raw"].append("-A %s -j %s_ZONES" % (chain, chain))
- self.our_chains["raw"].update(set(["%s_ZONES_SOURCE" % chain, "%s_ZONES" % chain]))
+ default_rules["raw"].append("-A %s_ZONES -g %s_ZONES_IFACES" % (chain, chain))
+ self.our_chains["raw"].update(set(["%s_ZONES" % chain, "%s_ZONES_IFACES" % chain]))
if self.get_available_tables("mangle"):
default_rules["mangle"] = [ ]
@@ -541,11 +541,11 @@ class ip4tables(object):
self.our_chains["mangle"].add("%s_direct" % chain)
if chain == "PREROUTING":
- default_rules["mangle"].append("-N %s_ZONES_SOURCE" % chain)
default_rules["mangle"].append("-N %s_ZONES" % chain)
- default_rules["mangle"].append("-A %s -j %s_ZONES_SOURCE" % (chain, chain))
+ default_rules["mangle"].append("-N %s_ZONES_IFACES" % chain)
default_rules["mangle"].append("-A %s -j %s_ZONES" % (chain, chain))
- self.our_chains["mangle"].update(set(["%s_ZONES_SOURCE" % chain, "%s_ZONES" % chain]))
+ default_rules["mangle"].append("-A %s_ZONES -g %s_ZONES_IFACES" % (chain, chain))
+ self.our_chains["mangle"].update(set(["%s_ZONES" % chain, "%s_ZONES_IFACES" % chain]))
if self.get_available_tables("nat"):
default_rules["nat"] = [ ]
@@ -556,22 +556,22 @@ class ip4tables(object):
self.our_chains["nat"].add("%s_direct" % chain)
if chain in [ "PREROUTING", "POSTROUTING" ]:
- default_rules["nat"].append("-N %s_ZONES_SOURCE" % chain)
default_rules["nat"].append("-N %s_ZONES" % chain)
- default_rules["nat"].append("-A %s -j %s_ZONES_SOURCE" % (chain, chain))
+ default_rules["nat"].append("-N %s_ZONES_IFACES" % chain)
default_rules["nat"].append("-A %s -j %s_ZONES" % (chain, chain))
- self.our_chains["nat"].update(set(["%s_ZONES_SOURCE" % chain, "%s_ZONES" % chain]))
+ default_rules["nat"].append("-A %s_ZONES -g %s_ZONES_IFACES" % (chain, chain))
+ self.our_chains["nat"].update(set(["%s_ZONES" % chain, "%s_ZONES_IFACES" % chain]))
default_rules["filter"] = [
"-N INPUT_direct",
- "-N INPUT_ZONES_SOURCE",
"-N INPUT_ZONES",
+ "-N INPUT_ZONES_IFACES",
"-A INPUT -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT",
"-A INPUT -i lo -j ACCEPT",
"-A INPUT -j INPUT_direct",
- "-A INPUT -j INPUT_ZONES_SOURCE",
"-A INPUT -j INPUT_ZONES",
+ "-A INPUT_ZONES -g INPUT_ZONES_IFACES",
]
if log_denied != "off":
default_rules["filter"].append("-A INPUT -m conntrack --ctstate INVALID %%LOGTYPE%% -j LOG --log-prefix 'STATE_INVALID_DROP: '")
@@ -582,18 +582,18 @@ class ip4tables(object):
default_rules["filter"] += [
"-N FORWARD_direct",
- "-N FORWARD_IN_ZONES_SOURCE",
"-N FORWARD_IN_ZONES",
- "-N FORWARD_OUT_ZONES_SOURCE",
"-N FORWARD_OUT_ZONES",
+ "-N FORWARD_IN_ZONES_IFACES",
+ "-N FORWARD_OUT_ZONES_IFACES",
"-A FORWARD -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT",
"-A FORWARD -i lo -j ACCEPT",
"-A FORWARD -j FORWARD_direct",
- "-A FORWARD -j FORWARD_IN_ZONES_SOURCE",
"-A FORWARD -j FORWARD_IN_ZONES",
- "-A FORWARD -j FORWARD_OUT_ZONES_SOURCE",
"-A FORWARD -j FORWARD_OUT_ZONES",
+ "-A FORWARD_IN_ZONES -g FORWARD_IN_ZONES_IFACES",
+ "-A FORWARD_OUT_ZONES -g FORWARD_OUT_ZONES_IFACES",
]
if log_denied != "off":
default_rules["filter"].append("-A FORWARD -m conntrack --ctstate INVALID %%LOGTYPE%% -j LOG --log-prefix 'STATE_INVALID_DROP: '")
@@ -609,10 +609,10 @@ class ip4tables(object):
"-A OUTPUT -j OUTPUT_direct",
]
- self.our_chains["filter"] = set(["INPUT_direct", "INPUT_ZONES_SOURCE", "INPUT_ZONES",
- "FORWARD_direct", "FORWARD_IN_ZONES_SOURCE",
- "FORWARD_IN_ZONES", "FORWARD_OUT_ZONES_SOURCE",
- "FORWARD_OUT_ZONES", "OUTPUT_direct"])
+ self.our_chains["filter"] = set(["INPUT_direct", "INPUT_ZONES", "INPUT_ZONES_IFACES"
+ "FORWARD_direct", "FORWARD_IN_ZONES",
+ "FORWARD_IN_ZONES_IFACES" "FORWARD_OUT_ZONES",
+ "FORWARD_OUT_ZONES_IFACES", "OUTPUT_direct"])
final_default_rules = []
for table in default_rules:
@@ -639,9 +639,8 @@ class ip4tables(object):
return {}
- def build_zone_source_interface_rules(self, enable, zone, zone_target,
- interface, table, chain,
- append=False):
+ def build_zone_source_interface_rules(self, enable, zone, interface,
+ table, chain, append=False):
# handle all zones in the same way here, now
# trust and block zone targets are handled now in __chain
opt = {
@@ -654,22 +653,20 @@ class ip4tables(object):
}[chain]
target = DEFAULT_ZONE_TARGET.format(chain=SHORTCUTS[chain], zone=zone)
- if zone_target == DEFAULT_ZONE_TARGET:
- action = "-g"
- else:
- action = "-j"
+ action = "-g"
+
if enable and not append:
- rule = [ "-I", "%s_ZONES" % chain, "1" ]
+ rule = [ "-I", "%s_ZONES_IFACES" % chain, "1" ]
elif enable:
- rule = [ "-A", "%s_ZONES" % chain ]
+ rule = [ "-A", "%s_ZONES_IFACES" % chain ]
else:
- rule = [ "-D", "%s_ZONES" % chain ]
+ rule = [ "-D", "%s_ZONES_IFACES" % chain ]
rule += [ "-t", table, opt, interface, action, target ]
return [rule]
- def build_zone_source_address_rules(self, enable, zone, zone_target,
+ def build_zone_source_address_rules(self, enable, zone,
address, table, chain):
- add_del = { True: "-A", False: "-D" }[enable]
+ add_del = { True: "-I", False: "-D" }[enable]
opt = {
"PREROUTING": "-s",
@@ -681,10 +678,7 @@ class ip4tables(object):
}[chain]
target = DEFAULT_ZONE_TARGET.format(chain=SHORTCUTS[chain], zone=zone)
- if zone_target == DEFAULT_ZONE_TARGET:
- action = "-g"
- else:
- action = "-j"
+ action = "-g"
if address.startswith("ipset:"):
name = address[6:]
@@ -694,7 +688,7 @@ class ip4tables(object):
opt = "src"
flags = ",".join([opt] * self._fw.ipset.get_dimension(name))
rule = [ add_del,
- "%s_ZONES_SOURCE" % chain, "-t", table,
+ "%s_ZONES" % chain, "-t", table,
"-m", "set", "--match-set", name,
flags, action, target ]
else:
@@ -703,12 +697,12 @@ class ip4tables(object):
if opt == "-d":
return ""
rule = [ add_del,
- "%s_ZONES_SOURCE" % chain, "-t", table,
+ "%s_ZONES" % chain, "-t", table,
"-m", "mac", "--mac-source", address.upper(),
action, target ]
else:
rule = [ add_del,
- "%s_ZONES_SOURCE" % chain, "-t", table,
+ "%s_ZONES" % chain, "-t", table,
opt, address, action, target ]
return [rule]
diff --git a/src/firewall/core/nftables.py b/src/firewall/core/nftables.py
index bf41ed98a542..0fe686a01878 100644
--- a/src/firewall/core/nftables.py
+++ b/src/firewall/core/nftables.py
@@ -358,11 +358,11 @@ class nftables(object):
IPTABLES_TO_NFT_HOOK["raw"][chain][0],
IPTABLES_TO_NFT_HOOK["raw"][chain][1]))
- default_rules.append("add chain inet %s raw_%s_ZONES_SOURCE" % (TABLE_NAME, chain))
default_rules.append("add chain inet %s raw_%s_ZONES" % (TABLE_NAME, chain))
- default_rules.append("add rule inet %s raw_%s jump raw_%s_ZONES_SOURCE" % (TABLE_NAME, chain, chain))
+ default_rules.append("add chain inet %s raw_%s_ZONES_IFACES" % (TABLE_NAME, chain))
default_rules.append("add rule inet %s raw_%s jump raw_%s_ZONES" % (TABLE_NAME, chain, chain))
- OUR_CHAINS["inet"]["raw"].update(set(["%s_ZONES_SOURCE" % chain, "%s_ZONES" % chain]))
+ default_rules.append("add rule inet %s raw_%s_ZONES goto raw_%s_ZONES_IFACES" % (TABLE_NAME, chain, chain))
+ OUR_CHAINS["inet"]["raw"].update(set(["%s_ZONES_IFACES" % chain, "%s_ZONES" % chain]))
OUR_CHAINS["inet"]["mangle"] = set()
for chain in IPTABLES_TO_NFT_HOOK["mangle"].keys():
@@ -371,11 +371,11 @@ class nftables(object):
IPTABLES_TO_NFT_HOOK["mangle"][chain][0],
IPTABLES_TO_NFT_HOOK["mangle"][chain][1]))
- default_rules.append("add chain inet %s mangle_%s_ZONES_SOURCE" % (TABLE_NAME, chain))
default_rules.append("add chain inet %s mangle_%s_ZONES" % (TABLE_NAME, chain))
- default_rules.append("add rule inet %s mangle_%s jump mangle_%s_ZONES_SOURCE" % (TABLE_NAME, chain, chain))
+ default_rules.append("add chain inet %s mangle_%s_ZONES_IFACES" % (TABLE_NAME, chain))
default_rules.append("add rule inet %s mangle_%s jump mangle_%s_ZONES" % (TABLE_NAME, chain, chain))
- OUR_CHAINS["inet"]["mangle"].update(set(["%s_ZONES_SOURCE" % chain, "%s_ZONES" % chain]))
+ default_rules.append("add rule inet %s mangle_%s_ZONES goto mangle_%s_ZONES_IFACES" % (TABLE_NAME, chain, chain))
+ OUR_CHAINS["inet"]["mangle"].update(set(["%s_ZONES_IFACES" % chain, "%s_ZONES" % chain]))
OUR_CHAINS["ip"]["nat"] = set()
OUR_CHAINS["ip6"]["nat"] = set()
@@ -386,11 +386,11 @@ class nftables(object):
IPTABLES_TO_NFT_HOOK["nat"][chain][0],
IPTABLES_TO_NFT_HOOK["nat"][chain][1]))
- default_rules.append("add chain %s %s nat_%s_ZONES_SOURCE" % (family, TABLE_NAME, chain))
default_rules.append("add chain %s %s nat_%s_ZONES" % (family, TABLE_NAME, chain))
- default_rules.append("add rule %s %s nat_%s jump nat_%s_ZONES_SOURCE" % (family, TABLE_NAME, chain, chain))
+ default_rules.append("add chain %s %s nat_%s_ZONES_IFACES" % (family, TABLE_NAME, chain))
default_rules.append("add rule %s %s nat_%s jump nat_%s_ZONES" % (family, TABLE_NAME, chain, chain))
- OUR_CHAINS[family]["nat"].update(set(["%s_ZONES_SOURCE" % chain, "%s_ZONES" % chain]))
+ default_rules.append("add rule %s %s nat_%s_ZONES goto nat_%s_ZONES_IFACES" % (family, TABLE_NAME, chain, chain))
+ OUR_CHAINS[family]["nat"].update(set(["%s_ZONES_IFACES" % chain, "%s_ZONES" % chain]))
OUR_CHAINS["inet"]["filter"] = set()
for chain in IPTABLES_TO_NFT_HOOK["filter"].keys():
@@ -400,12 +400,12 @@ class nftables(object):
IPTABLES_TO_NFT_HOOK["filter"][chain][1]))
# filter, INPUT
- default_rules.append("add chain inet %s filter_%s_ZONES_SOURCE" % (TABLE_NAME, "INPUT"))
default_rules.append("add chain inet %s filter_%s_ZONES" % (TABLE_NAME, "INPUT"))
+ default_rules.append("add chain inet %s filter_%s_ZONES_IFACES" % (TABLE_NAME, "INPUT"))
default_rules.append("add rule inet %s filter_%s ct state established,related accept" % (TABLE_NAME, "INPUT"))
default_rules.append("add rule inet %s filter_%s iifname lo accept" % (TABLE_NAME, "INPUT"))
- default_rules.append("add rule inet %s filter_%s jump filter_%s_ZONES_SOURCE" % (TABLE_NAME, "INPUT", "INPUT"))
default_rules.append("add rule inet %s filter_%s jump filter_%s_ZONES" % (TABLE_NAME, "INPUT", "INPUT"))
+ default_rules.append("add rule inet %s filter_%s_ZONES goto filter_%s_ZONES_IFACES" % (TABLE_NAME, "INPUT", "INPUT"))
if log_denied != "off":
default_rules.append("add rule inet %s filter_%s ct state invalid %%%%LOGTYPE%%%% log prefix '\"STATE_INVALID_DROP: \"'" % (TABLE_NAME, "INPUT"))
default_rules.append("add rule inet %s filter_%s ct state invalid drop" % (TABLE_NAME, "INPUT"))
@@ -414,16 +414,16 @@ class nftables(object):
default_rules.append("add rule inet %s filter_%s reject with icmpx type admin-prohibited" % (TABLE_NAME, "INPUT"))
# filter, FORWARD
- default_rules.append("add chain inet %s filter_%s_IN_ZONES_SOURCE" % (TABLE_NAME, "FORWARD"))
default_rules.append("add chain inet %s filter_%s_IN_ZONES" % (TABLE_NAME, "FORWARD"))
- default_rules.append("add chain inet %s filter_%s_OUT_ZONES_SOURCE" % (TABLE_NAME, "FORWARD"))
+ default_rules.append("add chain inet %s filter_%s_IN_ZONES_IFACES" % (TABLE_NAME, "FORWARD"))
default_rules.append("add chain inet %s filter_%s_OUT_ZONES" % (TABLE_NAME, "FORWARD"))
+ default_rules.append("add chain inet %s filter_%s_OUT_ZONES_IFACES" % (TABLE_NAME, "FORWARD"))
default_rules.append("add rule inet %s filter_%s ct state established,related accept" % (TABLE_NAME, "FORWARD"))
default_rules.append("add rule inet %s filter_%s iifname lo accept" % (TABLE_NAME, "FORWARD"))
- default_rules.append("add rule inet %s filter_%s jump filter_%s_IN_ZONES_SOURCE" % (TABLE_NAME, "FORWARD", "FORWARD"))
default_rules.append("add rule inet %s filter_%s jump filter_%s_IN_ZONES" % (TABLE_NAME, "FORWARD", "FORWARD"))
- default_rules.append("add rule inet %s filter_%s jump filter_%s_OUT_ZONES_SOURCE" % (TABLE_NAME, "FORWARD", "FORWARD"))
default_rules.append("add rule inet %s filter_%s jump filter_%s_OUT_ZONES" % (TABLE_NAME, "FORWARD", "FORWARD"))
+ default_rules.append("add rule inet %s filter_%s_IN_ZONES goto filter_%s_IN_ZONES_IFACES" % (TABLE_NAME, "FORWARD", "FORWARD"))
+ default_rules.append("add rule inet %s filter_%s_OUT_ZONES goto filter_%s_OUT_ZONES_IFACES" % (TABLE_NAME, "FORWARD", "FORWARD"))
if log_denied != "off":
default_rules.append("add rule inet %s filter_%s ct state invalid %%%%LOGTYPE%%%% log prefix '\"STATE_INVALID_DROP: \"'" % (TABLE_NAME, "FORWARD"))
default_rules.append("add rule inet %s filter_%s ct state invalid drop" % (TABLE_NAME, "FORWARD"))
@@ -452,16 +452,16 @@ class nftables(object):
return {}
- def build_zone_source_interface_rules(self, enable, zone, zone_target,
- interface, table, chain,
- append=False, family="inet"):
+ def build_zone_source_interface_rules(self, enable, zone, interface,
+ table, chain, append=False,
+ family="inet"):
# nat tables needs to use ip/ip6 family
if table == "nat" and family == "inet":
rules = []
rules.extend(self.build_zone_source_interface_rules(enable, zone,
- zone_target, interface, table, chain, append, "ip"))
+ interface, table, chain, append, "ip"))
rules.extend(self.build_zone_source_interface_rules(enable, zone,
- zone_target, interface, table, chain, append, "ip6"))
+ interface, table, chain, append, "ip6"))
return rules
# handle all zones in the same way here, now
@@ -479,36 +479,34 @@ class nftables(object):
interface = interface[:len(interface)-1] + "*"
target = DEFAULT_ZONE_TARGET.format(chain=SHORTCUTS[chain], zone=zone)
- if zone_target == DEFAULT_ZONE_TARGET:
- action = "goto"
- else:
- action = "jump"
+ action = "goto"
+
if enable and not append:
- rule = ["insert", "rule", family, "%s" % TABLE_NAME, "%s_%s_ZONES" % (table, chain)]
+ rule = ["insert", "rule", family, "%s" % TABLE_NAME, "%s_%s_ZONES_IFACES" % (table, chain)]
elif enable:
- rule = ["add", "rule", family, "%s" % TABLE_NAME, "%s_%s_ZONES" % (table, chain)]
+ rule = ["add", "rule", family, "%s" % TABLE_NAME, "%s_%s_ZONES_IFACES" % (table, chain)]
else:
- rule = ["delete", "rule", family, "%s" % TABLE_NAME, "%s_%s_ZONES" % (table, chain)]
+ rule = ["delete", "rule", family, "%s" % TABLE_NAME, "%s_%s_ZONES_IFACES" % (table, chain)]
if interface == "*":
rule += [action, "%s_%s" % (table, target)]
else:
rule += [opt, "\"" + interface + "\"", action, "%s_%s" % (table, target)]
return [rule]
- def build_zone_source_address_rules(self, enable, zone, zone_target,
+ def build_zone_source_address_rules(self, enable, zone,
address, table, chain, family="inet"):
# nat tables needs to use ip/ip6 family
if table == "nat" and family == "inet":
rules = []
if check_address("ipv4", address) or check_mac(address):
rules.extend(self.build_zone_source_address_rules(enable, zone,
- zone_target, address, table, chain, "ip"))
+ address, table, chain, "ip"))
if check_address("ipv6", address) or check_mac(address):
rules.extend(self.build_zone_source_address_rules(enable, zone,
- zone_target, address, table, chain, "ip6"))
+ address, table, chain, "ip6"))
return rules
- add_del = { True: "add", False: "delete" }[enable]
+ add_del = { True: "insert", False: "delete" }[enable]
opt = {
"PREROUTING": "saddr",
@@ -520,10 +518,7 @@ class nftables(object):
}[chain]
target = DEFAULT_ZONE_TARGET.format(chain=SHORTCUTS[chain], zone=zone)
- if zone_target == DEFAULT_ZONE_TARGET:
- action = "goto"
- else:
- action = "jump"
+ action = "goto"
if address.startswith("ipset:"):
ipset = address[len("ipset:"):]
@@ -541,7 +536,7 @@ class nftables(object):
rule_family = "ip6"
rule = [add_del, "rule", family, "%s" % TABLE_NAME,
- "%s_%s_ZONES_SOURCE" % (table, chain),
+ "%s_%s_ZONES" % (table, chain),
rule_family, opt, address, action, "%s_%s" % (table, target)]
return [rule]
diff --git a/src/tests/firewall-cmd.at b/src/tests/firewall-cmd.at
index a3844151aeb3..0f9cac204ccd 100644
--- a/src/tests/firewall-cmd.at
+++ b/src/tests/firewall-cmd.at
@@ -138,14 +138,14 @@ FWD_START_TEST([zone interfaces])
FWD_CHECK([--add-interface=foobar+++], 0, ignore)
FWD_CHECK([--add-interface=foobar+], 0, ignore)
m4_if(nftables, FIREWALL_BACKEND, [
- NFT_LIST_RULES([inet], [filter_INPUT_ZONES], 0, [dnl
+ NFT_LIST_RULES([inet], [filter_INPUT_ZONES_IFACES], 0, [dnl
table inet firewalld {
- chain filter_INPUT_ZONES {
+ chain filter_INPUT_ZONES_IFACES {
iifname "foobar*" goto filter_IN_public
iifname "foobar++*" goto filter_IN_public
- jump filter_IN_trusted
+ goto filter_IN_trusted
iifname "perm_dummy" goto filter_IN_work
- iifname "perm_dummy2" jump filter_IN_trusted
+ iifname "perm_dummy2" goto filter_IN_trusted
goto filter_IN_public
}
}
--
2.20.1