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