On Monday 29 October 2018 05:24 AM, Daniel Kahn Gillmor wrote: [...] >> # usermod -s /usr/sbin/nologin monkeysphere >> # su monkeysphere ls >> This account is currently not available. >> # runuser --user monkeysphere ls >> <output> > > reading the documentation for runuser: > > -s, --shell=shell > Run the specified shell instead of the default. The shell to > run is selected according to the following rules, in order: > > o the shell specified with --shell > > o the shell specified in the environment variable SHELL > if the --preserve-environment option is used > > o the shell listed in the passwd entry of the target > user > > o /bin/sh > > If the target user has a restricted shell (i.e. not listed in > /etc/shells) the --shell option and the SHELL environment vari‐ > ables are ignored unless the calling user is root. > > > which of these cases do you think is triggering the shell selection in > run_as_monkeysphere_user? >
I believe this option is only applicable if we run `runuser - monkeysphere` and not when we run `runuser --user monkeysphere`. From runuser man page: "runuser allows to run commands with a substitute user and group ID. If the option -u is not given, it falls back to su-compatible semantics and a shell is executed." Running with -u --------------- # usermod -s /bin/dash monkeysphere # runuser -u monkeysphere --shell /bin/dash sleep 100 runuser: options --{shell,fast,command,session-command,login} and --user are mutually exclusive # runuser -u monkeysphere sleep 100 # pstree -ap 8969 bash,8969 `-runuser,32377 -u monkeysphere sleep 100 `-sleep,32378 100 Running with - -------------- # usermod -s /bin/dash monkeysphere # runuser monkeysphere -s /bin/dash -c 'sleep 100' # pstree -ap 8969 bash,8969 `-runuser,4677 monkeysphere -s /bin/dash -c sleep 100 `-dash,4678 -c sleep 100 `-sleep,4679 100 # runuser monkeysphere -c 'sleep 100' # pstree -ap 8969 bash,8969 `-runuser,5403 monkeysphere -c sleep 100 `-dash,5404 -c sleep 100 `-sleep,5405 100 [..] >> # TESTVAR1='Hello!' runuser -u monkeysphere -- bash -c 'set' |grep TESTVAR1 >> TESTVAR1='Hello!' > > while we certainly *can* do this, this change means that runuser itself > will see the changed environment variable, which might affect runuser's > behavior, or the behavior of the (arbitrary, unknown!) pam stack that > runuser executes. Is that really a desirable outcome? in general, i'd > lean toward the smallest perturbation possible. Can we avoid that > problem? > > What if we replaced "su_monkeysphere_user KEY=value foo arg" with > "run_as_monkeysphere_user env KEY=value foo arg" instead? I agree that it is better to not expose the environment to runuser. I will make the change to use 'env' instead. A bigger concern should be to scrub all environment from the parent root user process except for the values that need to be passed down. Unfortunately, this is an issue equally problematic in the earlier and proposed code. # TESTVAR1='test' su -s /bin/bash -c 'set' |grep TESTVAR TESTVAR1=test # TESTVAR1='test' runuser -u monkeysphere -- bash -c set |grep TESTVAR TESTVAR1=test I will try to scrub the environment using 'env -i' and see if that introduces any breakages. > >> 4) Redirection expressions like what you have described above currently >> do not exist. I added documentation that bash expressions should not be >> run using the method. > > I didn't mean to imply that redirection was the only thing. what about > other shell metacharacters, like ';' or '||' ? > > When you say "no shell expressions" in the comments, does that include > function invocations? variable assignments? Yes, that includes: - No function invocations which have been done explicitly using bash -c '. <source>; <function>'. - No shell metacharacters such as '&', ';', '|', '||', '<', '>' etc. - Variable assignments were done but handled differently in my patch (above discussion). > >> PS: I ran into issues with TMPDIR as described in #656750 and I am >> currently working around them. > > hm, this sounds like it might be an interaction across your changes made > to point 3, above, but i'm not sure how. can you explain the issues > you're running into in more detail? can you explain your workaround? > I belive this is because I have libpam-tmpdir installed on my test VM (FreedomBox). I will test my next patch without this. Thanks, -- Sunil
signature.asc
Description: OpenPGP digital signature