[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: OpenLDAP 2.0.x + pam_ldap + cyrus-imapd-2.0.x



On Mon, 6 Aug 2001, David Wright wrote:

> I and quite a few other users of the cyrus-imapd system have found a
> problem which occurs exclusively when we authenticate using the PAM
> module pam_ldap linked against the OpenLDAP 2.0.x libraries. I am
> writing to ask whether this bug and any potential solutions are known to
> the wider OpenLDAP and pam_ldap communities.
>
> The basic problem is that, with the authentication scheme mentioned,
> imapd segfaults when pam_ldap returns success. Like anyone presented
> with this problem, I initially presumed the problem lay with cyrus-imapd
> (or with the cyrus-sasl library it uses). More careful investigation
> tends to case suspicion elsewhere:

I'm quite sure this problems is within the cyrus-sasl 1.5.x library,
and is, as was mentioned by somebody else on the OpenLDAP list, related
to the memory allocations done within the cyrus-sasl library.

When cyrus-imapd starts, it sets the memory allocation functions for the
sasl library using sasl_set_alloc(), calls sasl_server_init() and starts
using the library functions.   When the sasl library is configured to
use pam, which again uses pam_ldap, the OpenLDAP 2.x library is loaded.
This library also calls sasl_set_alloc(), followed by sasl_client_init().
Now the sasl library behaves as a client (from within the ldap library),
memory is (de)allocated using the memory management functions installed by
the OpenLDAP library, and everything still works as it should.

The problem arises when pam_ldap returns to the sasl library, now acting
as a server again.  It deallocates the memory it previously allocated
(which was done with the functions installed by imapd), but this is now
done with the incompatible functions installed by the OpenLDAP library,
and havoc breaks lose.

A real fix would involve a redesign of the sasl library.  At least it
should have separate sets of server- and client global variables, or
preferable, store everything it needs in a context created by the
sasl_*_init() functions.  No, I haven't looked at the 2.x version
of the library, so I don't know if something like this is what we might
expect there.  And no, I won't disagree with anyone stating that calls
to sasl_set_alloc() are best left to the application and should not be
done by libraries.

In the mean time, you may try the patch to the sasl 1.5.24 library I have
attached.  In effect, it makes sasl_set_alloc() a one-time-only function
that must be called before sasl_*_init().  We have been using the
cyrus-imapd-2.x, cyrus-sasl-1.5.x, pam_ldap, nss_ldap and OpenLDAP 2.x
combination on 64bit Solaris8 systems for some time now without any big
problems.

Apply the patch from the top of the cyrus-sasl-1.5.24 directory, with
the -p3 option to the patch program.

--
Rein Tollevik				Email: rein@basefarm.no
Senior System Administrator		Phone: +47 22 95 81 96
Basefarm AS				Fax:   +47 22 95 82 10


Index: drift/lib/cyrus-sasl/lib/client.c
diff -c drift/lib/cyrus-sasl/lib/client.c:1.1.1.1 drift/lib/cyrus-sasl/lib/client.c:1.2
*** drift/lib/cyrus-sasl/lib/client.c:1.1.1.1	Mon Jul 10 20:16:27 2000
--- drift/lib/cyrus-sasl/lib/client.c	Tue Aug 21 21:57:13 2001
***************
*** 351,356 ****
--- 351,358 ----
  {
    int ret;
  
+   _sasl_allocation_locked++;
+ 
    _sasl_client_cleanup_hook = &client_done;
    _sasl_client_idle_hook = &client_idle;
  
Index: drift/lib/cyrus-sasl/lib/common.c
diff -c drift/lib/cyrus-sasl/lib/common.c:1.1.1.2 drift/lib/cyrus-sasl/lib/common.c:1.2
*** drift/lib/cyrus-sasl/lib/common.c:1.1.1.2	Tue Aug 22 11:31:53 2000
--- drift/lib/cyrus-sasl/lib/common.c	Tue Aug 21 21:57:13 2001
***************
*** 180,185 ****
--- 180,186 ----
      }
  }
  
+ int _sasl_allocation_locked = 0;
  
  void
  sasl_set_alloc(sasl_malloc_t *m,
***************
*** 187,192 ****
--- 188,196 ----
  	       sasl_realloc_t *r,
  	       sasl_free_t *f)
  {
+   if (_sasl_allocation_locked++)
+     return;
+ 
    _sasl_allocation_utils.malloc=m;
    _sasl_allocation_utils.calloc=c;
    _sasl_allocation_utils.realloc=r;
Index: drift/lib/cyrus-sasl/lib/saslint.h
diff -c drift/lib/cyrus-sasl/lib/saslint.h:1.1.1.1 drift/lib/cyrus-sasl/lib/saslint.h:1.2
*** drift/lib/cyrus-sasl/lib/saslint.h:1.1.1.1	Mon Jul 10 20:16:27 2000
--- drift/lib/cyrus-sasl/lib/saslint.h	Tue Aug 21 21:57:13 2001
***************
*** 131,136 ****
--- 131,137 ----
  } sasl_log_utils_t;
  
  extern sasl_allocation_utils_t _sasl_allocation_utils;
+ extern int _sasl_allocation_locked;
  
  #define sasl_ALLOC(__size__) (_sasl_allocation_utils.malloc((__size__)))
  #define sasl_CALLOC(__nelem__, __size__) \
Index: drift/lib/cyrus-sasl/lib/server.c
diff -c drift/lib/cyrus-sasl/lib/server.c:1.1.1.2 drift/lib/cyrus-sasl/lib/server.c:1.2
*** drift/lib/cyrus-sasl/lib/server.c:1.1.1.2	Tue Aug 22 11:31:53 2000
--- drift/lib/cyrus-sasl/lib/server.c	Tue Aug 21 21:57:13 2001
***************
*** 633,638 ****
--- 633,640 ----
    int ret;
    const sasl_callback_t *vf;
  
+   _sasl_allocation_locked++;
+ 
    /* we require the appname to be non-null */
    if (appname==NULL) return SASL_BADPARAM;