Re: [PATCH 09/15] util: auth: Introduce virAuthAskCredential
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(a)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);