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

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from
+152 −0

Conversation

picnoir
Copy link
Member

@picnoir picnoir 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.

Sorry, something went wrong.

@picnoir
Copy link
Member Author

picnoir commented Jul 1, 2019

CC @andir @flokli @grahamc

@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Jul 1, 2019
@picnoir picnoir force-pushed the nin-wg-networkd branch 2 times, most recently from 27e8226 to 7763fd5 Compare July 1, 2019 17:37
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jul 1, 2019
eadwu
eadwu reviewed Jul 1, 2019
View reviewed changes
nixos/modules/system/boot/networkd.nix Outdated
"PrivateKeyFile" "ListenPort" "FwMark"
])
(assertInt "ListenPort")
(assertInt "FwMark")
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@picnoir picnoir Jul 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

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 :)

@picnoir
systemd-networkd: Add wireguard-related options.
ec073e4 
Add wireguard-related `netdev` options and their associated nixos
test.
@picnoir picnoir force-pushed the nin-wg-networkd branch from 7763fd5 to ec073e4 Compare July 1, 2019 18:39
@sjau
Copy link

sjau 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
Copy link
Contributor

flokli commented Jul 5, 2019

I agree - we should include them, but warn.

@zarelit zarelit mentioned this pull request Aug 7, 2019
Merged
9 tasks
@arianvp
Copy link
Member

arianvp commented Aug 7, 2019

Note that there is a PrivateKeyFile option in 242

@flokli
Copy link
Contributor

flokli 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 ;-)

@picnoir
Copy link
Member Author

picnoir commented Aug 21, 2019

Closing in favor of #45392

@picnoir picnoir closed this Aug 21, 2019
@picnoir picnoir deleted the nin-wg-networkd branch December 11, 2019 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eadwu eadwu

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@picnoir @sjau @flokli @arianvp @eadwu