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

Shall we disable scripts (run by openvpn.exe) by default ? #270

Open
selvanair opened this issue Jun 21, 2018 · 18 comments
Open

Shall we disable scripts (run by openvpn.exe) by default ? #270

selvanair opened this issue Jun 21, 2018 · 18 comments

Comments

@selvanair
Copy link
Collaborator

Considering the recent report on newer ways to exploit scripts in the ovpn file, I propose to disable all such scripts when a connection is started using the GUI (easy to do). We can still allow scripts that are executed by the GUI itself as those require explicitly named batch files to exist in the config folder.

Note that, in 2.4.x releases, we use the interactive service to start openvpn.exe, so privilege escalation though scripts is not a concern[*]. But exploits through commands embedded in the config (.ovpn) files is a valid threat as we currently do not have a way of signing config files.

[*] Some other GUI implementations disable scripts because of the possibility of privilege escalation as they run openvpn.exe with high privileges even if the user does not have those rights.

@chipitsine
Copy link
Contributor

we do not use interactive service if user is administrator

@selvanair
Copy link
Collaborator Author

selvanair commented Jun 21, 2018 via email

@cron2
Copy link
Contributor

cron2 commented Jun 21, 2018 via email

@selvanair
Copy link
Collaborator Author

By "easy to do" what I had in mind was exactly that: pass --script-security 0 to the end of the command line with an menu option to change that to 1 or 2 or to disable the override (letting the config file value take over). The menu option will have to be at a per config level to be really useful. But to avoid breaking existing configs we could decide to impose this only for newly added ones (added through the import menu).

On Windows we already have some kind of policy restrictions in the config (assuming iservice is in use): if the user is not admin (or not an OpenVPN Administrator) the config file should be installed in the global location (required admin rights) or only a limited set of options are allowed in the config or command line.

But extending that to handle the threat at hand (which is not privilege related) will put put too much burden on users: to be effective we'll have to lock down all configs by default and then the user has to tweak some flags to allow each extra feature. When a config is edited we'll have to revoke the rights and/or add some sophisticated parsing to detect what changed. Sounds too complex to me.

@cron2
Copy link
Contributor

cron2 commented Jun 21, 2018 via email

@selvanair
Copy link
Collaborator Author

selvanair commented Jun 21, 2018

Agreed, treating all configs including global ones as suspicious is safer.

Let's force all configs to script-security = 1 by default (0 may break many setups).

Deciding what defines newly discovered configs is trickier than I thought. Currently
we don't save a list of all configs, so what's the cut-off date for new?

Not sure I'd consider "editing an existing config to add bad stuff to it"
something a user is easily tricked into doing. And if yes, they could be tricked
as easily into "copy this into a .bat file and double click".

Suppose the user already has myprovider.ovpn installed and gets an unverified
email or link with an updated myprovider.ovpn which they unwittingly copies over
the existing one. It would be hard to figure out whether the config was slightly edited
or copied over, so we'll have to invalidate permissions granted to a config when its
modified. At least until we have a built-in config editor.

@cron2
Copy link
Contributor

cron2 commented Jun 29, 2018 via email

@selvanair
Copy link
Collaborator Author

selvanair commented Jun 29, 2018 via email

@dsommers
Copy link
Member

dsommers commented Jul 3, 2018

I've been pondering a lot on these challenges as well, mostly from an OpenVPN 3 perspective (where I am mostly focused these days). In OpenVPN 3, we've basically killed the possibility for external scripts to be run [1]. None of the OpenVPN Connect clients in the mobile world will ever provide such a feature. There has been some discussions what we would allow on the OpenVPN Connect clients on Windows and macOS. For Windows, this gets a bit more tricky when considering UWP and a possibility to get OpenVPN Connect into the Microsoft appstore, but we generally try to avoid running script from an OpenVPN scope.

The openvpn3-linux client (which is my main project) does also not support scripts out-of-the box. But, as this is a D-Bus based client it will issue D-Bus signals when certain events happens, like connecting, connected, paused, resumed, restarted and disconnected. With this in mind, it is possible to develop an "executor" which starts various scripts based on these signals. But that is never executed directly by OpenVPN itself, OpenVPN only triggers the execution. If this "executor" starts running scripts with elevated privileges or highly reduced privileges, is completely up to the implementer.

This ensures a strict separation between OpenVPN and its configuration vs the scripts being run. Which scripts to be run are also never defined via the OpenVPN configuration file (unless the "executor" parses that config file).

So I'm wondering if a similar path would be better for Windows as well: Configure which scripts to be run in certain scenarios in the GUI but outside of the OpenVPN configuration file and let the GUI be the one deciding when and how to run these scripts. If GUI is unprivileged, then scripts are definitely run unprivileged [2].

I know this will break some client configurations used today. So having an option to "re-enable" the old behaviour (by using the --script-security approach discussed here) is needed for some time, or if the GUI imports these scripts to such a new configuration model.

Bottom line is: Starting a path forward where scripts to be run is not taken from the main OpenVPN configuration is actually better to prepare users for what is coming the future.

[1] OpenVPN 3 is "OpenVPN as a library", so the client implementation can implement similar behaviour.
[2] I know OpenVPN-GUI runs openvpn.exe unprivileged normally (unless being run with admin privs), but I'm thinking further ... move away from defined scripts in OpenVPN configuration files.

@dsommers
Copy link
Member

dsommers commented Jul 3, 2018

One more point, if comparing to Linux. Users starting VPN tunnels via NetworkManager also looses the possibility to run any type of scripts. That is simply not supported in NetworkManager.

@cron2
Copy link
Contributor

cron2 commented Jul 3, 2018 via email

@selvanair
Copy link
Collaborator Author

@dsommers

Yes, but :)

The openvpn3-linux client (which is my main project) does also not support scripts out-of-the box. But, as this is a D-Bus based client it will issue D-Bus signals when certain events happens, like connecting, connected, paused, resumed, restarted and disconnected. With this in mind, it is possible to develop an "executor" which starts various scripts based on these signals. But that is never executed directly by OpenVPN itself, OpenVPN only triggers the execution. If this "executor" starts running scripts with elevated privileges or highly reduced privileges, is completely up to the implementer.

This ensures a strict separation between OpenVPN and its configuration vs the scripts being run. Which scripts to be run are also never defined via the OpenVPN configuration file (unless the "executor" parses that config file).

So I'm wondering if a similar path would be better for Windows as well: Configure which scripts to be run in certain scenarios in the GUI but outside of the OpenVPN configuration file and let the GUI be the one deciding when and how to run these scripts. If GUI is unprivileged, then scripts are definitely run unprivileged

The current situation in Windows is better (IMO) than the D-bus based separation. We run only the service with elevated privileges and the whole openvpn process is run as user from start to end. So privilege separation is less of a concern. In any case, the kind of threat that triggered this thread (remote shell) is valid irrespective of the privileges under which the code runs.

So transferring the job of running scripts from openvpn to the GUI is not going to help by itself. We need to treat scripts like how Windows treats macros and embedded scripts in some MS Office files --- (disabled on downloaded files unless the user explicitly enables them). The proposal here is to override the script-security setting so that we can provide an extra safety net to common users who may just download a config file and may not have the expertise to parse it and understand it before using it. In my view that would be "most" users in both Linux and Windows today. On Linux such users would likely use NM which doesn't support scripts, as you mentioned.

Now, in the long run, removing the ability to directly run scripts from openvpn is a good goal, but if the responsibility is transferred to a UI that we distribute, we still have to worry about how to protect users against script-based exploits.

@dsommers
Copy link
Member

dsommers commented Jul 3, 2018

Now, in the long run, removing the ability to directly run scripts from openvpn is a good goal, but if the responsibility is transferred to a UI that we distribute, we still have to worry about how to protect users against script-based exploits.

Regardless of implementation, if OpenVPN runs or triggers a run of scripts, there will always be a concern related to script-based exploits. But there are some use-cases where I believe, with the proper isolation of the execution, the risks are acceptable.

What I (probably not too clear) tried to say, was that scripts to be run must be configured outside of the main OpenVPN configuration (*.ovpn). This could be a separate configuration file, a specific GUI interface or something similar where you can provide the proper paths to the scripts to run for specific events. Any --up, --down, --route-up or --route-pre-down script hooks would simply just be ignored in the main configuration file. Where these (and similiar) script hooks exists today, they should merely just emit some signals that this phase has been reached (via management interface?). And to keep this simple, just ignore any feedback from the scripts back to OpenVPN (at least for now).

@cron2
Copy link
Contributor

cron2 commented Jul 3, 2018 via email

@dsommers
Copy link
Member

dsommers commented Jul 3, 2018

For a server-side (or automated on-router / backgrounded) "I know what I'm doing and this is why I use OpenVPN" setup, these features are useful and important to keep.

This can "easily" be managed by using the management interface instead. Where the execution of scripts can even run as a different user from the OpenVPN process. We're working on some code targeting user authentication for test environments, we can easily enough (once done) shape that code up to something which can at least serve as a good demonstration how to do this. That said, we might need a few extra "signals" in the management interface to be fully compliant with today's features.

But I see the server side aspect of this challenge more as a phase 2. I have primarily focused on the client side, as that is what the vast majority of OpenVPN GUI users uses - which is much more fragile and easier target for script abuse.

Filtering out scripting capabilities can probably in the beginning be tied to --tls-client in the first phase. This ignores that scripts will be available in --mode p2p with static keys; not sure now if it's worth the efforts to even plug that one.

@cron2
Copy link
Contributor

cron2 commented Jul 3, 2018 via email

@selvanair
Copy link
Collaborator Author

But I see the server side aspect of this challenge more as a phase 2. I have primarily focused on the client side, as that is what the vast majority of OpenVPN GUI users uses - which is much more fragile and easier target for script abuse.

Filtering out scripting capabilities can probably in the beginning be tied to --tls-client in the first phase. This ignores that scripts will be available in --mode p2p with static keys; not sure now if it's worth the efforts to plug even that one.

I'm absolutely opposed to breaking OpenVPN core functionality.

Teaching NM, OpenVPN-GUI, Tunnelblick to do the right thing is the right
way to protect "I have no idea what I'm doing" users - but breaking core
functionality would be like "we cannot have sudo anymore, because people
can do things like 'curl ... | sudo bash' with it". No.

I tend to agree with this. There is no need to completely remove the script capability from OpenVPN. Many installations would need both the server and client to be able to execute scripts. Converting that into signals with the execve job delegated elsewhere may make the code more modular, but it does not improve security. Recall that we are not talking about privilege separation here -- that's a different topic.

What we should do is to eliminate "unexpected" surprises. So make it a little harder and require explicit user action to run scripts when an automated, "user-friendly" approach is in use, e.g., when using a GUI. We could have an option like --management-query-scripts to optionally ask the management before running a script. A UI/GUI could start the daemon with such an option, get prompted before a script is run so that the UI can brief the user and take their response or use a previously saved preference.

Keep it simple, complicating things only increases the attack surface, not protect anyone.

As for scripts run by the GUI, on Windows we already have pre-connect, connect and disconnect scripts with hard coded names.

@cron2
Copy link
Contributor

cron2 commented Jul 4, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants