Question about sasl_client_start() and SASL_OK

[Alexey Melnikov]

On 04/11/2013 09:01, Christophe Fergeau wrote:
> Hey,
Hi,
> I'm currently working on an application making use of cyrus-sasl for
> authentication (spice-gtk[1] for the client side, and spice-server[2] for the
> server side).
> Code is working nicely when using digest-md5, but we got a report about
> auth failing with plain [3]. The proposed patch checks for
> sasl_client_start() returning SASL_OK (which happens with plain), and
> handles this similarly to what we do when sasl_client_step() returns
> SASL_OK.
> Doing this makes sense to me, but the documentation about this is not very
> clear, so I prefer to ask on the mailing list first if this is indeed what
> should be done.
>
> If I look at sasl_client_start() man page, it says:
> "RETURN VALUE
>         sasl_client_start returns an integer which corresponds to one of the following
>         codes.  SASL_CONTINUE indicates success and that there are more steps needed in
>         the authentication. All other return codes indicate errors and should either be
>         handled or the authentication session should be quit."
> If I follow this strictly, then SASL_OK should be considered an error,
> which is probably not what is intended (?).
>
> sasl/sasl.h says:
>   * Basic client model:
>   *  1. client calls sasl_client_init() at startup to load plug-ins
>   *  2. when connection formed, call sasl_client_new()
>   *  3. once list of supported mechanisms received from server, client
>   *     calls sasl_client_start().  goto 4a
>   *  4. client calls sasl_client_step()
>   * [4a. If SASL_INTERACT, fill in prompts and goto 4
>   *      -- doesn't happen if callbacks provided]
>   *  4b. If SASL error, goto 7 or 3
>   *  4c. If SASL_OK, continue or goto 6 if last server response was success
>
> ie SASL_OK would be a valid return value from sasl_client_start() (by going 1,
> 2, 3, 4a, 4c)
>
> Then, if I look at the documentation [5] it does:
> do {
>        result=sasl_client_start(conn,
>                                 mechlist,
>                                 &client_interact,
>                                 &out,
>                                 &outlen,
>                                 &mechusing);
>
>        if (result==SASL_INTERACT)
>        {
>           [deal with the interactions. See interactions section below]
>        }
> } while (result==SASL_INTERACT); /* the mechanism may ask us to fill
>                                      in things many times. result is
>                                      SASL_CONTINUE on success */
> if (result!=SASL_CONTINUE) [failure]
>
> so SASL_OK would be treated as a failure here.
>
>
> Finally, looking at the sample-client.c code from cyrus-sasl [6]:
>
> result = sasl_client_start(...)
> if (result != SASL_OK && result != SASL_CONTINUE) {
>        printf("error was %s\n", sasl_errdetail(conn));
>        saslfail(result, "Starting SASL negotiation", NULL);
> }
>
> so SASL_OK is not an error there.
Correct.
> I assume I should be handling sasl_client_start() returning SASL_OK and that
> this should not be an error, but a confirmation about that would be very nice!
> It would be even better if the documentation could get fixed ;) I can
> submit a patch if needed once this is clarified.
Please do :-).
> Thanks in advance for any answer!
>
> Christophe
>
>
> [1] http://cgit.freedesktop.org/spice/spice-gtk/tree/gtk/spice-channel.c#n1277
> [2] http://cgit.freedesktop.org/spice/spice/tree/server/reds.c#n2142
> [3] http://lists.freedesktop.org/archives/spice-devel/2013-October/015122.html
> [4] http://lists.freedesktop.org/archives/spice-devel/attachments/20131022/410110a9/attachment.ksh
> [5] http://cyrusimap.web.cmu.edu/docs/cyrus-sasl/2.1.25/programming.php#client_code
>