Fail2ban can use legacy iptables

[dguerri]

On some appliance (e.g., QNAP NAS') nftables on docker is not
available.
This commit adds FAIL2BAN_LEGACY_IPTABLES environment variable to allow configuring fail2ban to use iptables-legacy.

Fixes #2661

Description

Fixes #

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Improvement (non-breaking change that does improve existing functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (README.md or the documentation under docs/)
• [N] If necessary I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[github-actions]

Documentation preview for this PR is ready! 🎉

Built with commit: 25e901e

[polarathene]

TL;DR:

• Defer contribution to our docs to share with other users (Dockerfile edits + user-patches.sh), rather than maintain legacy support via ENV. If more users express a need for this support, we can consider the ENV approach then.
• Problem is likely due to NAS products (even those released last year) shipping kernels that are known to be 5 years old. Customers should engage with the product vendors to resolve that.

---

Looking at the PR that swapped iptables to nftables for v11, the fail2ban debug setup.sh command for debugging had this iptables specific logic, which may be helpful context if your setup would run into the issue?

Otherwise this mostly looks like it covers the relevant changes from that PR, our tests don't however, and there was quite a bit of difference there in syntax.

I'm not sure if other maintainers would want to carry this legacy support even if equivalent test support was added. As the PR shows, you could extend the image to add the relevant Dockerfile lines, and use user-patches.sh script to apply the single sed edit to /etc/fail2ban/jail.local?

While your time spent looking into the support and creating a PR is appreciated, the contribution may be better served as docs only contribution for how to solve this problem. We're trying to avoid adding more niche-case (and especially legacy support related ones) features when possible, and since the required changes to support this are small, the other maintainers likely share this opinion to defer to docs only.

I do understand that's probably frustrating, since needing to extend the image is more bothersome than carrying around user-patches.sh alone and maintaining compatibility with upstream over time. I believe we're more open to accepting this support when there's enough demand expressed for it, otherwise the preference is to defer to documenting it for others until then.

---

I will note that the PR which switched us to nftables had a discussion stating a iptables host + nftables container worked fine, and presumably the other way around as we've had no complaints since.

So I'm not sure why that differs with your QNAP host? Another docker project has this issue too, the users affected are QNAP and DSM (both NAS products), where at the end it's said that at least DSM 7 (released June 2021) was found to be using an extremely old kernel (4.4, Jan 2016)... that is not encouraging, and probably applies to your QNAP system? You should probably raise a complaint as a customer if you're on a kernel that old as well..

[dguerri]

@polarathene everything you said totally makes sense to me.
I am happy to have this issue and its solution mentioned in the documentation.

For completeness, QNAP does use a recent (longterm supported) kernel. They seem to have intentionally opted for the legacy iptables, probably because of some of the software running on these appliances.

[/] # cat /etc/os-release
NAME="QTS"
VERSION="5.0.0 (20220531)"
ID=qts
PRETTY_NAME="QTS 5.0.0 (20220531)"
VERSION_ID="5.0.0"
[/] # cat /proc/version
Linux version 5.10.60-qnap (root@U16BuildServer172) (x86_64-QNAP-linux-gnu-gcc (toolchain config: [gcc-4.9.2 binutils-2.25 glibc-2.21]) 4.9.2, GNU ld (GNU Binutils) 2.25) #1 SMP Tue May 31 01:57:44 CST 2022
[/] # lsmod | grep table
iptable_nat            16384  2
nf_nat                 36864  5 ip_vs_ftp,xt_nat,xt_MASQUERADE,xt_REDIRECT,iptable_nat
[/] # ls /lib/modules/5.10.60-qnap/*table*
/lib/modules/5.10.60-qnap/ip6table_filter.ko  /lib/modules/5.10.60-qnap/ip6table_mangle.ko  /lib/modules/5.10.60-qnap/ip6_tables.ko  /lib/modules/5.10.60-qnap/iptable_nat.ko
[/] #

[dguerri]

@polarathene I commented on PR #2505. The author of that patch was probably using the new iptables drop-in replacement (which actually uses nftables)

[georglauterbach]

@dguerri was indeed right, my host was running the iptables version that under the hood already used nftables. Id I'm not mistaken, this had already begun shipping for some 4.x.x Linux kernels (Linux 3.13 introduced nftables).

Nevertheless, I agree with @polarathene in this case, i.e. this PR should become a documentation-only PR. I have a bit of experience with these NAS systems, and I know that they are not that nice to handle, but I think this is exactly the reason we should not provide extra support in our code base - it makes support and maintainability more difficult than it already is.

I'd be happy to see this PR being merged with new documentation (only) though! @dguerri could you update this PR, we will gladly help you with our docs if some questions come up :)

[dguerri]

@georglauterbach SGTM. Will do asap

[casperklein]

fyi: This should also be done to /etc/fail2ban/action.d/iptables-common.local if ${FAIL2BAN_LEGACY_IPTABLES} -eq 1.

[georglauterbach]

ping @dguerri

[dguerri]

Sorry guys, something came up and I don't have much time right now to reflector the PR.
Shall we close it and I will submit a documentation one as soon as I can?

[georglauterbach]

Sorry guys, something came up and I don't have much time right now to reflector the PR. Shall we close it and I will submit a documentation one as soon as I can?

Yes, this sounds good to me :) Feel free to open a new PR in the future :D