Thread: configure openldap crash warning
configure can report this: configure: WARNING: *** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend *** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and *** also uses LDAP will crash on exit. The source code also says # PostgreSQL sometimes loads libldap_r and plain libldap into the same # process. Check for OpenLDAP versions known not to tolerate doing so; assume # non-OpenLDAP implementations are safe. The dblink test suite exercises the # hazardous interaction directly. The libldap installation that comes with the macOS operating system reports itself as version 2.4.28, so this warning comes up every time, but the dblink test suite passes without problem. I looked into this a bit further. I checked by installing openldap 2.4.31 from source on an centos 7 instance, and indeed the dblink test suite crashes right away. So the test is still good. I think it's very likely that Apple has patched around in their libldap code without changing the version. I wonder whether we can do something to not make this warning appear when it's not necessary. The release of openldap 2.4.32 (the first good one) is now ten years ago, so seeing faulty versions in the wild should be very rare. I'm tempted to suggest that we can just remove the warning and rely on the dblink test to catch any stragglers. We could also disable the test on macOS only. Thoughts?
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes: > configure can report this: > configure: WARNING: > *** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend > *** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and > *** also uses LDAP will crash on exit. > I wonder whether we can do something to not make this warning appear > when it's not necessary. The release of openldap 2.4.32 (the first good > one) is now ten years ago, so seeing faulty versions in the wild should > be very rare. I'm tempted to suggest that we can just remove the > warning and rely on the dblink test to catch any stragglers. We could > also disable the test on macOS only. Thoughts? I'm not that excited about getting rid of this warning, because to the extent that anyone notices it at all, it'll motivate them to get OpenLDAP from Homebrew or MacPorts, which seems like a good thing. This configure warning is already far less in-your-face than the compile-time deprecation warnings that get spewed at you when you use Apple's headers; their slapd doesn't really work as far as we can tell; and whether they fixed this issue or not, it's a safe bet that there are a lot of other unfixed bugs in their ancient copy of OpenLDAP. The deprecation warnings seem like clear evidence that Apple intends to remove the bundled libldap at some point, so I don't think we should put effort into encouraging people to use it. There's certainly a conversation to be had about whether this configure warning is still worth the cycles it takes to run it; maybe it isn't. But I don't think that we should be looking at it from the standpoint of silencing the complaint on macOS. regards, tom lane
On 02.05.22 16:03, Tom Lane wrote: > I'm not that excited about getting rid of this warning, because to the > extent that anyone notices it at all, it'll motivate them to get OpenLDAP > from Homebrew or MacPorts, which seems like a good thing. I tried building with Homebrew-supplied openldap. What ends up happening is that the postgres binary is indeed linked with openldap, but libpq still is linked against the OS-supplied LDAP framework. (Checked with "otool -L" in each case.) Can someone else reproduce this, too?
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes: > On 02.05.22 16:03, Tom Lane wrote: >> I'm not that excited about getting rid of this warning, because to the >> extent that anyone notices it at all, it'll motivate them to get OpenLDAP >> from Homebrew or MacPorts, which seems like a good thing. > I tried building with Homebrew-supplied openldap. What ends up > happening is that the postgres binary is indeed linked with openldap, > but libpq still is linked against the OS-supplied LDAP framework. > (Checked with "otool -L" in each case.) Can someone else reproduce > this, too? Hmm, I just tried it with up-to-date MacPorts, and it was a *complete* fail: I got all the deprecation warnings (so the system include headers were used), and both postgres and libpq.dylib still ended up linked against /System/Library/Frameworks/LDAP.framework/Versions/A/LDAP. But then I went "doh!" and added --with-includes=/opt/local/include --with-libraries=/opt/local/lib to the configure call, and everything built the way I expected. I'm not sure offhand if the docs include a reminder to do that when using stuff out of MacPorts, or the equivalent for Homebrew. We still have a bit of work to do, because this setup isn't getting all the way through src/test/ldap/: 2022-05-04 11:01:33.407 EDT [21312] [unknown] LOG: connection received: host=[local] 2022-05-04 11:01:33.457 EDT [21312] [unknown] LOG: could not start LDAP TLS session: Operations error 2022-05-04 11:01:33.457 EDT [21312] [unknown] DETAIL: LDAP diagnostics: TLS already started 2022-05-04 11:01:33.457 EDT [21312] [unknown] FATAL: LDAP authentication failed for user "test1" 2022-05-04 11:01:33.457 EDT [21312] [unknown] DETAIL: Connection matched pg_hba.conf line 1: "local all all ldap ldapurl="ldaps://localhost:51335/dc=example,dc=net??sub?(uid=$username)"ldaptls=1" 2022-05-04 11:01:33.459 EDT [21304] LOG: server process (PID 21312) was terminated by signal 11: Segmentation fault: 11 Many of the test cases pass, but it looks like ldaps-related ones don't. The stack trace isn't very helpful: (lldb) bt * thread #1, stop reason = ESR_EC_DABORT_EL0 (fault address: 0x0) * frame #0: 0x00000001b5bfc628 libsystem_pthread.dylib`pthread_rwlock_rdlock frame #1: 0x00000001054a74c4 libcrypto.3.dylib`CRYPTO_THREAD_read_lock + 12 regards, tom lane
I wrote: > Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes: >> I tried building with Homebrew-supplied openldap. What ends up >> happening is that the postgres binary is indeed linked with openldap, >> but libpq still is linked against the OS-supplied LDAP framework. >> (Checked with "otool -L" in each case.) Can someone else reproduce >> this, too? > [ it works with MacPorts ] Oh, I have a theory about this: I bet your Homebrew installation has a recent OpenLDAP version that only supplies libldap not libldap_r. In that case, configure will still find libldap_r available and will bind libpq to it, and you get the observed result. The configure check is not sophisticated enough to realize that it's finding chunks of two different OpenLDAP installations. Not sure about a good fix. If we had a way to detect which library file AC_CHECK_LIB finds, we could verify that libldap and libldap_r come from the same directory ... but I don't think we have that. regards, tom lane
I wrote: > Oh, I have a theory about this: I bet your Homebrew installation > has a recent OpenLDAP version that only supplies libldap not libldap_r. > In that case, configure will still find libldap_r available and will > bind libpq to it, and you get the observed result. The configure > check is not sophisticated enough to realize that it's finding chunks > of two different OpenLDAP installations. After thinking about this for awhile, it seems like the best solution is to make configure proceed like this: 1. Find libldap. 2. Detect whether it's OpenLDAP 2.5 or newer. 3. If not, try to find libldap_r. There are various ways we could perform step 2, but I think the most reliable is to try to link to some function that's present in 2.5 but not before. (In particular, this doesn't require any strong assumptions about whether the installation's header files match the library.) After a quick dig in 2.4 and 2.5, it looks like ldap_verify_credentials() would serve. Barring objections, I'll make a patch for that. BTW, I was a little distressed to read this in the 2.4 headers: ** If you fail to define LDAP_THREAD_SAFE when linking with ** -lldap_r or define LDAP_THREAD_SAFE when linking with -lldap, ** provided header definations and declarations may be incorrect. That's not something we do or ever have done, AFAIK. Given the lack of complaints and the fact that 2.4 is more or less EOL, I don't feel a strong need to worry about it; but it might be something to keep in mind in case we get bug reports. regards, tom lane
I wrote: > We still have a bit of work to do, because this setup isn't getting > all the way through src/test/ldap/: > 2022-05-04 11:01:33.459 EDT [21304] LOG: server process (PID 21312) was terminated by signal 11: Segmentation fault: 11 > Many of the test cases pass, but it looks like ldaps-related ones don't. Sadly, this still happens with MacPorts' build of openldap 2.6.1. I was able to get a stack trace from the point of the segfault this time: (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) * frame #0: 0x000000018f120628 libsystem_pthread.dylib`pthread_rwlock_rdlock frame #1: 0x00000001019174c4 libcrypto.3.dylib`CRYPTO_THREAD_read_lock + 12 frame #2: 0x00000001019099d0 libcrypto.3.dylib`ossl_lib_ctx_get_data + 56 frame #3: 0x00000001019144d0 libcrypto.3.dylib`get_provider_store + 28 frame #4: 0x000000010191641c libcrypto.3.dylib`ossl_provider_deregister_child_cb + 32 frame #5: 0x0000000101909748 libcrypto.3.dylib`OSSL_LIB_CTX_free + 48 frame #6: 0x000000010aa982d8 legacy.dylib`legacy_teardown + 24 frame #7: 0x0000000101914840 libcrypto.3.dylib`ossl_provider_free + 76 frame #8: 0x00000001018ec404 libcrypto.3.dylib`evp_cipher_free_int + 48 frame #9: 0x0000000101472804 libssl.3.dylib`SSL_CTX_free + 420 frame #10: 0x00000001013a4768 libldap.2.dylib`ldap_int_tls_destroy + 40 frame #11: 0x000000018f00fdd0 libsystem_c.dylib`__cxa_finalize_ranges + 464 frame #12: 0x000000018f00fb74 libsystem_c.dylib`exit + 44 frame #13: 0x0000000100941cb8 postgres`proc_exit(code=<unavailable>) at ipc.c:152:2 [opt] frame #14: 0x000000010096d804 postgres`PostgresMain(dbname=<unavailable>, username=<unavailable>) at postgres.c:4756:5[opt] frame #15: 0x00000001008d7730 postgres`BackendRun(port=<unavailable>) at postmaster.c:4489:2 [opt] frame #16: 0x00000001008d6ff4 postgres`ServerLoop [inlined] BackendStartup(port=<unavailable>) at postmaster.c:4217:3[opt] frame #17: 0x00000001008d6fcc postgres`ServerLoop at postmaster.c:1791:7 [opt] frame #18: 0x00000001008d474c postgres`PostmasterMain(argc=<unavailable>, argv=<unavailable>) at postmaster.c:1463:11[opt] frame #19: 0x000000010083a248 postgres`main(argc=<unavailable>, argv=<unavailable>) at main.c:202:3 [opt] frame #20: 0x000000010117908c dyld`start + 520 So (1) libldap relies on libssl to implement ldaps ... no surprise there, and (2) something's going wrong in the atexit callback that it seemingly installs to close down its SSL context. It's not clear from this whether this is purely libldap's fault or if there is something we're doing that sends it off the rails. I could believe that the problem is essentially a double shutdown of libssl, except that there doesn't seem to be any reason why PG itself would have touched libssl; this isn't an SSL-enabled build. (Adding --with-openssl doesn't make it better, either.) On the whole I'm leaning to the position that this is openldap's fault not ours. regards, tom lane
I wrote: > After thinking about this for awhile, it seems like the best solution > is to make configure proceed like this: > 1. Find libldap. > 2. Detect whether it's OpenLDAP 2.5 or newer. > 3. If not, try to find libldap_r. Here's a proposed patch for that. It seems to do the right thing with openldap 2.4.x and 2.6.x, but I don't have a 2.5 installation at hand to try. regards, tom lane diff --git a/configure b/configure index 364f37559d..274e0db8c5 100755 --- a/configure +++ b/configure @@ -13410,7 +13410,18 @@ _ACEOF fi done - if test "$enable_thread_safety" = yes; then + # The separate ldap_r library only exists in OpenLDAP < 2.5, and if we + # have 2.5 or later, we shouldn't even probe for ldap_r (we might find a + # library from a separate OpenLDAP installation). The most reliable + # way to check that is to check for a function introduced in 2.5. + ac_fn_c_check_func "$LINENO" "ldap_verify_credentials" "ac_cv_func_ldap_verify_credentials" +if test "x$ac_cv_func_ldap_verify_credentials" = xyes; then : + thread_safe_libldap=yes +else + thread_safe_libldap=no +fi + + if test "$enable_thread_safety" = yes -a "$thread_safe_libldap" = no; then # Use ldap_r for FE if available, else assume ldap is thread-safe. # On some platforms ldap_r fails to link without PTHREAD_LIBS. LIBS="$_LIBS" diff --git a/configure.ac b/configure.ac index bfd8b713a9..bba1ac5878 100644 --- a/configure.ac +++ b/configure.ac @@ -1374,7 +1374,14 @@ if test "$with_ldap" = yes ; then LDAP_LIBS_BE="-lldap $EXTRA_LDAP_LIBS" # This test is carried out against libldap. AC_CHECK_FUNCS([ldap_initialize]) - if test "$enable_thread_safety" = yes; then + # The separate ldap_r library only exists in OpenLDAP < 2.5, and if we + # have 2.5 or later, we shouldn't even probe for ldap_r (we might find a + # library from a separate OpenLDAP installation). The most reliable + # way to check that is to check for a function introduced in 2.5. + AC_CHECK_FUNC([ldap_verify_credentials], + [thread_safe_libldap=yes], + [thread_safe_libldap=no]) + if test "$enable_thread_safety" = yes -a "$thread_safe_libldap" = no; then # Use ldap_r for FE if available, else assume ldap is thread-safe. # On some platforms ldap_r fails to link without PTHREAD_LIBS. LIBS="$_LIBS"
On 06.05.22 21:25, Tom Lane wrote: > I wrote: >> After thinking about this for awhile, it seems like the best solution >> is to make configure proceed like this: > >> 1. Find libldap. >> 2. Detect whether it's OpenLDAP 2.5 or newer. >> 3. If not, try to find libldap_r. > > Here's a proposed patch for that. It seems to do the right thing > with openldap 2.4.x and 2.6.x, but I don't have a 2.5 installation > at hand to try. This patch works for me. I think it's a good solution.
On 04.05.22 17:16, Tom Lane wrote: > Hmm, I just tried it with up-to-date MacPorts, and it was a*complete* > fail: I got all the deprecation warnings (so the system include headers > were used), and both postgres and libpq.dylib still ended up linked > against /System/Library/Frameworks/LDAP.framework/Versions/A/LDAP. Btw., I was a bit puzzled about all this talk about deprecation warnings, which I have not seen. I turns out that you only get those if you use the OS compiler, not a third-party gcc installation. So in terms of my original message, my installation is clearly niche. The possibly false-positive configure warning is a drop in the bucket compared to the deprecation warnings from the compiler. So it's probably okay to leave this as is and encourage users to use openldap.
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes: > Btw., I was a bit puzzled about all this talk about deprecation > warnings, which I have not seen. I turns out that you only get those if > you use the OS compiler, not a third-party gcc installation. Ah-hah. > So in terms of my original message, my installation is clearly niche. > The possibly false-positive configure warning is a drop in the bucket > compared to the deprecation warnings from the compiler. So it's > probably okay to leave this as is and encourage users to use openldap. OK. I will push the configure change once the release freeze lifts. regards, tom lane
I wrote: > OK. I will push the configure change once the release freeze lifts. And done. regards, tom lane
Hi, On 2022-05-06 14:00:43 -0400, Tom Lane wrote: > I wrote: > > Oh, I have a theory about this: I bet your Homebrew installation > > has a recent OpenLDAP version that only supplies libldap not libldap_r. > > In that case, configure will still find libldap_r available and will > > bind libpq to it, and you get the observed result. The configure > > check is not sophisticated enough to realize that it's finding chunks > > of two different OpenLDAP installations. > > After thinking about this for awhile, it seems like the best solution > is to make configure proceed like this: > > 1. Find libldap. > 2. Detect whether it's OpenLDAP 2.5 or newer. > 3. If not, try to find libldap_r. > > There are various ways we could perform step 2, but I think the most > reliable is to try to link to some function that's present in 2.5 > but not before. (In particular, this doesn't require any strong > assumptions about whether the installation's header files match the > library.) After a quick dig in 2.4 and 2.5, it looks like > ldap_verify_credentials() would serve. Why do we continue to link the backend to ldap when we find ldap_r, given that we know that it can cause problems for extension libraries using libpq? I did a cursory search of the archives without finding an answer. ISTM that we'd be more robust if we used your check from above to decide when to use ldap (so we don't use an older ldap_r), and use ldap_r for both FE and BE otherwise. Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes: > Why do we continue to link the backend to ldap when we find ldap_r, given that > we know that it can cause problems for extension libraries using libpq? Uh ... if we know that, it's news to me. I think we might've avoided ldap_r for fear of pulling libpthread into the backend; per recent discussion, it's not clear that avoiding that is possible anyway. But you didn't make a case for changing this. regards, tom lane
Hi, On 2022-08-29 23:18:23 -0400, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > Why do we continue to link the backend to ldap when we find ldap_r, given that > > we know that it can cause problems for extension libraries using libpq? > > Uh ... if we know that, it's news to me. Isn't that what the configure warning Peter mentioned upthread is about? # PGAC_LDAP_SAFE # -------------- # PostgreSQL sometimes loads libldap_r and plain libldap into the same # process. Check for OpenLDAP versions known not to tolerate doing so; assume # non-OpenLDAP implementations are safe. The dblink test suite exercises the # hazardous interaction directly. The patch applied as a result of this thread dealt with a different version of the problem, with -lldap_r picking up a different library version than -lldap. Leaving that aside it also doesn't seem like a great idea to have two different copies of the nearly same library loaded for efficiency reasons, not that it'll make a large difference... Greetings, Andres Freund