Skip to content

nixos/nat: Don't flush tables, create subchains for autogenerated rules#4121

Merged
wkennington merged 3 commits intoNixOS:masterfrom
wkennington:master.nat
Sep 18, 2014
Merged

nixos/nat: Don't flush tables, create subchains for autogenerated rules#4121
wkennington merged 3 commits intoNixOS:masterfrom
wkennington:master.nat

Conversation

@wkennington
Copy link
Contributor

No description provided.

@wkennington wkennington added work-in-progress 0.kind: enhancement Add something new or improve an existing system. 9.needs: reporter feedback This issue needs the person who filed it to respond labels Sep 17, 2014
@edolstra
Copy link
Member

It may be a bit simpler to set the default policy of the table to DROP (iptables -P ... DROP), flush the table and run the start script. That way, packets won't get through while the table is being reloaed.

@wkennington
Copy link
Contributor Author

I was thinking about it, I just don't want to stomp on the default policies anyone has loaded into iptables. I know this shouldn't be much of a concern as we drop packets at the end of our chain anyway.

However, we would still need to reset this policy to ACCEPT in case the firewall fails to load, so I don't necessarily see a reduction in complexity aside from the creation of another chain.

It also means that users have the choice to set their policy in the failure case. Some may choose to ACCEPT and have access to their box on failure, and others may choose to lock out all access with a DROP policy if the firewall fails to load.

@lucabrunox
Copy link
Contributor

I think we should rather use iptables-restore (which in theory should be
atomic) and force the users to put every iptables rule in the nixos
configuration.

On Thu, Sep 18, 2014 at 8:57 PM, William A. Kennington III <
notifications@github.com> wrote:

I was thinking about it, I just don't want to stomp on the default
policies anyone has loaded into iptables. I know this shouldn't be much of
a concern as we drop packets at the end of our chain anyway.

However, we would still need to reset this policy to ACCEPT in case the
firewall fails to load, so I don't necessarily see a reduction in
complexity aside from the creation of another chain.

It also means that users have the choice to set their policy in the
failure case. Some may choose to ACCEPT and have access to their box on
failure, and others may choose to lock out all access with a DROP policy if
the firewall fails to load.


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

www.debian.org - The Universal Operating System

@wkennington
Copy link
Contributor Author

@lethalman For the long term, yes I agree as this would better roll over to nftables. The problem is that this would be a pretty breaking change in the short term and we would need to implement a significantly more useful interface for the firewall in the nix config.

@wkennington
Copy link
Contributor Author

I think if we are going to make a change that disruptive, we should consider creating a firewall2.0 type module, which combines the functionality of firewall.nix and nat.nix with a more descriptive interface that conflicts with both of the old modules. This way we won't disrupt any of the old users and can eventually roll over the changes when we feel they are safe enough.

@wkennington
Copy link
Contributor Author

Keeping in mind the longterm desgin changes, does this patch seem reasonable for the time being? It would greenlight the build until we actually come up with a nicer firewall interface.

@lucabrunox
Copy link
Contributor

Sure talking about a new interface is not the right place to do here,
sorry. Btw, instead of creating another interface we may have a
services.iptables as a bridge for firewall and nat, and mutableRules =
false; . Needs deep thoughts :-)

On Thu, Sep 18, 2014 at 11:04 PM, William A. Kennington III <
notifications@github.com> wrote:

Keeping in mind the longterm desgin changes, does this patch seem
reasonable for the time being? It would greenlight the build until we
actually come up with a nicer firewall interface.


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

www.debian.org - The Universal Operating System

@wkennington
Copy link
Contributor Author

So are we okay with this going through? I'll spin up an issue for nftables / iptables-restore based firewalls.

@wkennington
Copy link
Contributor Author

There seem to be no objections, I'm going to push this so that we can see a release of nixos unstable.

wkennington added a commit that referenced this pull request Sep 18, 2014
nixos/nat: Don't flush tables, create subchains for autogenerated rules
@wkennington wkennington merged commit 1ff027b into NixOS:master Sep 18, 2014
@wkennington wkennington deleted the master.nat branch January 7, 2015 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0.kind: enhancement Add something new or improve an existing system. 9.needs: reporter feedback This issue needs the person who filed it to respond

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants