[jira] [Comment Edited] (PROTON-992) Proton's use of Cyrus SASL is not thread-safe.

["Andrew Stitcher (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/PROTON-992?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15274623#comment-15274623 ] 

Andrew Stitcher edited comment on PROTON-992 at 5/6/16 7:54 PM:
----------------------------------------------------------------

For the record here is the current plan to fix this issue:
* Short term there is a fix for the cyrus sasl code in Proton which uses pthreads to avoid all of the multithreading insanity as mentioned in this bug report. This fix does not require any change to the user facing API and so no code that uses Proton will have to change.
This solution does require pthreads for the cyrus sasl implementation, and so is perhaps not ideal from this respect. However it seems that the cyrus sasl library relies on Posix in any case, so it's likely that pthreads is available in most places that cyrus sasl is available.
* Long term the solution will be to change the Proton SASL implementation so that the default SASL implementation is active until the application explicitly requests extended SASL support.
The default implementation supports ANONYMOUS, PLAIN and EXTERNAL on the client side and ANONYMOUS and EXTERNAL on the server side.
This change will allow us to fix the erroneous API for setting the server name and configuration path for the Cyrus configuration file which is really global, but which currently has a per transport API.
This approach will still allow applications to function without change, but they will only have access to very basic SASL.
Another advantage of this approach is that it will allow us to test basic SASL support in the same build as extended SASL support.


was (Author: astitcher):
For the record here is the current plan to fix this issue:
* Short term there is a fix for the cyrus sasl code in Proton which uses pthreads to avoid all of the multithreading insanity as mentioned in this bug report. This fix does not require any change to the user facing API and so no code that uses Proton will have to change.

  This solution does require pthreads for the cyrus sasl implementation. 
* Long term 

> Proton's use of Cyrus SASL is not thread-safe.
> ----------------------------------------------
>
>                 Key: PROTON-992
>                 URL: https://issues.apache.org/jira/browse/PROTON-992
>             Project: Qpid Proton
>          Issue Type: Bug
>          Components: proton-c
>    Affects Versions: 0.10
>            Reporter: michael goulish
>            Assignee: Andrew Stitcher
>            Priority: Critical
>
> Documentation for the Cyrus SASL library says that the library is believed to be thread-safe only if the code that uses it meets several requirements.
> The requirements are:
>     * you supply mutex functions (see sasl_set_mutex())
>     * you make no libsasl calls until sasl_client/server_init() completes
>     * no libsasl calls are made after sasl_done() is begun
>     * when using GSSAPI, you use a thread-safe GSS / Kerberos 5 library.
> It says explicitly that that sasl_set* calls are not thread safe, since they set global state.
> The proton library makes calls to sasl_set* functions in :
>           pni_init_client()
>           pni_init_server(), and
>           pni_process_init()
> Since those are internal functions, there is no way for code that uses Proton to lock around those calls.
> I think proton needs a new API call to let applications call sasl_set_mutex().  Or something.
> We probably also need other protections to meet the other requirements specified in the Cyrus documentation (and quoted above).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org