Skip to content

Commit

Permalink
fix(zone): target: default is now similar to reject
Browse files Browse the repository at this point in the history 
The only difference is default implicitly allows ICMP.

BREAKING CHANGE:

Firstly, this affects a combination in which the ingress zone uses a
default target and the egress zone uses ACCEPT. In this scenario packets
would "fall through" and the egress zone would be responsible for the
decision to accept/reject. A good example is public --> trusted. This is
now disallowed. It's unexpected and a security concern.

If users desire the old behavior it can be re-added via a policy.

  # firewall-cmd --permanent --new-policy allowForward
  # firewall-cmd --permanent --policy allowForward --set-target ACCEPT
  # firewall-cmd --permanent --policy allowForward --add-ingress-zone public
  # firewall-cmd --permanent --policy allowForward --add-egress-zone trusted

Secondly this has implications for things that occur after the
filter/forward chain: masquerade, tcp-mss-clamp. In the past the default
target (e.g. public zone) would allow the traffic to "fall through" to
zones that may be using these features. This is very similar to the
issue mentioned directly above and has the same cause. It is now
disallowed.

Note: masquerade is unaffected for ingress zones that use ACCEPT. As
such if your LAN is in the internal or trusted zone and your uplink is
in the external zone then it will still masquerade as expected. This
change only affects the case when ingress zone target == default.

If users desire the old behavior it can be re-added via a policy.

  # firewall-cmd --permanent --new-policy masqueradePolicy
  # firewall-cmd --permanent --policy masqueradePolicy --add-ingress-zone public
  # firewall-cmd --permanent --policy masqueradePolicy --add-egress-zone external
  # firewall-cmd --permanent --policy masqueradePolicy --add-masquerade

or for tcp-mss-clamp,

  # firewall-cmd --permanent --new-policy clampTcpMss
  # firewall-cmd --permanent --policy clampTcpMss --add-ingress-zone public
  # firewall-cmd --permanent --policy clampTcpMss --add-egress-zone external
  # firewall-cmd --permanent --policy clampTcpMss --add-rich-rule='rule tcp-mss-clamp'

Fixes: firewalld#177
  • Loading branch information
@erig0
erig0 committed Mar 3, 2021
1 parent d1a0f2d commit f2896e4
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 7 deletions.
11 changes: 8 additions & 3 deletions src/firewall/core/ipXtables.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from firewall.errors import FirewallError, INVALID_PASSTHROUGH, INVALID_RULE, UNKNOWN_ERROR, INVALID_ADDR
from firewall.core.rich import Rich_Accept, Rich_Reject, Rich_Drop, Rich_Mark, \
Rich_Masquerade, Rich_ForwardPort, Rich_IcmpBlock, Rich_Tcp_Mss_Clamp
from firewall.core.base import DEFAULT_ZONE_TARGET
import string

POLICY_CHAIN_PREFIX = ""
Expand Down Expand Up @@ -972,7 +973,7 @@ def build_policy_chain_rules(self, enable, policy, table, chain):

if self._fw.get_log_denied() != "off":
if table == "filter":
if target in [ "REJECT", "%%REJECT%%" ]:
if target in [DEFAULT_ZONE_TARGET, "REJECT", "%%REJECT%%" ]:
rules.append([ add_del_rule, _policy, "-t", table, "%%LOGTYPE%%",
"-j", "LOG", "--log-prefix",
"\"%s_REJECT: \"" % _policy ])
Expand All @@ -982,8 +983,12 @@ def build_policy_chain_rules(self, enable, policy, table, chain):
"\"%s_DROP: \"" % _policy ])

if table == "filter" and \
target in [ "ACCEPT", "REJECT", "%%REJECT%%", "DROP" ]:
rules.append([ add_del_rule, _policy, "-t", table, "-j", target ])
target in [DEFAULT_ZONE_TARGET, "ACCEPT", "REJECT", "%%REJECT%%", "DROP" ]:
if target in [DEFAULT_ZONE_TARGET]:
_target = "REJECT"
else:
_target = target
rules.append([ add_del_rule, _policy, "-t", table, "-j", _target ])

if not enable:
rules.reverse()
Expand Down
9 changes: 5 additions & 4 deletions src/firewall/core/nftables.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from firewall.core.rich import Rich_Accept, Rich_Reject, Rich_Drop, Rich_Mark, \
Rich_Masquerade, Rich_ForwardPort, Rich_IcmpBlock, \
Rich_Tcp_Mss_Clamp
from firewall.core.base import DEFAULT_ZONE_TARGET
from nftables.nftables import Nftables

TABLE_NAME = "firewalld"
Expand Down Expand Up @@ -890,9 +891,9 @@ def build_policy_chain_rules(self, enable, policy, table, chain):

if self._fw.get_log_denied() != "off":
if table == "filter":
if target in ["REJECT", "%%REJECT%%", "DROP"]:
if target in [DEFAULT_ZONE_TARGET, "REJECT", "%%REJECT%%", "DROP"]:
log_suffix = target
if target == "%%REJECT%%":
if target in [DEFAULT_ZONE_TARGET, "%%REJECT%%"]:
log_suffix = "REJECT"
rules.append({add_del: {"rule": {"family": "inet",
"table": TABLE_NAME,
Expand All @@ -901,8 +902,8 @@ def build_policy_chain_rules(self, enable, policy, table, chain):
{"log": {"prefix": "\"filter_%s_%s: \"" % (_policy, log_suffix)}}]}}})

if table == "filter" and \
target in ["ACCEPT", "REJECT", "%%REJECT%%", "DROP"]:
if target in ["%%REJECT%%", "REJECT"]:
target in [DEFAULT_ZONE_TARGET, "ACCEPT", "REJECT", "%%REJECT%%", "DROP"]:
if target in [DEFAULT_ZONE_TARGET, "%%REJECT%%", "REJECT"]:
target_fragment = self._reject_fragment()
else:
target_fragment = {target.lower(): None}
Expand Down
6 changes: 6 additions & 0 deletions src/tests/cli/firewall-cmd.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1381,6 +1381,7 @@ FWD_START_TEST([rich rules priority])
jump filter_IN_public_allow
jump filter_IN_public_post
meta l4proto { icmp, ipv6-icmp } accept
reject with icmpx type admin-prohibited
}
}
])
Expand All @@ -1393,6 +1394,7 @@ FWD_START_TEST([rich rules priority])
jump filter_FWDI_public_allow
jump filter_FWDI_public_post
meta l4proto { icmp, ipv6-icmp } accept
reject with icmpx type admin-prohibited
}
}
])
Expand All @@ -1403,6 +1405,7 @@ FWD_START_TEST([rich rules priority])
IN_public_allow all -- 0.0.0.0/0 0.0.0.0/0
IN_public_post all -- 0.0.0.0/0 0.0.0.0/0
ACCEPT icmp -- 0.0.0.0/0 0.0.0.0/0
REJECT all -- 0.0.0.0/0 0.0.0.0/0 reject-with icmp-port-unreachable
])
IPTABLES_LIST_RULES([filter], [FWDI_public], 0, [dnl
FWDI_public_pre all -- 0.0.0.0/0 0.0.0.0/0
Expand All @@ -1411,6 +1414,7 @@ FWD_START_TEST([rich rules priority])
FWDI_public_allow all -- 0.0.0.0/0 0.0.0.0/0
FWDI_public_post all -- 0.0.0.0/0 0.0.0.0/0
ACCEPT icmp -- 0.0.0.0/0 0.0.0.0/0
REJECT all -- 0.0.0.0/0 0.0.0.0/0 reject-with icmp-port-unreachable
])
IP6TABLES_LIST_RULES([filter], [IN_public], 0, [dnl
IN_public_pre all ::/0 ::/0
Expand All @@ -1419,6 +1423,7 @@ FWD_START_TEST([rich rules priority])
IN_public_allow all ::/0 ::/0
IN_public_post all ::/0 ::/0
ACCEPT icmpv6 ::/0 ::/0
REJECT all ::/0 ::/0 reject-with icmp6-port-unreachable
])
IP6TABLES_LIST_RULES([filter], [FWDI_public], 0, [dnl
FWDI_public_pre all ::/0 ::/0
Expand All @@ -1427,6 +1432,7 @@ FWD_START_TEST([rich rules priority])
FWDI_public_allow all ::/0 ::/0
FWDI_public_post all ::/0 ::/0
ACCEPT icmpv6 ::/0 ::/0
REJECT all ::/0 ::/0 reject-with icmp6-port-unreachable
])

dnl priority 0 (or not specified) is special:
Expand Down

0 comments on commit f2896e4

Please sign in to comment.