On 04/14/2016 11:22 AM, Daniel P. Berrange wrote:
> The virt-login-shell program is supposed to look like a
> regular shell to clients. Login services like sshd
> expect the shell to accept a '-c cmdstring' argument to
> specify a command to launch instead of presenting an
> interactive prompt.
>
> We can implement this by simply passing the '-c cmdstring'
> data straight through to the real shell we use. This does
> not open any security holes, since the command is not run
> until we're inside the container namespaces. This allows
> scp to work for users with virt-login-shell.
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> tools/virt-login-shell.c | 70 +++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 54 insertions(+), 16 deletions(-)
>
> diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
> index ec759dc..34a8836 100644
> --- a/tools/virt-login-shell.c
> +++ b/tools/virt-login-shell.c
> @@ -100,20 +100,19 @@ static int virLoginShellAllowedUser(virConfPtr conf,
> return ret;
> }
>
> -static char **virLoginShellGetShellArgv(virConfPtr conf)
> +static int virLoginShellGetShellArgv(virConfPtr conf,
> + char ***retshargv,
> + size_t *retshargvlen)
> {
> size_t i;
> + size_t len;
> char **shargv = NULL;
> - virConfValuePtr p;
> + virConfValuePtr p, pp;
>
> p = virConfGetValue(conf, "shell");
> - if (!p)
> - return virStringSplit("/bin/sh -l", " ", 3);
> -
> - if (p->type == VIR_CONF_LIST) {
> - size_t len;
> - virConfValuePtr pp;
> -
> + if (!p) {
> + len = 2; /* /bin/sh -l */
> + } else if (p->type == VIR_CONF_LIST) {
> /* Calc length and check items */
> for (len = 0, pp = p->list; pp; len++, pp = pp->next) {
> if (pp->type != VIR_CONF_STRING) {
> @@ -122,18 +121,41 @@ static char **virLoginShellGetShellArgv(virConfPtr conf)
> goto error;
> }
> }
> + } else {
> + virReportSystemError(EINVAL, "%s",
> + _("shell must be a list of strings"));
> + goto error;
> + }
>
> - if (VIR_ALLOC_N(shargv, len + 1) < 0)
> + len++; /* NULL terminator */
> +
> + if (VIR_ALLOC_N(shargv, len) < 0)
> + goto error;
> +
> + i = 0;
> + if (!p) {
> + if (VIR_STRDUP(shargv[i++], "/bin/sh") < 0)
> goto error;
> - for (i = 0, pp = p->list; pp; i++, pp = pp->next) {
> - if (VIR_STRDUP(shargv[i], pp->str) < 0)
> + 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)
> goto error;
> }
> }
> - return shargv;
> +
> + shargv[i] = NULL;
> +
> + *retshargvlen = i;
> + *retshargv = shargv;
> +
> + return 0;
> error:
> + *retshargv = NULL;
> + *retshargvlen = 0;
> virStringFreeList(shargv);
> - return NULL;
> + return -1;
> }
>
> static char *progname;
Right in here somewhere there's some 'usage()' that should probably be
updated.
> @@ -177,6 +199,7 @@ main(int argc, char **argv)
> gid_t gid = getgid();
> char *name = NULL;
> char **shargv = NULL;
> + size_t shargvlen = 0;
> virSecurityModelPtr secmodel = NULL;
> virSecurityLabelPtr seclabel = NULL;
> virDomainPtr dom = NULL;
> @@ -190,6 +213,7 @@ main(int argc, char **argv)
> int *fdlist = NULL;
> int openmax;
> size_t i;
> + const char *cmdstr = NULL;
>
> struct option opt[] = {
> {"help", no_argument, NULL, 'h'},
Should this add a "-c" w/ required_argument?
I also see the "V" lists "optional_argument", but that's not part of the
"hV" below, so shouldn't it be no_argument?
ACK with the changes -
John
> @@ -220,7 +244,7 @@ main(int argc, char **argv)
> return ret;
> }
>
> - while ((arg = getopt_long(argc, argv, "hV", opt, &longindex)) != -1) {
> + while ((arg = getopt_long(argc, argv, "hVc:", opt, &longindex)) != -1) {
> switch (arg) {
> case 'h':
> usage();
> @@ -230,6 +254,10 @@ main(int argc, char **argv)
> show_version();
> exit(EXIT_SUCCESS);
>
> + case 'c':
> + cmdstr = optarg;
> + break;
> +
> case '?':
> default:
> usage();
> @@ -265,7 +293,7 @@ main(int argc, char **argv)
> if (virLoginShellAllowedUser(conf, name, groups) < 0)
> goto cleanup;
>
> - if (!(shargv = virLoginShellGetShellArgv(conf)))
> + if (virLoginShellGetShellArgv(conf, &shargv, &shargvlen) < 0)
> goto cleanup;
>
> conn = virConnectOpen("lxc:///");
> @@ -318,6 +346,16 @@ main(int argc, char **argv)
> goto cleanup;
> }
>
> + if (cmdstr) {
> + if (VIR_REALLOC_N(shargv, shargvlen + 3) < 0)
> + goto cleanup;
> + if (VIR_STRDUP(shargv[shargvlen++], "-c") < 0)
> + goto cleanup;
> + if (VIR_STRDUP(shargv[shargvlen++], cmdstr) < 0)
> + goto cleanup;
> + shargv[shargvlen] = NULL;
> + }
> +
> /* A fork is required to create new process in correct pid namespace. */
> if ((cpid = virFork()) < 0)
> goto cleanup;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list