| Message ID | 1531491395-29049-1-git-send-email-selva.nair@gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [Openvpn-devel,v3] Make up/down script errors not FATAL | expand |
Hi, On 13-07-18 16:16, selva.nair@gmail.com wrote: > From: Selva Nair <selva.nair@gmail.com> > > The error is treated as a warning only if its triggered due > to script_security < SSEC_SCRIPTS. > > This helps user interfaces enforce a safer script-security setting > without causing a FATAL error. But does it make sense at all to accept configs that have a --up script without a sufficiently-high script-security set? I do agree that the current place where the code checks this not nice, so maybe we should perform this check somewhere in the options_postprocess_verify* functions? That way we error out early, instead of only when trying to execute the script. -Steffan
Hi, On Fri, Oct 5, 2018 at 5:44 AM Steffan Karger <steffan@karger.me> wrote: > Hi, > > On 13-07-18 16:16, selva.nair@gmail.com wrote: > > From: Selva Nair <selva.nair@gmail.com> > > > > The error is treated as a warning only if its triggered due > > to script_security < SSEC_SCRIPTS. > > > > This helps user interfaces enforce a safer script-security setting > > without causing a FATAL error. > > But does it make sense at all to accept configs that have a --up script > without a sufficiently-high script-security set? > This came out of a proposed patch for the GUI to protect lay users from malicious scripts embedded in config files. Recall the ado about exploiting scripts using unsuspecting "inline" commands. To defeat such exploits we want to enforce a safer script security setting from the GUI but do not want to cause a FATAL error as that would be counter productive. Please see GUI PR #271 https://github.com/OpenVPN/openvpn-gui/pull/271 and my comment dated Jul 3 under it. The discussion that led to this is here: https://github.com/OpenVPN/openvpn-gui/issues/270 Thanks, Selva <div dir="ltr"><div dir="ltr"><div dir="ltr">Hi,<br><div><br><div class="gmail_quote"><div dir="ltr">On Fri, Oct 5, 2018 at 5:44 AM Steffan Karger <<a href="mailto:steffan@karger.me" target="_blank">steffan@karger.me</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br> <br> On 13-07-18 16:16, <a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a> wrote:<br> > From: Selva Nair <<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>><br> > <br> > The error is treated as a warning only if its triggered due<br> > to script_security < SSEC_SCRIPTS.<br> > <br> > This helps user interfaces enforce a safer script-security setting<br> > without causing a FATAL error.<br> <br> But does it make sense at all to accept configs that have a --up script<br> without a sufficiently-high script-security set?<br></blockquote><div><br>This came out of a proposed patch for the GUI to protect lay users from malicious<br></div><div>scripts embedded in config files.<br></div><div><br>Recall the ado about exploiting scripts using unsuspecting "inline" commands.<br>To defeat such exploits we want to enforce a safer script security setting from<br>the GUI but do not want to cause a FATAL error as that would be counter productive.<br><br></div><div>Please see GUI PR #271 <a href="https://github.com/OpenVPN/openvpn-gui/pull/271" target="_blank">https://github.com/OpenVPN/openvpn-gui/pull/271</a> and <br>my comment dated Jul 3 under it.<br></div><div><br></div><div>The discussion that led to this is here:<br><a href="https://github.com/OpenVPN/openvpn-gui/issues/270">https://github.com/OpenVPN/openvpn-gui/issues/270</a><br><br></div><div>Thanks,<br></div><br></div><div class="gmail_quote">Selva<br></div></div></div></div></div>
Hi, I've postponed replying to this mail a couple of times because I felt I missed something and needed to look closer, but today once again I don't get it. So here goes for a potentially stupid reply: On 05-10-18 17:30, Selva Nair wrote: > On Fri, Oct 5, 2018 at 5:44 AM Steffan Karger <steffan@karger.me > <mailto:steffan@karger.me>> wrote: > > Hi, > > On 13-07-18 16:16, selva.nair@gmail.com > <mailto:selva.nair@gmail.com> wrote: > > From: Selva Nair <selva.nair@gmail.com <mailto:selva.nair@gmail.com>> > > > > The error is treated as a warning only if its triggered due > > to script_security < SSEC_SCRIPTS. > > > > This helps user interfaces enforce a safer script-security setting > > without causing a FATAL error. > > But does it make sense at all to accept configs that have a --up script > without a sufficiently-high script-security set? > > > This came out of a proposed patch for the GUI to protect lay users from > malicious > scripts embedded in config files. > > Recall the ado about exploiting scripts using unsuspecting "inline" > commands. > To defeat such exploits we want to enforce a safer script security > setting from > the GUI but do not want to cause a FATAL error as that would be counter > productive. > > Please see GUI PR #271 https://github.com/OpenVPN/openvpn-gui/pull/271 and > my comment dated Jul 3 under it. > > The discussion that led to this is here: > https://github.com/OpenVPN/openvpn-gui/issues/270 I fail to understand why you believe a fatal error in this case is counterproductive. A config file that has a script configured, but is not allowed to run that script is a faulty setup that might result is connections that are not working properly or maybe even not providing the expected security level. Consider for example someone changing DNS or proxy settings in a --up script to avoid leaking data. Most users will never reads warnings if the connection pretends to be set up correctly. This makes me believe that the best thing we can do is to error out if someone tries to connect using such a configuration, and have them review their configuration. Why is a fatal error not exactly what you would want? -Steffan
Hi Steffan, On Sun, Nov 11, 2018 at 8:43 AM Steffan Karger <steffan@karger.me> wrote: > Hi, > > I've postponed replying to this mail a couple of times because I felt I > missed something and needed to look closer, but today once again I don't > get it. So here goes for a potentially stupid reply: > Not so fast, likely its my stupidity to push for this. > > On 05-10-18 17:30, Selva Nair wrote: > > On Fri, Oct 5, 2018 at 5:44 AM Steffan Karger <steffan@karger.me > > <mailto:steffan@karger.me>> wrote: > > > > Hi, > > > > On 13-07-18 16:16, selva.nair@gmail.com > > <mailto:selva.nair@gmail.com> wrote: > > > From: Selva Nair <selva.nair@gmail.com <mailto: > selva.nair@gmail.com>> > > > > > > The error is treated as a warning only if its triggered due > > > to script_security < SSEC_SCRIPTS. > > > > > > This helps user interfaces enforce a safer script-security setting > > > without causing a FATAL error. > > > > But does it make sense at all to accept configs that have a --up > script > > without a sufficiently-high script-security set? > > > > > > This came out of a proposed patch for the GUI to protect lay users from > > malicious > > scripts embedded in config files. > > > > Recall the ado about exploiting scripts using unsuspecting "inline" > > commands. > > To defeat such exploits we want to enforce a safer script security > > setting from > > the GUI but do not want to cause a FATAL error as that would be counter > > productive. > > > > Please see GUI PR #271 https://github.com/OpenVPN/openvpn-gui/pull/271 > and > > my comment dated Jul 3 under it. > > > > The discussion that led to this is here: > > https://github.com/OpenVPN/openvpn-gui/issues/270 > > I fail to understand why you believe a fatal error in this case is > counterproductive. > > A config file that has a script configured, but is not allowed to run > that script is a faulty setup that might result is connections that are > not working properly or maybe even not providing the expected security > level. Consider for example someone changing DNS or proxy settings in a > --up script to avoid leaking data. Most users will never reads warnings > if the connection pretends to be set up correctly. This makes me believe > that the best thing we can do is to error out if someone tries to > connect using such a configuration, and have them review their > configuration. > > Why is a fatal error not exactly what you would want? > When you put it that way I can't disagree. Either I hadn't thought through it properly, or have forgotten my reasoning (it has been a while since I tested the GUI patch that prompted this). I will drop this patch at least for now and go ahead with the feature to override script security setting from the GUI. Thanks for your time, and sorry for wasting it. Selva <div dir="ltr"><br>Hi Steffan,<div><br><div class="gmail_quote"><div dir="ltr">On Sun, Nov 11, 2018 at 8:43 AM Steffan Karger <<a href="mailto:steffan@karger.me">steffan@karger.me</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br> <br> I've postponed replying to this mail a couple of times because I felt I<br> missed something and needed to look closer, but today once again I don't<br> get it. So here goes for a potentially stupid reply:<br></blockquote><div><br></div><div>Not so fast, likely its my stupidity to push for this.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br> On 05-10-18 17:30, Selva Nair wrote:<br> > On Fri, Oct 5, 2018 at 5:44 AM Steffan Karger <<a href="mailto:steffan@karger.me" target="_blank">steffan@karger.me</a><br> > <mailto:<a href="mailto:steffan@karger.me" target="_blank">steffan@karger.me</a>>> wrote:<br> > <br> > Hi,<br> > <br> > On 13-07-18 16:16, <a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a><br> > <mailto:<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>> wrote:<br> > > From: Selva Nair <<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a> <mailto:<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>>><br> > ><br> > > The error is treated as a warning only if its triggered due<br> > > to script_security < SSEC_SCRIPTS.<br> > ><br> > > This helps user interfaces enforce a safer script-security setting<br> > > without causing a FATAL error.<br> > <br> > But does it make sense at all to accept configs that have a --up script<br> > without a sufficiently-high script-security set?<br> > <br> > <br> > This came out of a proposed patch for the GUI to protect lay users from<br> > malicious<br> > scripts embedded in config files.<br> > <br> > Recall the ado about exploiting scripts using unsuspecting "inline"<br> > commands.<br> > To defeat such exploits we want to enforce a safer script security<br> > setting from<br> > the GUI but do not want to cause a FATAL error as that would be counter<br> > productive.<br> > <br> > Please see GUI PR #271 <a href="https://github.com/OpenVPN/openvpn-gui/pull/271" rel="noreferrer" target="_blank">https://github.com/OpenVPN/openvpn-gui/pull/271</a> and<br> > my comment dated Jul 3 under it.<br> > <br> > The discussion that led to this is here:<br> > <a href="https://github.com/OpenVPN/openvpn-gui/issues/270" rel="noreferrer" target="_blank">https://github.com/OpenVPN/openvpn-gui/issues/270</a><br> <br> I fail to understand why you believe a fatal error in this case is<br> counterproductive.<br> <br> A config file that has a script configured, but is not allowed to run<br> that script is a faulty setup that might result is connections that are<br> not working properly or maybe even not providing the expected security<br> level. Consider for example someone changing DNS or proxy settings in a<br> --up script to avoid leaking data. Most users will never reads warnings<br> if the connection pretends to be set up correctly. This makes me believe<br> that the best thing we can do is to error out if someone tries to<br> connect using such a configuration, and have them review their<br> configuration.<br> <br> Why is a fatal error not exactly what you would want?<br></blockquote><div> </div><div>When you put it that way I can't disagree. Either I hadn't thought through</div><div>it properly, or have forgotten my reasoning (it has been a while since I</div><div>tested the GUI patch that prompted this).</div><div><br></div><div>I will drop this patch at least for now and go ahead with the</div><div>feature to override script security setting from the GUI.</div><div><br></div><div>Thanks for your time, and sorry for wasting it.</div><div><br></div><div>Selva</div></div></div></div>
diff --git a/src/openvpn/init.c b/src/openvpn/init.c index d28d1fd..4969565 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -169,13 +169,14 @@ run_up_down(const char *command, if (command) { struct argv argv = argv_new(); + int flags = (script_security() >= SSEC_SCRIPTS)? S_FATAL : 0; ASSERT(arg); setenv_str(es, "script_type", script_type); argv_parse_cmd(&argv, 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, flags, "--up/--down"); argv_reset(&argv); }