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
