WireGuard Archive on lore.kernel.org
 help / color / Atom feed
* PostUp/PreUp/PostDown/PreDown Dangerous?
@ 2018-06-22  1:34 Jason A. Donenfeld
  2018-06-22  1:35 ` Jason A. Donenfeld
  0 siblings, 1 reply; 19+ messages in thread
From: Jason A. Donenfeld @ 2018-06-22  1:34 UTC (permalink / raw)
  To: WireGuard mailing list

Hey list,

wg(8) is the main wireguard tool. It takes a fairly strict set of
inputs, and is supposed to perform acceptable input validation on
them.


wg-quick(8) is a dinky bash script, that is useful for some limited use cases.


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22  1:34 PostUp/PreUp/PostDown/PreDown Dangerous? Jason A. Donenfeld
@ 2018-06-22  1:35 ` Jason A. Donenfeld
  2018-06-22  1:41   ` Jason A. Donenfeld
  0 siblings, 1 reply; 19+ messages in thread
From: Jason A. Donenfeld @ 2018-06-22  1:35 UTC (permalink / raw)
  To: WireGuard mailing list

Yikes, hit send by accident. Give me a minute to finish writing that.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22  1:35 ` Jason A. Donenfeld
@ 2018-06-22  1:41   ` Jason A. Donenfeld
  2018-06-22  1:55     ` logcabin
                       ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Jason A. Donenfeld @ 2018-06-22  1:41 UTC (permalink / raw)
  To: WireGuard mailing list; +Cc: baines.jacob

Hey list,

wg(8) is the main WireGuard configuration tool. It takes a fairly
strict set of inputs, and is supposed to perform acceptable input
validation on them.

https://git.zx2c4.com/WireGuard/about/src/tools/man/wg.8

wg-quick(8), on the other and, is a dinky bash script, that is useful
for making some common limited use cases a bit easier.

https://git.zx2c4.com/WireGuard/about/src/tools/man/wg-quick.8

wg-quick(8) has the very handy feature of allowing
PostUp/PostDown/PreUp/PreDown directives, to execute some helpers,
such as iptables or whatever else you want in a custom setup. These
have proven very useful to folks. And because these allow arbitrary
execution anyway, wg-quick(8) doesn't try very hard to do proper input
validation either.

I just saw this nice post pointing out a problem in OpenVPN:
https://medium.com/tenable-techblog/reverse-shell-from-an-openvpn-configuration-file-73fd8b1d38da

The same thing applies to wg-quick(8) with
PostUp/PostDown/PreUp/PreDown. The question is how seriously we should
take the problem presented by this blog post. Namely, you can't trust
configuration files given to you by outside parties. Maybe you
shouldn't reconfigure your network without inspecting what those
reconfigurations are first. However, one could argue that code
execution is a bit beyond networking config.

So, the question we need to ask is whether this problem is important
enough that these useful features should be _removed_? Or if there's a
way to make them safer? Or if it just doesn't matter that much and we
shouldn't do anything.

Thoughts?

Jason

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22  1:41   ` Jason A. Donenfeld
@ 2018-06-22  1:55     ` logcabin
  2018-06-22  1:56     ` Antonio Quartulli
                       ` (4 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: logcabin @ 2018-06-22  1:55 UTC (permalink / raw)
  To: wireguard

I'm in favor of keeping the features. A competent sysadmin or netadmin should know not to put questionable material on their systems, or at the very least, try it on a test bed where it can't do any damage.

On Thu, Jun 21, 2018, at 9:41 PM, Jason A. Donenfeld wrote:
> Hey list,
> 
> wg(8) is the main WireGuard configuration tool. It takes a fairly
> strict set of inputs, and is supposed to perform acceptable input
> validation on them.
> 
> https://git.zx2c4.com/WireGuard/about/src/tools/man/wg.8
> 
> wg-quick(8), on the other and, is a dinky bash script, that is useful
> for making some common limited use cases a bit easier.
> 
> https://git.zx2c4.com/WireGuard/about/src/tools/man/wg-quick.8
> 
> wg-quick(8) has the very handy feature of allowing
> PostUp/PostDown/PreUp/PreDown directives, to execute some helpers,
> such as iptables or whatever else you want in a custom setup. These
> have proven very useful to folks. And because these allow arbitrary
> execution anyway, wg-quick(8) doesn't try very hard to do proper input
> validation either.
> 
> I just saw this nice post pointing out a problem in OpenVPN:
> https://medium.com/tenable-techblog/reverse-shell-from-an-openvpn-configuration-file-73fd8b1d38da
> 
> The same thing applies to wg-quick(8) with
> PostUp/PostDown/PreUp/PreDown. The question is how seriously we should
> take the problem presented by this blog post. Namely, you can't trust
> configuration files given to you by outside parties. Maybe you
> shouldn't reconfigure your network without inspecting what those
> reconfigurations are first. However, one could argue that code
> execution is a bit beyond networking config.
> 
> So, the question we need to ask is whether this problem is important
> enough that these useful features should be _removed_? Or if there's a
> way to make them safer? Or if it just doesn't matter that much and we
> shouldn't do anything.
> 
> Thoughts?
> 
> Jason
> _______________________________________________
> WireGuard mailing list
> WireGuard@lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22  1:41   ` Jason A. Donenfeld
  2018-06-22  1:55     ` logcabin
@ 2018-06-22  1:56     ` Antonio Quartulli
  2018-06-22 10:46       ` Jordan Glover
  2018-06-22  4:01     ` Matthias Urlichs
                       ` (3 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Antonio Quartulli @ 2018-06-22  1:56 UTC (permalink / raw)
  To: Jason A. Donenfeld, WireGuard mailing list; +Cc: baines.jacob

Hi,

On 22/06/18 09:41, Jason A. Donenfeld wrote:
> Hey list,
> 
> wg(8) is the main WireGuard configuration tool. It takes a fairly
> strict set of inputs, and is supposed to perform acceptable input
> validation on them.
> 
> https://git.zx2c4.com/WireGuard/about/src/tools/man/wg.8
> 
> wg-quick(8), on the other and, is a dinky bash script, that is useful
> for making some common limited use cases a bit easier.
> 
> https://git.zx2c4.com/WireGuard/about/src/tools/man/wg-quick.8
> 
> wg-quick(8) has the very handy feature of allowing
> PostUp/PostDown/PreUp/PreDown directives, to execute some helpers,
> such as iptables or whatever else you want in a custom setup. These
> have proven very useful to folks. And because these allow arbitrary
> execution anyway, wg-quick(8) doesn't try very hard to do proper input
> validation either.
> 
> I just saw this nice post pointing out a problem in OpenVPN:
> https://medium.com/tenable-techblog/reverse-shell-from-an-openvpn-configuration-file-73fd8b1d38da
> 
> The same thing applies to wg-quick(8) with
> PostUp/PostDown/PreUp/PreDown. The question is how seriously we should
> take the problem presented by this blog post. Namely, you can't trust
> configuration files given to you by outside parties. Maybe you
> shouldn't reconfigure your network without inspecting what those
> reconfigurations are first. However, one could argue that code
> execution is a bit beyond networking config.
> 
> So, the question we need to ask is whether this problem is important
> enough that these useful features should be _removed_? Or if there's a
> way to make them safer? Or if it just doesn't matter that much and we
> shouldn't do anything.
> 
> Thoughts?

In case this might be useful: in OpenVPN there is an additional
parameter called "--script-security" that requires to be set to a
certain level before allowing configured scripts to be executed.

Unfortunately there is no real protection against the clueless user, who
can and will blindly enable that setting if asked by a $random VPN provider.

However, I still believe (and hope) that forcing the user to enable a
specific knob may raise the level of attention.

Maybe something similar could be added as a command line parameter to
wg/wg-quick so that it will execute the various
PostUp/PreUp/PostDown/PreDown only if allowed to?


Just as a side note: this is not a VPN specific problem, this is
something users can end up with everytime they execute some binary with
a configuration they have not inspected. So, be careful out there ;-)


Cheers,


-- 
Antonio Quartulli

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22  1:41   ` Jason A. Donenfeld
  2018-06-22  1:55     ` logcabin
  2018-06-22  1:56     ` Antonio Quartulli
@ 2018-06-22  4:01     ` Matthias Urlichs
  2018-06-22  5:44     ` Reto Brunner
                       ` (2 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Matthias Urlichs @ 2018-06-22  4:01 UTC (permalink / raw)
  To: wireguard

On 22.06.2018 03:41, Jason A. Donenfeld wrote:
> So, the question we need to ask is whether this problem is important
> enough that these useful features should be _removed_? Or if there's a
> way to make them safer? Or if it just doesn't matter that much and we
> shouldn't do anything.

User is able to shoot themselves in the foot. Film at 11.

Seriously. If you accept untrusted "secure" network configuration
scripts from anybody without checking them, you deserve to lose. Also,
OpenVPN config is somewhat more arcane+verbose than wireguard's, thus
it's much harder to hide random script calls from even cursory glances
in a wg config file.

If we want to, we could emit a big fat warning and/or ask the user to
confirm whenever their wg-quick script is newer than
/etc/wireguard/.{NAME].warned (or we could copy the contents of the
up/down script options to that file and warn when they've changed).

That being said, tools like systemd-networkd or NetworkManager do a good
job of making that option (and thus wg-quick itself) unnecessary
altogether, so I consider that to be a mostly-non-problem.

-- 
-- Matthias Urlichs

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22  1:41   ` Jason A. Donenfeld
                       ` (2 preceding siblings ...)
  2018-06-22  4:01     ` Matthias Urlichs
@ 2018-06-22  5:44     ` Reto Brunner
  2018-06-22 14:07     ` Andy Dorman
  2018-06-22 19:26     ` Lonnie Abelbeck
  5 siblings, 0 replies; 19+ messages in thread
From: Reto Brunner @ 2018-06-22  5:44 UTC (permalink / raw)
  To: wireguard

On Fri, Jun 22, 2018 at 03:41:03AM +0200, Jason A. Donenfeld wrote:
> The same thing applies to wg-quick(8) with
> PostUp/PostDown/PreUp/PreDown. The question is how seriously we should
> take the problem presented by this blog post. Namely, you can't trust
> configuration files given to you by outside parties. Maybe you
> shouldn't reconfigure your network without inspecting what those
> reconfigurations are first. However, one could argue that code
> execution is a bit beyond networking config.

You should never run *any* config from the internet without inspecting
it...
Even if it isn't a reverse shell directly, you can still for example end
up with questionable cipher suit choices in say openvpn or openssh if
you just blindly do that.

So please don't remove the hooks. They are very useful for many reasons
and adding an additional knob will not make it any more secure.

As others already said those users anyhow just run random commands from
$blog (heck they even copy / paste fork bombs and stuff like `rm -rf /*`)

In my view the whole point of wg-quick is that it can do things like the
post-up hook, to enter things like firewall rules etc.


Kind regards,
Reto

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22  1:56     ` Antonio Quartulli
@ 2018-06-22 10:46       ` Jordan Glover
  2018-06-22 10:53         ` Antonio Quartulli
  0 siblings, 1 reply; 19+ messages in thread
From: Jordan Glover @ 2018-06-22 10:46 UTC (permalink / raw)
  To: Antonio Quartulli; +Cc: baines.jacob, WireGuard mailing list

On June 22, 2018 3:56 AM, Antonio Quartulli <a@unstable.cc> wrote:
>=20
> In case this might be useful: in OpenVPN there is an additional
>=20
> parameter called "--script-security" that requires to be set to a
>=20
> certain level before allowing configured scripts to be executed.
>=20
> Unfortunately there is no real protection against the clueless user, who
>=20
> can and will blindly enable that setting if asked by a $random VPN provid=
er.
>=20
> However, I still believe (and hope) that forcing the user to enable a
>=20
> specific knob may raise the level of attention.
>=20
> Maybe something similar could be added as a command line parameter to
>=20
> wg/wg-quick so that it will execute the various
>=20
> PostUp/PreUp/PostDown/PreDown only if allowed to?
>=20
> Just as a side note: this is not a VPN specific problem, this is
>=20
> something users can end up with everytime they execute some binary with
>=20
> a configuration they have not inspected. So, be careful out there ;-)
>=20
> Cheers,
>=20

Attacker can pass appropriate "--script-security" level with the very same =
config
containing malicious commands so this isn't solving problem of not looking =
at
the content of config files. I think blindly using untrusted files from the=
 web is
indefensible. Sure, we could throw away this functionality completely  but =
then
we will punish people who bother to look at the configs before using them a=
nd
make their life little harder while the others will still find their footgu=
n somewhere
else as this is rather generic issue not limited to wireguard or even netwo=
rking.

Jordan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22 10:46       ` Jordan Glover
@ 2018-06-22 10:53         ` Antonio Quartulli
  2018-06-22 13:08           ` Jacob Baines
  0 siblings, 1 reply; 19+ messages in thread
From: Antonio Quartulli @ 2018-06-22 10:53 UTC (permalink / raw)
  To: Jordan Glover; +Cc: baines.jacob, WireGuard mailing list

[-- Attachment #1.1: Type: text/plain, Size: 1568 bytes --]



On 22/06/18 18:46, Jordan Glover wrote:
> On June 22, 2018 3:56 AM, Antonio Quartulli <a@unstable.cc> wrote:
>>
>> In case this might be useful: in OpenVPN there is an additional
>>
>> parameter called "--script-security" that requires to be set to a
>>
>> certain level before allowing configured scripts to be executed.
>>
>> Unfortunately there is no real protection against the clueless user, who
>>
>> can and will blindly enable that setting if asked by a $random VPN provider.
>>
>> However, I still believe (and hope) that forcing the user to enable a
>>
>> specific knob may raise the level of attention.
>>
>> Maybe something similar could be added as a command line parameter to
>>
>> wg/wg-quick so that it will execute the various
>>
>> PostUp/PreUp/PostDown/PreDown only if allowed to?
>>
>> Just as a side note: this is not a VPN specific problem, this is
>>
>> something users can end up with everytime they execute some binary with
>>
>> a configuration they have not inspected. So, be careful out there ;-)
>>
>> Cheers,
>>
> 
> Attacker can pass appropriate "--script-security" level with the very same config
> containing malicious commands so this isn't solving problem of not looking at
> the content of config files. 

that's why I suggested to implement it as a command line knob for
wg/wg-quick.

But I totally agree with you that against this kind of issues there is
not really a lot the developer can do - each of us is free to shoot
himself in the foot.

Regards,

-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22 10:53         ` Antonio Quartulli
@ 2018-06-22 13:08           ` Jacob Baines
  2018-06-22 14:47             ` Andy Dorman
                               ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Jacob Baines @ 2018-06-22 13:08 UTC (permalink / raw)
  To: Antonio Quartulli; +Cc: WireGuard mailing list

[-- Attachment #1: Type: text/plain, Size: 3304 bytes --]

Hey all,

  I'm not familiar with WireGuard so I can only speak in an OpenVPN
context. If the following doesn't apply to WireGuard at all than feel free
to ignore me.

  My main problem with "--script-security" is that its useless. It just
causes OpenVPN to spit a line into a log file (and by that time its too
late anyways). That isn't a security mechanism.

  There also seems to be an expectation that users should understand these
config files. I'm not sure why that is. Excuse my speaking in generalities
but a majority of users aren't going to understand how OpenVPN works, let
alone how the configuration file affects the program. Many users (myself
included) simply receive config files from our bosses or our IT guy and
trust that they aren't malicious. Call me naive or foolish but I don't
review every single file passed my way for malicious content.

   Furthermore, I've heard a few times now "config files exec things". Off
the top of my head, I can't think of any other applications that execute
shell commands listed in their configuration file. I have no idea where
that is coming from. Its not a smart practice from a security point of view.

   I'm not an OpenVPN expert. I'm just some guy that spent a little time
reviewing the source. So I accept that my opinions might be bad. But if I
was I dev I'd deprecate the entire system of executing programs listed in
the configuration file. I'd try to migrate code into OpenVPN itself or the
plugin system. At the very least, prompting the user and asking them for
permission to execute whatever command seems like an improvement to me.

-Jake



On Fri, Jun 22, 2018 at 6:53 AM, Antonio Quartulli <a@unstable.cc> wrote:

>
>
> On 22/06/18 18:46, Jordan Glover wrote:
> > On June 22, 2018 3:56 AM, Antonio Quartulli <a@unstable.cc> wrote:
> >>
> >> In case this might be useful: in OpenVPN there is an additional
> >>
> >> parameter called "--script-security" that requires to be set to a
> >>
> >> certain level before allowing configured scripts to be executed.
> >>
> >> Unfortunately there is no real protection against the clueless user, who
> >>
> >> can and will blindly enable that setting if asked by a $random VPN
> provider.
> >>
> >> However, I still believe (and hope) that forcing the user to enable a
> >>
> >> specific knob may raise the level of attention.
> >>
> >> Maybe something similar could be added as a command line parameter to
> >>
> >> wg/wg-quick so that it will execute the various
> >>
> >> PostUp/PreUp/PostDown/PreDown only if allowed to?
> >>
> >> Just as a side note: this is not a VPN specific problem, this is
> >>
> >> something users can end up with everytime they execute some binary with
> >>
> >> a configuration they have not inspected. So, be careful out there ;-)
> >>
> >> Cheers,
> >>
> >
> > Attacker can pass appropriate "--script-security" level with the very
> same config
> > containing malicious commands so this isn't solving problem of not
> looking at
> > the content of config files.
>
> that's why I suggested to implement it as a command line knob for
> wg/wg-quick.
>
> But I totally agree with you that against this kind of issues there is
> not really a lot the developer can do - each of us is free to shoot
> himself in the foot.
>
> Regards,
>
> --
> Antonio Quartulli
>
>

[-- Attachment #2: Type: text/html, Size: 4297 bytes --]

<div dir="ltr">Hey all,<div><br></div><div>  I&#39;m not familiar with WireGuard so I can only speak in an OpenVPN context. If the following doesn&#39;t apply to WireGuard at all than feel free to ignore me.</div><div><br></div><div>  My main problem with &quot;--script-security&quot; is that its useless. It just causes OpenVPN to spit a line into a log file (and by that time its too late anyways). That isn&#39;t a security mechanism.</div><div><br></div><div>  There also seems to be an expectation that users should understand these config files. I&#39;m not sure why that is. Excuse my speaking in generalities but a majority of users aren&#39;t going to understand how OpenVPN works, let alone how the configuration file affects the program. Many users (myself included) simply receive config files from our bosses or our IT guy and trust that they aren&#39;t malicious. Call me naive or foolish but I don&#39;t review every single file passed my way for malicious content.</div><div><br></div><div>   Furthermore, I&#39;ve heard a few times now &quot;config files exec things&quot;. Off the top of my head, I can&#39;t think of any other applications that execute shell commands listed in their configuration file. I have no idea where that is coming from. Its not a smart practice from a security point of view.</div><div><br></div><div>   I&#39;m not an OpenVPN expert. I&#39;m just some guy that spent a little time reviewing the source. So I accept that my opinions might be bad. But if I was I dev I&#39;d deprecate the entire system of executing programs listed in the configuration file. I&#39;d try to migrate code into OpenVPN itself or the plugin system. At the very least, prompting the user and asking them for permission to execute whatever command seems like an improvement to me.</div><div><br></div><div>-Jake</div><div><br></div><div>  </div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jun 22, 2018 at 6:53 AM, Antonio Quartulli <span dir="ltr">&lt;<a href="mailto:a@unstable.cc" target="_blank">a@unstable.cc</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
On 22/06/18 18:46, Jordan Glover wrote:<br>
&gt; On June 22, 2018 3:56 AM, Antonio Quartulli &lt;a@unstable.cc&gt; wrote:<br>
&gt;&gt;<br>
&gt;&gt; In case this might be useful: in OpenVPN there is an additional<br>
&gt;&gt;<br>
&gt;&gt; parameter called &quot;--script-security&quot; that requires to be set to a<br>
&gt;&gt;<br>
&gt;&gt; certain level before allowing configured scripts to be executed.<br>
&gt;&gt;<br>
&gt;&gt; Unfortunately there is no real protection against the clueless user, who<br>
&gt;&gt;<br>
&gt;&gt; can and will blindly enable that setting if asked by a $random VPN provider.<br>
&gt;&gt;<br>
&gt;&gt; However, I still believe (and hope) that forcing the user to enable a<br>
&gt;&gt;<br>
&gt;&gt; specific knob may raise the level of attention.<br>
&gt;&gt;<br>
&gt;&gt; Maybe something similar could be added as a command line parameter to<br>
&gt;&gt;<br>
&gt;&gt; wg/wg-quick so that it will execute the various<br>
&gt;&gt;<br>
&gt;&gt; PostUp/PreUp/PostDown/PreDown only if allowed to?<br>
&gt;&gt;<br>
&gt;&gt; Just as a side note: this is not a VPN specific problem, this is<br>
&gt;&gt;<br>
&gt;&gt; something users can end up with everytime they execute some binary with<br>
&gt;&gt;<br>
&gt;&gt; a configuration they have not inspected. So, be careful out there ;-)<br>
&gt;&gt;<br>
&gt;&gt; Cheers,<br>
&gt;&gt;<br>
&gt; <br>
&gt; Attacker can pass appropriate &quot;--script-security&quot; level with the very same config<br>
&gt; containing malicious commands so this isn&#39;t solving problem of not looking at<br>
&gt; the content of config files. <br>
<br>
</span>that&#39;s why I suggested to implement it as a command line knob for<br>
wg/wg-quick.<br>
<br>
But I totally agree with you that against this kind of issues there is<br>
not really a lot the developer can do - each of us is free to shoot<br>
himself in the foot.<br>
<br>
Regards,<br>
<span class="HOEnZb"><font color="#888888"><br>
-- <br>
Antonio Quartulli<br>
<br>
</font></span></blockquote></div><br></div>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22  1:41   ` Jason A. Donenfeld
                       ` (3 preceding siblings ...)
  2018-06-22  5:44     ` Reto Brunner
@ 2018-06-22 14:07     ` Andy Dorman
  2018-06-23 19:16       ` Reto Brunner
  2018-06-22 19:26     ` Lonnie Abelbeck
  5 siblings, 1 reply; 19+ messages in thread
From: Andy Dorman @ 2018-06-22 14:07 UTC (permalink / raw)
  To: wireguard

On 6/21/18 8:41 PM, Jason A. Donenfeld wrote:
> So, the question we need to ask is whether this problem is important
> enough that these useful features should be_removed_? Or if there's a
> way to make them safer? Or if it just doesn't matter that much and we
> shouldn't do anything.

We use wg-quick with PostUp/PostDown/PreUp/PreDown and would prefer that 
feature be retained.

However, looking ahead I believe Wireguard's speed, simplicity, and 
simple, straightforward configuration and operation is going to attract 
marginally competent amateur users that definitely do not qualify as a 
system or network admin.

So, while it should be obvious, it wouldn't hurt to add a short warning 
(in bold) to the wg-quick man page that lets these "amateur" users know 
of the potential danger.  Something along the lines of "Using a config 
written by someone else that you do not understand and have not vetted 
for security is stupid and can be dangerous. For example, the 
PostUp/PostDown/PreUp/PreDown commands can be used to enable malicious 
code.  So always be certain your configuration and the code it executes 
does only what you expect."

Sincere regards,

-- 
Andy Dorman
Ironic Design, Inc.
AnteSpam.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22 13:08           ` Jacob Baines
@ 2018-06-22 14:47             ` Andy Dorman
  2018-06-22 15:14             ` Matthias Urlichs
  2018-06-22 17:11             ` Jason A. Donenfeld
  2 siblings, 0 replies; 19+ messages in thread
From: Andy Dorman @ 2018-06-22 14:47 UTC (permalink / raw)
  To: wireguard

On 6/22/18 8:08 AM, Jacob Baines wrote:
> There also seems to be an expectation that users should understand these 
> config files. I'm not sure why that is. Excuse my speaking in 
> generalities but a majority of users aren't going to understand how 
> OpenVPN works, let alone how the configuration file affects the program. 
> Many users (myself included) simply receive config files from our bosses 
> or our IT guy and trust that they aren't malicious. Call me naive or 
> foolish but I don't review every single file passed my way for malicious 
> content.

We run an email security service and we MUST understand the action of 
every line in the config files on our servers. Of course we also 
carefully limit the applications on these servers so we have a limited 
set of config files to deal with. ;-)

But I agree that perhaps the warning should be to either know what the 
config is doing OR it should be from a trusted source (ie, your boss or 
IT person or some other known, trusted entity).

The main point about a warning for wg-quick is to try and help the 
average user know that there can be a danger in the pre- and post- 
commands so they will perhaps look twice before using a config that they 
grabbed off an untrusted source.

-- 
Andy Dorman
Ironic Design, Inc.
AnteSpam.com

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22 13:08           ` Jacob Baines
  2018-06-22 14:47             ` Andy Dorman
@ 2018-06-22 15:14             ` Matthias Urlichs
  2018-06-22 17:11             ` Jason A. Donenfeld
  2 siblings, 0 replies; 19+ messages in thread
From: Matthias Urlichs @ 2018-06-22 15:14 UTC (permalink / raw)
  To: wireguard

On 22.06.2018 15:08, Jacob Baines wrote:
> Excuse my speaking in generalities but a majority of users aren't
> going to understand how OpenVPN works, let alone how the configuration
> file affects the program.

Fortunately, WireGuard is a lot more approachable. All you really need
is a basic understanding of PK crypto, i.e. you need a private key for
yourself and the public key of whoever you want to talk with, both of
which can be generated with very simple commands. You can learn how to
set it up in half an hour.

In contrast, understanding SSL and OpenVPN well enough to be able to
generate a config file, let alone know how to debug it, takes a day –
and then you don't know how to debug it. With WireGuard you need to
answer three questions – do the endpoints see each others' packets? do
the public keys match? are the remote IP addresses correct (plus routed
to the WG network interface, not filtered, etc.)? If "yes", it'll work.
Dead simple.

-- 
-- Matthias Urlichs

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22 13:08           ` Jacob Baines
  2018-06-22 14:47             ` Andy Dorman
  2018-06-22 15:14             ` Matthias Urlichs
@ 2018-06-22 17:11             ` Jason A. Donenfeld
  2 siblings, 0 replies; 19+ messages in thread
From: Jason A. Donenfeld @ 2018-06-22 17:11 UTC (permalink / raw)
  To: Jacob Baines; +Cc: WireGuard mailing list

Hi Jacob,

Thanks for writing in, and for your original blog post. It looks to me
like most people on the mailing list don't know how to use their email
clients, and so you're getting cut off from the responses. However, if
you're interested, they are at least in the archives, starting with
the parent post:
https://lists.zx2c4.com/pipermail/wireguard/2018-June/003032.html

On Fri, Jun 22, 2018 at 3:08 PM Jacob Baines <baines.jacob@gmail.com> wrote=
:
>   My main problem with "--script-security" is that its useless. It just c=
auses OpenVPN to spit a line into a log file (and by that time its too late=
 anyways). That isn't a security mechanism.

Yes, --script-security doesn't solve much at all for this use case,
especially because it can be put inside the config file itself, so
that by the time somebody runs `openvpn --config pwnd.conf`, it's too
late. I think the suggestion in this case would be to add something to
wg-quick(8) along the lines of:

$ wg-quick up --i-recognize-this-config-file-might-murder-kittens pwnd.conf

I don't actually think that helps much though. The users you want to
protect from pwnd.conf will still paste that line verbatim anyway, and
it's just another nob that risks making the whole thing a hassle (like
OpenVPN is with all of those nobs).

>   There also seems to be an expectation that users should understand thes=
e config files. I'm not sure why that is. Excuse my speaking in generalitie=
s but a majority of users aren't going to understand how OpenVPN works, let=
 alone how the configuration file affects the program. Many users (myself i=
ncluded) simply receive config files from our bosses or our IT guy and trus=
t that they aren't malicious. Call me naive or foolish but I don't review e=
very single file passed my way for malicious content.

Yes, that's a very fair assessment of the situation with OpenVPN.
There are a million nobs, and including massive x509 certs inline with
the config makes the whole thing an unscanable mess. I've read the
entire code base of OpenVPN several times and played with nearly every
feature, but I still can't remember off the topic of my head the
semantics of all those nobs. By contrast, a typical wg-quick(8) config
file looks like:

zx2c4@thinkpad ~ $ cat /etc/wireguard/demo.conf
[Interface]
PrivateKey =3D gP+/hJKAhmXKmewrgpkrOKYwvwGwHQ3i5bQqVzWpoEI=3D
Address =3D 192.168.4.207/24
DNS =3D 8.8.8.8, 8.8.4.4, 1.1.1.1, 1.0.0.1

[Peer]
PublicKey =3D JRI8Xc0zKP9kXk8qP84NdUQA04h6DLfFbwJn4g+/PFs=3D
Endpoint =3D demo.wireguard.com:12912
AllowedIPs =3D 0.0.0.0/0

Turns out, though, that our [Interface] section above can also contain
{Post,Pre}{Up,Down}, hence this email thread.

>    Furthermore, I've heard a few times now "config files exec things". Of=
f the top of my head, I can't think of any other applications that execute =
shell commands listed in their configuration file. I have no idea where tha=
t is coming from. Its not a smart practice from a security point of view.

Just going haphazardly through the various random applications on my
laptop, I can think of quite a few actually:

vim
git
mutt
msmtp
chromium
firefox
kde
qtcreator
bash (haha)
portage
systemd
dhcpcd
texstudio
networkmanager
apache
emacs
Most things in /etc/...

I think it's pretty typical for application configuration files on
unix systems to wind up exec'ing things. Configuration is often seen
as a combination of code and data.

On the other hand, there's a set of applications that take purely data
input, and we certainly expect them to not execute anything. This
would apply to things like:

pdf docs
msoffice docs (turing complete!)
jpeg images
text files
sound files
video files
archives

One might conclude from this distinction that configuration files are
not like word files, and therefore should be permitted to execute
things. But if that's the case, is there any real difference between a
configuration file and just shipping around a bash script or elf
binary? Or you could argue that a configuration file is sometimes just
sort of a domain-specific command executor. This is also an
unsatisfying conclusion. So I'm not sure what formal distinction one
should draw in the end, except, "sometimes on unix, it's traditional
and common for configuration files to execute things." Hm.

> I'd try to migrate code into OpenVPN itself or the plugin system

It's worth noting that OpenVPN's configuration file can load
plugins... So... Here's a fun exercise: can you make a polyglot file
that's parsed both as an openvpn configuration file and as an
ELF/PE/Mach-O shared library that the loader will execute code from?
Then you could just specify "plugin /proc/self/fd/3" or the like. Or
there's always the drive-by download trick, where you get a .notadll
file put in the user's download folder for free (and since it isn't a
.dll, it's not handled specially), and then benefit from the fact that
OpenVPN's plugin directive doesn't check file extension. Or, ... okay,
I'll stop now.

Jason

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22  1:41   ` Jason A. Donenfeld
                       ` (4 preceding siblings ...)
  2018-06-22 14:07     ` Andy Dorman
@ 2018-06-22 19:26     ` Lonnie Abelbeck
  2018-06-22 22:13       ` Jordan Glover
  5 siblings, 1 reply; 19+ messages in thread
From: Lonnie Abelbeck @ 2018-06-22 19:26 UTC (permalink / raw)
  To: WireGuard mailing list; +Cc: baines.jacob


> On Jun 21, 2018, at 8:41 PM, Jason A. Donenfeld <Jason@zx2c4.com> =
wrote:
>=20
> Hey list,
>=20
> wg(8) is the main WireGuard configuration tool. It takes a fairly
> strict set of inputs, and is supposed to perform acceptable input
> validation on them.
>=20
> https://git.zx2c4.com/WireGuard/about/src/tools/man/wg.8
>=20
> wg-quick(8), on the other and, is a dinky bash script, that is useful
> for making some common limited use cases a bit easier.
>=20
> https://git.zx2c4.com/WireGuard/about/src/tools/man/wg-quick.8
>=20
> wg-quick(8) has the very handy feature of allowing
> PostUp/PostDown/PreUp/PreDown directives, to execute some helpers,
> such as iptables or whatever else you want in a custom setup. These
> have proven very useful to folks. And because these allow arbitrary
> execution anyway, wg-quick(8) doesn't try very hard to do proper input
> validation either.
>=20
> I just saw this nice post pointing out a problem in OpenVPN:
> =
https://medium.com/tenable-techblog/reverse-shell-from-an-openvpn-configur=
ation-file-73fd8b1d38da
>=20
> The same thing applies to wg-quick(8) with
> PostUp/PostDown/PreUp/PreDown.

How about not supporting direct execution of commands in the config =
[Interface] section but rather support an optional path to where a fixed =
command (ex. wireguard.script) is found...
--
ActionScriptDir =3D /usr/local/bin
--

Then instead of executing the PostUp/PostDown/PreUp/PreDown data, the =
wg-quick script would call:
--
/usr/local/bin/wireguard.script PRE_UP|PRE_DOWN|POST_UP|POST_DOWN =
"$INTERFACE"
--

1) When called, the first argument would be one of: =
PRE_UP|PRE_DOWN|POST_UP|POST_DOWN

2) When called, the second argument would be the wireguard interface.

3) If ActionScriptDir is not defined, then wireguard.script is not =
called.


This requires an extra step to be taken to create a wireguard.script =
file with execute permissions and possibly require specific ownership.

Lonnie

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22 19:26     ` Lonnie Abelbeck
@ 2018-06-22 22:13       ` Jordan Glover
  2018-06-23  2:36         ` Antonio Quartulli
  0 siblings, 1 reply; 19+ messages in thread
From: Jordan Glover @ 2018-06-22 22:13 UTC (permalink / raw)
  To: Lonnie Abelbeck; +Cc: baines.jacob, WireGuard mailing list

On June 22, 2018 9:26 PM, Lonnie Abelbeck <lists@lonnie.abelbeck.com> wrote=
:

> How about not supporting direct execution of commands in the config [Inte=
rface] section but rather support an optional path to where a fixed command=
 (ex. wireguard.script) is found...
>=20
>=20
> -------------------------------------------------------------------------=
---------------------------------------------------------------------------=
---------------------------------------
>=20
> ActionScriptDir =3D /usr/local/bin
> --------------------------------
>=20
> Then instead of executing the PostUp/PostDown/PreUp/PreDown data, the wg-=
quick script would call:
>=20
>=20
> -------------------------------------------------------------------------=
----------------------------
>=20
> /usr/local/bin/wireguard.script PRE_UP|PRE_DOWN|POST_UP|POST_DOWN "$INTER=
FACE"
> -------------------------------------------------------------------------=
-----
>=20
> 1.  When called, the first argument would be one of: PRE_UP|PRE_DOWN|POST=
_UP|POST_DOWN
> 2.  When called, the second argument would be the wireguard interface.
> 3.  If ActionScriptDir is not defined, then wireguard.script is not calle=
d.
>    =20
>     This requires an extra step to be taken to create a wireguard.script =
file with execute permissions and possibly require specific ownership.
>    =20
>     Lonnie
>    =20

But attacker will helpfully provide you customized 'wireguard.script'  as w=
ell
and even tell you how to use it by setting 'chmod 4777 wireguard.script'.

Jordan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22 22:13       ` Jordan Glover
@ 2018-06-23  2:36         ` Antonio Quartulli
  2018-06-23  7:02           ` Dario Bosch
  0 siblings, 1 reply; 19+ messages in thread
From: Antonio Quartulli @ 2018-06-23  2:36 UTC (permalink / raw)
  To: Jordan Glover, Lonnie Abelbeck; +Cc: baines.jacob, WireGuard mailing list

[-- Attachment #1.1: Type: text/plain, Size: 1863 bytes --]

Hi,

On 23/06/18 06:13, Jordan Glover wrote:
[cut]
> 
> But attacker will helpfully provide you customized 'wireguard.script'  as well
> and even tell you how to use it by setting 'chmod 4777 wireguard.script'.
> 

An attacker will also tell you to run "rm -Rf /" :-P


Jokes apart, I was talking to Jason on IRC and I suggested an idea that
might be worth sharing.

A network device driver in the kernel is free to send events to
userspace with any custom set of properties/values.

Most of you have already seen and played with those typically thrown
when an interface goes up and down, with udev normally handling them by
executing some (user-)configured action.

These events can be easily created and customized by any kernel module
and associated to a network interface.
Wireguard could generate preup/postup/etc.. uevents and send them to
userspace.

It will then be udev to decide how to handle those.
Specific scripts could be installed by the admin, or udev could come
with its own default ones.

In any case, this would delegate the execution of scripts to a component
that is in charge of doing exactly that.

This would remove the risk of sneaking malicious things into the
configuration file, which is what people do not expect and is the core
of the issue discussed here.

(Yeah, I already hear people saying "but the malicious attacker will
tell the clueless user to install this script in udev", but I think that
by then, the problem has moved to another plane)

My experience with this mechanism comes from batman-adv[1], where it
used to report special routing events to the user so that he could react
accordingly (if desired).


just my 2 cents.


Cheers,

[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/batman-adv/sysfs.c#n1209

-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-23  2:36         ` Antonio Quartulli
@ 2018-06-23  7:02           ` Dario Bosch
  0 siblings, 0 replies; 19+ messages in thread
From: Dario Bosch @ 2018-06-23  7:02 UTC (permalink / raw)
  To: wireguard

Hi all,

I've been following this discussion and now also want to add my 2 cents.

In my opinion wireguard's biggest strength is the simplicity of its
config. One can write a 8-line config file that works reliably, is
secure and also can take care of ip tables rules, start services etc.
Even a user who only has a rough understanding about their system can
achieve this.
Everything that makes using wireguard more complicated should be
avoided. Setting up other VPNs such as openVPN is a hassle simply
because they are *not* designed to be simple.

We are discussing about an attack scenario, where the attacker convinces
a clueless user to compile/install a kernel module, we always should
keep that in mind. Most installation instructions advise people to add a
custom repository or clone a git repository, so they're already running
foreign, untrusted code with root rights.

In my opinion the only viable option would be to computer a check-sum
over the {Pre/Post}{Up/Down} lines in the config files i wg-quick. In
case the check-sum has changed, the user could then be asked to trust
this file. I imagine it a bit like that:

    [root@machine /]$ wg-quick up myWG
    [#] *New Post-Up rule detected*
    [#] Warning: The selected config file will execute the following
    [#] commands when loaded:
    [#]      'rm -rf /'
    [#] Do you want to trust this config file? [y]/[n]
       > y
    [#] Rule added to list of trusted config, continuing
    ...

Again, I think making wireguard usage more complicated and annoying than
necessary should be avoided.

Cheers,
Dario




On 06/23/2018 04:36 AM, Antonio Quartulli wrote:
> Hi,
> 
> On 23/06/18 06:13, Jordan Glover wrote:
> [cut]
>>
>> But attacker will helpfully provide you customized 'wireguard.script'  as well
>> and even tell you how to use it by setting 'chmod 4777 wireguard.script'.
>>
> 
> An attacker will also tell you to run "rm -Rf /" :-P
> 
> 
> Jokes apart, I was talking to Jason on IRC and I suggested an idea that
> might be worth sharing.
> 
> A network device driver in the kernel is free to send events to
> userspace with any custom set of properties/values.
> 
> Most of you have already seen and played with those typically thrown
> when an interface goes up and down, with udev normally handling them by
> executing some (user-)configured action.
> 
> These events can be easily created and customized by any kernel module
> and associated to a network interface.
> Wireguard could generate preup/postup/etc.. uevents and send them to
> userspace.
> 
> It will then be udev to decide how to handle those.
> Specific scripts could be installed by the admin, or udev could come
> with its own default ones.
> 
> In any case, this would delegate the execution of scripts to a component
> that is in charge of doing exactly that.
> 
> This would remove the risk of sneaking malicious things into the
> configuration file, which is what people do not expect and is the core
> of the issue discussed here.
> 
> (Yeah, I already hear people saying "but the malicious attacker will
> tell the clueless user to install this script in udev", but I think that
> by then, the problem has moved to another plane)
> 
> My experience with this mechanism comes from batman-adv[1], where it
> used to report special routing events to the user so that he could react
> accordingly (if desired).
> 
> 
> just my 2 cents.
> 
> 
> Cheers,
> 
> [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/batman-adv/sysfs.c#n1209
> 
> 
> 
> _______________________________________________
> WireGuard mailing list
> WireGuard@lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/wireguard
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PostUp/PreUp/PostDown/PreDown Dangerous?
  2018-06-22 14:07     ` Andy Dorman
@ 2018-06-23 19:16       ` Reto Brunner
  0 siblings, 0 replies; 19+ messages in thread
From: Reto Brunner @ 2018-06-23 19:16 UTC (permalink / raw)
  To: wireguard

On Fri, Jun 22, 2018 at 09:07:43AM -0500, Andy Dorman wrote:
> So, while it should be obvious, it wouldn't hurt to add a short warning (in
> bold) to the wg-quick man page that lets these "amateur" users know of the
> potential danger.

The issue being that people do not tend to read the man pages.
I don't disagree that this might be useful for some, but don't get your
hopes up too much...

The type of user who actually needs this warning tends to not want to
put in any effort whatsoever in understanding how stuff works, they just want the end result.

This is at least my experience from several Linux irc channels, where
half the questions could be answered by `man $program_I_have_issues_with`

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, back to index

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-22  1:34 PostUp/PreUp/PostDown/PreDown Dangerous? Jason A. Donenfeld
2018-06-22  1:35 ` Jason A. Donenfeld
2018-06-22  1:41   ` Jason A. Donenfeld
2018-06-22  1:55     ` logcabin
2018-06-22  1:56     ` Antonio Quartulli
2018-06-22 10:46       ` Jordan Glover
2018-06-22 10:53         ` Antonio Quartulli
2018-06-22 13:08           ` Jacob Baines
2018-06-22 14:47             ` Andy Dorman
2018-06-22 15:14             ` Matthias Urlichs
2018-06-22 17:11             ` Jason A. Donenfeld
2018-06-22  4:01     ` Matthias Urlichs
2018-06-22  5:44     ` Reto Brunner
2018-06-22 14:07     ` Andy Dorman
2018-06-23 19:16       ` Reto Brunner
2018-06-22 19:26     ` Lonnie Abelbeck
2018-06-22 22:13       ` Jordan Glover
2018-06-23  2:36         ` Antonio Quartulli
2018-06-23  7:02           ` Dario Bosch

WireGuard Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/wireguard/0 wireguard/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 wireguard wireguard/ https://lore.kernel.org/wireguard \
		wireguard@lists.zx2c4.com zx2c4-wireguard@archiver.kernel.org
	public-inbox-index wireguard


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/com.zx2c4.lists.wireguard


AGPL code for this site: git clone https://public-inbox.org/ public-inbox