[PATCH] treewide: more portable bash shebangs

[PATCH] treewide: more portable bash shebangs

[Jörg Thalheim]

While /usr/bin/env is more or less available on all POSIX systems
/bin/bash might not be. This is particular the case on NixOS and the BSD
family (/usr/local/bin/bash). Downstream packagers would often rewrite
those shebangs back automatically as they can rely on absolute paths
but having portable shebangs in the repository helps to run the code
without any further modification.

Signed-off-by: Jörg Thalheim <joerg@thalheim.io>
---
 contrib/examples/dns-hatchet/apply.sh                     | 2 +-
 contrib/examples/extract-handshakes/extract-handshakes.sh | 2 +-
 contrib/examples/json/wg-json                             | 2 +-
 contrib/examples/ncat-client-server/client.sh             | 2 +-
 contrib/examples/ncat-client-server/server.sh             | 2 +-
 contrib/examples/reresolve-dns/reresolve-dns.sh           | 2 +-
 contrib/examples/synergy/synergy-client.sh                | 2 +-
 contrib/examples/synergy/synergy-server.sh                | 2 +-
 contrib/kernel-tree/create-patch.sh                       | 2 +-
 contrib/kernel-tree/filter-compat-defines.sh              | 2 +-
 contrib/kernel-tree/jury-rig.sh                           | 2 +-
 src/tests/netns.sh                                        | 2 +-
 src/tools/wg-quick/linux.bash                             | 2 +-
 13 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/contrib/examples/dns-hatchet/apply.sh b/contrib/examples/dns-hatchet/apply.sh
index 2bf002d..fb4ca34 100755
--- a/contrib/examples/dns-hatchet/apply.sh
+++ b/contrib/examples/dns-hatchet/apply.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
diff --git a/contrib/examples/extract-handshakes/extract-handshakes.sh b/contrib/examples/extract-handshakes/extract-handshakes.sh
index f794ffe..289907e 100755
--- a/contrib/examples/extract-handshakes/extract-handshakes.sh
+++ b/contrib/examples/extract-handshakes/extract-handshakes.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
diff --git a/contrib/examples/json/wg-json b/contrib/examples/json/wg-json
index 8b35521..f4d86a1 100755
--- a/contrib/examples/json/wg-json
+++ b/contrib/examples/json/wg-json
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
diff --git a/contrib/examples/ncat-client-server/client.sh b/contrib/examples/ncat-client-server/client.sh
index dd0f575..1a5f3ed 100755
--- a/contrib/examples/ncat-client-server/client.sh
+++ b/contrib/examples/ncat-client-server/client.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
diff --git a/contrib/examples/ncat-client-server/server.sh b/contrib/examples/ncat-client-server/server.sh
index 38a69e1..e710661 100755
--- a/contrib/examples/ncat-client-server/server.sh
+++ b/contrib/examples/ncat-client-server/server.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
diff --git a/contrib/examples/reresolve-dns/reresolve-dns.sh b/contrib/examples/reresolve-dns/reresolve-dns.sh
index e579f86..28ee389 100755
--- a/contrib/examples/reresolve-dns/reresolve-dns.sh
+++ b/contrib/examples/reresolve-dns/reresolve-dns.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
diff --git a/contrib/examples/synergy/synergy-client.sh b/contrib/examples/synergy/synergy-client.sh
index de358ef..ecaf337 100755
--- a/contrib/examples/synergy/synergy-client.sh
+++ b/contrib/examples/synergy/synergy-client.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
diff --git a/contrib/examples/synergy/synergy-server.sh b/contrib/examples/synergy/synergy-server.sh
index e04b1f4..0404996 100755
--- a/contrib/examples/synergy/synergy-server.sh
+++ b/contrib/examples/synergy/synergy-server.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
diff --git a/contrib/kernel-tree/create-patch.sh b/contrib/kernel-tree/create-patch.sh
index 4a4ad98..cc7e35f 100755
--- a/contrib/kernel-tree/create-patch.sh
+++ b/contrib/kernel-tree/create-patch.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
diff --git a/contrib/kernel-tree/filter-compat-defines.sh b/contrib/kernel-tree/filter-compat-defines.sh
index d083bf5..b1747b5 100755
--- a/contrib/kernel-tree/filter-compat-defines.sh
+++ b/contrib/kernel-tree/filter-compat-defines.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
diff --git a/contrib/kernel-tree/jury-rig.sh b/contrib/kernel-tree/jury-rig.sh
index 2b16f88..021830a 100755
--- a/contrib/kernel-tree/jury-rig.sh
+++ b/contrib/kernel-tree/jury-rig.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
diff --git a/src/tests/netns.sh b/src/tests/netns.sh
index 7cbbfce..7e0e166 100755
--- a/src/tests/netns.sh
+++ b/src/tests/netns.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
diff --git a/src/tools/wg-quick/linux.bash b/src/tools/wg-quick/linux.bash
index 2f36dee..6a101f3 100755
--- a/src/tools/wg-quick/linux.bash
+++ b/src/tools/wg-quick/linux.bash
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 # SPDX-License-Identifier: GPL-2.0
 #
 # Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
-- 
2.22.0

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: [PATCH] treewide: more portable bash shebangs

[Jordan Glover]

On Tuesday, July 16, 2019 12:21 PM, Jörg Thalheim <joerg@higgsboson.tk> wrote:

> While /usr/bin/env is more or less available on all POSIX systems
> /bin/bash might not be. This is particular the case on NixOS and the BSD
> family (/usr/local/bin/bash). Downstream packagers would often rewrite
> those shebangs back automatically as they can rely on absolute paths
> but having portable shebangs in the repository helps to run the code
> without any further modification.
>

The reason almost everyone hardcodes bash to /bin/bash is the potential
environment attack where someone create malicious "bash" and export it in PATH:

https://developer.apple.com/library/archive/documentation/OpenSource/Conceptual/ShellScripting/ShellScriptSecurity/ShellScriptSecurity.html

Obviously wg scripts are handling quite sensitive data like private keys...

Seriously if you except that downstream packagers would rewrite it back to
/bin/bash then why the others can't rewrite it to /usr/bin/env bash right
now if this is something they want?

Jordan
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: [PATCH] treewide: more portable bash shebangs

[Janne Johansson]

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

Den tis 16 juli 2019 kl 19:34 skrev Jordan Glover <
Golden_Miller83@protonmail.ch>:

> > While /usr/bin/env is more or less available on all POSIX systems
> > /bin/bash might not be. This is particular the case on NixOS and the BSD
> > family (/usr/local/bin/bash). Downstream packagers would often rewrite
> > those shebangs back automatically as they can rely on absolute paths
> > but having portable shebangs in the repository helps to run the code
> > without any further modification.
> >
>
> The reason almost everyone hardcodes bash to /bin/bash is the potential
> environment attack where someone create malicious "bash" and export it in
> PATH:
>
>
> https://developer.apple.com/library/archive/documentation/OpenSource/Conceptual/ShellScripting/ShellScriptSecurity/ShellScriptSecurity.html


Well, if they rewrite your env and PATH you can't trust anything you do on
that box ever. If wg is started with a malicious environment where IFS is
set to "/" so that
"/bin/bash" (or any absolute-path-named-program) turns into " bin bash"
then an evil PATH pointing to that "bin" would still start a bad script for
you.

https://books.google.se/books?id=-aIKj0lbADIC&pg=PT182&lpg=PT182&dq=set+IFS+to+slash&source=bl&ots=cNQdBQUJEv&sig=ACfU3U0apkUJWhJRjnJMgKlRBFBPD5nZ6g&hl=en&sa=X&ved=2ahUKEwiP0Ka8nrrjAhVOwsQBHZOtC08Q6AEwBHoECAgQAQ#v=onepage&q=set%20IFS%20to%20slash&f=false


-- 
May the most significant bit of your life be positive.

[-- Attachment #1.2: Type: text/html, Size: 2493 bytes --]

[-- Attachment #2: Type: text/plain, Size: 148 bytes --]

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: [PATCH] treewide: more portable bash shebangs

[Jörg Thalheim]

On 16/07/2019 18.32, Jordan Glover wrote:
> On Tuesday, July 16, 2019 12:21 PM, Jörg Thalheim <joerg@higgsboson.tk> wrote:
>
>> While /usr/bin/env is more or less available on all POSIX systems
>> /bin/bash might not be. This is particular the case on NixOS and the BSD
>> family (/usr/local/bin/bash). Downstream packagers would often rewrite
>> those shebangs back automatically as they can rely on absolute paths
>> but having portable shebangs in the repository helps to run the code
>> without any further modification.
>>
> The reason almost everyone hardcodes bash to /bin/bash is the potential
> environment attack where someone create malicious "bash" and export it in PATH:
>
> https://developer.apple.com/library/archive/documentation/OpenSource/Conceptual/ShellScripting/ShellScriptSecurity/ShellScriptSecurity.html
>
> Obviously wg scripts are handling quite sensitive data like private keys...
>
> Seriously if you except that downstream packagers would rewrite it back to
> /bin/bash then why the others can't rewrite it to /usr/bin/env bash right
> now if this is something they want?
>
> Jordan


This argument does not apply here since all commands internally could
be redirected by a PATH change as well since the PATH is not set in the scripts.
I am also not quite sure what threat model here is?
The scripts changed here are not designed to run from a CGI context
and are not hardened for that purpose.
The idea is that you can run the scripts unmodified from the repository
without having to alter the files, which is convenient for development w.r.t to git.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: [PATCH] treewide: more portable bash shebangs

[Jordan Glover]

On Tuesday, July 16, 2019 10:08 PM, Jörg Thalheim <joerg@higgsboson.tk> wrote:

> On 16/07/2019 18.32, Jordan Glover wrote:
>
> > On Tuesday, July 16, 2019 12:21 PM, Jörg Thalheim joerg@higgsboson.tk wrote:
> >
> > > While /usr/bin/env is more or less available on all POSIX systems
> > > /bin/bash might not be. This is particular the case on NixOS and the BSD
> > > family (/usr/local/bin/bash). Downstream packagers would often rewrite
> > > those shebangs back automatically as they can rely on absolute paths
> > > but having portable shebangs in the repository helps to run the code
> > > without any further modification.
> >
> > The reason almost everyone hardcodes bash to /bin/bash is the potential
> > environment attack where someone create malicious "bash" and export it in PATH:
> > https://developer.apple.com/library/archive/documentation/OpenSource/Conceptual/ShellScripting/ShellScriptSecurity/ShellScriptSecurity.html
> > Obviously wg scripts are handling quite sensitive data like private keys...
> > Seriously if you except that downstream packagers would rewrite it back to
> > /bin/bash then why the others can't rewrite it to /usr/bin/env bash right
> > now if this is something they want?
> > Jordan
>
> This argument does not apply here since all commands internally could
> be redirected by a PATH change as well since the PATH is not set in the scripts.

Yep, that's something to actually fix as you won't fix things by making it worse.

> I am also not quite sure what threat model here is?
> The scripts changed here are not designed to run from a CGI context
> and are not hardened for that purpose.
> The idea is that you can run the scripts unmodified from the repository
> without having to alter the files, which is convenient for development w.r.t to git.

Then simply run "bash <script.sh>" and call it a day.

Jordan


_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: [PATCH] treewide: more portable bash shebangs

[Jörg Thalheim]

On 17/07/2019 18.32, Jordan Glover wrote:
> On Tuesday, July 16, 2019 10:08 PM, Jörg Thalheim <joerg@higgsboson.tk> wrote:
>
>> On 16/07/2019 18.32, Jordan Glover wrote:
>>
>>> On Tuesday, July 16, 2019 12:21 PM, Jörg Thalheim joerg@higgsboson.tk wrote:
>>>
>>>> While /usr/bin/env is more or less available on all POSIX systems
>>>> /bin/bash might not be. This is particular the case on NixOS and the BSD
>>>> family (/usr/local/bin/bash). Downstream packagers would often rewrite
>>>> those shebangs back automatically as they can rely on absolute paths
>>>> but having portable shebangs in the repository helps to run the code
>>>> without any further modification.
>>> The reason almost everyone hardcodes bash to /bin/bash is the potential
>>> environment attack where someone create malicious "bash" and export it in PATH:
>>> https://developer.apple.com/library/archive/documentation/OpenSource/Conceptual/ShellScripting/ShellScriptSecurity/ShellScriptSecurity.html
>>> Obviously wg scripts are handling quite sensitive data like private keys...
>>> Seriously if you except that downstream packagers would rewrite it back to
>>> /bin/bash then why the others can't rewrite it to /usr/bin/env bash right
>>> now if this is something they want?
>>> Jordan
>> This argument does not apply here since all commands internally could
>> be redirected by a PATH change as well since the PATH is not set in the scripts.
> Yep, that's something to actually fix as you won't fix things by making it worse.
It does not make anything worse. Your threat model is unreasonable and out of scope
of what the scripts are intended to guarantee.
There are tones of other environment variables like LD_PRELOAD or LD_LIBRARY_PATH

that would also need to be sanitized so they could run in an untrusted environment.

Hard-coding all these would make the scripts unportable for no good reason.

>
>> I am also not quite sure what threat model here is?
>> The scripts changed here are not designed to run from a CGI context
>> and are not hardened for that purpose.
>> The idea is that you can run the scripts unmodified from the repository
>> without having to alter the files, which is convenient for development w.r.t to git.
> Then simply run "bash <script.sh>" and call it a day.

I came across this because wireguard was used in a third-party build system not controlled by me,
so this is not a general solution.


>
> Jordan
>
>
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: [PATCH] treewide: more portable bash shebangs

[Jordan Glover]

On Wednesday, July 17, 2019 6:39 PM, Jörg Thalheim <joerg@higgsboson.tk> wrote:
>
> It does not make anything worse. Your threat model is unreasonable and out of scope
> of what the scripts are intended to guarantee.
> There are tones of other environment variables like LD_PRELOAD or LD_LIBRARY_PATH

And how exactly you get into position to decide what
scope is reasonable for wireguard scripts?

Same as you I can clam that your use-case is out of scope and instead
of forcing everyone else to patch-out your changes you can go back to
patching the code by yourself in a way you wish and with accordance to
threat model you may or not have. Good luck.

Jordan
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard