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

[Eric Blake]

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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131122/379fa89b/attachment-0001.sig>