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

[Jim Meyering]

"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.

> +            break;
> +        }
> +
> +        if (STREQ(bufptr, "") && cred[i].defresult)
> +            cred[i].result = strdup(cred[i].defresult);
> +        else
> +            cred[i].result = strdup(bufptr);
> +        if (!cred[i].result)
> +            return -1;
> +        cred[i].resultlen = strlen(cred[i].result);
> +    }
> +
> +    return 0;
> +}