Horton

Thread

Commits

  1. configure: don't probe for libldap_r if libldap is 2.5 or newer.

  1. configure openldap crash warning

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-05-02T13:24:30Z

    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?

  2. Re: configure openldap crash warning

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-02T14:03:58Z

    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

  3. Re: configure openldap crash warning

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-05-04T14:05:45Z

    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?

  4. Re: configure openldap crash warning

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-04T15:16:12Z

    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

  5. Re: configure openldap crash warning

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-04T15:30:12Z

    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

  6. Re: configure openldap crash warning

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-06T18:00:43Z

    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

  7. Re: configure openldap crash warning

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-06T19:17:04Z

    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

  8. Re: configure openldap crash warning

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-06T19:25:07Z

    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

  9. Re: configure openldap crash warning

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-05-09T12:28:13Z

    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.

  10. Re: configure openldap crash warning

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-05-09T12:36:47Z

    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.

  11. Re: configure openldap crash warning

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-09T13:47:59Z

    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

  12. Re: configure openldap crash warning

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-10T22:46:26Z

    I wrote:
> OK.  I will push the configure change once the release freeze lifts.

And done.

			regards, tom lane

  13. Re: configure openldap crash warning

    Andres Freund <andres@anarazel.de> — 2022-08-30T03:08:17Z

    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

  14. Re: configure openldap crash warning

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-08-30T03:18:23Z

    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

  15. Re: configure openldap crash warning

    Andres Freund <andres@anarazel.de> — 2022-08-30T03:48:30Z

    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