Do not make mksh the default shell on macOS

[Demetrius Iatrakis <demetrius.iatrakis%gmail.com@localhost>]

Hello everyone,

I would like to bring revision 1.286 to bootstrap/bootstrap (Enable
mksh by default on macOS 10.11+) to the community's attention, as
the reason for this change is invalid.  The committer claims to
have addressed my concerns, and is currently not responding.  I
asked for a second opinion on IRC and received it, but my response
to it has not been acknowledged. See https://git.io/JJO9N.

I will restate my case here, in a manner hopefully understandable
by those not familiar with macOS:

1.  There is a feature on macOS systems called System Integrity
Protection (SIP).  SIP is claimed by Apple to protect the runtime
integrity of software by preventing certain classes of exploits,
like code injection, dynamically linked library (DLL) hijacking,
and process memory space tampering.

2.  When an app is launched, the macOS kernel loads the app's code
and data into the address space of a new process.  The kernel also
loads the dynamic linker (dyld) into the process and passes control
to it.  The dynamic linker then loads the app's dependent libraries.

3.  The dynamic linker checks a number of environment variables
during the launch of each process, including DYLD_LIBRARY_PATH.
DYLD_LIBRARY_PATH is a colon separated list of directories that
contain libraries.  The dynamic linker searches these directories
before it searches the default locations for libraries.  It allows
one to test new versions of existing libraries.  For each library
that a program uses, the dynamic linker looks for its leaf name in
each directory in DYLD_LIBRARY_PATH.

4.  Any dynamic linker (dyld) environment variables, such as
DYLD_LIBRARY_PATH, are purged when launching SIP-protected processes.

5.  All system shells are SIP-protected.

6.  Commands executed by the shell inherit the environment.

7.  A wrapper is a shell script that embeds a system command or
utility, and accepts and passes a set of parameters to that command.

8.  pkg_alternatives is a tool to manage the alternatives system
provided by pkgsrc.  It creates, configures, and destroys generic
wrappers used to run programs with similar interfaces.

9.  It follows from the foregoing that the program embedded in the
wrapper will not inherit any dynamic linker environment variables
if those are passed directly to the wrapper.  This behaviour hinders
the legitimate use of those variables for programs embedded in the
wrappers.

10.  Dynamic linker environment variables can be passed to shells
that are not SIP-protected.  Revision 1.286 to bootstrap/bootstrap
makes mksh, a non-system shell, the default shell for macOS, citing
the above as justification and claiming that this change negates
the aforementioned issue.

11.  One must take into account that dynamic linker environment
variables affect the shell as well.  For example,
`DYLD_LIBRARY_PATH=/path_with_dylibs ctest` is a legitimate use of
ctest.  However, if ctest is in fact a pkg_alternatives wrapper,
the dynamic linker will load the libraries in /path_with_dylibs for
the shell.  This means that if a library with the same leaf name
as one the shell requires is in /path_with_dylibs, the dynamic
linker will load that one instead.  This is bound to change the
shell's behaviour and is clearly not the user's intention.  This
also affects auxiliary programs used in the wrapper, such as grep
and sed.  There are few such libraries in this particular case, but
this is a coincidence due to pkg_alternatives' implementation; this
issue affects all instances where a dynamic linker environment
variable is incorrectly passed to the parent of a process instead
of the process itself.  As it is impossible to guarantee that a
directory in DYLD_LIBRARY_PATH will not contain such libraries,
this revision does not fix the aforementioned issue, but instead
encourages the dangerous use of dynamic linker environment variables.

12.  As the only reason for this revision is fixing the aforementioned
issue, this leaves no valid reasons for the revision.  Therefore,
it should be reverted.

At the time of writing, the committer, jperkin@, has not responded
to point 11.  The committer claims that possible regressions would
occur if this change were reverted, and has cited Ninja, Meson, and
GObject Introspection as examples.  However, the fix for GObject
Introspection has already been imported in the tree, as can be seen
in this thread:
https://mail-index.netbsd.org/tech-pkg/2020/02/16/msg022627.html.
mksh also seems to be the default shell for Ninja, the Meson backend:
https://git.io/JJO9A.  Whether this should be the case given that
Meson itself is supposed to have fixed the issue in https://git.io/JJO9h,
and whether DYLD_FALLBACK_LIBRARY_PATH is a better idea since
LD_LIBRARY_PATH only works for leaf names (i.e. LD_LIBRARY_PATH
does not affect libraries with a slash in their path) on macOS is
outside the scope of this e-mail.  Since it is difficult to test
for any regressions for all packages, as bulk builds cannot be
performed on SIP-enabled platforms, it is better to fix any affected
packages on a case-by-case basis to avoid the risks outlined in
point 11.

--
Demetrius Iatrakis