V-delete-firewall-ban very slow because of looping at fail2ban

Community Support Firewall
1

v-delete-firewall-ban

There is a code block

for f2bjail in $(fail2ban-client status ...); do
    if fail2ban-client get $f2bjail actions | grep -Eq "^hestia-$chain"; then
        fail2ban-client set $f2bjail unbanip <ip>    # ← recursion
    fi
done

which makes unblock IP very slow and timeout when run from fail2ban (while CLI and webui is ok to do its job to clear the ban)

Cause:
PR #5139:
loop: v-delete-firewall-ban - fail2ban itself runs the script, that loop re-enters fail2ban → the deadlock

2

Also inside this loop block:

for f2bjail in $("$f2bcli" status | grep -i 'Jail list:' | cut -d':' -f2- | sed -E 's/\s*//g' | tr ',' ' '); do
		if "$f2bcli" get "$f2bjail" actions | grep -Eq "^hestia-$chain" 2> /dev/null; then
			"$f2bcli" set "$f2bjail" unbanip "$sip" &> /dev/null
		fi
	done

$ipv4_cidr should be used instead of $sip? because $sip is an escaped value only applicable to sed line but should not be used in fail2ban unban action

3

Devs, pelase review the change below to see if correctly fix this issue.

v-delete-firewall-ban

#!/bin/bash
# info: delete firewall blocking rule
# options: IPV4_CIDR CHAIN
#
# example: v-delete-firewall-ban 198.11.130.250 MAIL
#
# This function deletes blocking rule from system firewall

#----------------------------------------------------------#
#                Variables & Functions                     #
#----------------------------------------------------------#

# Argument definition
ipv4_cidr=$1
chain=$(echo "$2" | tr '[:lower:]' '[:upper:]')

# Defining absolute path for iptables and fail2ban-client
iptables="/sbin/iptables"
f2bcli="/usr/bin/fail2ban-client"

# Includes
# shellcheck source=/etc/hestiacp/hestia.conf
source /etc/hestiacp/hestia.conf
# shellcheck source=/usr/local/hestia/func/main.sh
source $HESTIA/func/main.sh
# shellcheck source=/usr/local/hestia/func/firewall.sh
source $HESTIA/func/firewall.sh
# load config file
source_conf "$HESTIA/conf/hestia.conf"

#----------------------------------------------------------#
#                    Verifications                         #
#----------------------------------------------------------#

check_args '2' "$#" 'IPV4_CIDR CHAIN'
is_format_valid 'ipv4_cidr' 'chain'
is_system_enabled "$FIREWALL_SYSTEM" 'FIREWALL_SYSTEM'

# Perform verification if read-only mode is enabled
check_hestia_demo_mode

#----------------------------------------------------------#
#                       Action                             #
#----------------------------------------------------------#

# Self heal iptables links
heal_iptables_links

conf="$HESTIA/data/firewall/banlist.conf"
if [ "$chain" == "ALL" ]; then
	check_ip=$(grep "IP='$ipv4_cidr' CHAIN='*'" $conf)
	if [ -z "$check_ip" ]; then
		exit
	fi
	grep "IP='$ipv4_cidr' CHAIN='*'" $conf | while read -r line; do
		parse_object_kv_list $line

		# Deleting ip from banlist
		sip=$(echo "$IP" | sed "s|/|\\\/|g")
		sed -i "/IP='$sip' CHAIN='$CHAIN'/d" $conf
		b=$($iptables -L fail2ban-$CHAIN --line-number -n | grep -w $ipv4_cidr | awk '{print $1}')
		$iptables -D fail2ban-$CHAIN $b 2> /dev/null
		# --- [PATCH] skip fail2ban sync when invoked by fail2ban (prevents recursive deadlock)
		if [ -z "${HESTIA_F2B_CALLER:-}" ]; then
			for f2bjail in $("$f2bcli" status | grep -i 'Jail list:' | cut -d':' -f2- | sed -E 's/\s*//g' | tr ',' ' '); do
				if "$f2bcli" get "$f2bjail" actions | grep -Eq "^hestia-$CHAIN" 2> /dev/null; then
					# --- [PATCH] pass raw IP (not sed-escaped $sip) so CIDR bans sync correctly
					"$f2bcli" set "$f2bjail" unbanip "$IP" &> /dev/null
				fi
			done
		fi
	done
else
	# Checking ip in banlist
	check_ip=$(grep "IP='$ipv4_cidr' CHAIN='$chain'" $conf 2> /dev/null)
	if [ -z "$check_ip" ]; then
		exit
	fi

	# Deleting ip from banlist
	sip=$(echo "$ipv4_cidr" | sed "s|/|\\\/|g")
	sed -i "/IP='$sip' CHAIN='$chain'/d" $conf
	b=$($iptables -L fail2ban-$chain --line-number -n | grep -w $ipv4_cidr | awk '{print $1}')
	$iptables -D fail2ban-$chain $b 2> /dev/null
	# --- [PATCH] skip fail2ban sync when invoked by fail2ban (prevents recursive deadlock)
	if [ -z "${HESTIA_F2B_CALLER:-}" ]; then
		for f2bjail in $("$f2bcli" status | grep -i 'Jail list:' | cut -d':' -f2- | sed -E 's/\s*//g' | tr ',' ' '); do
			if "$f2bcli" get "$f2bjail" actions | grep -Eq "^hestia-$chain" 2> /dev/null; then
				# --- [PATCH] pass raw IP (not sed-escaped $sip) so CIDR bans sync correctly
				"$f2bcli" set "$f2bjail" unbanip "$ipv4_cidr" &> /dev/null
			fi
		done
	fi

fi

# Changing permissions
chmod 660 $conf

#----------------------------------------------------------#
#                       Hestia                             #
#----------------------------------------------------------#

# Logging
$BIN/v-log-action "system" "Info" "Firewall" "Removed IP from ban list (IP: $ipv4_cidr, Service: $chain)."
log_event "$OK" "$ARGUMENTS"

exit

/etc/fail2ban/action.d/hestia.conf

[Init]
timeout = 30

[Definition]

actionstart = flock -w 30 /run/f2b-hestia.lock /usr/local/hestia/bin/v-add-firewall-chain <name>
actionstop  = flock -w 30 /run/f2b-hestia.lock /usr/local/hestia/bin/v-delete-firewall-chain <name>
actionban   = flock -w 30 /run/f2b-hestia.lock /usr/local/hestia/bin/v-add-firewall-ban <ip> <name>
actionunban = flock -w 30 /run/f2b-hestia.lock env HESTIA_F2B_CALLER=1 /usr/local/hestia/bin/v-delete-firewall-ban <ip> <name>
4

If you have Github please create a PR against the main branch.

5

just want to seek for dev team review first, see if i overlooked anything.

And as I know, some .sh should also be amended for upgrade script? which I am not sure how

6

It works slowly for me too.

7

I am testing with my revised code code. It just add a IF wrapping the loop, and also fix the escaped CIDR fixing. Then apply the guard on hestia.local at fail2ban action.d folder to merge to exisitng one

Do a compare with the original and it’s a simple fix.

But I am not sure if there are anything i overlook, so seeking for devs to review. Also I dont know how to add those upgrade sh script as well

8

Ask @eris @sahsanu

They will do it, I think.

9

I’ll take a look to understand what this does and how it works. Once validated, you can open a PR, and if any changes are required during the Hestia upgrade, I’ll add a commit to take care of them.

Basically, the process is to create or modify the upgrade script for the next Hestia version, for example, install/upgrade/versions/1.10.5.sh, and add the required changes there. This could include checking whether Fail2Ban is installed, modifying the actions, restarting or reloading the service, etc.

10

I’ve been checking it, and it looks good so far (I need to do more tests), but as I said, it looks good.

Regarding the actions, I would keep the Hestia comment, use the full paths for the flock and env commands, and remove the init section. You should not add a 30 seconds timeout to Fail2Ban if flock already has a 30 seconds timeout. As I said, I would remove the init section, and Fail2Ban will use the default timeout of 60 seconds.

# Fail2Ban configuration file for hestia

[Definition]

actionstart = /usr/bin/flock -w 30 /run/f2b-hestia.lock /usr/local/hestia/bin/v-add-firewall-chain <name>
actionstop  = /usr/bin/flock -w 30 /run/f2b-hestia.lock /usr/local/hestia/bin/v-delete-firewall-chain <name>
actionban   = /usr/bin/flock -w 30 /run/f2b-hestia.lock /usr/local/hestia/bin/v-add-firewall-ban <ip> <name>
actionunban = /usr/bin/flock -w 30 /run/f2b-hestia.lock /usr/bin/env HESTIA_F2B_CALLER=1 /usr/local/hestia/bin/v-delete-firewall-ban <ip> <name>

Create the PR, and we can continue the conversation on GitHub. Thank you very much! :wink:

11

Thanks @sahsanu for reviewing. During the study, v-update-firewall is also risky when being triggered at the same time with ban/unban actions. I’ve put the fix together with the above (also changed the lock file name to cover the whole finding better)

Please review at Implement locking mechanism for Fail2Ban actions and firewall state + raw IP CIDR fix - Pull Request #5699 - hestiacp/hestiacp - GitHub