Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/{firewall,nat}: Standardize around an iptables-restore / nftables solution #4155

Open
wkennington opened this issue Sep 18, 2014 · 29 comments

Comments

@wkennington
Copy link
Contributor

Right now we use a command defined set of firewall rules. This allows for mutability of rules during normal operation and a simple, flexible interface for users to add new firewall rules using iptables commands.

While this approach has worked in the past, it would be nice to standardize on a cleaner firewall interface using the iptables-restore or nftables utilities.

Personally I'd like to see nftables used for this, as it seems to be the future direction of the linux packet filtering stack.

@wkennington wkennington changed the title nixos/{firewall,nat}: Standardize around an iptables-restore / nftables Solution nixos/{firewall,nat}: Standardize around an iptables-restore / nftables solution Sep 18, 2014
@Shados
Copy link
Member

Shados commented Sep 18, 2014

I have a working iptables-restore based firewall.nix/nat.nix that I've been playing with/testing for a few weeks, could clean it up in the next few days and push it to github if you like.

I spoke to @edolstra about the idea previously and he mentioned there would be issues with packages that add/change iptables rules (e.g. libvirtd) getting their rules trashed by iptables-restore, currently my implementation just ignores this issue.

@lucabrunox
Copy link
Contributor

I think there should be a common services.iptables configuration, used by
firewall, nat and others. Then iptables-restore shall be done in
services.iptables once for the unified rules.

On Fri, Sep 19, 2014 at 1:02 AM, Alexei Robyn notifications@github.com
wrote:

I have a working iptables-restore based firewall.nix/nat.nix that I've
been playing with/testing for a few weeks, could clean it up in the next
few days and push it to github if you like.

I spoke to @edolstra https://github.com/edolstra about the idea
previously and he mentioned there would be issues with packages that
add/change iptables rules (e.g. libvirtd) getting their rules trashed by
iptables-restore, currently my implementation just ignores this issue.


Reply to this email directly or view it on GitHub
#4155 (comment).

www.debian.org - The Universal Operating System

@Shados
Copy link
Member

Shados commented Sep 18, 2014

@lethalman I kind of like that the modules' naming in this case is more semantic rather than specifying the technologies involved, especially because it does mean they won't need to be arbitrarily renamed if the backend is changed.
OTOH, having them be named after the technology does mean you can keep around multiple backends simultaneously, which could I guess be useful for transitioning between them.

Also, keep in mind you can call iptables-restore just once from just one service but still have the configuration be spread across multiple modules if that provides for a cleaner interface.

@wmertens
Copy link
Contributor

Iptables-restore trashes any stateful rules that were added. Running
containers adds rules, for example.

Wout.
On Sep 19, 2014 1:04 AM, "lethalman" notifications@github.com wrote:

I think there should be a common services.iptables configuration, used by
firewall, nat and others. Then iptables-restore shall be done in
services.iptables once for the unified rules.

On Fri, Sep 19, 2014 at 1:02 AM, Alexei Robyn notifications@github.com
wrote:

I have a working iptables-restore based firewall.nix/nat.nix that I've
been playing with/testing for a few weeks, could clean it up in the next
few days and push it to github if you like.

I spoke to @edolstra https://github.com/edolstra about the idea
previously and he mentioned there would be issues with packages that
add/change iptables rules (e.g. libvirtd) getting their rules trashed by
iptables-restore, currently my implementation just ignores this issue.


Reply to this email directly or view it on GitHub
#4155 (comment).

www.debian.org - The Universal Operating System


Reply to this email directly or view it on GitHub
#4155 (comment).

@CMCDragonkai
Copy link
Member

Currently, are changes made to firewall/nat applied atomically (to prevent packet drops)? (this could be done using iptables-restore).

To deal with external services also modifying iptables, there might be a way to merge the modifications together. Would the ipset utility help? http://ipset.netfilter.org/index.html

@wkennington
Copy link
Contributor Author

ipset only helps for ip address matches, not for generic iptables rules.
Unfortunately just using iptables-restore doesn't actually give you
atomicity in your rule chain as it just processes the clears / adds
internally. You would need to use a modern kernel with nftables to achieve
this effect. Unfortunately that is going to require users to port their
current config to a next generation nftables firewall. I think there is
something to be gained by having both implementations side by side and
phasing out the old one over the period of say a year.

On Mon, Sep 14, 2015 at 11:13 PM Roger Qiu notifications@github.com wrote:

Currently, are changes made to firewall/nat applied atomically (to prevent
packet drops)? (this could be done using iptables-restore).

To deal with external services also modifying iptables, there might be a
way to merge the modifications together. Would the ipset utility help?
http://ipset.netfilter.org/index.html


Reply to this email directly or view it on GitHub
#4155 (comment).

@Shados
Copy link
Member

Shados commented Sep 15, 2015

@wkennington iptables-restore does actually clear and apply the rules in a single transaction, and any errors simply result in the previous state continuing on as normal. There's no opportunity for packets to be incorrectly accepted or dropped. In what way is it not atomic?

@wkennington
Copy link
Contributor Author

Ah, I didn't realize we had true atomic support with iptables-restore. At
any rate, I'm still not sure how you would account for externally applied
for rules yet.

On Tue, Sep 15, 2015, 03:57 Alexei Robyn notifications@github.com wrote:

@wkennington https://github.com/wkennington iptables-restore does
actually clear and apply the rules in a single transaction, and any errors
simply result in the previous state continuing on as normal. There's no
opportunity for packets to be incorrectly accepted or dropped. In what way
is it not atomic?


Reply to this email directly or view it on GitHub
#4155 (comment).

@lucabrunox
Copy link
Contributor

Let's try to shrink the problem at least.

To address the external iptables rule I guess we need some intelligence in parsing iptables-save, in order to readd them in an eventual iptables-restore. Is that the only approach possible?

@wkennington
Copy link
Contributor Author

What I'm worried about is how this will interact with
networking.firewall.extraCommands which expect the state of the firewall to
be flushed prior to being executed and therefore cannot be saved. Given
that this conflicts with the idea of parsing running rules to be saved, I
don't think you can come up with a strategy here that doesn't break
something. It would be better to create an entirely new interface with no
surprises and have users migrate.

On Tue, Sep 15, 2015, 11:09 lethalman notifications@github.com wrote:

Let's try to shrink the problem at least.

To address the external iptables rule I guess we need some intelligence in
parsing iptables-save, in order to readd them in an eventual
iptables-restore. Is that the only approach possible?


Reply to this email directly or view it on GitHub
#4155 (comment).

@edolstra
Copy link
Member

Sacrificing extraCommands sounds acceptable if we get an atomically updated firewall in exchange.

@wkennington
Copy link
Contributor Author

That's utterly terrible given the only way to configure a marginally
complex firewall with our current options is to use extraCommands. It would
make way more sense to drop all runtime supplied firewall rules before
breaking extraCommands, as users should not expect NixOS to preserve
stateful configuration which it already manages.

Again, this is why a new interface needs to be made. There is no reason why
we can't create options which have the ability to define complex nftables
or iptables-restore rules. This would then replace the current firewall
with something flexible that shouldn't need extraCommands.

On Tue, Sep 15, 2015, 11:42 Eelco Dolstra notifications@github.com wrote:

Sacrificing extraCommands sounds acceptable if we get an atomically
updated firewall in exchange.


Reply to this email directly or view it on GitHub
#4155 (comment).

@CMCDragonkai
Copy link
Member

What about when you need atomicity between iproute2 mutation and iptables mutation? Often for complex networks you would setup an virtual interfaces, network namespaces... etc, and then change the iptable rules. How can we compose these actions together into a single atomic change?

@wkennington a new interface in the Nix language that abstracts over iptables/nftables? Would this mean that all servers will need to migrate to to this nix abstraction so that the point of synchronisation is at the nix language and not all over the place, where the network rules can diverge from what is specified in the nix configuration?

@Profpatsch
Copy link
Member

(triage) any new leads?

@moretea
Copy link
Contributor

moretea commented Feb 16, 2017

It would be quite nice to have something like mutableFirewall akin to the multipleUsers option. I'm not sure if/how we can enforce this.

@moretea
Copy link
Contributor

moretea commented Feb 16, 2017

I had a short discussion about this yesterday on the IRC channel with @cleverca22.

There are other issues here like how this would interact with docker or other services that dynamically create firewall rules.

AFAICS there are basically two options:

  1. Just accept that IP table rules will be modified by other systems. Parse iptables-save and diff it with the previously saved version to see which rules were manually added. Then generate a new set of firewall rules from the configuration (and the "impure" rules) and apply it atomically. Unfortunately, this will cause a race issue between e.g. docker issuing new iptables commands and the activation of a new firewall.
  2. Creating an iptables wrapper program.

For the iptables wrapper, again, there are two options:

  • A thin iptables wrapper would just flock() on some file to prevent this race issue. The firewall activation script must flock() this as well to make it act like a mutex. Parsing would work the same as in (1).
  • Creating a fat iptables wrapper that would manage "custom" iptables state in some /var/run/firewall. flock() is still required. There are two ways to implement this:
    1. Just add the rules to /var/run/firewall. If it's a rule that would remove or modify a chain, check which would match and remove/modify it.
    2. Interpret the rules ourselves and store them in a database. Then generate input for iptables-restore from this. We might do fancy stuff like creating a iptablesCommandFor "daemonName", that would allow us to track by which daemon a firewall rule was created. It even could do some form of access control to only allow a daemon to modify certain rules through the iptables wrapper.

@CMCDragonkai
Copy link
Member

There should be a way for a program to lock iptables, while changing something else at the same time, and then unlock it. Other programs should then queue their iptable changes. Perhaps this can allow one to compose atomic network changes?

@Nadrieril
Copy link
Member

Nadrieril commented Feb 16, 2017

Hmm, this sounds overkill and risky. Since firewall rules usually don't change often, wouldn't it be enough to apply our rules with iptables-restore and then reload the services that need to add custom rules ?
This is how I used to do it on debian + iptables-restore + docker IIRC

@cleverca22
Copy link
Contributor

that is one of the options i had on IRC

a related option, is to patch the docker scripts to make a hook that returns a fragment of an iptables-restore file, and then you concat the output of all of the hooks and the real firewall, and pipe the final result into iptables-restore, but now docker has to provide that hook, and reload the firewall.service target to apply changes

@Nadrieril
Copy link
Member

This is not a new problem. Every daemon that changes firewall rules has to deal with admins trashing the rules (just google "<service> iptables-restore" to see). Services usually recommend some kind of workaround: libvirt supports reloading its rules on receiving a HUP signal; fail2ban makes it easy to grep on f2b-* chains; etc.

Rather than trying to make service-specific workarounds, I think it would be sufficient to make declarative rules easier to filter out.

Here is what I suggest:

  • add a common service.iptables option with a thin wrapper around iptables-restore rules that would force any defined chain name to start with "nixos-". It would need to redirect the usual INPUT, FOWARD, etc. chains accordingly (see also Firewall: Add support for arbitrary rules for input, output and forward #12940).
    The interface could look something like:

    service.iptables.filter.nixos-forward = ''
      -s 10.0.0.0/24 -j ACCEPT
      -j DROP
    '';

    There's probably a better interface than this but the idea is to be able to filter out nixos-defined rules easily

  • make firewall.nix and nat.nix use this new interface

  • on firewall reload:

    • filter out nixos-specific chains from the output of iptables-save
    • generate the new nixos-specific rules as an iptables-restore file fragment
    • call iptables-restore with the concatenation of the two previous steps

This would allow arbitrarily complex firewall setups, apply them atomically, and interact seamlessly with daemons like fail2ban.
This may be a good rfc to submit to https://github.com/NixOS/rfcs.

Further steps may be:

@Nadrieril
Copy link
Member

Even simpler: we could instead append -m comment --comment "nixos-fw" to each nixos-defined rule

@mmahut
Copy link
Member

mmahut commented Aug 18, 2019

Any news on this issue?

@wmertens
Copy link
Contributor

wmertens commented Aug 19, 2019 via email

@stale
Copy link

stale bot commented Jun 2, 2020

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.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 2, 2020
@Thesola10
Copy link
Contributor

still definitely important

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 17, 2021
@stale
Copy link

stale bot commented Sep 14, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 14, 2021
@IreneKnapp
Copy link
Contributor

Still important. I have my plate full with other RFC stuff right now, but am happy to help however I can short of driving that process.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Feb 5, 2022
@flokli
Copy link
Contributor

flokli commented Feb 22, 2022

Is it possible to atomically replace individual tables inside nftables?

The currently proposed iptables-{save,restore} approach indeed has a lot of problems, similar to exclusively replacing all nftable rules in our nftables module - there's many different things in the system, except from the NixOS firewall with legitimate cases to insert things into the firewall.

If that's possible, one way out of this could be to have a NixOS-firewall specific nftables table. Other things could still mock around with iptables or their own nftables table, but they would stomp less on each others' feet.

@IreneKnapp
Copy link
Contributor

That's a good idea. I don't know offhand, I'll wait for someone else to chime in and if nobody else does we should research it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests