On Fri, 10 Jun 2016 16:01:22 +0200
Quentin Glidic <[email protected]> wrote:

> From: Quentin Glidic <[email protected]>
> 
> This avoids the need to maintain two parallel shell profile files for
> users with a compatible shell (at least bash, sh and zsh are).
> 
> There is no major security issue here, as the shell is the one returned
> from the password database, and thus is retricted by /etc/shells (or
> root override).
> 
> Signed-off-by: Quentin Glidic <[email protected]>
> ---
> 
> v2: Added some rationale to the commit message

Hi Quentin,

thanks for that, this patch is now:
Reviewed-by: Pekka Paalanen <[email protected]>

However, I would like to have someone say they would benefit from this
patch. Then it would be easy to land this. Otherwise I'm a bit torn.
Yes, it's a simple addition and it looks safe, but it's still adding a
new optional path to a setuid-root binary, so it's not with zero cost.


Thanks,
pq

>  src/weston-launch.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/src/weston-launch.c b/src/weston-launch.c
> index 140fde1..c206094 100644
> --- a/src/weston-launch.c
> +++ b/src/weston-launch.c
> @@ -108,6 +108,7 @@ struct weston_launch {
>       pid_t child;
>       int verbose;
>       char *new_user;
> +     int use_user_shell;
>  };
>  
>  union cmsg_data { unsigned char b[4]; int fd; };
> @@ -613,7 +614,10 @@ setup_session(struct weston_launch *wl, char 
> **child_argv)
>        * We open a new session, so it makes sense
>        * to run a new login shell
>        */
> -     child_argv[0] = "/bin/sh";
> +     if (wl->use_user_shell)
> +             child_argv[0] = wl->pw->pw_shell;
> +     else
> +             child_argv[0] = "/bin/sh";
>       child_argv[1] = "-l";
>       child_argv[2] = "-c";
>       child_argv[3] = BINDIR "/weston \"$@\"";
> @@ -675,10 +679,11 @@ static void
>  help(const char *name)
>  {
>       fprintf(stderr, "Usage: %s [args...] [-- [weston args..]]\n", name);
> -     fprintf(stderr, "  -u, --user      Start session as specified 
> username\n");
> -     fprintf(stderr, "  -t, --tty       Start session on alternative tty\n");
> -     fprintf(stderr, "  -v, --verbose   Be verbose\n");
> -     fprintf(stderr, "  -h, --help      Display this help message\n");
> +     fprintf(stderr, "  -u, --user              Start session as specified 
> username\n");
> +     fprintf(stderr, "  -s, --use-user-shell    Use the user login shell 
> (from PAM) instead of /bin/sh, only has effect with --user\n");
> +     fprintf(stderr, "  -t, --tty               Start session on alternative 
> tty\n");
> +     fprintf(stderr, "  -v, --verbose           Be verbose\n");
> +     fprintf(stderr, "  -h, --help              Display this help 
> message\n");
>  }
>  
>  int
> @@ -688,11 +693,12 @@ main(int argc, char *argv[])
>       int i, c;
>       char *tty = NULL;
>       struct option opts[] = {
> -             { "user",    required_argument, NULL, 'u' },
> -             { "tty",     required_argument, NULL, 't' },
> -             { "verbose", no_argument,       NULL, 'v' },
> -             { "help",    no_argument,       NULL, 'h' },
> -             { 0,         0,                 NULL,  0  }
> +             { "user",           required_argument, NULL, 'u' },
> +             { "use-user-shell", no_argument,       NULL, 's' },
> +             { "tty",            required_argument, NULL, 't' },
> +             { "verbose",        no_argument,       NULL, 'v' },
> +             { "help",           no_argument,       NULL, 'h' },
> +             { 0,                0,                 NULL,  0  }
>       };
>  
>       memset(&wl, 0, sizeof wl);
> @@ -704,6 +710,9 @@ main(int argc, char *argv[])
>                       if (getuid() != 0)
>                               error(1, 0, "Permission denied. -u allowed for 
> root only");
>                       break;
> +             case 's':
> +                     wl.use_user_shell = 1;
> +                     break;
>               case 't':
>                       tty = optarg;
>                       break;

Attachment: pgp_9qVlCuhhk.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to