With some authentication mechanism (PLAIN for example),
sasl_client_start()
can return SASL_OK, which translates to virNetSASLSessionClientStart()
returning VIR_NET_SASL_COMPLETE.
cyrus-sasl documentation is a bit vague as to what to do in such situation,
but upstream clarified this a bit in
http://asg.andrew.cmu.edu/archive/message.php?mailbox=archive.cyrus-sasl&...
When we got VIR_NET_SASL_COMPLETE after virNetSASLSessionClientStart() and
if the remote also tells us that authentication is complete, then we should
end the authentication procedure rather than forcing a call to
virNetSASLSessionClientStep(). Without this patch, when trying to use SASL
PLAIN, I get:
errorĀ :authentication failed : Failed to step SASL negotiation: -1
(SASL(-1): generic failure: Unable to find a callback: 32775)
This patch is based on a spice-gtk patch by Dietmar Maurer.
---
Changes since v1:
- adjust comment before the for(;;) code block
- add comment before the if (complete && ...) test as this is what is done
for these tests in this block.
src/remote/remote_driver.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 7181949..38da119 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -4122,9 +4122,16 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv,
complete, serverinlen, serverin);
/* Loop-the-loop...
- * Even if the server has completed, the client must *always* do at least one step
- * in this loop to verify the server isn't lying about something. Mutual auth
*/
+ * Even if the server has completed, the client must loop until sasl_client_start()
or
+ * sasl_client_step() return SASL_OK to verify the server isn't lying
+ * about something. Mutual auth
+ * */
for (;;) {
+ /* Previous server call showed completion & sasl_client_start()
+ * told us we are locally complete too */
+ if (complete && err == VIR_NET_SASL_COMPLETE)
+ break;
+
We already have this check at the end of the for(;;) loop. We only need
to deal with the first iteration. So I think this code should be placed
prior to the for(;;) loop, rather than inside it.
restep:
if ((err = virNetSASLSessionClientStep(sasl,
serverin,