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

wireguard service: allow empty interfaces #61743

Merged
merged 1 commit into from May 25, 2019

Conversation

abbradar
Copy link
Member

@abbradar abbradar commented May 20, 2019

This is needed in case one wants to use wg-quick on NixOS.

Motivation for this change
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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@@ -301,8 +301,10 @@ in

networking.wireguard = {

enable = mkEnableOption "WireGuard";
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd add default = cfg.interfaces != {}; here, instead of having the logic inside the mkIf of the implementation (and switch on cfg.enable there).

Thus, setting networking.wireguard.enable to false also will disable potentially configured interfaces, which might be intended anyways.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, done!

@flokli
Copy link
Contributor

flokli commented May 22, 2019

This is also useful when using systemd-networkd to setup wireguard interfaces.

Currently, one has to manually set

boot.extraModulePackages = [ kernel.wireguard ];, and optionally environment.systemPackages = [ pkgs.wireguard-tools ];

To load the kernel module - having networking.wireguard.enable available for that is quite convenient.

cc @Mic92

This is needed in case one wants to use wg-quick on NixOS.
@Ma27
Copy link
Member

Ma27 commented May 25, 2019

This is needed in case one wants to use wg-quick on NixOS.

I'm probably missing something, but can't we use netwokring.wg-quick (nixos/modules/services/networking/wg-quick.nix) for that?

@flokli
Copy link
Contributor

flokli commented May 25, 2019

Uff, yet another NixOS module to setup wireguard - we now have networking.wireguard, networking.wg-quick, and systemd.network.netdevs.<name> (with netdevConfig.Kind = "wireguard") 😄

With the plans to switch more of the networking.* module to systemd-networkd, I still think this PR makes sense - I consider wg-quick a specific way of setting up wireguard.

Can we unify the kernel and userland part from nixos/modules/services/networking/wg-quick.nix

    boot.extraModulePackages = [ kernel.wireguard ];
    environment.systemPackages = [ pkgs.wireguard-tools ];

with the one already present in nixos/modules/services/networking/wireguard.nix?

nixos/modules/services/networking/wg-quick.nix should simply set networking.wireguard.enable = true; inside the config = mkIf (cfg.interfaces != {}), together with setting up the wg-quick specific units.

@abbradar
Copy link
Member Author

Uhhh, didn't know about this module. Long term I think we obviously should leave only one method of doing things. I see wg-quick was added in abf6079 because existing wireguard support didn't cover all needed cases. Perhaps we should start deprecating one of them for the next release.

I see wg-quick has a quirk of having to set networking.firewall.checkReversePath = false but otherwise it's more feature-rich than wireguard. I propose this: reimplement networking.wireguard via wg-quick saving backwards compatibility but copying features and code from the wg-quick module. Then deprecate networking.wg-quick as I think networking.wireguard is more well-known.

Anyway, out of scope of this PR ;)

@flokli
Copy link
Contributor

flokli commented May 25, 2019

Well, we can remove the duplications mentioned at a later point, too - nothing that needs to prevent this PR from being merged.

@flokli flokli merged commit e4de353 into NixOS:master May 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants