Draft PR: https://github.com/vyos/vyos-1x/pull/1033

Would appreciate any help testing this before marking the PR ready for merge.

@sdev: in your original commit for this task, recent rules are somehow semi-discarded (the time/counter condition will not be written out; however, the action will be written out) because of an apparent problem with nftables in this area.

My question here is how to deal with this: would it be possible (and desirable) to flag rules with recent conditions and discard them *entirely* (while issuing a warning) until a fix for this part of the code exists?

Presently, it's not clear to a user that recent won't work and will most likely even break a “typical” setup, for example by blocking all traffic matching the remaining conditions. In my opinion, asking to change all rule sets with recent conditions is impractical for something that ought to work anyway.

Background:

In python/vyos/firewall.py on L160-L186 one finds the following code & comment (abridged):

if 'recent' in rule_conf:
    count = rule_conf['recent']['count']
    time = rule_conf['recent']['time']
    # output.append(f'meter {fw_name}_{rule_id} {{ ip saddr and 255.255.255.255 limit rate over {count}/{time} burst {count} packets }}')
    # Waiting on input from nftables developers due to
    # bug with above line and atomic chain flushing.

# [...]

if 'set' in rule_conf:
    output.append(parse_policy_set(rule_conf['set'], def_suffix))

if 'action' in rule_conf:
    output.append(nft_action(rule_conf['action']))
else:
    output.append('return')

output.append(f'comment "{fw_name}-{rule_id}"')

This currently leads to a situation where a rule set like the following will lead to a complete drop of *any* packet that matches the remaining conditions (e.g. port/state in the example) because the recent counter / meter will be missing in the rule set.

set firewall name OUTSIDE-LOCAL rule 30 action 'drop'
set firewall name OUTSIDE-LOCAL rule 30 destination port '22'
set firewall name OUTSIDE-LOCAL rule 30 protocol 'tcp'
set firewall name OUTSIDE-LOCAL rule 30 recent count '4'
set firewall name OUTSIDE-LOCAL rule 30 recent time '60'
set firewall name OUTSIDE-LOCAL rule 30 state new 'enable'

Obviously, this breaks many existing rule sets with recent on one hand. But on the other, it's not a bug in VyOS (technically speaking) but a limitation of nftables for which you are waiting on input from the nftables project to solve / work-around it.

@johannrichard Hey sorry I didn't see your comment, I suggest we move the discussion to the dedicated task: https://phabricator.vyos.net/T4209

But you're right, we do need to figure this out asap.