RE: ldap_sasl_interactive_bind_s leaks? (ITS#2423)

[igor@ipass.net]

On Mon, 14 Apr 2003, Howard Chu wrote:

> > -----Original Message-----
> > From: Igor Brezac [mailto:igor@ipass.net]
>
> > It turns out cyrus DIGEST-MD5 is not leaking.  Openldap API
> > does not call
> > sasl_done() to clear cyrus-sasl buffers.  Your ldapsearch.c
> > patch includes
> > sasl_done(), but I think this is a wrong solution: ldapsearch
> > needs to be
> > explicitely linked with -lsasl2 and if cyrus sasl is not
> > configured with
> > openldap, the compile will fail.
>
> You're right, the patch is bad, it should be conditional #ifdef
> HAVE_CYRUS_SASL.

I still think sasl_done() belongs in LDAP API somewhere.  Otherwise this
fix needs to be duplicated in a bunch of places.

> >
> > I think sasl_done() needs to be called during ldap_unbind() and
> > ldap_int_sasl_init() needs to be called every time ldap_init(ialize)()
> > runs rather than just once.  Please see attached patch.  My patch also
> > fixes threadsafe issue in ldap_int_sasl_init().
>
> This solution isn't any better. My interpretation of the SASL docs is that
> sasl_done() only needs to be called once, at the end of the particular

This is probably true until cyrus-sasl bug 1963 is developed.
sasl_done() clears digest-md5 reauth buffer.  This is what causes the
leak, the buffer is never cleared.

> application. The LDAP API doesn't provide a similar ldap_done() function to
> cleanup its library, though it certainly needs one. The big problem with your
> patch is if any client uses two (or more) LDAP sessions at once with SASL,
> calling ldap_unbind on any one of them will tear down the SASL library for
> all of them. That's certainly not correct.

True, but the current code leaks memory a fair amount.  At the same time,
if an app uses SASL API as well as LDAP API and if the app calls
sasl_done(), openldap session will break with and without the patch.

This problem will be solved once the cyrus-sasl bug is fixed:
http://bugzilla.andrew.cmu.edu/show_bug.cgi?id=1963
Patch like the one I proposed still needs to be applied to openldap.

-- 
Igor