[PATCH] virauth: Report error on empty auth result

Michal Privoznik mprivozn at redhat.com
Mon Mar 27 11:21:51 UTC 2023


When opening a connection, it may be necessary to provide user
credentials, or some additional info (e.g. whether to trust an
ssh key). We have a special API for that: virConnectOpenAuth()
where and additional callback can be passed. This callback is
then called with _virConnectCredential struct filled partially
and it's callback's responsibility to get desired data (e.g. by
prompting user) and store it into .result member of the struct.

But we document the callback behaviour as:

   If an interaction cannot be filled, fill in NULL and 0.

(meaning fill in NULL and return 0)

But this does not really fly with the way callback is called. So
far, we have three places where the callback is called:

  1) remoteAuthInteract()
  2) virAuthGetUsernamePath()
  3) virAuthAskCredential()

Now, 1) just grabs whatever the client side provided and sends it
to the other side via RPC. All interesting work takes place at 2)
and 3). And the usual pattern in which these two are called is:

  if (!(cred = wrapper()))
      return -1;

where wrapper() is a function that firstly tries to get data from
a config file or URI itself. The wrappers are named
virAuthGetUsername() for virAuthGetUsernamePath() and
virAuthGetPassword() for virAuthAskCredential().

All wrappers do report error on failure. Except, when the user
provided callback set the struct member to NULL. Then they both
return NULL without setting any error. This then leads to the
generic error message:

  error : An error occurred, but the cause is unknown

Since we failed to get desired credential data, we can't proceed
(what data should we pass down to the auth layer, say ssh?) and
have to fail. But, we can produce an error message that'll
hopefully point users in the right direction.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2181235
Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/util/virauth.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/util/virauth.c b/src/util/virauth.c
index 7b4a1bd8a5..d1f32f8e6d 100644
--- a/src/util/virauth.c
+++ b/src/util/virauth.c
@@ -176,7 +176,8 @@ virAuthGetUsernamePath(const char *path,
         cred.result = NULL;
         cred.resultlen = 0;
 
-        if ((*(auth->cb))(&cred, 1, auth->cbdata) < 0) {
+        if ((*(auth->cb))(&cred, 1, auth->cbdata) < 0 ||
+            !cred.result) {
             virReportError(VIR_ERR_AUTH_FAILED, "%s",
                            _("Username request failed"));
             VIR_FREE(cred.result);
@@ -310,7 +311,8 @@ virAuthAskCredential(virConnectAuthPtr auth,
 
     ret->prompt = prompt;
 
-    if (auth->cb(ret, 1, auth->cbdata) < 0) {
+    if (auth->cb(ret, 1, auth->cbdata) < 0 ||
+        !ret->result) {
         virReportError(VIR_ERR_OPERATION_FAILED, "%s",
                        _("failed to retrieve user response for authentication callback"));
         return NULL;
-- 
2.39.2



More information about the libvir-list mailing list