Crash on backend exit w/ OpenLDAP [2.4.24, 2.4.31]

With LDAP support enabled, we link the backend with libldap, and we link libpq
with libldap_r. Modules like dblink and postgres_fdw link to libpq, so
loading them results in a backend having both libldap and libdap_r loaded.
Those libraries export the same symbols, and the load order rule gives
priority to the libldap symbols. So far, so good. However, both libraries
declare a GCC destructor, ldap_int_destroy_global_options(), to free memory
reachable from a global variable, ldap_int_global_options. In OpenLDAP
versions 2.4.24 through 2.4.31, that variable's structure type has an
incompatible layout in libldap vs. libldap_r. When the libldap_r build of the
destructor uses pointers from the ldap_int_global_options of libldap, SIGSEGV
ensues. This does not arise if nothing in the process ever caused OpenLDAP to
initialize itself, because the relevant bytes then happen to be all-zero in
either structure layout. OpenLDAP 2.4.32 fixed this by making the libldap
structure a strict prefix of the libldap_r structure. The OpenLDAP change log
merely says "Fixed libldap sasl handling (ITS#7118, ITS#7133)"; here are the
relevant commits:

http://www.openldap.org/devel/gitweb.cgi?p=openldap.git;a=commitdiff;h=270ef33acf18dc13bfd07f8a8e66b446f80e7d27
http://www.openldap.org/devel/gitweb.cgi?p=openldap.git;a=commitdiff;h=7ff18967d7d2e49baa9d80f1b9408b276f3982e0

You can cause the at-exit crash by building PostgreSQL against OpenLDAP
2.4.31, connecting with LDAP authentication, and issuing "LOAD 'dblink'".
Alternately, connect with any authentication method and create a dblink
connection that uses an LDAP-based pg_service.conf entry. I'm attaching a
test suite patch to illustrate that second vector.

The popularity of the affected OpenLDAP versions is not clear to me. RHEL 5,
6 and 7 all ship OpenLDAP either old enough or new enough to miss the problem.
Debian wheezy ships 2.4.31, an affected version. I have not looked beyond
that. I pondered what we could do short of treating this as a pure OpenLDAP
bug for packagers to fix in their OpenLDAP builds, but I did not come up with
anything singularly attractive. Some possibilities:

1. Link the backend with libldap_r, so we never face the mismatch. On some
platforms, this means also linking in threading libraries.

2. Link a second copy of libpq against plain libldap and use that in server
modules like dblink.

3. Wipe the memory of ldap_int_global_options so the destructor will have
nothing to do. A challenge here is that we don't know the structure size;
it's not part of any installed header, and it varies in response to OpenLDAP
configuration options. Still, this is achievable in principle. This would be
easy if the destructor function weren't static, alas.

4. Detect older OpenLDAP versions at runtime, just before we would otherwise
initialize OpenLDAP, and raise an error. Possibly make the same check at
compile time, for packager convenience.

5. Use only _exit(), not exit().

Hopefully I'm missing a great alternative, because each of those has something
substantial to dislike about it. Thoughts welcome, especially from packagers
dealing with platforms that use affected OpenLDAP versions.

Thanks,
nm

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

On Thu, Jun 12, 2014 at 05:02:19PM -0400, Noah Misch wrote:
> You can cause the at-exit crash by building PostgreSQL against OpenLDAP
> 2.4.31, connecting with LDAP authentication, and issuing "LOAD 'dblink'".

> 4. Detect older OpenLDAP versions at runtime, just before we would otherwise
> initialize OpenLDAP, and raise an error. Possibly make the same check at
> compile time, for packager convenience.

Having pondered this some more, I lean toward the following conservative fix.
Add to all supported branches a test case that triggers the crash and a
configure-time warning if the OpenLDAP version falls in the vulnerable range.
So long as those who build from source monitor either "configure" output or
test suite failures, they'll have the opportunity to head off the problem.

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

Noah Misch <noah(at)leadboat(dot)com> writes:
> On Thu, Jun 12, 2014 at 05:02:19PM -0400, Noah Misch wrote:
>> You can cause the at-exit crash by building PostgreSQL against OpenLDAP
>> 2.4.31, connecting with LDAP authentication, and issuing "LOAD 'dblink'".

>> 4. Detect older OpenLDAP versions at runtime, just before we would otherwise
>> initialize OpenLDAP, and raise an error. Possibly make the same check at
>> compile time, for packager convenience.

> Having pondered this some more, I lean toward the following conservative fix.
> Add to all supported branches a test case that triggers the crash and a
> configure-time warning if the OpenLDAP version falls in the vulnerable range.
> So long as those who build from source monitor either "configure" output or
> test suite failures, they'll have the opportunity to head off the problem.

+1 for a configure warning, but I share your concern that it's likely to
go unnoticed (sometimes I wish autoconf were not so chatty...).

Keep in mind that some distros patch bugs without changing the reported
version number, so I'm afraid we couldn't adopt the easy solution of
making configure give a hard error when the version is suspicious; and
for the same reason your #4 above is unworkable.

I'm not sure about the practicality of adding a test case --- how will we
test that if no LDAP server is at hand?

I concur with not working much harder than this, in any case. It's really
OpenLDAP's bug to fix.

regards, tom lane

On Thu, Jun 19, 2014 at 01:01:34PM -0400, Tom Lane wrote:
> Noah Misch <noah(at)leadboat(dot)com> writes:
> > On Thu, Jun 12, 2014 at 05:02:19PM -0400, Noah Misch wrote:
> >> You can cause the at-exit crash by building PostgreSQL against OpenLDAP
> >> 2.4.31, connecting with LDAP authentication, and issuing "LOAD 'dblink'".
>
> >> 4. Detect older OpenLDAP versions at runtime, just before we would otherwise
> >> initialize OpenLDAP, and raise an error. Possibly make the same check at
> >> compile time, for packager convenience.
>
> > Having pondered this some more, I lean toward the following conservative fix.
> > Add to all supported branches a test case that triggers the crash and a
> > configure-time warning if the OpenLDAP version falls in the vulnerable range.
> > So long as those who build from source monitor either "configure" output or
> > test suite failures, they'll have the opportunity to head off the problem.
>
> +1 for a configure warning, but I share your concern that it's likely to
> go unnoticed (sometimes I wish autoconf were not so chatty...).

> I'm not sure about the practicality of adding a test case --- how will we
> test that if no LDAP server is at hand?

Merely attempting an LDAP connection (to a closed port, for example)
initializes the library far enough to trigger the problem. Here's a patch
implementing the warning and test case. The test case is based on the one I
posted upthread, modified to work with installcheck, work with non-LDAP
builds, and close a race condition.

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com