nftables + multiport is broken

[hobbes1069]

I'm not sure if this has been discussed before but I could not find anything related by searching through the issues.

The default allports setting in jail.conf is:

port = 0:65535

This works fine for iptables but nftables does not accept ":" for port ranges and only accepts "-". Obviously I can easily work around this in the packaging for Fedora/EPEL but should fail2ban handle this "automagically"?

https://bugzilla.redhat.com/show_bug.cgi?id=1850164

[sebres]

The default allports setting in jail.conf is:
port = 0:65535

Strictly speaking, this is not an allports setting (in sense of action), but default port setting for multiport actions (which are also default for all new jails).
Known allports-action have normally no reference for this parameter, and so should not use it at all.
So as different by multiport vs. allports (executed with current version of nftables-action):

$ fail2ban-client --dp | grep -P 'actionstart|65535'
     ...
-['set', 'test-jail', 'addaction', 'nftables-multiport']
+['set', 'test-jail', 'addaction', 'nftables-allports']
     ...
-    ['actionstart', "nft add table inet f2b-table\nnft -- add chain inet f2b-table f2b-chain \\{ type filter hook input priority -1 \\; \\}\nnft add set inet f2b-table <addr_set> \\{ type <addr_type>\\; \\}\nfor proto in $(echo 'tcp' | sed 's/,/ /g'); do\nnft add rule inet f2b-table f2b-chain $proto dport \\{ 0:65535 \\} <addr_family> saddr @<addr_set> reject\ndone"],
+    ['actionstart', 'nft add table inet f2b-table\nnft -- add chain inet f2b-table f2b-chain \\{ type filter hook input priority -1 \\; \\}\nnft add set inet f2b-table <addr_set> \\{ type <addr_type>\\; \\}\n\nnft add rule inet f2b-table f2b-chain meta l4proto \\{ tcp \\} <addr_family> saddr @<addr_set> reject\n'],
     ['port', '0:65535'],

As one can see, there is no port usage in nftables-allports case.

As for nftables-multiport action (or nftables[type=multiport] in new version) - well, basically it is not really intended for the usage like that.
Either one would set banaction = nftables-allports (or banaction = %(banaction_allports)s) for such jails.
But to do it suitable for some port ranges, given in old (iptables compatible) format with :, we could indeed replace : with - inside the nftables action for multiport type.
Just in case like this such port range is mostly set by user in jail.local as local configuration, so it can be given already as correct range for nftables.

[hobbes1069]

Ok, I'm still pretty new to fail2ban so I had to read that a few times over to understand it.

From an end user POV (and consumer of a distro package) the expectation is for it to just "work". My options for the packaging side of things is:

1. Patch jail.conf but then things would break if the end user preferred iptables over nftables.
2. Create a fail2ban-nftables subpackage which overrides the port settings in jail.conf and provide a jail.d/00-nftables.conf file.

However, before creating a new package that may be obsoleted by a fix in upstream I wanted to discuss here first.

[sebres]

I had to read that a few times over to understand it.

Well there are 3 simple possibilities the user have to define such jails (besides other settings), so for example as in this jail.local:

[DEFAULT]
banaction = nftables[type=multiport]
banaction_allports = nftables[type=allports]

# jail1, some new jail with port range (user must set anyway):
[jail1_some_port_range]
# banaction remans nftables[type=multiport]
port = 123-125,321-323
enabled = true

# jail2, some new jail with allports:
[jail2_allports]
banaction = %(banaction_allports)s
# port is irrelevant
enabled = true

# jail3, already available and it uses allports banaction (in stock jail.conf):
[pam-generic]
# nothing to specify (port is irrelevant, banaction is already allports)
enabled = true

the expectation is for it to just "work"

Sure, just in case of a new jail, the necessary settings are obvious and either the port(s) must be set, or the banaction must be set properly.

However, before creating a new package that may be obsoleted by a fix in upstream I wanted to discuss here first.

Yes, it is better. Perhaps we'd find some better solution (e. g. as already said, replace : with - inside the nftables action for multiport type).
For which jails you had an intention to change that (or did you mean the default setting)?
Because if not default and you meant some concrete (allports) jails, the simple fix would be to specify banaction = %(banaction_allports)s in these jails.

[hobbes1069]

Yes, as the default is Fedora 32 and up (and RHEL/CentOS 8 and up) is nftables, I'd like to change the default. Based on our discussion the simplest change is to patch jail.conf during the build process.

[sebres]

the simplest change is to patch jail.conf during the build process

wait a bit...
would try to provide simple fix (for the port) in nftables action... something like $(echo <port> | sed s/:/-/g) instead of <port>

[sebres]

First shot is implemented in 309c8dd
I think it is enough safe and simple, so I'll merge it hereafter to all major branches starting with 0.10.

[hobbes1069]

I'll give it a try. I'm having an issue with one of the tests (posted to the mailing list) that I need to figure out.

Thanks!

[sebres]

Closed in 62a6771