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

Jump to bottom

nixos/mullvad-vpn: enable iproute2 config #91929

Closed
wants to merge 1 commit into from
Closed

nixos/mullvad-vpn: enable iproute2 config #91929

wants to merge 1 commit into from

Conversation

06kellyjac
Copy link
Member

@06kellyjac 06kellyjac commented Jul 1, 2020 •
edited

Motivation for this change

Since 2020.5 mullvad-daemon will try to write to /etc/iproute2/rt_tables
This change enables the copying of those config files over to /etc

Resolves #91923

I'm not sure if this is an ideal solution or a particularly NixOS-ie solution so input would be greatly appreciated.

Waiting on feedback from Mullvad as to double check whether it's actually necessary to write to /etc/iproute2/rt_tables
mullvad/mullvadvpn-app#1891

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Br1ght0ne
Br1ght0ne reviewed Jul 2, 2020
View reviewed changes
Copy link
Member

@Br1ght0ne Br1ght0ne left a comment •
edited

Choose a reason for hiding this comment

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

The idea - LGTM, diff - please see my comment.

I'm also not sure if it's the one and only right solution, but seems like it might be. If it makes the app work, why not?

nixos/modules/services/networking/mullvad-vpn.nix Outdated Show resolved Hide resolved
@06kellyjac
nixos/mullvad-vpn: enable iproute2 config
ac7cad7 
Since 2020.5 mullvad-daemon will try to write to /etc/iproute2/rt_tables
This change enables the copying of those config files over to /etc
@06kellyjac
Copy link
Member Author

After discussions on mullvad/mullvadvpn-app#1891 another potential fix would be to patch out writing to rt_tables and define the route in nix.
Could be done with adding the route to networking.iproute2.rttablesExtraConfig but would still require copying over the config (which is currently always writable). IDK if networking.interfaces.<name?>.ipv4.routes would work, I'm not really au fait with networking in NixOS.

This method is easy enough for now & saves maintaining a patch

@06kellyjac 06kellyjac marked this pull request as ready for review July 2, 2020 09:25
@06kellyjac 06kellyjac closed this Aug 10, 2020
@06kellyjac 06kellyjac deleted the mullvad_service_iproute2 branch August 10, 2020 08:20
@Artturin
Copy link
Member

mullvad remains broken unless iproute2 is enabled manually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Br1ght0ne Br1ght0ne Br1ght0ne left review comments

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.

mullvad-vpn 2020.5 broken
3 participants
@06kellyjac @Artturin @Br1ght0ne