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

systemd-networkd: Add wireguard-related options. #64040

Closed
wants to merge 1 commit into from

Conversation

@NinjaTrappeur
Copy link
Contributor

commented Jul 1, 2019

Motivation for this change

The systemd.network nixos module is lacking the wireguard-related options.

Things done

Add wireguard-related netdev options and their associated nixos test.

Note: I omitted both the PrivateKey and PresharedKey options to prevent the user from leaking private keys to the store.

  • 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.

@NinjaTrappeur

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

@NinjaTrappeur NinjaTrappeur force-pushed the NinjaTrappeur:nin-wg-networkd branch 2 times, most recently from 27e8226 to 7763fd5 Jul 1, 2019
"PrivateKeyFile" "ListenPort" "FwMark"
])
(assertInt "ListenPort")
(assertInt "FwMark")

This comment has been minimized.

Copy link
@eadwu

eadwu Jul 1, 2019

Contributor

ListenPort takes either value between 1 and 65535 or auto [1]. FwMark takes a number between 1 and 4294967295 according to man/systemd.netdev.xml [2]. Also looks like it got renamed to FirewallMark but FwMark looks like it still exists [3].

[1] https://github.com/NixOS/systemd/blob/5c20aab77900f478fd380ab189787d80e4a35963/man/systemd.netdev.xml#L1254
[2] https://github.com/systemd/systemd/blob/master/man/systemd.netdev.xml#L1490
[3] https://github.com/systemd/systemd/blob/master/test/fuzz/fuzz-netdev-parser/directives.netdev#L13

This comment has been minimized.

Copy link
@NinjaTrappeur

NinjaTrappeur Jul 1, 2019

Author Contributor

Just corrected the PR according to your remarks regarding ListenPort and the extra spaces.

As for FwMark, it seems that the current pinned systemd is not supporting this new syntax yet.

[Edit]: I forgot the obvious: thanks for the review :)

nixos/modules/system/boot/networkd.nix Show resolved Hide resolved
Add wireguard-related `netdev` options and their associated nixos
test.
@NinjaTrappeur NinjaTrappeur force-pushed the NinjaTrappeur:nin-wg-networkd branch from 7763fd5 to ec073e4 Jul 1, 2019
@sjau

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2019

As for PrivateKey and PreshareKey, why not include them also but give a warning in the description? There might be good reason to use them even if they become world-accessible in the nix store.

@flokli

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2019

I agree - we should include them, but warn.

@zarelit zarelit referenced this pull request Aug 7, 2019
2 of 9 tasks complete
@arianvp

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2019

Note that there is a PrivateKeyFile option in 242

@flokli

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2019

@arianvp this PR adds it already - I still think we shouldn't artificially limit what a user can do. Warning if sb shoots into is food should probably be enough ;-)

@NinjaTrappeur

This comment has been minimized.

Copy link
Contributor Author

commented Aug 21, 2019

Closing in favor of #45392

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.