`set-only` mode causes crowdsec-firewall-bouncer to flood crowdsec-firewall-bouncer.log

[jarppiko]

Summary

If crowdsec-firewall-bouncer uses mode=nftables and set-only=true, metrics collection floods crowdsec-firewall-bouncer.log with lines like this every 10 seconds:

$ tail /var/log/crowdsec-firewall-bouncer.log
time="28-05-2023 13:36:43" level=error msg="can't collect dropped packets for ipv4 from nft: while running /usr/sbin/nft -j list chain ip filter crowdsec-chain-input: exit status 1"
time="28-05-2023 13:36:53" level=error msg="can't collect dropped packets for ipv4 from nft: while running /usr/sbin/nft -j list chain ip filter crowdsec-chain-input: exit status 1"
time="28-05-2023 13:37:03" level=error msg="can't collect dropped packets for ipv4 from nft: while running /usr/sbin/nft -j list chain ip filter crowdsec-chain-input: exit status 1"
time="28-05-2023 13:37:13" level=error msg="can't collect dropped packets for ipv4 from nft: while running /usr/sbin/nft -j list chain ip filter crowdsec-chain-input: exit status 1"
time="28-05-2023 13:37:23" level=error msg="can't collect dropped packets for ipv4 from nft: while running /usr/sbin/nft -j list chain ip filter crowdsec-chain-input: exit status 1"

set-only mode is intended for more complex firewall setups (e.g. in routers) where Crowdsec's the automated firewall rules cannot be used (e.g. there are multiple interfaces and crowdsec should control only some of those).

Root cause

This bug was probably introduced in #231 since the PR seemed to consider only "auto-mode" (set-only=false).

nftables.metrics.CollectMetrics() (line 118) does not handle set-only mode since it assumes there is a nftables chain with name chain-hook (where hook is prerouting, input, output, forward or ingress) to query from. In set-only mode crowdsec manages only blocked IPs in a named nftables set and the chain param is not used.

Workaround

It is not possible to disable metrics collection. Therefore, as a workaround an admin has to create/rename a chain according to an undocumented naming convention $nftables.ipv4.chain-$nftables_hooks.hook (referring to bouncer config file yaml fields here).

For example, if nftables config in bouncer config looks like this:

# /etc/crowdsec/bouncers/crowdsec-firewall-bouncer.yaml
...
nftables:
  ipv4:
    enabled: true
    set-only: true
    table: filter
    chain: existing-chain-without-its-input-postfix

nftables_hooks:
  - input

Then there needs to be a nftables chain in the right table with name : existing-chain-without-its-input-postfix-input.

Solution options

I could come up with couple of options how to solve this issue.

1. Add disable_metrics option
2. Disable metrics collection in set-only mode automatically
3. Add support for metrics collection in set-only mode

Technical considerations

1. Add disable_metrics bouncer config option

This would require admin to manually fix the issue.

Commit 9619b01 introduced this functionality, but in the same PR #282 the commit was later reversed (d17dafa). Maybe @mmetc knows why the change was reversed.

2. Disable metrics collection in set-only mode

This can be done by editing one line in collectDropped() in nftables/metrics.go.

func (c *nftContext) collectDropped(path string, hooks []string) (int, int, int) {
	if c.conn == nil || c.setOnly {     // "|| c.setOnly" added
		return 0, 0, 0
	}
        ...

3. Add support for metrics collection in set-only mode

This should not be a big change.

• Modify collectDroppedPackets() in metrics.go to take chain name directly instead of doing string concatenation magic within the function.
• Modify collectDropped() in metrics.go to handle set-only=true mode:
  • set-only=false: create chain name here with the string concatenation magic
  • set-only=true: use chain name directly from bouncer config

Drawback

A drawback of this implementation is that it would collect metrics only from one chain.

Collecting metrics from all chains / hooks in set-only mode would require a way to tell the bouncer what chains are being used. If nftables_hooks config option could be changed to contain actual chain names and the string concatenation could be scrapped, then the nftables_hooks config option could be used also in set-only mode to configure metrics collection right.

Disclaimer: I am the author of set-only mode

[mmetc]

Commit 9619b01 introduced this functionality,
but in the same PR #282 the commit was later reversed (d17dafa).
Maybe @mmetc knows why the change was reversed.

There was never a "disable_metrics" option. The metrics depend on the "nft -j" command being available. The absence of the "-j" option is not considered a transient error (command nft is older than we need), and metrics are disabled to avoid spamming the logs.

We should probably extend or generalize this behavior.

[mmetc]

Merged. Thanks!

[jarppiko]

Thank you so much 🎉 This is my 2nd merged PR into Crowdsec project 🎂

I think the bouncer config file would benefit of some harmonization between the modes (automatic/normal, hooks, set-only). It's not a major issue, but then all the features could be available to each of the mode. Changing config file formats will always cause deployment issues and are a huge headache. Therefore those should be thought well ahead instead of feature-by-feature. If this is something Crowdsec team is interested, I can try to chip in a proposal and let's see where does it go.

[mmetc]

I think the bouncer config file would benefit of some harmonization between the modes (automatic/normal, hooks, set-only). It's not a major issue, but then all the features could be available to each of the mode. Changing config file formats will always cause deployment issues and are a huge headache. Therefore those should be thought well ahead instead of feature-by-feature. If this is something Crowdsec team is interested, I can try to chip in a proposal and let's see where does it go.

We have 5 official bouncers written in go and some from third parties, they are undergoing a refactoring to use common methods for build/CI/packaging/configuration etc. We are definitely interested in proposals to improve the user experience but the keywords are "backward compatibility" and "least surprise" o.O

[mstilkerich]

Just in case someone else still stumbles across this while using the version 0.25 packaged in Debian 12: There are a couple of further constraints with version 0.25 of the bouncer to work around the log being flooded with errors:

• The configured table will be honored when looking for the chain, however, it must be an ip/ip6 type table for IPv4/IPv6. A table of type inet will not work.
• Version 0.25 does not yet append the hook type as described above to the chain name. It takes the chain name as configured with the chain setting. The magic appending of the hook type only happens starting with version 0.26 of the bouncer, where Nftables: Add configurable hooks #231 was added to allow defining several hooks for nftables, whereas with 0.25 we are still limited to the input hook only.

In the end, I ended up creating separate tables for crowdsec essentially the same as recent versions of crowdsec-firewall-bouncer create in managed mode, as my own rules live in a table of type inet which I like to keep that way.

table ip crowdsec {
    set crowdsec-blacklists {
        type ipv4_addr
        flags timeout
        comment "dynamically managed by crowdsec; includes blacklisted IPv4"
    }

    # common chain for input and forward hooks so metric collection accumulates dropped packets in both hooks
    chain crowdsec-chain {
        ip saddr @crowdsec-blacklists counter drop
    }

    chain crowdsec-chain-input {
        type filter hook input priority filter; policy accept;
        goto crowdsec-chain
    }

    chain crowdsec-chain-forward {
        type filter hook forward priority filter; policy accept;
        goto crowdsec-chain
    }
}

table ip6 crowdsec6 {
    set crowdsec6-blacklists {
        type ipv6_addr
        flags timeout
        comment "dynamically managed by crowdsec; includes blacklisted IPv6"
    }

    # common chain for input and forward hooks so metric collection accumulates dropped packets in both hooks
    chain crowdsec6-chain {
        ip6 saddr @crowdsec6-blacklists counter drop
    }

    chain crowdsec6-chain-input {
        type filter hook input priority filter; policy accept;
        goto crowdsec6-chain
    }

    chain crowdsec6-chain-forward {
        type filter hook forward priority filter; policy accept;
        goto crowdsec6-chain
    }
}

This works with 0.25 together with the following config overrides in /etc/crowdsec/bouncers/crowdsec-firewall-bouncer.yaml.local:

nftables:
  ipv4:
    set-only: true
  ipv6:
    set-only: true

Since the nftables objects are all named according to the standard naming of crowdsec-firewall-bouncer (as defined in /etc/crowdsec/bouncers/crowdsec-firewall-bouncer.yaml), no further configuration is needed in the bouncer.

Maybe it's useful to someone else.