Skip to content

[Feature] Add firewall backend selection (iptables/nftables/none) to Fail2ban/IP Limit installer#5905

Open
hermit-turtle wants to merge 8 commits into
MHSanaei:mainfrom
hermit-turtle:main
Open

[Feature] Add firewall backend selection (iptables/nftables/none) to Fail2ban/IP Limit installer#5905
hermit-turtle wants to merge 8 commits into
MHSanaei:mainfrom
hermit-turtle:main

Conversation

@hermit-turtle

Copy link
Copy Markdown

Summary

Added support for choosing the firewall backend for Fail2ban/IP Limit during installation: iptables (default), nftables, or none (skip). This makes the panel more compatible with modern systems while keeping full backward compatibility.

Why

  • Many modern distributions (especially Debian 12+, Ubuntu 22.04+, newer Fedora/RHEL) prefer or default to nftables.
  • Users on minimal or containerized environments often want to skip Fail2ban entirely or use nftables.
  • Previously the installer always forced nftables + iptables mix, which could cause confusion or unnecessary dependencies.
  • Improves user experience and reduces potential conflicts on reinstall/upgrade.

Type of change

  • New feature
  • Refactoring (no behavior change for default users)

Areas affected

  • Install / upgrade script
  • Docker image (indirectly — through install logic)

How was this tested?

  • Fresh installation on Ubuntu 22.04 and Debian 12 with all three options (iptables, nftables, none).
  • Re-installation (upgrade scenario) with different backend choices.
  • Verified that IP Limit works correctly with both backends.
  • Checked that skipping Fail2ban doesn't break panel startup.
  • Tested x-ui menu → IP Limit section after installation.

Breaking changes

None.
Default behavior remains exactly the same (iptables). Existing installations are not affected unless user explicitly chooses a different backend on reinstall.

Checklist

  • I tested the change locally and confirmed the described behavior.
  • I have no unrelated changes mixed into this PR.

@github-actions

Copy link
Copy Markdown
Contributor

Summary
This PR adds a firewall-backend choice (iptables/nftables/none) to the Fail2ban/IP Limit installer in install.sh and x-ui.sh. The feature idea is reasonable and the non-interactive/interactive prompt handling follows the existing conventions already used elsewhere in the script, but the implementation has several concrete, verifiable bugs: a function that is still called gets deleted, the new nftables action file contains an unfinished placeholder command, distro-specific package logic that a prior documented fix relied on was dropped (this affects the default iptables path too), and the chosen backend is never persisted so later operations silently fall back to iptables. This should not be merged as is.

Findings

Finding 1 - default "iptables" choice no longer installs nftables; EPEL and CentOS 7 handling dropped
Severity: Critical | Confidence: High | Category: correctness / regression
Location: x-ui.sh:2271-2297 (deleted by this diff, hunk @@ -2249,119 +2249,81 @@)
Problem: The rewritten setup_fail2ban_iplimit() only installs the nftables package when the user explicitly picks "nftables" (apt-get/dnf install -y fail2ban nftables); picking "iptables" (the documented default) now runs "... install -y fail2ban iptables" and never installs nftables. The comment deleted from the old x-ui.sh:2263-2270 explains that nftables was previously installed unconditionally because recent fail2ban packages default to banaction = nftables-multiport in jail.conf, and without nft in PATH the default sshd jail fails to ban with stderr "/bin/sh: 1: nft: not found", even though the 3x-ipl jail itself uses iptables. The same hunk also removes the EPEL-enablement logic for fedora | amzn | virtuozzo | rhel | almalinux | rocky | ol (old x-ui.sh:2290-2296) and merges centos into the same dnf-only branch as the rest of the RHEL family (old x-ui.sh:2298-2305), even though CentOS 7 does not ship dnf by default. The untouched remove_iplimit() uninstall path at x-ui.sh:2411-2418 still special-cases VERSION_ID =~ ^7 for exactly that reason.
Why it matters: fail2ban is EPEL-only on RHEL-family distros other than Fedora, so a fresh install with the default iptables choice on RHEL, AlmaLinux, Rocky, Amazon Linux, Virtuozzo, or OL will fail to install fail2ban at all, and on CentOS 7 it will fail with "dnf: command not found". On Debian, Ubuntu, and Fedora, the default choice reintroduces the previously-fixed "nft: not found" log spam for the default fail2ban jails. This directly contradicts the description text stating that default behavior remains exactly the same and existing installations are not affected unless the user explicitly chooses a different backend.
Recommendation: Keep installing nftables unconditionally regardless of the 3x-ipl action backend, and restore the EPEL-enablement and CentOS 7 yum/dnf branching before collapsing the distro cases.

Finding 2 - iplimit_remove_conflicts is called but its definition is deleted
Severity: High | Confidence: High | Category: correctness
Location: x-ui.sh:2331 (call site, unchanged and retained by this diff) and x-ui.sh:2557-2570 (definition, deleted by hunk @@ -2550,23 +2531,9 @@)
Problem: setup_fail2ban_iplimit() still calls iplimit_remove_conflicts (kept as an unchanged context line in the diff), but this diff deletes the entire iplimit_remove_conflicts() function and does not redefine it anywhere else in either file.
Why it matters: every run of setup_fail2ban_iplimit (fresh install, x-ui setup-fail2ban, or the interactive "Install Fail2ban and configure IP Limit" menu item) hits "iplimit_remove_conflicts: command not found". The script has no set -e, so execution continues, but the cleanup step described right above the call ("Remove old conflicting 3x-ipl configs, safe on reinstall") silently stops running, undermining the reinstall and backend-switch safety the description text claims was tested.
Recommendation: Restore the iplimit_remove_conflicts() function, or inline its jail.conf/jail.local cleanup at the call site.

Finding 3 - nftables action definition is incomplete and appears syntactically invalid
Severity: High | Confidence: High | Category: correctness
Location: x-ui.sh, new create_iplimit_jails() nftables branch (diff hunk @@ -2522,7 +2485,25 @@)
Problem: The generated /etc/fail2ban/action.d/3x-ipl.conf for the nftables backend defines:

actionunban = delete rule ip f2b- ip saddr ...

with a literal, unfinished "..." instead of real clauses, and only one line total (no UDP counterpart, unlike actionban which has two lines). Separately, actionstart/actionstop run " add chain ip f2b- { ... }" and " delete chain ip f2b-", omitting the table argument that nft add/delete chain

requires (no add table is issued anywhere in this action).
Why it matters: as written this is not valid input to nft. actionstart, and therefore the jail itself, is likely to fail to initialize, and actionunban will fail on the literal "..." every time. The nftables backend this PR advertises is very likely non-functional, meaning it will not actually ban or unban anyone, which conflicts with the description text claiming both backends were verified to work correctly.
Recommendation: Base the nftables action on the nftables*.conf templates shipped by fail2ban itself, which use a named set plus add element/delete element for ban and unban since nft cannot delete a rule by match spec, and exercise an actual ban/unban cycle with "nft list ruleset" before merging.

Finding 4 - chosen backend is not persisted; later operations silently revert to iptables
Severity: High | Confidence: High | Category: correctness
Location: x-ui.sh:2175 (create_iplimit_jails ${NUM}, unchanged call site in the "Change Ban Duration" option inside iplimit_main) and x-ui.sh:2371-2374 (install_iplimit)
Problem: create_iplimit_jails() now takes an optional backend argument defaulting to "iptables" (local backend="${2:-iptables}"), and the chosen backend only ever exists as the XUI_FAIL2BAN_BACKEND environment variable set during the initial install.sh run; it is never written to disk. The "Change Ban Duration" menu option calls create_iplimit_jails ${NUM} with a single argument, and the interactive "Install Fail2ban and configure IP Limit" menu item (install_iplimit calling setup_fail2ban_iplimit) runs in a fresh shell with no XUI_FAIL2BAN_BACKEND set.
Why it matters: a user who chose nftables at install time and later changes the ban duration from the panel menu, or simply re-runs Fail2ban setup from the menu, gets /etc/fail2ban/action.d/3x-ipl.conf silently regenerated for iptables, even though, per Finding 1, only fail2ban and nftables were installed and not iptables. The jail then references an iptables-based action against a binary that may not exist, silently breaking IP Limit enforcement with no warning.
Recommendation: Persist the chosen backend, for example a small state file under /etc/fail2ban/, or detect it by reading back the existing action.d/3x-ipl.conf, and have both call sites use it instead of relying on a defaulted, session-only environment variable.

Positive observations
The NONINTERACTIVE and XUI_FAIL2BAN_BACKEND prompt in install.sh correctly follows the same pattern already used for other prompts elsewhere in the script (gated on the same tty and NONINTERACTIVE check), and the SSH/panel-port exemption logic in create_iplimit_jails is preserved unchanged for both backends, avoiding a regression of a security-relevant detail (locking out SSH or the panel).

Verdict
Request changes - @MHSanaei. The feature is a reasonable idea, but the current implementation contains a deleted-yet-still-called function, an incomplete and likely non-functional nftables action definition, and distro package-install regressions that affect the default iptables path on several previously-supported distros, contradicting the backward-compatibility claim in the description. These need to be fixed before merge.

This review was generated automatically; a maintainer may follow up.

@hermit-turtle

Copy link
Copy Markdown
Author

FIX #1: x-ui.sh:2306-2309

Remove call to deleted function. Inline cleanup:

rm -f /etc/fail2ban/jail.d/3x-ipl.conf 2>/dev/null
rm -f /etc/fail2ban/action.d/3x-ipl.conf 2>/dev/null
rm -f /etc/fail2ban/filter.d/3x-ipl.conf 2>/dev/null

FIX #2: x-ui.sh:2272-2298

  • Always install nftables (not conditional)
  • Add EPEL-release for RHEL family
  • Use yum for CentOS 7 (not dnf)
  • Separate Fedora branch

FIX #3: x-ui.sh:2493-2507

Replace nftables action with named-set syntax:

actionban = nft add element ip f2b 3x-ipl { <ip> }
actionunban = nft delete element ip f2b 3x-ipl { <ip> }
actionstart = nft add table ip f2b ... | nft add set ... | nft add chain ... | nft add rule ...
actionstop = nft delete chain ... | nft delete set ... | nft delete table ...

FIX #4a: install.sh:~1360

After export XUI_FAIL2BAN_BACKEND:

mkdir -p /etc/x-ui
echo "$XUI_FAIL2BAN_BACKEND" > /etc/x-ui/fail2ban_backend
chmod 600 /etc/x-ui/fail2ban_backend

FIX #4b: x-ui.sh:~2314

After create_iplimit_jails 30 "$backend":

mkdir -p /etc/x-ui
echo "$backend" > /etc/x-ui/fail2ban_backend
chmod 600 /etc/x-ui/fail2ban_backend

FIX #4c: x-ui.sh:2450-2453

At start of create_iplimit_jails() after variable declarations:

if [[ $# -lt 2 && -f /etc/x-ui/fail2ban_backend ]]; then
    backend=$(cat /etc/x-ui/fail2ban_backend)
fi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant