[Libvir] PATCH: 6/10: remote driver auth callback API

[Daniel P. Berrange]

On Mon, Dec 03, 2007 at 11:53:04PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange at redhat.com> wrote:
> > This patch implements internal driver API for authentication callbacks
> > in the remote driver. It is basically a bunch of code to bridge from
> > the libvirt public API for auth/credentials and the SASL equivalent
> > API. The libvirt API is very close in style to the SASL API so it is
> > a fairly mechanical mapping.
> 
> Hi Dan,
> 
> I have to start by admitting I've never used or even looked at
> policykit before.
> 
> > diff -r 98599cfde033 src/libvirt.c
> > --- a/src/libvirt.c	Wed Nov 28 23:01:08 2007 -0500
> > +++ b/src/libvirt.c	Wed Nov 28 23:29:58 2007 -0500
> > @@ -62,6 +62,78 @@ static int initialized = 0;
> >  #define DEBUG0
> >  #define DEBUG(fs,...)
> >  #endif /* !ENABLE_DEBUG */
> > +
> > +static int virConnectAuthCallbackDefault(virConnectCredentialPtr cred,
> > +                                         unsigned int ncred,
> > +                                         void *cbdata ATTRIBUTE_UNUSED) {
> > +    int i;
> > +
> > +    for (i = 0 ; i < ncred ; i++) {
> > +        char buf[1024];
> > +        char *bufptr = buf;
> > +
> > +        printf("%s:", cred[i].prompt);
> > +        fflush(stdout);
> 
> If printf or fflush fails, this probably return -1.
> 
> > +        switch (cred[i].type) {
> > +        case VIR_CRED_USERNAME:
> > +        case VIR_CRED_AUTHNAME:
> > +        case VIR_CRED_ECHOPROMPT:
> > +        case VIR_CRED_REALM:
> > +            if (!fgets(buf, sizeof(buf), stdin)) {
> > +                return -1;
> > +            }
> 
> A consistency nit: you might want to make EOF be treated the same as
> an empty name.  Currently typing EOF to fgets (which then returns NULL)
> makes this code return -1, while entering an empty line doesn't.
> At least with passwords, I confirmed that cvs login treats ^D like
> the empty string.
> 
> On the other hand, an empty name probably makes no sense in many
> applications.
> 
> > +            if (buf[strlen(buf)-1] == '\n')
> > +                buf[strlen(buf)-1] = '\0';
> > +            break;
> > +
> > +        case VIR_CRED_PASSPHRASE:
> > +        case VIR_CRED_NOECHOPROMPT:
> > +            bufptr = getpass("");
> 
> If getpass fails (it'd return NULL), return -1.
> Otherwise, the following strdup would segfault.

Committed with this & the other suggested fixes all included.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|