[Toybox] [patch] add su
Rob Landley
rob at landley.net
Wed Aug 14 19:55:17 PDT 2013
On 08/12/2013 11:20:40 PM, Strake wrote:
> =46rom 4693d474e5cdb95898bb0759eaac6a0d693ee33d Mon Sep 17 00:00:00
> 2001
> From: Strake <strake888 at gmail.com>
> Date: Mon, 12 Aug 2013 23:08:53 +0000
> Subject: [PATCH] add su
>
> ---
> toys/pending/su.c | 98
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 98 insertions(+)
> create mode 100644 toys/pending/su.c
>
> diff --git a/toys/pending/su.c b/toys/pending/su.c
> new file mode 100644
> index 0000000..7f0866d
> --- /dev/null
> +++ b/toys/pending/su.c
> @@ -0,0 +1,98 @@
> +/* su.c - switch user
> + *
> + * Copyright 2013 CE Strake <strake888 at gmail.com>
> + *
> + * See
> http://refspecs.linuxfoundation.org/LSB_4.1.0/LSB-Core-generic/LSB-Core-generic/su.html
> +
> +USE_SU(NEWTOY(su, "lmpc:s:", TOYFLAG_BIN))
> +
> +config SU
> + bool "su"
> + default n
> + help
> + usage: su [-lmp] [-c cmd] [-s shell] [user [argu...]]
I tend to have things like CMD SHELL USER be all caps to show it's not
a literal argument. I also tend to say ARGS instead of argu (there's a
couple existing instances in the tree, trying to be consistent).
I need to do a cleanup pass actually. df has [FILESYSTEM ...] when it
should probably have FILESYSTEM[...]. And of course chgrp has file...
chmod has FILE... cut has [FILE]... xzcat has [filename...] and so on.
(It'd be nice there was a standard on this. I'm sort of copying what
the man pages do but it's cargo cult documenting...)
Right, throw another todo item on the heap.
> + Switch to given user, or root if not given, and call a shell with
> the given arguments.
> +
> + options:
> + -s shell to call
> + -c command to pass to shell with -c
> +
> + flags:
> + -l login shell
> + -(m|p) preserve environment
> +*/
> +
> +#define FOR_su
> +#include "toys.h"
> +
> +GLOBALS(
> + char *sArgu;
> + char *cArgu;
> +)
I'm leaning more and more towards just having the argument variable be
the argument letter unless there's an immediately more obvious name.
TT.s is sort of its own namespace...
> +extern char **environ;
That should be in toys.h
> +static void deny () {
> + printf ("Denied\n");
> + xexit ();
> +}
> +
> +void su_main () {
> + char *name, *passhash, **argu, **argv;
> + struct passwd *up;
> + struct spwd *shp;
> + long ii;
> +
> + if (toys.optc && strcmp ("-", toys.optargs[0]) == 0) {
> + toys.optflags |= FLAG_l;
> + toys.optc--; toys.optargs++;
> + }
> +
> + if (toys.optc) {
> + name = toys.optargs[0];
> + toys.optc--; toys.optargs++;
> + }
This is a common enough idiom I'm starting to think there should be an
xwrap function for it. Hmmm...
> + else name = "root";
> + shp = getspnam (name);
> + if (!shp) perror_exit ("failed to find password");
See earlier discussion on minimal error messages aimed at non-english
speakers who have a hard enough time learning the posix command names
in a non-native alphabet. It's linked from
http://landley.net/toybox/cleanup.html which I _really_ need to catch
up on the writeups for...
The root user is uid 0, which could be _named_ anything. (Crazy people
say "wheel".) So we need a getwpuid(0) else getpwnam(name) and then do
the shadow lookup based on that name.
I'm curious what bionic/android does with this. First version I checked
user accounts weren't really implemented. I know they don't have an
/etc/passwd file. (Last time I implemented this stuff I parsed
/etc/passwd and /etc/shadow myself, but when I looked into android they
had some funky database stored in extended attributes or something; I
forget the details. Everybody keeps thinking the windows registry was
somehow a good idea...)
> + switch (shp -> sp_pwdp[0]) {
> + case '!': deny ();
> + case '$': break;
> + default : error_exit ("bad password format");
> + }
> +
> + if (read_password (toybuf, sizeof (toybuf), "Password: ") != 0)
> perror_exit ("failed to read password");
> +
> + passhash = crypt (toybuf, shp -> sp_pwdp);
> + if (!passhash) perror_exit ("failed to compute password hash");
> + for (ii = 0; toybuf[ii]; ii++) toybuf[ii] = 0;
Proper paranoia would probably swap those last two lines.
Hmmm, should the suid plumbing in main.c mlockall() when we don't drop
privs?
> + if (strcmp (passhash, shp -> sp_pwdp) != 0) deny ();
> +
> + up = getpwnam (name);
> + if (!up) perror_exit ("failed to getpwnam");
> +
> + if (setuid (up -> pw_uid) < 0) perror_exit ("failed to setuid");
> + if (chdir (up -> pw_dir) < 0) perror_exit ("failed to chdir");
xsetuid() and xchdir() are in lib/xwrap.c.
> + argu = xmalloc (sizeof (char *)*(toys.optc + 4));
> + argv = argu;
> + argv[0] = toys.optflags & FLAG_s ? TT.sArgu : up -> pw_shell;
Reading man page, "restricted shell", ala /etc/shell. Lovely. (If you
set a shell to /dev/null the user can't log in, _and_ can't run
anything via su either...)
I note that bash's behavior changes if argv[0] starts with "-".
> + if (toys.optflags & FLAG_c) {
> + argv[1] = toys.optflags & FLAG_l ? "-lc" : "-c";
> + argv[2] = TT.cArgu;
> + argv += 2;
> + }
> + else if (toys.optflags & FLAG_l) (argv++)[1] = "-l";
Is there a reason you can't do -l -c as two arguments and just test
once?
> + for (ii = 0; ii < toys.optc; ii++) argv[ii + 1] = toys.optargs[ii];
Probably memcpy.
> + if (execve (argu[0], argu,
> + toys.optflags & FLAG_l ? (char *[]){
> + xmsprintf ( "HOME=%s", up -> pw_dir),
> + xmsprintf ("SHELL=%s", up -> pw_shell),
> + xmsprintf ( "USER=%s", up -> pw_name),
> + xmsprintf ( "TERM=%s", getenv ("TERM")),
> + 0
> + } : environ) < 0) perror_exit ("failed to exec %s",
> argu[0]);
You never have to check the return value of execve(). I dunno why they
bother having one. If it returns, it failed.
I'll try to take a swipe at cleaning this up and moving it out of
pending.
Thanks,
Rob
1376535317.0
More information about the Toybox
mailing list