NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
17.7k stars 13.84k forks source link

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

Open wkennington opened 10 years ago

wkennington commented 10 years ago

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.

Shados commented 10 years ago

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 commented 10 years ago

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 https://github.com/NixOS/nixpkgs/issues/4155#issuecomment-56115638.

www.debian.org - The Universal Operating System

Shados commented 10 years ago

@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 commented 10 years ago

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 https://github.com/NixOS/nixpkgs/issues/4155#issuecomment-56115638.

www.debian.org - The Universal Operating System

— Reply to this email directly or view it on GitHub https://github.com/NixOS/nixpkgs/issues/4155#issuecomment-56115792.

CMCDragonkai commented 9 years ago

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 commented 9 years ago

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 https://github.com/NixOS/nixpkgs/issues/4155#issuecomment-140290913.

Shados commented 9 years ago

@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 commented 9 years ago

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 https://github.com/NixOS/nixpkgs/issues/4155#issuecomment-140353336.

lucabrunox commented 9 years ago

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 commented 9 years ago

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 https://github.com/NixOS/nixpkgs/issues/4155#issuecomment-140486482.

edolstra commented 9 years ago

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

wkennington commented 9 years ago

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 https://github.com/NixOS/nixpkgs/issues/4155#issuecomment-140494742.

CMCDragonkai commented 9 years ago

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 commented 8 years ago

(triage) any new leads?

moretea commented 7 years ago

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 commented 7 years ago

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:

CMCDragonkai commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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:

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 commented 7 years ago

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

mmahut commented 5 years ago

Any news on this issue?

wmertens commented 5 years ago

Some should combine these comments into an RFC so we can collaborate there and close this issue. I have my plate full right now, any volunteers?

stale[bot] commented 4 years ago

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.
Thesola10 commented 3 years ago

still definitely important

stale[bot] commented 3 years ago

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

IreneKnapp commented 2 years ago

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.

flokli commented 2 years ago

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 commented 2 years ago

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.