[libvirt] [PATCH v3] sasl: Fix authentication when using PLAIN mechanism

[Christophe Fergeau]

On Fri, Nov 22, 2013 at 12:58:27PM -0700, Eric Blake wrote:
> On 11/22/2013 10:26 AM, 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.
> > ---
> > Change since v2:
> >   - move the added test out of the for(;;) loop
> 
> >      /* 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 (;;) {
> > +
> 
> This blank line seems spurious
> 
> >      restep:
> 
> now that you aren't modifying the head of the loop, you could follow my
> earlier suggestion of dropping the 'restep' label and replacing 'goto
> restep' with 'continue'.  But that's trivial, so I don't care either
> way, and don't need to see a v4 if you choose to change before pushing.
> 
> ACK.

I've pushed this after removing the blank line, and I've pushed an
additional patch replacing restep with continue;

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131126/7cd639cd/attachment-0001.sig>