On Fri, Nov 22, 2013 at 10:59:20AM +0100, Christophe Fergeau wrote: > 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&msg=10104 > > 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, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list