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

  From: Selva Nair <selva.nair@gmail.com>

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 <selva.nair@gmail.com>
---
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(-)
  

Hi.

On Mon, Jul 2, 2018 at 9:24 PM, <selva.nair@gmail.com> wrote:
>
> From: Selva Nair <selva.nair@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.

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
  

Hi Jon,

On Mon, Jul 2, 2018 at 11:13 PM, Jonathan K. Bullard <jkbullard@gmail.com>
wrote:
> Hi.
>
> On Mon, Jul 2, 2018 at 9:24 PM, <selva.nair@gmail.com> wrote:
>>
>> From: Selva Nair <selva.nair@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.

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
<div dir="ltr">Hi Jon,<br><br>On Mon, Jul 2, 2018 at 11:13 PM, Jonathan K. Bullard &lt;<a href="mailto:jkbullard@gmail.com" target="_blank">jkbullard@gmail.com</a>&gt; wrote:<br>&gt; Hi.<br>&gt;<br>&gt; On Mon, Jul 2, 2018 at 9:24 PM, &lt;<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>&gt; wrote:<br>&gt;&gt;<br>&gt;&gt; From: Selva Nair &lt;<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>&gt;<br>&gt;&gt;<br>&gt;&gt; Instead log only a warning.<br>&gt;&gt;<br>&gt;&gt; This helps user interfaces enforce a safer script-security setting<br>&gt;&gt; without causing a FATAL error.<br>&gt;<br>&gt;<br>&gt; Can you expand on that? What &quot;safer script secuity settings&#39; do you<br>&gt; have in mind? Tunnelblick (and I think all Linux) use script-security<br>&gt; 2 to allow for up/down scripts that implement DNS and other settings.<br>&gt;<br>&gt; My initial reaction is that I&#39;d rather a problem in the up/down<br>&gt; scripts generates a fatal error, so if there&#39;s a problem in the<br>&gt; Tunnelblick scripts somebody will report it. In my experience, almost<br>&gt; nobody pays attention to warnings, and mostly, those who do are<br>&gt; worried about warning that don&#39;t matter.<br><br>This is in reaction to<br><br><a href="https://medium.com/tenable-techblog/reverse-shell-from-an-openvpn-configuration-file-73fd8b1d38da" target="_blank">https://medium.com/tenable-tec<wbr>hblog/reverse-shell-from-an-<wbr>openvpn-configuration-file-73f<wbr>d8b1d38da</a><br><br>In OpenVPN Windows GUI I&#39;m considering to enforce &quot;--script-security 1&quot; (SSEC_BUILT_IN). See the discussion here:<br><br><a href="https://github.com/OpenVPN/openvpn-gui/issues/270" target="_blank">https://github.com/OpenVPN/ope<wbr>nvpn-gui/issues/270</a><div><br><div>Selva</div></div></div>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
  

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
  

On 03/07/18 09:49, Selva Nair wrote:
> Hi Jon,
> 
> On Mon, Jul 2, 2018 at 11:13 PM, Jonathan K. Bullard <jkbullard@gmail.com
> <mailto:jkbullard@gmail.com>> wrote:
>> Hi.
>>
>> On Mon, Jul 2, 2018 at 9:24 PM, <selva.nair@gmail.com
> <mailto:selva.nair@gmail.com>> wrote:
>>>
>>> From: Selva Nair <selva.nair@gmail.com <mailto:selva.nair@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
> <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.
  

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,
  

Hi,

On Tue, Jul 3, 2018 at 3:09 AM, Gert Doering <gert@greenie.muc.de> 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(&argv, es, 0, "--up/--down");
+       openvpn_run_script(&argv, 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
<div dir="ltr">Hi,<br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jul 3, 2018 at 3:09 AM, Gert Doering <span dir="ltr">&lt;<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<span class="m_4065982707826998152gmail-"><br>
On Mon, Jul 02, 2018 at 11:13:01PM -0400, Jonathan K. Bullard wrote:<br>
&gt; My initial reaction is that I&#39;d rather a problem in the up/down<br>
&gt; scripts generates a fatal error, so if there&#39;s a problem in the<br>
&gt; Tunnelblick scripts somebody will report it. In my experience, almost<br>
&gt; nobody pays attention to warnings, and mostly, those who do are<br>
&gt; worried about warning that don&#39;t matter.<br>
<br>
</span>From how I read Selva&#39;s mail, an error in the script will still create<br>
a fatal error.  <br>
<br>
The difference is that today, if you have --script-security 1 and a --up <br>
config, that combination will cause an error, while after the change, this <br>
will only cause a warning.<br>
<br>
Selva, did I read that correctly?<br></blockquote><div><br></div><div>Unfortunately no. This patch will trigger only a warning for both a script error</div><div>and inability execute the script due to script-security setting.</div><div><br></div><div>If actual errors in up/down scripts should trigger M_FATAL, we can change the</div><div>patch to just bypass the script execution if script security is &lt; 2. It would be a</div><div>bit ugly like this:</div><div><br></div><div><div>-        openvpn_run_script(&amp;argv, es, 0, &quot;--up/--down&quot;);</div><div>+       openvpn_run_script(&amp;argv, es, (script_security &gt;= SSEC_SCRIPTS)? S_FATAL : 0, &quot;--up/--down&quot;);</div></div><div><br></div><div><br></div><div>For some reason the code path involved is somewhat convoluted:</div><div><br></div><div>First we log a warning that external scripts require script_security &gt;= 2.</div><div>But fully knowing its going to fail we still call openvpn_run_script(). The flag</div><div>that say error out or warn is set in this call and script permission is</div><div>checked just before executing:</div><div><br></div><div>openvpn_run_script() --&gt; openvpn_execve_check() --&gt; openvpn_execve_allowed()</div><div><br></div><div>When the latter returns an error due to script-security, openvpn_execve_check()</div><div>fails with a slightly misleading message.</div><div><br></div><div>Selva</div></div></div></div>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot