Re: [PATCH 12/17] virt-login-shell: change way we request a login shell

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 04/14/2016 11:22 AM, Daniel P. Berrange wrote:
> Currently we request a login shell by passing the -l argument
> to the shell. This is either hardcoded, or required to be
> specified by the user in the virt-login-shell.conf file.
> 
> The standard way for login programs to request a shell run
> as a login shell is to modify the argv passed to execve()
> so that argv[0] contains the relative shell filename
> prefixed with a zero. eg instead of doing
> 
>   const char **shellargs = ["/bin/bash", "-l", NULL];
>   execve(shellargs[0], shellargs, env);
> 
> We should be doing
> 
>   const char **shellargs = ["-bash", NULL];
>   execve("/bin/bash", shellargs, env);
> 
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
>  tools/virt-login-shell.c    | 27 ++++++++++++++++++++++-----
>  tools/virt-login-shell.conf |  6 +++++-
>  tools/virt-login-shell.pod  |  2 +-
>  3 files changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
> index 34a8836..1ca58ee 100644
> --- a/tools/virt-login-shell.c
> +++ b/tools/virt-login-shell.c
> @@ -111,7 +111,7 @@ static int virLoginShellGetShellArgv(virConfPtr conf,
>  
>      p = virConfGetValue(conf, "shell");
>      if (!p) {
> -        len = 2; /* /bin/sh -l */
> +        len = 1; /* /bin/sh */
>      } else if (p->type == VIR_CONF_LIST) {
>          /* Calc length and check items */
>          for (len = 0, pp = p->list; pp; len++, pp = pp->next) {
> @@ -136,8 +136,6 @@ static int virLoginShellGetShellArgv(virConfPtr conf,
>      if (!p) {
>          if (VIR_STRDUP(shargv[i++], "/bin/sh") < 0)
>              goto error;
> -        if (VIR_STRDUP(shargv[i++], "-l") < 0)
> -            goto error;
>      } else if (p->type == VIR_CONF_LIST) {
>          for (pp = p->list; pp; pp = pp->next) {
>              if (VIR_STRDUP(shargv[i++], pp->str) < 0)
> @@ -200,6 +198,7 @@ main(int argc, char **argv)
>      char *name = NULL;
>      char **shargv = NULL;
>      size_t shargvlen = 0;
> +    char *shcmd = NULL;
>      virSecurityModelPtr secmodel = NULL;
>      virSecurityLabelPtr seclabel = NULL;
>      virDomainPtr dom = NULL;
> @@ -214,6 +213,7 @@ main(int argc, char **argv)
>      int openmax;
>      size_t i;
>      const char *cmdstr = NULL;
> +    char *tmp;
>  
>      struct option opt[] = {
>          {"help", no_argument, NULL, 'h'},
> @@ -356,6 +356,22 @@ main(int argc, char **argv)
>          shargv[shargvlen] = NULL;
>      }
>  
> +    /* We need to modify the first elementin shargv
> +     * so that it has the relative filename and has
> +     * a leading '-' to indicate it is a login shell
> +     */
> +    shcmd = shargv[0];
> +    if (shcmd[0] != '/') {
> +        virReportSystemError(errno,
> +                             _("Shell '%s' should have absolute path"),
> +                             shcmd);
> +        goto cleanup;
> +    }
> +    tmp = strrchr(shcmd, '/');
> +    if (VIR_STRDUP(shargv[0], tmp) < 0)
> +        goto cleanup;
> +    shargv[0][0] = '-';
> +
>      /* A fork is required to create new process in correct pid namespace.  */
>      if ((cpid = virFork()) < 0)
>          goto cleanup;
> @@ -367,9 +383,9 @@ main(int argc, char **argv)
>              tmpfd = i;
>              VIR_MASS_CLOSE(tmpfd);
>          }
> -        if (execv(shargv[0], (char *const*) shargv) < 0) {
> +        if (execv(shcmd, (char *const*) shargv) < 0) {
>              virReportSystemError(errno, _("Unable to exec shell %s"),
> -                                 shargv[0]);
> +                                 shcmd);
>              virDispatchError(NULL);
>              return errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE;
>          }
> @@ -388,6 +404,7 @@ main(int argc, char **argv)
>      if (conn)
>          virConnectClose(conn);
>      virStringFreeList(shargv);
> +    VIR_FREE(shcmd);
>      VIR_FREE(name);
>      VIR_FREE(homedir);
>      VIR_FREE(seclabel);
> diff --git a/tools/virt-login-shell.conf b/tools/virt-login-shell.conf
> index 835fd3f..67ca1cc 100644
> --- a/tools/virt-login-shell.conf
> +++ b/tools/virt-login-shell.conf
> @@ -8,7 +8,11 @@
>  # container.  Shell commands must be a list of commands/options separated by
>  # comma and delimited by square brackets. Defaults to: /bin/sh -l.

Need to adjust the "Defaults to:", right?


ACK w/ that.

John
>  # Modify and uncomment the following to modify the login shell.
> -# shell = [ "/bin/sh",  "-l" ]
> +#
> +# shell = [ "/bin/bash" ]
> +#
> +# Note there is no need to pass a '--login' / '-l' argument since
> +# virt-login-shell will always request a login shell
>  
>  # allowed_users specifies the user names of all users that are allowed to
>  # execute virt-login-shell.  You can specify the users as a comma
> diff --git a/tools/virt-login-shell.pod b/tools/virt-login-shell.pod
> index 56861f7..99eeed8 100644
> --- a/tools/virt-login-shell.pod
> +++ b/tools/virt-login-shell.pod
> @@ -39,7 +39,7 @@ By default, virt-login-shell will execute the /bin/sh program for the user.
>  You can modify this behaviour by defining the shell variable in
>  /etc/libvirt/virt-login-shell.conf.
>  
> -eg.  shell = [ "/bin/ksh", "--login"]
> +eg.  shell = [ "/bin/bash" ]
>  
>  By default no users are allowed to use virt-login-shell, if you want to allow
>  certain users to use virt-login-shell, you need to modify the allowed_users
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]