| Message ID | 1530581086-7825-1-git-send-email-selva.nair@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Openvpn-devel] Make up/down script errors not FATAL | expand |
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 <<a href="mailto:jkbullard@gmail.com" target="_blank">jkbullard@gmail.com</a>> wrote:<br>> Hi.<br>><br>> On Mon, Jul 2, 2018 at 9:24 PM, <<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>> wrote:<br>>><br>>> From: Selva Nair <<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>><br>>><br>>> Instead log only a warning.<br>>><br>>> This helps user interfaces enforce a safer script-security setting<br>>> without causing a FATAL error.<br>><br>><br>> Can you expand on that? What "safer script secuity settings' do you<br>> have in mind? Tunnelblick (and I think all Linux) use script-security<br>> 2 to allow for up/down scripts that implement DNS and other settings.<br>><br>> My initial reaction is that I'd rather a problem in the up/down<br>> scripts generates a fatal error, so if there's a problem in the<br>> Tunnelblick scripts somebody will report it. In my experience, almost<br>> nobody pays attention to warnings, and mostly, those who do are<br>> worried about warning that don'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'm considering to enforce "--script-security 1" (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"><<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>></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> > My initial reaction is that I'd rather a problem in the up/down<br> > scripts generates a fatal error, so if there's a problem in the<br> > Tunnelblick scripts somebody will report it. In my experience, almost<br> > nobody pays attention to warnings, and mostly, those who do are<br> > worried about warning that don't matter.<br> <br> </span>From how I read Selva'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 < 2. It would be a</div><div>bit ugly like this:</div><div><br></div><div><div>- openvpn_run_script(&argv, es, 0, "--up/--down");</div><div>+ openvpn_run_script(&argv, es, (script_security >= SSEC_SCRIPTS)? S_FATAL : 0, "--up/--down");</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 >= 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() --> openvpn_execve_check() --> 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
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(&argv, "%s %d %d %s %s %s", arg, tun_mtu, link_mtu, ifconfig_local, ifconfig_remote, context); argv_msg(M_INFO, &argv); - openvpn_run_script(&argv, es, S_FATAL, "--up/--down"); + openvpn_run_script(&argv, es, 0, "--up/--down"); argv_reset(&argv); }