[libvirt] [PATCH 16/17] virt-login-shell: add ability to auto-detect shell from container

[John Ferlan]

On 04/14/2016 11:22 AM, Daniel P. Berrange wrote:
> Currently the shell must be looked up from the config setting in
> /etc/libvirt/virt-login-shell.conf. This is inflexible if there
> are containers where different users need different shells. Add
> add a new 'auto-shell' config parameter which instructs us to
> query the containers' /etc/passwd for the shell to be exec'd.
> 
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  tools/virt-login-shell.c    | 33 +++++++++++++++++++++++++++++++++
>  tools/virt-login-shell.conf | 14 ++++++++++++++
>  2 files changed, 47 insertions(+)
> 

Should there be a virt-login-shell.pod change too in the CONFIG section?
  Hmmm.. it just dawned on me, should there be virt-login-shell.pod
change for the -c addition?

> diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
> index 6e8056d..7680266 100644
> --- a/tools/virt-login-shell.c
> +++ b/tools/virt-login-shell.c
> @@ -100,6 +100,24 @@ static int virLoginShellAllowedUser(virConfPtr conf,
>      return ret;
>  }
>  
> +static int virLoginShellGetAutoShell(virConfPtr conf, bool *autoshell)

I see virLoginShellAllowedUser goes w/ multiple lies for each argument...

> +{
> +    virConfValuePtr p;
> +
> +    p = virConfGetValue(conf, "auto_shell");
> +    if (!p) {
> +        autoshell = false;

*autoshell

> +    } else if (p->type == VIR_CONF_LONG ||
> +               p->type == VIR_CONF_ULONG) {
> +        *autoshell = (p->l != 0);
> +    } else {
> +        virReportSystemError(EINVAL, "%s",
> +                             _("auto-shell must be a boolean value"));

s/-/_/

> +        return -1;
> +    }
> +    return 0;
> +}
> +
>  static int virLoginShellGetShellArgv(virConfPtr conf,
>                                       char ***retshargv,
>                                       size_t *retshargvlen)
> @@ -221,6 +239,7 @@ main(int argc, char **argv)
>      char *tmp;
>      char *term = NULL;
>      virErrorPtr saved_err = NULL;
> +    bool autoshell = false;
>  
>      struct option opt[] = {
>          {"help", no_argument, NULL, 'h'},
> @@ -300,6 +319,8 @@ main(int argc, char **argv)
>  
>      if (virLoginShellGetShellArgv(conf, &shargv, &shargvlen) < 0)
>          goto cleanup;

Extra line between to follow existing...

> +    if (virLoginShellGetAutoShell(conf, &autoshell) < 0)
> +        goto cleanup;
>  
>      conn = virConnectOpen("lxc:///");
>      if (!conn)
> @@ -351,6 +372,18 @@ main(int argc, char **argv)
>          goto cleanup;
>      }
>  
> +    if (autoshell) {
> +        tmp = virGetUserShell(uid);
> +        if (tmp) {
> +            virStringFreeList(shargv);
> +            shargvlen = 1;
> +            if (VIR_ALLOC_N(shargv[0], shargvlen + 1) < 0)

tmp will be leaked.  It's 'overloaded' with the subsequent strrchr


ACK - with the obvious adjustments...

John
> +                goto cleanup;
> +            shargv[0] = tmp;
> +            shargv[1] = NULL;
> +        }
> +    }
> +
>      if (cmdstr) {
>          if (VIR_REALLOC_N(shargv, shargvlen + 3) < 0)
>              goto cleanup;
> diff --git a/tools/virt-login-shell.conf b/tools/virt-login-shell.conf
> index e29d222..4a504b3 100644
> --- a/tools/virt-login-shell.conf
> +++ b/tools/virt-login-shell.conf
> @@ -18,6 +18,20 @@
>  # Note there is no need to pass a '--login' / '-l' argument since
>  # virt-login-shell will always request a login shell
>  
> +# Normally virt-login-shell will always use the shell identified
> +# by the 'shell' configuration setting above. If the container
> +# is running a full OS, it might be desirable to allow the choice
> +# of shell to be delegated to the owner of the shell, by querying
> +# the /etc/passwd file inside the container
> +#
> +# To allow for that, uncomment the following:
> +# auto_shell = 1
> +#
> +# NB, this should /not/ be used if any container is sharing the
> +# host filesystem /etc, as this would cause virt-login-shell to
> +# look at the host's /etc/passwd finding itself as the listed
> +# shell. Hilarious recursion would then ensue.
> +
>  # 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
>  # separated list of usernames or user groups.
>