From: "Daniel P. Berrange" <berrange@redhat.com> If passing a 'credtype' parameter which was an empty list to the python openAuth() API, the 'credtype' field in the virConnectAuth struct would not be initialized. This lead to a crash when later trying to free that field. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- python/libvirt-override.c | 1 + 1 file changed, 1 insertion(+) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 65e8c69..edaa2a4 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -1820,6 +1820,7 @@ libvirt_virConnectOpenAuth(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *pycredtype; virConnectAuth auth; + memset(&auth, 0, sizeof(auth)); if (!PyArg_ParseTuple(args, (char *)"zOi:virConnectOpenAuth", &name, &pyauth, &flags)) return NULL; -- 1.7.11.2

From: "Daniel P. Berrange" <berrange@redhat.com> If an exception occurs in the python callback for openAuth() the stack trace isn't seen by the apps, since this code is called from libvirt context. To aid diagnostics, print the error to stderr at least Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- python/libvirt-override.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index edaa2a4..74a8abf 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -1776,8 +1776,10 @@ static int virConnectCredCallbackWrapper(virConnectCredentialPtr cred, PyErr_Clear(); pyret = PyEval_CallObject(pycb, list); - if (PyErr_Occurred()) + if (PyErr_Occurred()) { + PyErr_Print(); goto cleanup; + } ret = PyLong_AsLong(pyret); if (ret == 0) { -- 1.7.11.2

From: "Daniel P. Berrange" <berrange@redhat.com> When deciding whether to provide an auth function callback in openAuth(), credcb was checked against NULL, when it really needs to be checked against Py_None Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- python/libvirt-override.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 74a8abf..bf0b45b 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -1840,7 +1840,8 @@ libvirt_virConnectOpenAuth(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { auth.credtype[i] = (int)PyLong_AsLong(val); } } - auth.cb = pycredcb ? virConnectCredCallbackWrapper : NULL; + if (pycredcb && pycredcb != Py_None) + auth.cb = virConnectCredCallbackWrapper; auth.cbdata = pyauth; LIBVIRT_BEGIN_ALLOW_THREADS; -- 1.7.11.2

From: "Daniel P. Berrange" <berrange@redhat.com> All public API functions must call virResetLastError to clear out any previous error. The virConnectOpen* functions forgot todo this. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libvirt.c b/src/libvirt.c index b034ed6..31656dc 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1329,6 +1329,7 @@ virConnectOpen (const char *name) goto error; VIR_DEBUG("name=%s", name); + virResetLastError(); ret = do_open (name, NULL, 0); if (!ret) goto error; @@ -1363,6 +1364,7 @@ virConnectOpenReadOnly(const char *name) goto error; VIR_DEBUG("name=%s", name); + virResetLastError(); ret = do_open (name, NULL, VIR_CONNECT_RO); if (!ret) goto error; @@ -1401,6 +1403,7 @@ virConnectOpenAuth(const char *name, goto error; VIR_DEBUG("name=%s, auth=%p, flags=%x", NULLSTR(name), auth, flags); + virResetLastError(); ret = do_open (name, auth, flags); if (!ret) goto error; -- 1.7.11.2

From: "Daniel P. Berrange" <berrange@redhat.com> The remote driver first looks at the libvirt auth config file to fill in any credentials. It then invokes the auth callback for any remaining credentials. It was accidentally invoking the auth callback even if there were not any more credentials required. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/remote/remote_driver.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index cf1f079..6169243 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3326,29 +3326,38 @@ static int remoteAuthInteract(virConnectPtr conn, VIR_DEBUG("Starting SASL interaction"); remoteAuthInteractStateClear(state, false); + /* Fills state->interact with any values from the auth config file */ if (remoteAuthFillFromConfig(conn, state) < 0) goto cleanup; + /* Populates state->cred for anything not found in the auth config */ if (remoteAuthMakeCredentials(state->interact, &state->cred, &state->ncred) < 0) { virReportError(VIR_ERR_AUTH_FAILED, "%s", _("Failed to make auth credentials")); goto cleanup; } - /* Run the authentication callback */ - if (!auth || !auth->cb) { - virReportError(VIR_ERR_AUTH_FAILED, "%s", - _("No authentication callback available")); - goto cleanup; - } + /* If there was anything not in the auth config, we need to + * run the interactive callback + */ + if (state->ncred) { + /* Run the authentication callback */ + if (!auth || !auth->cb) { + virReportError(VIR_ERR_AUTH_FAILED, "%s", + _("No authentication callback available")); + goto cleanup; + } - if ((*(auth->cb))(state->cred, state->ncred, auth->cbdata) < 0) { - virReportError(VIR_ERR_AUTH_FAILED, "%s", - _("Failed to collect auth credentials")); - goto cleanup; + if ((*(auth->cb))(state->cred, state->ncred, auth->cbdata) < 0) { + virReportError(VIR_ERR_AUTH_FAILED, "%s", + _("Failed to collect auth credentials")); + goto cleanup; + } + + /* Copy user's responses from cred into interact */ + remoteAuthFillInteract(state->cred, state->interact); } - remoteAuthFillInteract(state->cred, state->interact); /* * 'interact' now has pointers to strings in 'state->cred' * so we must not free state->cred until the *next* -- 1.7.11.2