Nixpkgs: 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:

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.