The global chain contains INPUT specific rules

[LionelCons]

The current global chain is documented as "something we want for all" and it is used in the 3 master chains (INPUT, OUTPUT and FORWARD).

However, it contains rules that are specific to the INPUT chain, for instance:

ip protocol icmp icmp type echo-request limit rate 4/second accept

It does not really make sense to rate limit the echo request in the OUTPUT or FORWARD chain. So this rule is more for the INPUT chain.

IMHO, only the rules which are always valid for all the master chains should end up in the global chain. And I wonder whether state tracking (ct state established,related accept) makes sense in the FORWARD chain...

In any case, ICMP handling (both as input and as output) should IMHO be moved into specific rules like we have for most protocols.

[LionelCons]

BTW, the default ICMP configuration is somehow broken with echo request allowed both in and out but no echo reply allowed in either direction. So ping does not work (in either direction).

One more reason to move the ICMP rules away from the global chain and let users define which kind of ICMP traffic they want to allow.

[duritong]

yeah sounds good to me

[nbarrientos]

I'd suggest to have the ICMPv4/v6 packet types allowed for each chain as parameters of nftables, something like:

class nftables (
  Array[String] $in_icmp6
  Array[String] $out_icmp6
    ...
  ]

with perhaps default lists of ICMPv4/ICMPv6 packets to allow in each direction or simply a default that accepts all ICMP traffic. Then use these parameters to add individual rules in inet_filter.

If I understand the proposal correctly the global chain will be reduced to just:

        chain global {
                ct state invalid drop
        }

as ct state established,related accept will be moved to INPUT and OUTPUT. Comments?

@LionelCons Perhaps if @duritong and @keachi like the proposal you could produce a proposal for the default values of

• nftables::in_icmpv6
• nftables::in_icmpv4
• nftables::out_icmpv6
• nftables::out_icmpv4

[nbarrientos]

If I understand the proposal correctly the global chain will be reduced to just:

Not really because in FORWARD perhaps it makes no sense to have any rules involving conntrack.

So, remove global?

[duritong]

or just keep it empty.

[LionelCons]

It could indeed be handy to have a common chain shared by all the master chains (BTW, should it be called common rather than global?).

But this common chain should be empty by default.

The ICMP rules should end up in separate files and the connection tracking should be added to the INPUT and OUTPUT chains, probably using variables like we have already for most of the rules.

For maximum flexibility, it should be possible to get a configuration with no rules at all by changing a few variables.

[LionelCons]

I can confirm that the conntrack rules moved out of the global chain work fine.

As for the default ICMP rules, this is really a matter of taste. I am in favour of a very limited set of rules but others prefer to allow everything, only limiting the rate. See for instance https://wiki.nftables.org/wiki-nftables/index.php/Simple_ruleset_for_a_server.

[nbarrientos]

@LionelCons could you please share with us your preferred defaults for ICMP4/6 in/out traffic so we can use them as a base to start discussing?

[LionelCons]

Here is what I allow for the incoming traffic:

• IPv4: echo-request, echo-reply, time-exceeded, destination-unreachable
• IPv6: IPv4 + neighbour-advertisement, neighbour-solicitation, router-advertisement

This mainly allows ping and traceroute to work.

I do not filter the outgoing traffic.

[LionelCons]

The main question is: what do we want in the API?

nftables::in_icmpv4 and nftables::out_icmpv4 or nftables::in_icmp_ping4 and nftables::out_icmp_ping4?

An incoming ping allowed means echo-request allowed in and echo-reply allowed out...

[nbarrientos]

Ok, thanks. I'll check the RHEL8 defaults (as suggested by @Feandil) and come back with a proposal in code.

[nbarrientos]

If I'm not reading it wrong, the RHEL8 default (checked on a 8.2 image) is:

• In
  • ICMPv6: All accepted (no rate limit for any type)
  • ICMPv4: Al accepted (no rate limit for any type)
• Out: All traffic accepted so no ICMP-specific rules apply.

RHEL8.2 rules: http://cern.ch/stikked/view/70c7ee72

@keachi @duritong, comments?

[nbarrientos]

There's a proposal in code available.

[nbarrientos]

@LionelCons For info the proposed patch allows setting your configuration by doing:

nftables::rules::icmp::v4_types:
  - "echo-request limit rate 4/second"
  - "echo-reply"
  - "time-exceeded"
  - "destination-unreachable"
nftables::rules::icmp::v6_types:
  - "echo-request limit rate 4/second"
  - "echo-reply"
  - "time-exceeded"
  - "destination-unreachable"
  - "nd-neighbor-advert"
  - "nd-neighbor-solicit"
  - "nd-router-advert"

leading to this ruleset:

...
        chain default_in {
                ip protocol icmp icmp type destination-unreachable accept
                ip protocol icmp icmp type echo-reply accept
                ip protocol icmp icmp type echo-request limit rate 4/second accept
                ip protocol icmp icmp type time-exceeded accept
                ip6 nexthdr ipv6-icmp icmpv6 type destination-unreachable accept
                ip6 nexthdr ipv6-icmp icmpv6 type echo-reply accept
                ip6 nexthdr ipv6-icmp icmpv6 type echo-request limit rate 4/second accept
                ip6 nexthdr ipv6-icmp icmpv6 type nd-neighbor-advert accept
                ip6 nexthdr ipv6-icmp icmpv6 type nd-neighbor-solicit accept
                ip6 nexthdr ipv6-icmp icmpv6 type nd-router-advert accept
                ip6 nexthdr ipv6-icmp icmpv6 type time-exceeded accept
...

[LionelCons]

Very good!

Note that it seems that one could also write:

nftables::rules::icmp::v4_types:
  - "{ destination-unreachable, time-exceeded, parameter-problem }"

I don't know if this is intended but this allows a more compact set of rules...

[keachi]

I really appreciate this idea of exposing the icmp types to class parameters.
echo-reply is allowed by conntrack with established,related. I never opened echo-reply but echo-request works fine.

[nbarrientos]

Great, thanks :) I'll remove the draft flag once #15 is merged (need that patch to run the test suite).