[Date Prev][Date Next]
RE: ldap_sasl_interactive_bind_s leaks? (ITS#2423)
On Mon, 14 Apr 2003, Howard Chu wrote:
> > -----Original Message-----
> > From: Igor Brezac [mailto:firstname.lastname@example.org]
> > 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
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:
Patch like the one I proposed still needs to be applied to openldap.