Conversation
drupol
left a comment
There was a problem hiding this comment.
Hello,
Thanks for your first PR.
I made some feedback, let me know if you need some help.
|
For such a security relevant package such as a vpn software, we should build from source if possible. This would also allow us to patch out quirks such as relying on /usr |
Your claims about salts are valid and correct! My earlier statement was based on an incorrect misunderstanding, and I've updated the comment accordingly. Another way to address quirks is to make changes directly in the NordVPN repository, which was my implicit assumption for the next release. I could update this to build from scratch, but I don’t see the added security benefit. All other Linux distributions download from |
You are right that most distributions fetch the However, the key issue here is trust and verifiability. At the moment, there's no reliable way to verify that the binaries provided on One of the strengths of Nix is its focus on reproducibility. Building from source allows us (most of the time) to produce reproducible outputs. This enables a verifiable 1-to-1 mapping between the source code and the resulting binaries, which significantly improves the security of the software supply chain. Fortunately, thanks to recent community efforts, we’re getting close to being able to build the client fully from source. That’s why I believe it’s worth pushing in that direction. |
Gotcha. A malicious attacker might somehow tamper with their binaries. Building from source is the secure way to go. Ok, will do, thanks! |
|
Modified the package to build from source instead of extracting the .deb file. Verified that core features function correctly. Thank you all for your time! |
6a79c37 to
30f70e6
Compare
|
I've reduced privileges by using a dedicated Additionally, the One more thing, Thanks again for the review! |
|
Any insight about why meshnet is not supported? I've been using the package just fine until I got hit with an error while trying to use meshnet and I could not find anything useful on the logs. |
@andersonjoseph , meshnet requires dependencies such as |
ffa143f to
0333c2f
Compare
|
Just a quick version bump to 4.3.1. Going to work on the Flutter GUI now. Hopefully it turns out not too difficult thanks to the prior effort of @ruffsl |
aba34f9 to
0f52b06
Compare
|
Ok, I think the previous build passed because of some cache weirdness. Strangely when I had tested the binary, it spat out that the binary had used version 4.3.1. Anyway, latest commits use the correct vendor and src hashes for 4.3.1. |
|
Attempting to build nordvpn's flutter gui. You can find what I have so far here. It does not build atm, complains with the following error: Seems like I need to add |
0f52b06 to
0a155fe
Compare
|
I've added flutter gui support to the package and removed the salt. Also to avoid rebuilding the cli twice, I've separated out the package into cli.nix and gui.nix. I had to patch their linux CMakeLists.txt to use pkgconfig to find the correct x11 path. Tested and verified that both standalone package and module work as intended over openvpn and nordlynx.
I intend to work on incorporating meshnet next which I would start on Dec 26th. Hopefully not too difficult thanks to the prior efforts of @dimkNevidimk! |
|
meshnet progress update:
https://github.com/different-error/nixpkgs/tree/nordvpn-meshnet |
|
update:
Surprisingly, I don't see "meshnet" in the nordvpn settings. https://github.com/different-error/nixpkgs/tree/nordvpn-meshnet I've caught the flu. I intend to get back to this when I feel healthy enough to do so. |
|
Sorry for not replying earlier, but I think NordVPN team dropped meshnet support completely: |
They changed their mind and decided to keep it. |
|
I could use some help getting meshnet working. Please base your changes around my feature branch |
I will take a look when I get home from holiday travel (in a couple of hours) 👌 TL;DR: I got Meshnet working, but it tries to edit /etc/hosts, which causes permission errors. If anyone knows a clean way to grant write permissions to /etc/hosts, it would be great. Another solution is to send a patch with a --no-ns-hosts flag to the upstream repo so we can disable the write attempts and handle hostnames declaratively. |
0a155fe to
5c45f40
Compare
|
Some nits. Tested successful connection using the GUI. While progress with meshnet continues, seeing that including it would cause this PR to increase significantly in size, I think we should PR the current changes without meshnet support and include meshnet in the next PR. I presently intend to break this PR into smaller, newer ones to facilitate review. |



Add the popular NordVPN to NixOS. Tested using the following configuration:
The configuration was tested by running:
There is another PR (#220616) for NordVPN, which is over two years old and has been stale for a year. Additionally, there are issues requesting NordVPN support for NixOS here and here.
I chose to extract the .deb package instead of building from source
to avoid modifying or leaking the salt. Meshnet is not yet supported, but core NordVPN features work. I’ll create another PR once the Meshnet issues are resolved.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.