[Pkg-openldap-devel] Bug#340601: Bug#340601: Bug#340601: ldapsearch hangs when using ldap for /etc/hosts

[Steve Langasek]

On Sun, Feb 10, 2008 at 10:06:14AM -0800, Russ Allbery wrote:
> > Ok, I've scaled back the patch a bit before committing it because a
> > deeper search leaves me uncertain that res_query and dn_expand are
> > thread-safe even in current versions of glibc.  Dropping the mutex for
> > getaddrinfo() and getnameinfo() is sufficient to fix this bug, in any
> > case.

> I have now discussed this and the related fact that we're using libldap_r
> for ldapsearch (which from upstream's perspective is the actual problem)
> with upstream.  Upstream's stance on this is:

>  * Using libldap_r for anything other than slapd is flatly unsupported and
>    considered a bug.  We should not be doing that.  We should be treating
>    libldap_r as a private library only for slapd.

>  * libldap has no supported thread-safe API.  Threaded programs that link
>    against libldap are required to handle locking themselves.

Wow, ok, I'm very not happy with this answer.  I think that *all* libraries
should have thread-safe APIs; and aside from this minor blemish, libldap_r
appears to already be a perfectly good thread-safe, reentrant library,
supported by upstream or not.

We have the following callers using libldap in Debian which also have need
of thread-safety and currently take no additional steps to ensure libldap is
thread-safe:

- libpq5
- libnss-ldapd
- evolution-data-server
- libexchange-storage1.2-3
- libcurl3
- kdepimlibs5
- claws-mail (for bonus points, this app launches each LDAP query in a
  separate thread... :)
- strongswan
- libnss-ldap
- libpam-ldap
- python-ldap
- openoffice.org-core
- libsasl2-modules-ldap
- libpam-smbpass
- libsmbclient
- freeradius-ldap (uses a mutex to ensure any given LDAP connection is only
  used by one thread at a time, but otherwise assumes that multiple LDAP
  connections can be going at once)

... and probably others besides, the list of reverse-depends is long and I
haven't looked at them all.

Moving the locking up the stack when we already have a perfectly thread-safe
libldap available would be a significant regression.

>  * The root underlying problem would then be trying to use libnss-ldap and
>    slapd together on the same system at the same time, because libnss-ldap
>    pulls libldap into slapd's namespace.  Upstream's opinion is that
>    libnss-ldap is broken and this regard and libnss-ldapd may be better.

This particular bug is not related to using libnss-ldap and slapd together
on the same system; it only requires one to try using an LDAP *client* on a
system that uses nss_ldap for hosts resolution.

The concerns about symbol collisions between libldap and libldap_r are
specific to slapd only if we concede that libldap_r should be treated as a
private library not intended for public consumption, which I am not because
the non-threaded libldap is not an adequate replacement for a number of
applications without duplicating a lot of work that's already been done
centrally.

>  * People really shouldn't put hosts into LDAP; LDAP is a heavy-weight
>    protocol that is not suited for use as a DNS resolver.

I happen to agree with that, but nss_ldap nevertheless implements all of the
NSS services, and supporting the hosts implementation for glibc-based
systems is a trivial change.  Eliminating a lock that we *know* we don't
need is hardly a bad thing, anyway.

> The last we can communicate back to the user and perhaps even put into the
> documentation for libnss-ldap and libnss-ldapd.  For the rest, here is the
> outline of an upstream-acceptable solution, which I'd love to be able to
> get at.

>  * Revert the change to link everything against libldap_r and ship only
>    libldap in the libldap package (which will require nasty transition
>    stuff, but putting that side for right now).  Adjust the shlibs in the
>    package accordingly, of course.

Frankly, I would rather continue to ship libldap_r publically, even without
upstream's blessing for this configuration, than inflict threading problems
on the reverse-dependencies.  We *know* that libldap_r is thread-safe
because slapd itself exercises it in the "supported" configuration, and we
already have to deal with frequently-changing libldap sonames primarily
because of the ABI changes that are specific to the internal ldap_pvt_*
functions - we ought to get some benefit for the migration troubles that's
going to cause.

>  * Ship libldap_r in the slapd package or in a separate package referenced
>    by slapd and clearly document in the README.Debian for that package
>    that those libraries are intended for use only with slapd and any other
>    use is not supported.

If that's really upstream's position, then they've been doing this wrong
from the start.  If libldap_r isn't supposed to be public, then they
should have been building it as a convenience library and linking it
statically into slapd; they should have kept all of the ldap_pvt_*/ldap_int_*
functions *private*, instead of exporting them in libldap; and they should
be maintaining a stable ABI for what is a pretty major system library
instead of forcing rebuilds for every slapd internal ABI change.

I'm dismayed.  All of the problems with using libldap_r as the system
libldap would be fixable over time, but it becomes considerably less
feasible without upstream's support of the effort.

Was your discussion with upstream somewhere public?

-- 
Steve Langasek                   Give me a lever long enough and a Free OS
Debian Developer                   to set it on, and I can move the world.
Ubuntu Developer                                    http://www.debian.org/
slangasek at ubuntu.com                                     vorlon at debian.org