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.

Sign up for GitHub

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

Jump to bottom

nixos/firewall: per-interface port options #41222

Merged
merged 1 commit into from Jul 3, 2018
Merged

nixos/firewall: per-interface port options #41222

merged 1 commit into from Jul 3, 2018

Conversation

ghost
Copy link

@ghost ghost commented May 29, 2018 •
edited by ghost

Motivation for this change

This adds a way to have different open ports on different interfaces with rules like:

  networking.firewall.interfaces = {
    "${eth}" = {
      allowedTCPPorts = [ 22 80 443 9418 ];
      allowedUDPPorts = [ 53 ];
    };
    "${veth}" = {
      allowedUDPPorts = [ 547 ];
      allowedTCPPorts = [ 8118 ];
    };
    "${deth}".allowedTCPPorts = [ 8118 ];   
    "${ieth}".allowedTCPPorts = [ 22 8118 ];
  };

As a side effect it will decline rules set via networking.firewall.allowed... and so also rules implicility set by services like sshd, which is fine as user wants direct control.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
  • Fits CONTRIBUTING.md.

@ghost ghost mentioned this pull request Jun 4, 2018
Closed
@andrew-d
Copy link
Contributor

This is awesome - I've wanted this for a while! Thanks 😀

cc @fpletz and @abbradar as some folks that have touched this recently

infinisil
infinisil approved these changes Jun 24, 2018
View reviewed changes
Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this with 2 interfaces myself on a machine, works just fine. It might be a bit troublesome to enable all ports you need manually, but I think that's okay for the people who need this control.

infinisil
infinisil requested changes Jun 24, 2018
View reviewed changes
nixos/modules/services/networking/firewall.nix Outdated
default = mapAttrs (name: value: cfg."${name}") commonOptions;
};
type = with types; attrsOf (submodule [ { options = commonOptions; } ]);
description = "Per interface options. Override action of global options.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll suggest a different wording though: "Interface-specific open ports. Setting this value will override all values of the networking.firewall.allowed* options."

Using the same formatting as the other options

infinisil
infinisil requested changes Jun 24, 2018
View reviewed changes
nixos/modules/services/networking/firewall.nix Outdated
default = mapAttrs (name: value: cfg."${name}") commonOptions;
};
type = with types; attrsOf (submodule [ { options = commonOptions; } ]);
description = "Interface-specific open ports. Setting this value will override all values of the networking.firewall.allowed* options.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant with '' quotes same as the other options, and the option networking.firewall.allowed* should be wrapped in at least <literal></literal>, also 80 chars max line length same as the other options.

@infinisil
Copy link
Member

Looking at the changes, it shouldn't change any existing configurations, but just to make sure:

@GrahamcOfBorg test firewall networking containers-restart-networking

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: tests.firewall, tests.networking

The following builds were skipped because they don't evaluate on aarch64-linux: tests.containers-restart-networking

Partial log (click to expand)

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

@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Attempted: tests.firewall, tests.networking

The following builds were skipped because they don't evaluate on x86_64-linux: tests.containers-restart-networking

Partial log (click to expand)

cannot build derivation '/nix/store/vl3zx1rl920jn11s6lsdmrma5mmwc33i-nixos-system-walled-18.09.git.5a49e15.drv': 5 dependencies couldn't be built
cannot build derivation '/nix/store/8nmn4lbkb30zczsn2m81vpb225rzs8z2-closure-info.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/yawyaniv5h17b9n3p3hshd00iimlwp65-closure-info.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/izb3297kvqz1whzs2zp8bl5jk8hx54dz-run-nixos-vm.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/kj2gg8m06q2nb4kcbj7j4wzrncb5biga-run-nixos-vm.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/4lkwg8139i16wysaa51z4zq1blv2k2kg-nixos-vm.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/qspz11pw1dz60pas6117hbs6vvra6jma-nixos-vm.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/vs10csbclnxx2i9qa0la91ksca93xbji-nixos-test-driver-firewall.drv': 4 dependencies couldn't be built
cannot build derivation '/nix/store/7k6k3pg1432p7a3395wvkaj3i8ihz9d9-vm-test-run-firewall.drv': 1 dependencies couldn't be built
error: build of '/nix/store/7k6k3pg1432p7a3395wvkaj3i8ihz9d9-vm-test-run-firewall.drv' failed

@infinisil
Copy link
Member

Ran the tests firewall, networking.scripted.static and containers-restart_networking locally, the first 2 still succeed and the third one never succeeded to start with.

@infinisil infinisil merged commit 59dd0e6 into NixOS:master Jul 3, 2018
@ghost
Copy link
Author

ghost commented Jul 3, 2018

@infinisil Thank you!

@ghost ghost deleted the firewall branch July 3, 2018 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@andrew-d @infinisil @GrahamcOfBorg