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

2014-07-11 Thread Noah Misch
On Thu, Jun 19, 2014 at 01:01:34PM -0400, Tom Lane wrote:
 Noah Misch n...@leadboat.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
diff --git a/configure b/configure
index 481096c..e2850e4 100755
--- a/configure
+++ b/configure
@@ -9464,6 +9464,17 @@ fi
 
 fi
 
+# 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.
+
+
+
+
+
 if test $with_ldap = yes ; then
   if test $PORTNAME != win32; then
  for ac_header in ldap.h
@@ -9480,6 +9491,47 @@ fi
 
 done
 
+ { $as_echo $as_me:${as_lineno-$LINENO}: checking for compatible LDAP 
implementation 5
+$as_echo_n checking for compatible LDAP implementation...  6; }
+if ${pgac_cv_ldap_safe+:} false; then :
+  $as_echo_n (cached)  6
+else
+  cat confdefs.h - _ACEOF conftest.$ac_ext
+/* end confdefs.h.  */
+#include ldap.h
+#if !defined(LDAP_VENDOR_VERSION) || \
+ (defined(LDAP_API_FEATURE_X_OPENLDAP)  \
+  LDAP_VENDOR_VERSION = 20424  LDAP_VENDOR_VERSION = 20431)
+choke me
+#endif
+int
+main ()
+{
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile $LINENO; then :
+  pgac_cv_ldap_safe=yes
+else
+  pgac_cv_ldap_safe=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+{ $as_echo $as_me:${as_lineno-$LINENO}: result: $pgac_cv_ldap_safe 5
+$as_echo $pgac_cv_ldap_safe 6; }
+
+if test $pgac_cv_ldap_safe != yes; then
+  { $as_echo $as_me:${as_lineno-$LINENO}: 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. 5
+$as_echo $as_me: 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. 2;}
+fi
   else
  for ac_header in winldap.h
 do :
diff --git a/configure.in b/configure.in
index c938a5d..9f324f0 100644
--- a/configure.in
+++ b/configure.in
@@ -1096,10 +1096,39 @@ if test $with_libxslt = yes ; then
   AC_CHECK_HEADER(libxslt/xslt.h, [], [AC_MSG_ERROR([header file 
libxslt/xslt.h is required for XSLT support])])
 fi
 
+# 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.
+
+AC_DEFUN([PGAC_LDAP_SAFE],
+[AC_CACHE_CHECK([for compatible LDAP implementation], [pgac_cv_ldap_safe],
+[AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
+[#include ldap.h
+#if !defined(LDAP_VENDOR_VERSION) || \
+ (defined(LDAP_API_FEATURE_X_OPENLDAP)  \
+  LDAP_VENDOR_VERSION = 20424  LDAP_VENDOR_VERSION = 20431)
+choke me
+#endif], [])],
+[pgac_cv_ldap_safe=yes],
+[pgac_cv_ldap_safe=no])])
+
+if test $pgac_cv_ldap_safe != yes; then
+  AC_MSG_WARN([
+*** 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.])
+fi])
+
+
+
 if test $with_ldap = yes ; then
   if test $PORTNAME != win32; then
  AC_CHECK_HEADERS(ldap.h, [],
   [AC_MSG_ERROR([header file ldap.h is required for 
LDAP])])
+ PGAC_LDAP_SAFE
   else
  

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

2014-06-19 Thread Noah Misch
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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2014-06-19 Thread Tom Lane
Noah Misch n...@leadboat.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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers