Re: [Openvpn-devel] [PATCH] Make up/down script errors not FATAL

[Selva Nair]

Hi,

On Tue, Jul 3, 2018 at 3:09 AM, Gert Doering  wrote:

> Hi,
>
> On Mon, Jul 02, 2018 at 11:13:01PM -0400, Jonathan K. Bullard wrote:
> > My initial reaction is that I'd rather a problem in the up/down
> > scripts generates a fatal error, so if there's a problem in the
> > Tunnelblick scripts somebody will report it. In my experience, almost
> > nobody pays attention to warnings, and mostly, those who do are
> > worried about warning that don't matter.
>
> From how I read Selva's mail, an error in the script will still create
> a fatal error.
>
> The difference is that today, if you have --script-security 1 and a --up
> config, that combination will cause an error, while after the change, this
> will only cause a warning.
>
> Selva, did I read that correctly?
>

Unfortunately no. This patch will trigger only a warning for both a script
error
and inability execute the script due to script-security setting.

If actual errors in up/down scripts should trigger M_FATAL, we can change
the
patch to just bypass the script execution if script security is < 2. It
would be a
bit ugly like this:

-openvpn_run_script(, es, 0, "--up/--down");
+   openvpn_run_script(, es, (script_security >= SSEC_SCRIPTS)?
S_FATAL : 0, "--up/--down");


For some reason the code path involved is somewhat convoluted:

First we log a warning that external scripts require script_security >= 2.
But fully knowing its going to fail we still call openvpn_run_script(). The
flag
that say error out or warn is set in this call and script permission is
checked just before executing:

openvpn_run_script() --> openvpn_execve_check() --> openvpn_execve_allowed()

When the latter returns an error due to script-security,
openvpn_execve_check()
fails with a slightly misleading message.

Selva
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Make up/down script errors not FATAL

[Antonio Quartulli]

Hi,

On 03/07/18 16:23, David Sommerseth wrote:
> TL;DR: Reduce the possibility to run scripts to an absolute minimum (if at
> all).  If having this possibility run them with as few privileges as possible,
> and scripts to run is preferred to be configured outside of the OpenVPN
> configuration file.
> 
> The latter argument of configuring scripts outside of the configuration file
> is simply trying to end up with a single configuration file which would be
> functional on all devices.  A configuration file with Windows scripts won't
> work on a non-Windows box and vice versa - some configuration files might not
> even work across Linux distributions even.  So let the OpenVPN configuration
> files be as generic as possible, focusing on getting a connection to a remote
> server.  And configure the rest outside of the OpenVPN configuration profile.
> 

I have previously proposed to use an udev-compatible mechanism to run
scripts.
In this scenario OpenVPN only needs to trigger "signals" and then
whoever is listening (i.e. udev/hotplug) will take care of handling
them. This could even be DBus driven.

However, this can work on Linux. Anybody knows of a similar mechanism
for Windows and macOS?


Cheers,


-- 
Antonio Quartulli



signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Make up/down script errors not FATAL

[David Sommerseth]

On 03/07/18 09:49, Selva Nair wrote:
> Hi Jon,
> 
> On Mon, Jul 2, 2018 at 11:13 PM, Jonathan K. Bullard  > wrote:
>> Hi.
>>
>> On Mon, Jul 2, 2018 at 9:24 PM,  > wrote:
>>>
>>> From: Selva Nair mailto:selva.n...@gmail.com>>
>>>
>>> Instead log only a warning.
>>>
>>> This helps user interfaces enforce a safer script-security setting
>>> without causing a FATAL error.
>>
>>
>> Can you expand on that? What "safer script secuity settings' do you
>> have in mind? Tunnelblick (and I think all Linux) use script-security
>> 2 to allow for up/down scripts that implement DNS and other settings.
>>
>> My initial reaction is that I'd rather a problem in the up/down
>> scripts generates a fatal error, so if there's a problem in the
>> Tunnelblick scripts somebody will report it. In my experience, almost
>> nobody pays attention to warnings, and mostly, those who do are
>> worried about warning that don't matter.

+1

> 
> This is in reaction to
> 
> https://medium.com/tenable-techblog/reverse-shell-from-an-openvpn-configuration-file-73fd8b1d38da
> 
> 
> In OpenVPN Windows GUI I'm considering to enforce "--script-security 1"
> (SSEC_BUILT_IN). See the discussion here:
>
> https://github.com/OpenVPN/openvpn-gui/issues/270


This I am much more in favour of.  I've already added a longer GitHub comment
with a bit different perspective, as well as looking more into the future of
what we're doing with OpenVPN 3 - where OpenVPN processes generally will not
run any scripts or even support it.

TL;DR: Reduce the possibility to run scripts to an absolute minimum (if at
all).  If having this possibility run them with as few privileges as possible,
and scripts to run is preferred to be configured outside of the OpenVPN
configuration file.

The latter argument of configuring scripts outside of the configuration file
is simply trying to end up with a single configuration file which would be
functional on all devices.  A configuration file with Windows scripts won't
work on a non-Windows box and vice versa - some configuration files might not
even work across Linux distributions even.  So let the OpenVPN configuration
files be as generic as possible, focusing on getting a connection to a remote
server.  And configure the rest outside of the OpenVPN configuration profile.


-- 
kind regards,

David Sommerseth
OpenVPN Inc




signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Make up/down script errors not FATAL

[Gert Doering]

Hi,

On Mon, Jul 02, 2018 at 11:13:01PM -0400, Jonathan K. Bullard wrote:
> My initial reaction is that I'd rather a problem in the up/down
> scripts generates a fatal error, so if there's a problem in the
> Tunnelblick scripts somebody will report it. In my experience, almost
> nobody pays attention to warnings, and mostly, those who do are
> worried about warning that don't matter.

From how I read Selva's mail, an error in the script will still create
a fatal error.  

The difference is that today, if you have --script-security 1 and a --up 
config, that combination will cause an error, while after the change, this 
will only cause a warning.

Selva, did I read that correctly?

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 g...@greenie.muc.de


signature.asc
Description: PGP signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Make up/down script errors not FATAL

[Selva Nair]

Hi Jon,

On Mon, Jul 2, 2018 at 11:13 PM, Jonathan K. Bullard 
wrote:
> Hi.
>
> On Mon, Jul 2, 2018 at 9:24 PM,  wrote:
>>
>> From: Selva Nair 
>>
>> Instead log only a warning.
>>
>> This helps user interfaces enforce a safer script-security setting
>> without causing a FATAL error.
>
>
> Can you expand on that? What "safer script secuity settings' do you
> have in mind? Tunnelblick (and I think all Linux) use script-security
> 2 to allow for up/down scripts that implement DNS and other settings.
>
> My initial reaction is that I'd rather a problem in the up/down
> scripts generates a fatal error, so if there's a problem in the
> Tunnelblick scripts somebody will report it. In my experience, almost
> nobody pays attention to warnings, and mostly, those who do are
> worried about warning that don't matter.

This is in reaction to

https://medium.com/tenable-techblog/reverse-shell-from-an-
openvpn-configuration-file-73fd8b1d38da

In OpenVPN Windows GUI I'm considering to enforce "--script-security 1"
(SSEC_BUILT_IN). See the discussion here:

https://github.com/OpenVPN/openvpn-gui/issues/270

Selva
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Make up/down script errors not FATAL

[Jonathan K. Bullard]

Hi.

On Mon, Jul 2, 2018 at 9:24 PM,  wrote:
>
> From: Selva Nair 
>
> Instead log only a warning.
>
> This helps user interfaces enforce a safer script-security setting
> without causing a FATAL error.


Can you expand on that? What "safer script secuity settings' do you
have in mind? Tunnelblick (and I think all Linux) use script-security
2 to allow for up/down scripts that implement DNS and other settings.

My initial reaction is that I'd rather a problem in the up/down
scripts generates a fatal error, so if there's a problem in the
Tunnelblick scripts somebody will report it. In my experience, almost
nobody pays attention to warnings, and mostly, those who do are
worried about warning that don't matter.

Best regards,

Jon Bullard

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Make up/down script errors not FATAL

[selva . nair]

From: Selva Nair 

Instead log only a warning.

This helps user interfaces enforce a safer script-security setting
without causing a FATAL error.

Signed-off-by: Selva Nair 
---
Note: All other scripts are called with flag = 0 and will only
trigger a warning message if openvpn_execve fails.

 src/openvpn/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index b748357..6673734 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -174,7 +174,7 @@ run_up_down(const char *command,
 argv_printf_cat(, "%s %d %d %s %s %s", arg, tun_mtu, link_mtu,
 ifconfig_local, ifconfig_remote, context);
 argv_msg(M_INFO, );
-openvpn_run_script(, es, S_FATAL, "--up/--down");
+openvpn_run_script(, es, 0, "--up/--down");
 argv_reset();
 }
 
-- 
2.1.4


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel