Firewall improvements/suggestions/discussion

[primeos]

Hey, I'd like to get some feedback on the following suggestions - thanks :)

Suggestion: Use allowPing for both IPv4 and IPv6

networking.firewall.allowPing = mkOption {
  type = types.bool;
  default = true;
  description =
    ''
      Whether to respond to incoming ICMPv4 echo requests
      ("pings").  ICMPv6 pings are always allowed because the
      larger address space of IPv6 makes network scanning much
      less effective.
    '';
};

Even without considering the scanning I would suggest we use this option for IPv6 as well or rename it to something like networking.firewall.allowIPv4Ping.

Regarding the scanning of the larger address space of IPv6: This isn't necessarily true since one can do way better than just brute-forcing a /64 subnet (e.g. via DNS, patterns in the interface identifiers). Some references:

• http://www.internetsociety.org/deploy360/blog/2015/02/ipv6-security-myth-4-ipv6-networks-are-too-big-to-scan/
• https://tools.ietf.org/html/rfc7707#section-6
• https://media.ccc.de/v/33c3-8061-you_can_-j_reject_but_you_can_not_hide_global_scanning_of_the_ipv6_internet

Suggestion: Use -w (wait for the xtables lock) everywhere

helpers =
  ''
    # Helper command to manipulate both the IPv4 and IPv6 tables.
    ip46tables() {
      iptables -w "$@"
      ${optionalString config.networking.enableIPv6 ''
        ip6tables -w "$@"
      ''}
    }
  '';

This covers most of the commands but there are iptables and ip6tables commands in the module where that -w option is missing. And users probably won't use that option as well (additionalCommands).

Not using that option could cause some problems (race conditions):

• https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=816087

Suggestion: Properly reset the firewall

# Flush the old firewall rules.  !!! Ideally, updating the
# firewall would be atomic.  Apparently that's possible
# with iptables-restore.
ip46tables -D INPUT -j nixos-fw 2> /dev/null || true
for chain in nixos-fw nixos-fw-accept nixos-fw-log-refuse nixos-fw-refuse; do
  ip46tables -F "$chain" 2> /dev/null || true
  ip46tables -X "$chain" 2> /dev/null || true
done

This method keeps quite some state (e.g. policies, additional chains/tables) which shouldn't be the case imho (this is a functional OS at least :).

Imho we should completely reset the firewall (also in the firewall-stop script and deprecate extraStopCommands). This could be done atomically with iptables-restore which could help us improving firewall-reload as well.

Currently firewall-reload uses an additional chain to drop all traffic while reloading, but if the firewall-start script fails firewall-stop will be called which by default results in accepting all traffic.

We could probably improve that situation by executing iptables-save before calling the firewall-start script and restoring the firewall to the previous state if the firewall-start script fails.

After that we could try to make the whole operation atomic (there are other issues on this).

Issue/Inconsistency: Connection tracking helpers (edit by @globin: FIXED in #22034 )

networking.firewall.connectionTrackingModules = mkOption {
  type = types.listOf types.str;
  default = [ "ftp" ];
  example = [ "ftp" "irc" "sane" "sip" "tftp" "amanda" "h323" "netbios_sn" "pptp" "snmp" ];
  description =
    ''
      List of connection-tracking helpers that are auto-loaded.
      The complete list of possible values is given in the example.

      As helpers can pose as a security risk, it is advised to
      set this to an empty list and disable the setting
      networking.firewall.autoLoadConntrackHelpers

      Loading of helpers is recommended to be done through the new
      CT target. More info:
      https://home.regit.org/netfilter-en/secure-use-of-helpers/
    '';
};

networking.firewall.autoLoadConntrackHelpers = mkOption {
  type = types.bool;
  default = true;
  description =
    ''
      Whether to auto-load connection-tracking helpers.
      See the description at networking.firewall.connectionTrackingModules

      (needs kernel 3.5+)
    '';
};

The description states that these helpers can pose a security risk and should therefore not be used.

However the default value of networking.firewall.connectionTrackingModules is [ "ftp" ] and networking.firewall.autoLoadConntrackHelpers is enabled by default.

Do we need the ftp connection tracking helper for anything? If not I suggest we use the following defaults:

networking.firewall.connectionTrackingModules = [ ];
networking.firewall.autoLoadConntrackHelpers = !kernelCanDisableHelpers;

Setting networking.firewall.autoLoadConntrackHelpers = false would be even better imho but it seems like that could lead to some problems:

{ assertion = cfg.autoLoadConntrackHelpers || kernelCanDisableHelpers;
  message = "This kernel does not support disabling conntrack helpers"; }

[abbradar]

If I understand correctly you can't just block ICMPv6 because that would break neighbour discovery and as the result you won't be able to connect to a host on the same subnet (it's something like ARP), discover a router and so on. I'm not sure but blocking non-fe00::/9 addresses may work though. But yes, if we can block IPv6 pings with this option it would be more consistent (I'm not a fan of ICMP blackholing myself but we are not talking about defaults here).

:+1: to other suggestions (I like autoloading conntrack helpers by default though).

[edolstra]

See also #4155.

[globin]

For the ICMP stuff see: http://shouldiblockicmp.com/

[primeos]

Regarding the ICMP discussion:

The idea wasn't to block all ICMPv6 messages (which would obviously be a very bad idea and break IPv6 as described by @abbradar :+1:) - only "ICMPv6 Echo Requests" (Type 128, Code 0).

Of course even just blocking ICMPv6 echo requests can cause (unexpected) problems, so that it should never be the default (which is why the default of networking.firewall.allowPing is true.

BTW: It's probably a good idea to include a statement, that this option shouldn't be disabled if the user isn't aware of the implications (especially on a server - on a laptop, etc. it could make sense (imho)).

AFAIK blocking ICMPv6 echo requests would cause the following problems:

• It breaks Teredo tunneling (this shouldn't be a problem for clients)
• It makes troubleshooting more difficult (this applies for ICMP(v4) as well - mainly an issue for servers)

But note that blocking ICMP(v4) echo request can "break" stuff too (IIRC DHCP (at least some implementations) use ICMP echo requests in order to avoid duplicate addresses if misconfigured)

From [https://tools.ietf.org/html/rfc4890#appendix-A.5](RFC 4890 - Recommendations for Filtering ICMPv6 Messages in Firewalls):

Echo Request (Type 128) uses unicast addresses as source addresses, but may be sent to any legal IPv6 address, including multicast and anycast addresses [RFC4443]. Echo Requests travel end-to-end. Similarly, Echo Responses (Type 129) travel end-to-end and would have a unicast address as destination and either a unicast or anycast address as source. They are mainly used in combination for monitoring and debugging connectivity. Their only role in establishing communication is that they are required when verifying connectivity through Teredo tunnels [RFC4380]: Teredo tunneling to IPv6 nodes on the site will not be possible if these messages are blocked. It is not thought that there is a significant risk from scanning attacks on a well-designed IPv6 network (see Section 3.2), and so connectivity checks should be allowed by default.

The RFC states "it is not thought that there is a significant risk from scanning attacks on a well-designed IPv6 network" but please note that that RFC is from 2007.

This is why I would still suggest we do one of the following:

• Use that option for ICMP and ICMPv6
• Rename that option
• Remove that option

[primeos]

See also #4155.

@edolstra Good point, I forgot to mention that issue (but included a statement that there are other issues - Unfortunately I didn't manage to read the complete issue yet...).

My idea was to break that transition into multiple steps (not mentioned above) by beginning with "Properly reset the firewall". However unfortunately this doesn't work as simple as I described above (at least not without breaking stuff).

(I'll probably update/change that suggestion later.)

[fpletz]

For disabling conntrack helper autloading, see PR #22034.

[globin]

(Moved the conntrack part to the bottom in the description to make reading the summary easier.)

[peterhoeg]

@primeos did you give up on this? I have seen some odd race conditions at times when rules are changed and would very much like to explore how we can make the firewall handling better.

[primeos]

@peterhoeg Unfortunately yes, at least for now... (I've had some more ideas and it got a bit too complicated)

I might have some time for this in ~1 month and could give it another try. The main problem is that we would probably need to restructure the firewall module(s) a bit (for other ideas, not mentioned in this issue). I'd like to have a generic firewall module (backwards compatible to the current one but can be used to switch between iptables, nftables, etc. - i.e. these modules would all have to implement the options from the generic firewall module but could also implement additional options that won't go into the generic module), two iptables modules (the current one and an atomic one - IMO it would most likely be too complex to use one module for both cases) and one nftable module (already exists but probably needs to be extended). Especially the atomic iptables module should solve all race conditions and be better suited for a declarative configuration but wouldn't work for all use-cases (e.g. dynamic rules from Virtualbox, etc. would get lost/overwritten).

And since this change wouldn't be minor I should formalize this idea (e.g. an extra issue - I guess (/hope :D) an RFC would be overkill) and ask for some feedback before implementing something like this. Any help with this as well as feedback/suggestions would be very welcome :smile:. But as mentioned above it would have to wait ~1 month, except someone else would take it over.

Edit: @peterhoeg And thanks for your comment, I'm glad to hear that there's some interest :smile:.

[Ekleog]

(triage) @primeos Did you find time to give it another try / do you still think you will be able to give it another try anytime “soon”? :)

[stale[bot]]

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
2. Ask on the NixOS Discourse.
3. Ask on the #nixos channel on irc.freenode.net.

[flokli]

@primeos is there something left from the initial improvements/suggestions/discussions?

Could this be split into individual issues, so it's easier to track? Also, how does some of this still apply with the recent switch from iptables to nftables?

[primeos]

Sorry, this got bigger than I had time for so I basically just dropped it. I haven't kept up with recent changes but I assume that many of the ideas are still relevant. I could try to split it up into individual issues but it might make more sense to just close this for now - unless someone else has time and motivation to work on a implementation.

Suggestion: Use allowPing for both IPv4 and IPv6

That's a low-hanging fruit. I'd like that setting to also apply to IPv6 for consistency but I don't know how others feel about it. It's not a big deal in any case (and IMO there are very few (if any) reasons for setting it to false in the first place).

Suggestion: Use -w (wait for the xtables lock) everywhere

The switch to iptables-nftables-compat may or may not fix that automatically (I haven't checked).

Suggestion: Properly reset the firewall

This was my main motivation and would require a complete rewrite of the firewall module. Or rather an additional module as it doesn't work for every use-case. The idea was that the firewall rules would be configured completely declarative via the NixOS configurations and all changes would be applied with an atomic operation (i.e., it either works completely or the operation fails and everything will remain as it is - currently this isn't the case). This is easy to implement when configuring everything through NixOS but it doesn't work when using tools like Docker and LXD as they modify firewall rules by default (can be disabled but I doubt that many users do it manually). The "problem" is that any firewall changes made outside the NixOS configuration will be discarded when reloading the firewall rules. (The nftables module might already use an atomic approach.) Edit: Yes, from a quick look it seems to be an atomic operation:

https://github.com/NixOS/nixpkgs/blob/4c21b7bf132c572b2a4b906d1be2449518ad47d1/nixos/modules/services/networking/nftables.nix#L115-L126

https://wiki.nftables.org/wiki-nftables/index.php/Atomic_rule_replacement

[flokli]

Suggestion: Use -w (wait for the xtables lock) everywhere

Doesn't seem to be a problem anymore, according to https://www.redhat.com/en/blog/using-iptables-nft-hybrid-linux-firewall:

In comparison to legacy iptables, iptables-nft has a few clear benefits: With back end transactions being atomic, there is no need for the global xtables lock which has proven problematic in environments with large and/or rapidly changing rulesets.

---

Suggestion: Use allowPing for both IPv4 and IPv6

I'd rather prefer renaming this to allowPing4 to make the distinction clear. Blocking ICMP is bad in general, and even worse for IPv6. If people really want to block ping6 without breaking the rest of icmp, they should probably configure things explicitly, and what ICMP messages to drop.

---

Suggestion: Properly reset the firewall / atomic replacement

I don't think it's feasible without a refactor of the firewall module (and probably switching to more nftables features). I'm also not sure declaratively managing all firewall rules exclusively will work - maybe we can do atomic replacements of individual "NixOS firewall" nf_tables. I propose we tackle https://github.com/NixOS/nixpkgs/issues/161428 first (which would further clarify networking.nftables.rulesetFile and then follow up on that. There's also already an issue on that, https://github.com/NixOS/nixpkgs/issues/4155#issuecomment-1048295371.