As best as I can recall, running ikiwiki-mass-rebuild as root has never worked for me on NetBSD or Mac OS X. On both platforms, it gives me a shell as each user in the system wikilist. This is due to non-portable arguments to su(1).

The following patch works much better on the aforementioned platforms, as well as CentOS 6:

diff --git ikiwiki-mass-rebuild ikiwiki-mass-rebuild
index ce4e084e8..2ff33b493 100755
--- ikiwiki-mass-rebuild
+++ ikiwiki-mass-rebuild
@@ -32,7 +32,7 @@ sub processuser {
    my $user=shift;
    return if $user=~/^-/ || $users{$user};
    $users{$user}=1;
-   my $ret=system("su", $user, "-s", "/bin/sh", "-c", "--", "$0 --nonglobal @ARGV");
+   my $ret=system("su", "-m", $user, "-c", "/bin/sh -c -- '$0 --nonglobal @ARGV'");
    if ($ret != 0) {
        print STDERR "warning: processing for $user failed with code $ret\n";
    }

The -m may be overzealous. I have some sites running as users with /sbin/nologin for a shell, and this allows running a command as those users, though without some typical environment variables. This is probably wrong. Maybe I should be doing something else to limit shell access for those users, and the su arg should instead be -.

--schmonz

To get some real-world and very cross-platform testing, I've committed a conservative version of this patch, with - in place of -m, to pkgsrc's ikiwiki package (rev 3.20180311nb1), and will report back. In the meanwhile, would this change cause any obvious regressions on Debian? --schmonz

su(1) does several things for us, not all of them completely obvious:

  • raise or drop privileges
  • avoid inheriting the controlling tty
  • alter the environment
  • run a PAM stack which can do more or less anything
  • execute the given command

Because it's a privileged program, and POSIX/SUS don't specify the behaviour of privileged operations, its behaviour is determined by tradition rather than standards.

Dropping privileges (in this case) is uncontroversial: clearly we want to do that.

Not inheriting the controlling tty is necessary to prevent tty hijacking when dropping privileges (CVE-2011-1408, Debian bug #628843). See ikiwiki-mass-rebuild's git history. It might also be possible to do this with POSIX::setsid, but I don't know whether that fully protects us on all platforms, and I would hope that every platform's su does the right things for that platform.

Altering the environment is less clear. I'm taking the su(1) from Debian as a reference because that's what Joey would have developed against, and it has several modes for how much it does to the environment:

  • with -m (or equivalently -p or --preserve-environment): reset only PATH and IFS; inherit everything else. I'm fairly sure we don't want this, because we don't want ikiwiki to run with root's HOME.
  • without -m or -: reset HOME, SHELL, USER, LOGNAME, PATH and IFS; inherit everything else.
  • with - (or equivalently -l or --login) but not -m: reset HOME, etc.; inherit TERM, COLORTERM, DISPLAY and XAUTHORITY; clear everything else.

Before Joey switched ikiwiki-mass-rebuild from dropping privileges itself to using su to fix CVE-2011-1408, it would reset HOME, inherit PATH (!) and clear everything else. Using plain su without - and without clearing the environment is increasingly discredited, because it isn't 1980 any more and a lot of programs respect environment variables whose correct values are user-specific, such as XDG_RUNTIME_DIR and DBUS_SESSION_BUS_ADDRESS. So I think using su - would be reasonable and perhaps preferable.

Running the PAM stack is essentially unavoidable when we're altering privileges like this, and it's what PAM is there for, so we should do it. I think some su implementations (although not the one in Debian) run different PAM stacks for su and su -.

Finally, running the command. su has two design flaws in this area:

  • The command is a string to be parsed by the shell, not an argument vector; on Linux, this design flaw can be avoided by using runuser -u USER ... -- COMMAND [ARGUMENT...] from util-linux instead (essentially a non-setuid fork of util-linux su with more reasonable command-line handling), and on many Unix systems it can be avoided by using sudo -u USER ... -- COMMAND [ARGUMENT...], but presumably neither is available as standard on all OSs because that would be far too helpful. runuser is also (still) vulnerable to TIOCSTI tty hijacking, because its developers think that ioctl has no legitimate uses and should be disabled or made a privileged operation in the Linux kernel, but the Linux kernel maintainers have rejected that solution and neither seems to be willing to back down.

    We might be able to bypass this with this trick:

    system('su', ..., '--', '-c', 'exec "$0" "$@"', $0, @ARGV);

    using the fact that arguments to a Bourne/POSIX shell after -c are set as $0, $1, ... in the shell. But the second design flaw makes this unreliable.

  • -c is specified to run the given command with the user's login shell from /etc/passwd (which might be nologin or csh or anything else), not a standardized Bourne/POSIX shell, so you can't predict what (if anything) the given command will actually do, or even how to quote correctly. On Linux, giving -s /bin/sh works around this design flaw, but apparently that's not portable or we wouldn't be having this discussion.

In principle ikiwiki-mass-rebuild was already wrong here, becase it receives arbitrary arguments and passes them to ikiwiki, but will do the wrong thing if they contain shell metacharacters (this is not a security vulnerability, because it's the unprivileged shell that will do the wrong thing; it's just wrong). Your proposed change makes it differently wrong, which I suppose is not necessarily worse, but I'd prefer it to be actually correct.

It seems that by using -m you're relying on root having a Bourne-compatible (POSIX) login shell, so that when SHELL is inherited from root's environment, it will parse the argument of -c according to /bin/sh rules. This is less reliable than Linux su -s /bin/sh and has more side-effects, but the man page collection on unix.com suggests that this meaning for -s is Linux-specific and has not been copied by any other OSs, which is depressing because that option seems to be the only way to achieve what we want.

In conclusion, non-interactive su is a disaster area, but we use it because traditional Unix terminal handling is also a disaster area, and I don't see a good solution. --smcv

After reading this, appreciating your effort writing it, and then ignoring it for a while, I think our easiest option might be to take a dependency on sudo. It's ubiquitous-ish, and where it's not already present the dependency feels more "suggested" than "required": ikiwiki is plenty useful for many/most uses without a working ikiwiki-mass-rebuild (as I can vouch). A slightly more annoying and thorough option might be to make the run-as-user command configurable, with some strong suggestions and warnings. Thoughts? --schmonz

Here's what I'm experimenting with now:

my $ret=system("sudo", "-n", "-s", "-u", $user, "/bin/sh", "-c", "--", "$0", "--nonglobal", @ARGV);

--schmonz

Available in a git repository branch.
Branch: schmonz/sudo-mass-rebuild
Author: schmonz

Works well for me on macOS and NetBSD. Does it look right? Can someone vouch that there is indeed no functional change on Debian? --schmonz