On Wed, Apr 27, 2016 at 02:28:33PM -0400, John Ferlan wrote:
>
>
> 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.
Yep.
> > 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?
The struct option entry is only required if we need to support a
--long argument alternative. These two only need short args so
are omitted here.
> ACK with the changes -
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list