On Mon, Mar 27, 2023 at 01:21:51PM +0200, Michal Privoznik wrote:
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)
No, not really, the whole docstring is: * When authentication requires one or more interactions, this callback * is invoked. For each interaction supplied, data must be gathered * from the user and filled in to the 'result' and 'resultlen' fields. * If an interaction cannot be filled, fill in NULL and 0. * * Returns 0 if all interactions were filled, or -1 upon error hence if interaction cannot be filled, then: .result = NULL; .resultlen = 0; and since interaction could not be filled, then the callback should not return 0 as that is the case only if all interactions were filled. However the fix is, I believe, actually correct. If we need some credentials and they were not provided (for example even the default one was not used -- that is also job of the callback) then we should still error out. We essentially do even before this patch, we just don't report an error. So I would scrap the rest of the commit and just say that we should also work with "buggy" auth callbacks. So with the commit message shortened: Reviewed-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2181235 Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> --- 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
Attachment:
signature.asc
Description: PGP signature
