[PATCH 09/15] util: auth: Introduce virAuthAskCredential

[Jonathon Jongsma]

On 1/17/23 10:20 AM, Peter Krempa wrote:
> The helper uses the user-provided auth callbacks to ask the user. The
> helper encapsulates the steps we do to query the user in few places into
> a common helper which can be then used further.
> 
> Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> ---
>   src/libvirt_private.syms |  2 ++
>   src/util/virauth.c       | 66 ++++++++++++++++++++++++++++++++++++++++
>   src/util/virauth.h       |  7 +++++
>   3 files changed, 75 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 576ec8f95f..5616c0d44c 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1850,6 +1850,8 @@ virAuditSend;
> 
> 
>   # util/virauth.h
> +virAuthAskCredential;
> +virAuthConnectCredentialFree;
>   virAuthGetConfigFilePath;
>   virAuthGetConfigFilePathURI;
>   virAuthGetPassword;
> diff --git a/src/util/virauth.c b/src/util/virauth.c
> index b9c2ae3ed1..aa1da80266 100644
> --- a/src/util/virauth.c
> +++ b/src/util/virauth.c
> @@ -31,6 +31,7 @@
>   #include "virerror.h"
>   #include "configmake.h"
>   #include "virauthconfig.h"
> +#include "virsecureerase.h"
> 
>   #define VIR_FROM_THIS VIR_FROM_AUTH
> 
> @@ -283,3 +284,68 @@ virAuthGetPassword(virConnectPtr conn,
> 
>       return virAuthGetPasswordPath(path, auth, servicename, username, hostname);
>   }
> +
> +
> +void
> +virAuthConnectCredentialFree(virConnectCredential *cred)
> +{
> +    if (cred->result) {
> +        virSecureErase(cred->result, cred->resultlen);
> +        g_free(cred->result);
> +    }
> +    g_free(cred);
> +}

As long as you're introducing this function, I'd personally like to see 
the open-coded versions replaced with a call to this function in other 
locations (such as remoteAuthInteractStateClear()).

> +
> +
> +/**
> + * virAuthAskCredential:
> + * @auth: authentication callback data
> + * @prompt: question string to ask the user
> + * @echo: true if user's reply should be considered sensitive and not echoed

The description of the @echo parameter seems inverted.

> + *
> + * Invoke the authentication callback for the connection @auth and ask the user
> + * the question in @prompt. If @echo is true user's reply should be collected
> + * as sensitive (user's input not printed on screen).

again

> + */
> +virConnectCredential *
> +virAuthAskCredential(virConnectAuthPtr auth,
> +                     const char *prompt,
> +                     bool echo)
> +{
> +    g_autoptr(virConnectCredential) ret = g_new0(virConnectCredential, 1);
> +    size_t i;
> +
> +    ret->type = -1;
> +
> +    for (i = 0; i < auth->ncredtype; ++i) {
> +        int type = auth->credtype[i];
> +        if (echo) {
> +            if (type == VIR_CRED_ECHOPROMPT) {
> +                ret->type = type;
> +                break;
> +            }
> +        } else {
> +            if (type == VIR_CRED_PASSPHRASE ||
> +                type == VIR_CRED_NOECHOPROMPT) {
> +                ret->type = type;
> +                break;
> +            }
> +        }
> +    }
> +
> +    if (ret->type == -1) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("no suitable callback authentication callback was found"));
> +        return NULL;
> +    }
> +
> +    ret->prompt = prompt;
> +
> +    if (auth->cb(ret, 1, auth->cbdata)) {

I suppose this works since negative values evaluate to true, but it 
*looks* like the logic is incorrect. It would be more readable as "if 
(cb() < 0)"

> +        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                       _("failed to retrieve user response for authentication callback"));
> +        return NULL;
> +    }
> +
> +    return g_steal_pointer(&ret);
> +}
> diff --git a/src/util/virauth.h b/src/util/virauth.h
> index a0fd84962b..3eaf40c626 100644
> --- a/src/util/virauth.h
> +++ b/src/util/virauth.h
> @@ -52,3 +52,10 @@ char * virAuthGetPasswordPath(const char *path,
>                                 const char *servicename,
>                                 const char *username,
>                                 const char *hostname);
> +
> +virConnectCredential *virAuthAskCredential(virConnectAuthPtr auth,
> +                                           const char *prompt,
> +                                           bool echo);
> +
> +void virAuthConnectCredentialFree(virConnectCredential *cred);
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virConnectCredential, virAuthConnectCredentialFree);