OpenVPN / openvpn-gui

OpenVPN GUI is a graphical frontend for OpenVPN running on Windows 7 / 8 / 10. It creates an icon in the notification area from which you can control OpenVPN to start/stop your VPN tunnels, view the log and do other useful things.
Other
1.42k stars 400 forks source link

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

Open selvanair opened 6 years ago

selvanair commented 6 years ago

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 commented 6 years ago

we do not use interactive service if user is administrator

selvanair commented 6 years ago

On Thu, Jun 21, 2018 at 1:15 PM, Ilya Shipitsin notifications@github.com wrote:

we do not use interactive service if user is administrator

As I tried to explain in the post "admin or not" is unlreated to why we may want to disable scripts. Sounds like I shouldn't have said anything about interactive service.

Anyway, not using the iservice when admin is by design. We could revisit this as Vista is no longer supported but let's discuss that in a separate thread.

Selva

cron2 commented 6 years ago

Hi,

On Thu, Jun 21, 2018 at 09:43:58AM -0700, Selva Nair wrote:

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.

Short version: yes.

Long version: I have been thinking how to tackle this, and one of the ideas floating around was to have a dial in the GUI that influences a --script-security command line being passed to the openvpn.exec process, at the end of the command line (overriding whatever is in the config).

Default would be --script-security 0 or 1 ("nothing" or "netsh.exe is permitted", according to the docs) but if a user really feels like they know what they are doing they can set it to 2 or 3.

Longer version: David wants full blown policy config ("this instance is allowed to do <list of things, nothing else>") which I find too heavy :-)

What are your ideas how to tackle this?

gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de

selvanair commented 6 years ago

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 commented 6 years ago

Hi,

On Thu, Jun 21, 2018 at 11:54:48AM -0700, Selva Nair wrote:

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.

Feature-ACK :-)

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 even a user that has "enough rights" might have been tricked into copying the config into c:\programs\openvpn\config\ (copy as admin), so openvpn-gui will "see" that config without having "imported" it - the user might be careful enough to run openvpn-gui as non-admin, so the assumption "if it's a non-admin, config files in the admin place have been properly scrutinized" won't hold.

I only have users that run "no scripts at all on windows", so I'm not sure how much breakage it would cause to make this "off by default for all newly discovered configs" (= not only imported, but also pre-existing and copied by hand). The "mount file systems after openvpn startup" example won't work with openvpn scripts anyway :-)

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.

Right.

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.

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".

OTOH the attack "here's a nice and shiny and complete .ovpn, just copy it and it will make your Internet access secure!" is easy enough that it will work in many cases...

gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de

selvanair commented 6 years ago

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 commented 6 years ago

Hi,

On Thu, Jun 21, 2018 at 02:23:20PM -0700, Selva Nair wrote: [..]

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.

Yep. Checking mtime, and revoking per-config permissions.

Do we want a global "I know what I'm doing leave me alone!" switch that disables --script-security overriding?

Most interesting question :-) - do you have time to whip up a prototype?

In theory, we have a release coming up "soonish" (if we ever get the TAP driver signing sorted out) and this would be a nice fit...

gert

-- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de

selvanair commented 6 years ago

Hi

On Fri, Jun 29, 2018 at 4:42 AM, Gert Doering notifications@github.com wrote:

Hi,

On Thu, Jun 21, 2018 at 02:23:20PM -0700, Selva Nair wrote: [..]

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.

Yep. Checking mtime, and revoking per-config permissions.

mtime may not be good enough as (i) I think its not updated until the file handle that makes the change is closed (ii) if FAT is in use filetimes change with location (local time), have low resolution etc (iii) when copied over, the mtime comes from the source so it could have any value including the original..

My prototype (see below) uses a file hash and locks the write access when a connection is active (as config is re-read during sighup restarts). Am I over-thinking this?

Do we want a global "I know what I'm doing leave me alone!" switch that disables --script-security overriding?

I would rather not have an easy to toggle a global switch. Unless users scream.. We could have a global registry entry (not exposed to the UI) to be set by an admin user.

Most interesting question :-) - do you have time to whip up a prototype?

Yes I already have something coded up waiting for the weekend to test.

Here is the current status (it builds but totally untested)

https://github.com/selvanair/openvpn-gui/commits/script-security

It has grown to something more complex than I anticipated, so any feedback would help.

Selva

dsommers commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Hi,

On Tue, Jul 03, 2018 at 01:12:07AM -0700, David Sommerseth wrote:

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.

This is what this PR is about :-)

gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de

selvanair commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

Hi,

On Tue, Jul 03, 2018 at 01:23:36PM -0700, David Sommerseth wrote:

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

Which is reasonable for a client-side "click-to-conenct" VPN thingie.

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.

So having the GUIs disable script execution (in an appropriate way) is a good way forward, but removing the script capabilities would be reducing OpenVPN 2.x usefulness.

gert

-- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de

dsommers commented 6 years ago

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 commented 6 years ago

Hi,

On Tue, Jul 03, 2018 at 01:36:51PM -0700, David Sommerseth wrote:

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.

This is far from "easy". Right now you have a single process, you start it with your config file, and it will do the job for you - in a very easy and straightforward way.

Requiring users to write their own management client to get access to pre-existing functionality would be a major step back.

[..]

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.

gert

-- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de

selvanair commented 6 years ago

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 commented 6 years ago

Hi,

On Tue, Jul 03, 2018 at 08:51:00PM -0700, Selva Nair wrote:

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.

This is actually quite an interesting idea.

And we should not dismiss what David proposed when this was discussed last round - have an external, admin-maintained, policy file that governs what OpenVPN is willing to do. I won't object to having a default policy on Linux installed by the distribution that states something like

default up, down: reject mynamedrule: up, down: permit

(or however we want to build the syntax of this, and reference particular policy rules from a given config file)

But make this optional, not "ripp out stuff that is the reason why OpenVPN is more useful than more basic VPN programs"

gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de