nixos/openvpn: user, forwarding and local state dir #65878
Conversation
|
can it work without net.ipv4.ip_forward ? or is like for firewalls and default to the secure version.
Sure it works, but without it you can only reach the machine running OpenVPN and nothing on the network behind it so both cases are equally valid.
what about ipv6 ?
Yeah, that's missing - nice catch. I have changed the PR but haven't tried it out yet.
|
|
that sounds similar to the openFirewall option: you very likely need it but we cna't enable it by default. Aren't there some UDP ports that should be open as well ? would it make sense to make ports configurable (I believe that's common to avoid scans on specific ports) ? |
|
that sounds similar to the openFirewall option: you very likely need it
but we cna't enable it by default.
I disagree. For random service foo, with openFirewall you *have* to either use that option *or* open the firewall yourself.
Without this option, things still work - you are probably likely to want to enable it but you don't have to and you don't have to do anything else.
Aren't there some UDP ports that should
be open as well ? would it make sense to make ports configurable (I
believe that's common to avoid scans on specific ports) ?
OpenVPN can run over TCP and UDP, but yes, we do need to open ports.
Currently, we expose very few of the options and basically have the user throw in a bunch of text that will end up in the configuration file. Whether that's by design or because the initial author just wanted to get up and running quickly I cannot say.
Now, port and protocol would make sense to promote to config options as it would allow us to open the firewalls through a toggle as you mention but I don't think that's part of the scope here. We definitely should do it (and add a test!) though.
|
|
Now, port and protocol would make sense to promote to config options as it
would allow us to open the firewalls through a toggle as you mention but I
don't think that's part of the scope here. We definitely should do it (and
add a test!) though.
Actually, just having thought of it a bit more, it's going to be tricky with the port/protocol.
We can run in either server or client mode and the proto/port is specified differently depending on the mode so we need to add a mode as well and act differently based on that. Considering the flexibility/complexity in the configuration file, it might not be worth trying to model that using module options.
|
| @@ -211,6 +226,24 @@ in | |||
|
|
|||
| boot.kernelModules = [ "tun" ]; | |||
|
|
|||
| boot.kernel.sysctl = lib.mkIf cfg.enableForwarding ({ | |||
| "net.ipv4.ip_forward" = true; | |||
| } // (if config.networking.enableIPv6 then { | |||
|
I just wanted to mention that since it's options I would typically expect in network-related modules. It's completely fine if it's too hard to implement or too convoluted, you know better :p |
I fully with you - it makes sense to have this type of stuff standard across the board but here I think it would just add complexity for not a lot of gain. |
|
For the author, reviewers, and committers: this PR was scanned and appears to add a use of the deprecated
|
|
Hello, I'm a bot and I thank you in the name of the community for your contributions. Nixpkgs is a busy repository, and unfortunately sometimes PRs get left behind for too long. Nevertheless, we'd like to help committers reach the PRs that are still important. This PR has had no activity for 180 days, and so I marked it as stale, but you can rest assured it will never be closed by a non-human. If this is still important to you and you'd like to remove the stale label, we ask that you leave a comment. Your comment can be as simple as "still important to me". But there's a bit more you can do: If you received an approval by an unprivileged maintainer and you are just waiting for a merge, you can @ mention someone with merge permissions and ask them to help. You might be able to find someone relevant by using Git blame on the relevant files, or via GitHub's web interface. You can see if someone's a member of the nixpkgs-committers team, by hovering with the mouse over their username on the web interface, or by searching them directly on the list. If your PR wasn't reviewed at all, it might help to find someone who's perhaps a user of the package or module you are changing, or alternatively, ask once more for a review by the maintainer of the package/module this is about. If you don't know any, you can use Git blame on the relevant files, or GitHub's web interface to find someone who touched the relevant files in the past. If your PR has had reviews and nevertheless got stale, make sure you've responded to all of the reviewer's requests / questions. Usually when PR authors show responsibility and dedication, reviewers (privileged or not) show dedication as well. If you've pushed a change, it's possible the reviewer wasn't notified about your push via email, so you can always officially request them for a review, or just @ mention them and say you've addressed their comments. Lastly, you can always ask for help at our Discourse Forum, or more specifically, at this thread or at #nixos' IRC channel. |
stale
bot
added
the
2.status: stale
stale
bot
removed
the
2.status: stale
ofborg
bot
added
10.rebuild-linux: 1-10
10.rebuild-linux: 1
and removed
10.rebuild-linux: 0
labels
|
I marked this as stale due to inactivity. → More info |
stale
bot
added
the
2.status: stale
stale
bot
removed
the
2.status: stale
|
Been running with this for ages. Should be good to go - just cleaned up the commit. |
We change a few things here: a) Create the local openvpn user and group so that openvpn can drop privileges. This is not switched on by default. b) Define an option to enable IP forwarding which would have to be defined outside of the openvpn module. c) Create a local state and runtime directories for anything openvpn might create (replay persistence logs, current connections, etc).
wegank
added
2.status: stale
stale
bot
removed
the
2.status: stale
Motivation for this change
We change a few things here:
a) Create the local openvpn user and group so that openvpn can drop privileges.
This is not switched on by default.
b) Define an option to enable IP forwarding which would have to be defined
outside of the openvpn module.
c) Create a local state and runtime directories for anything openvpn might create (replay
persistence logs, current connections, etc).
d) Launch openvpn in a separate
openvpn.sliceslice and activate from a dedicatedopenvpn.targettarget. The former allows one to easily check the logs across all instances, apply resource limitations and stop all instances. The latter allows you to easily start all the instances.NOTE: I strongly recommend looking at this diff while ignoring whitespace due to the indent changes (options were indented further due to the introduction of a new option) - https://github.com/NixOS/nixpkgs/pull/65878/files?w=1
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nix-review --run "nix-review wip"./result/bin/)nix path-info -Sbefore and after)Notify maintainers
cc @viric