mirror of
https://github.com/mailcow/mailcow-dockerized.git
synced 2026-08-10 16:26:19 -04:00
fix(netfilter): prevent duplicate bans from orphaning firewall rules
ban() had no guard against re-issuing a firewall ban for a net that is already actively banned, and NFTables.banIPv4/6() had no dedup check before inserting (unlike IPTables.py, which already guards this). nftables itself does not dedup on insert, so any double-fire of ban() left a second, identical DROP rule in place. Since unbanIPv4/6() only ever removes one matching rule per call, that second rule became permanently orphaned -- found 3 of these in production. Adds a 'banned' flag to the in-memory bans dict, checked before re-issuing a ban and cleared on unban, plus a is_already_banned() check in NFTables.py so banIPv4/6() skip inserting when a matching rule already exists.
This commit is contained in:
parent
ac4326f2c0
commit
ee3bbbfdd2
2 changed files with 42 additions and 1 deletions
|
|
@ -147,7 +147,7 @@ def ban(address):
|
|||
logdebug("Ban net: %s" % net)
|
||||
|
||||
if not net in bans:
|
||||
bans[net] = {'attempts': 0, 'last_attempt': 0, 'ban_counter': 0}
|
||||
bans[net] = {'attempts': 0, 'last_attempt': 0, 'ban_counter': 0, 'banned': False}
|
||||
logdebug("Initing new ban counter for %s" % net)
|
||||
|
||||
current_attempt = time.time()
|
||||
|
|
@ -162,6 +162,9 @@ def ban(address):
|
|||
logdebug("%s attempts now %d" % (net, bans[net]['attempts']))
|
||||
|
||||
if bans[net]['attempts'] >= MAX_ATTEMPTS:
|
||||
if bans[net].get('banned'):
|
||||
logdebug("%s is already actively banned -- skipping duplicate ban()" % net)
|
||||
return
|
||||
cur_time = int(round(time.time()))
|
||||
NET_BAN_TIME = calcNetBanTime(bans[net]['ban_counter'])
|
||||
logger.logCrit('Banning %s for %d minutes' % (net, NET_BAN_TIME / 60 ))
|
||||
|
|
@ -174,6 +177,7 @@ def ban(address):
|
|||
logdebug("Calling tables.banIPv6(%s)" % net)
|
||||
tables.banIPv6(net)
|
||||
|
||||
bans[net]['banned'] = True
|
||||
logdebug("Updating F2B_ACTIVE_BANS[%s]=%d" %
|
||||
(net, cur_time + NET_BAN_TIME))
|
||||
r.hset('F2B_ACTIVE_BANS', '%s' % net, cur_time + NET_BAN_TIME)
|
||||
|
|
@ -228,6 +232,7 @@ def unban(net):
|
|||
logdebug("Unban for %s, setting attempts=0, ban_counter+=1" % net_str)
|
||||
bans[net]['attempts'] = 0
|
||||
bans[net]['ban_counter'] += 1
|
||||
bans[net]['banned'] = False
|
||||
|
||||
def safe_unban(net, reason=''):
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -100,10 +100,18 @@ class NFTables:
|
|||
self.logger.logInfo(f"Clear completed: {_family}")
|
||||
|
||||
def banIPv4(self, source):
|
||||
# nft insert rule has no dedup: a second ban() call for a net that's
|
||||
# already banned would add a second identical DROP rule, which unban()
|
||||
# (single-match delete) can never fully clear -- an orphaned rule.
|
||||
if self.is_already_banned(source, "ip"):
|
||||
return False
|
||||
ban_dict = self.get_ban_ip_dict(source, "ip")
|
||||
return self.nft_exec_dict(ban_dict)
|
||||
|
||||
def banIPv6(self, source):
|
||||
# See banIPv4() -- same duplicate-insert/orphaned-rule risk applies.
|
||||
if self.is_already_banned(source, "ip6"):
|
||||
return False
|
||||
ban_dict = self.get_ban_ip_dict(source, "ip6")
|
||||
return self.nft_exec_dict(ban_dict)
|
||||
|
||||
|
|
@ -437,6 +445,34 @@ class NFTables:
|
|||
|
||||
return json_command
|
||||
|
||||
def is_already_banned(self, ipaddr: str, _family: str):
|
||||
_chain_opts = {'family': _family, 'table': 'filter', 'name': self.chain_name}
|
||||
command = self.get_base_dict()
|
||||
command['nftables'].append({'list': {'chain': _chain_opts} })
|
||||
kernel_ruleset = self.nft_exec_dict(command)
|
||||
if not kernel_ruleset:
|
||||
return False
|
||||
|
||||
candidate_net = ipaddress.ip_network(ipaddr, strict=False)
|
||||
for _object in kernel_ruleset["nftables"]:
|
||||
if not _object.get("rule"):
|
||||
continue
|
||||
rule = _object["rule"]["expr"][0]["match"]
|
||||
if not "payload" in rule["left"]:
|
||||
continue
|
||||
left_opt = rule["left"]["payload"]
|
||||
if left_opt["protocol"] != _family or left_opt["field"] != "saddr":
|
||||
continue
|
||||
rule_right = rule["right"]
|
||||
if isinstance(rule_right, dict):
|
||||
current_rule_ip = rule_right["prefix"]["addr"] + '/' + str(rule_right["prefix"]["len"])
|
||||
else:
|
||||
current_rule_ip = rule_right
|
||||
if ipaddress.ip_network(current_rule_ip) == candidate_net:
|
||||
return True
|
||||
|
||||
return False
|
||||
|
||||
def get_unban_ip_dict(self, ipaddr:str, _family: str):
|
||||
json_command = self.get_base_dict()
|
||||
# Command: 'nft list chain {s_family} filter MAILCOW'
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue