nixos/firewall: Always use global firewall.allowed rules #50641

blaxill · 2018-11-18T23:10:12Z

Apply global firewall.allowed* rules separately from the interface specific rules. This means setting interface specific rules (networking.firewall.INTERFACE.allowed*) will still apply global rules to all interfaces.

Motivation for this change

Currently global networking.firewall.allowed* are dropped for interfaces when they have interface specific rules. Although this was documented, the more intuitive option would be to merge the rules.

#47580
(This supersedes my pull request fail #50625 sorry for that)

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Fits CONTRIBUTING.md.

Mic92 · 2018-11-18T23:37:01Z

Yes. This also hit me. Nevertheless we should document this change as well in https://github.com/NixOS/nixpkgs/blob/master/nixos/doc/manual/release-notes/rl-1903.xml

infinisil · 2018-11-18T23:39:37Z

See my comment #50625 (comment)

blaxill · 2018-11-18T23:53:53Z

I'm unfamiliar with the nix language, but wouldn't this cause an entry "default" under network.interfaces.* ? That was the previous behavior (and along with the filtering ${optionalString (iface != "default") ), but since I moved the global rules out, creating a default interface would duplicate the iptables rules.

I've verified the correct iptables are generated with this patch.

infinisil · 2018-11-19T00:34:05Z

Oh, I see my mistake now.

I'm not so sure about this change though, it adds even more duplication to this script, which has a lot of similar calls to begin with.

blaxill · 2018-11-19T00:54:02Z

You're right that it adds to more duplication- what I was initially trying to do was keep the extra "default" named interface even when there were network.interfaces.* specified. Since the "default" named interface was created previously in the default constructor for network.interfaces, it was removed when any interfaces were set. Unfortunately I'm not too familiar with nix so I'm not sure how to merge the network.allow* into the "default" named interface, if there is something similar you could point me to I will copy that structure.

I'll have a tinker and see if I can get something with less duplication.

Apply global firewall.allowed* rules separately from the interface specific rules.

blaxill · 2018-11-19T03:51:43Z

Oops accidentally closed.

blaxill · 2018-11-19T03:53:33Z

So I've reduced the duplication back to what it originally was, while keeping the functionality of the pull request. Are there additional change requests?

blaxill · 2018-11-21T02:31:29Z

@infinisil any other changes you might want to this?

Mic92 · 2018-11-21T10:17:03Z

yes, release notes. This is a breaking change and people might have already adapted to the other behavior.

Mic92 · 2018-11-21T23:03:50Z

nixos/doc/manual/release-notes/rl-1903.xml

+    <para>
+     NixOS global firewall allow options (<literal>networking.firewall.allow*</literal>)
+     are now preserved when setting interface specific rules such as
+     <literal>networking.firewall.interfaces.en0.allow*</literal>.


Suggested change

<literal>networking.firewall.interfaces.en0.allow*</literal>.

<literal>networking.firewall.interfaces.en0.allow*</literal>.

The pseudo device <literal>networking.firewall.interfaces.default</literal> was removed

in the process.

It kind of still uses a pseudo device but named any which allows you to overwrite individual rules by assigning to networking.firewall.interfaces.any.allow*. Perhaps I should rename that back if it will be less confusing?

Yes. That would also increase compatibility.

Mic92 · 2018-11-23T11:36:18Z

@GrahamcOfBorg build nixosTests.firewall

GrahamcOfBorg · 2018-11-23T11:36:41Z

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: nixosTests.firewall

Partial log (click to expand)

while evaluating the attribute 'linux_4_14' at /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/pkgs/top-level/all-packages.nix:14333:3:
while evaluating 'callPackageWith' at /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/lib/customisation.nix:108:35, called from /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/pkgs/top-level/all-packages.nix:14333:16:
while evaluating 'makeOverridable' at /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/lib/customisation.nix:67:24, called from /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/lib/customisation.nix:112:8:
while evaluating anonymous function at /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/pkgs/os-specific/linux/kernel/linux-4.14.nix:1:1, called from /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/lib/customisation.nix:69:12:
while evaluating 'buildLinux' at /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/pkgs/top-level/all-packages.nix:14631:16, called from /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/pkgs/os-specific/linux/kernel/linux-4.14.nix:5:1:
while evaluating 'callPackageWith' at /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/lib/customisation.nix:108:35, called from /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/pkgs/top-level/all-packages.nix:14631:23:
while evaluating 'makeOverridable' at /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/lib/customisation.nix:67:24, called from /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/lib/customisation.nix:112:8:
while evaluating anonymous function at /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/pkgs/os-specific/linux/kernel/generic.nix:1:1, called from /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/lib/customisation.nix:69:12:
assertion failed at /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-ndndx-vm/pkgs/os-specific/linux/kernel/generic.nix:51:1

GrahamcOfBorg · 2018-11-23T11:39:15Z

Success on x86_64-linux (full log)

Attempted: nixosTests.firewall

Partial log (click to expand)

attacker: exit status 0
walled: running command: sync
walled: exit status 0
test script finished in 28.69s
cleaning up
killing attacker (pid 609)
killing walled (pid 597)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
/nix/store/49xlikmk07r9km40zvm5k1hkpgzng74v-vm-test-run-firewall

Mic92 · 2018-11-23T11:42:32Z

Thanks!

GrahamcOfBorg · 2018-11-23T11:57:09Z

Success on aarch64-linux (full log)

Attempted: nixosTests.firewall

Partial log (click to expand)

walled: exit status 0
attacker: running command: sync
attacker: exit status 0
test script finished in 43.67s
cleaning up
killing walled (pid 631)
killing attacker (pid 643)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
/nix/store/in6j35xxp1i6h5sf9zgsj3xvv99ln46g-vm-test-run-firewall

blaxill requested a review from infinisil as a code owner November 18, 2018 23:10

blaxill changed the title ~~Use global firewall.allowed rules, even with specific rules~~ nixos/firewall: Use global firewall.allowed rules, even with specific rules Nov 18, 2018

blaxill changed the title ~~nixos/firewall: Use global firewall.allowed rules, even with specific rules~~ nixos/firewall: Always use global firewall.allowed rules Nov 18, 2018

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Nov 18, 2018

Mic92 requested a review from fpletz November 18, 2018 23:37

Mic92 added this to the 19.03 milestone Nov 18, 2018

nixos/firewall: Always use global firewall.allowed rules

551d2f7

Apply global firewall.allowed* rules separately from the interface specific rules.

blaxill force-pushed the firewallMerge branch from b17e54a to 551d2f7 Compare November 19, 2018 03:50

blaxill closed this Nov 19, 2018

blaxill reopened this Nov 19, 2018

Refactor out the set operations

32779b4

Add release notes

b48c6d0

GrahamcOfBorg added 8.has: changelog 8.has: documentation labels Nov 21, 2018

Mic92 reviewed Nov 21, 2018

View reviewed changes

Rename back to default and better release notes

308ab4e

Mic92 merged commit d3aeed3 into NixOS:master Nov 23, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/firewall: Always use global firewall.allowed rules #50641

nixos/firewall: Always use global firewall.allowed rules #50641

blaxill commented Nov 18, 2018

Mic92 commented Nov 18, 2018

infinisil commented Nov 18, 2018

blaxill commented Nov 18, 2018 •

edited

infinisil commented Nov 19, 2018

blaxill commented Nov 19, 2018 •

edited

blaxill commented Nov 19, 2018

blaxill commented Nov 19, 2018

blaxill commented Nov 21, 2018

Mic92 commented Nov 21, 2018 •

edited

Mic92 Nov 21, 2018

blaxill Nov 21, 2018

Mic92 Nov 21, 2018

Mic92 commented Nov 23, 2018

GrahamcOfBorg commented Nov 23, 2018

GrahamcOfBorg commented Nov 23, 2018

Mic92 commented Nov 23, 2018

GrahamcOfBorg commented Nov 23, 2018

nixos/firewall: Always use global firewall.allowed rules #50641

nixos/firewall: Always use global firewall.allowed rules #50641

Conversation

blaxill commented Nov 18, 2018

Motivation for this change

Things done

Mic92 commented Nov 18, 2018

infinisil commented Nov 18, 2018

blaxill commented Nov 18, 2018 • edited

infinisil commented Nov 19, 2018

blaxill commented Nov 19, 2018 • edited

blaxill commented Nov 19, 2018

blaxill commented Nov 19, 2018

blaxill commented Nov 21, 2018

Mic92 commented Nov 21, 2018 • edited

Mic92 Nov 21, 2018

Choose a reason for hiding this comment

blaxill Nov 21, 2018

Choose a reason for hiding this comment

Mic92 Nov 21, 2018

Choose a reason for hiding this comment

Mic92 commented Nov 23, 2018

GrahamcOfBorg commented Nov 23, 2018

GrahamcOfBorg commented Nov 23, 2018

Mic92 commented Nov 23, 2018

GrahamcOfBorg commented Nov 23, 2018

blaxill commented Nov 18, 2018 •

edited

blaxill commented Nov 19, 2018 •

edited

Mic92 commented Nov 21, 2018 •

edited