Merge ~ahasenack/ubuntu/+source/openldap:xenial-slapd-sasl-segfault-1688575 into ubuntu/+source/openldap:ubuntu/xenial-devel

[Andreas Hasenack]

  * Import upstream patches to fix memory corruption caused by calling
    sasl_client_init() multiple times and possibly concurrently.
    (ITS#8648) (LP: #1688575)

 -- Ryan Tandy <email address hidden> Mon, 14 May 2018 23:03:54 -0700

For a test case, please see the linked bug and its SRU template.

Here is a PPA with fixed packages: https://launchpad.net/~ahasenack/+archive/ubuntu/slapd-sasl-segfault-1688575

sudo add-apt-repository ppa:ahasenack/slapd-sasl-segfault-1688575

If this is approved, I can dput the package, but I'll need the upload tag to be pushed for me.

[Christian Ehrhardt  (paelzer)]

Changelog: ok
Test and verification: ok
ppa fixing the bug: ok
ppa build has no other obvious issues: ok
Review changes: notes, see below

Note: I added the needed dependencies to the SRU template of your bug

I'd ask you to add a bit more to the dep3 headers, in particular a Origin with an URL.
I know your git exports files hold the sha, and if one knows the openldap repo he might find it.
But a real URL that gets you to the change would really be preferred.

Further we usually do a back-ref of the "Ubuntu-Bug: ..." from the .patch files to the bug number right? That would help to later maintain this more easily as well.

Finally the changelog doesn't refer to the .patch files. This last statement might be personal-style and not required - up to you.

review: Needs Fixing

[Christian Ehrhardt  (paelzer)]

Note: I know this comes from a 3rd party provided debdiff, but we could/should clean up on the upload IMHO.

[Andreas Hasenack (ahasenack)]

Thanks for the comments. Yeah, I always hesitate a bit with adding the ubuntu "touches" when a patch comes fully done like that from someone like that, but better clarity is good.

Changes pushed, please take another look.

[Christian Ehrhardt  (paelzer)]

Thanks for the Cleanup, LGTM now +1

review: Approve

[Christian Ehrhardt  (paelzer)]

upload/2.4.42+dfsg-2ubuntu3.3 on 499a8d56fdb3274996cdcac05d116db176aa6865 pushed if you want to upload.

[Andreas Hasenack (ahasenack)]

Thanks, uploaded.

diff --git a/debian/changelog b/debian/changelog
index b7a7257..3feead7 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
+openldap (2.4.42+dfsg-2ubuntu3.3) xenial; urgency=medium
+
+  [ Ryan Tandy ]
+  * d/p/ITS-8648-check-result-of-ldap_int_initialize-in-ldap.patch,
+    d/p/ITS-8648-init-SASL-library-in-global-init.patch: Import upstream
+    patches to fix memory corruption caused by calling sasl_client_init()
+    multiple times and possibly concurrently.  (ITS#8648) (LP: #1688575)
+
+ -- Andreas Hasenack <[e-mail]>  Tue, 22 May 2018 10:54:12 -0300
+
 openldap (2.4.42+dfsg-2ubuntu3.2) xenial-security; urgency=medium
 
   * SECURITY UPDATE: denial of service via search with page size of 0
diff --git a/debian/patches/ITS-8648-check-result-of-ldap_int_initialize-in-ldap.patch b/debian/patches/ITS-8648-check-result-of-ldap_int_initialize-in-ldap.patch
new file mode 100644
index 0000000..8829a6a
--- /dev/null
+++ b/debian/patches/ITS-8648-check-result-of-ldap_int_initialize-in-ldap.patch
@@ -0,0 +1,31 @@
+Description: check result of ldap_int_initialize in ldap_{get,set}_option
+Author: Ryan Tandy <[e-mail]>
+Origin: Upstream, http://www.openldap.org/devel/gitweb.cgi?p=openldap.git;a=commit;h=e437b12277c1cc8ec72e0f78f660137c60ffaad7
+Bug: http://www.openldap.org/its/index.cgi/Software%20Bugs?id=8648;selectid=8648
+Bug-Ubuntu: http://www.openldap.org/its/index.cgi/Software%20Bugs?id=8648;selectid=8648
+Last-Update: 2018-05-22
+
+---
+ libraries/libldap/options.c | 4 ++++
+ 1 file changed, 4 insertions(+)
+
+--- a/libraries/libldap/options.c
++++ b/libraries/libldap/options.c
+@@ -106,6 +106,8 @@
+ 
+ 	if( lo->ldo_valid != LDAP_INITIALIZED ) {
+ 		ldap_int_initialize(lo, NULL);
++		if ( lo->ldo_valid != LDAP_INITIALIZED )
++			return LDAP_LOCAL_ERROR;
+ 	}
+ 
+ 	if(ld != NULL) {
+@@ -446,6 +448,8 @@
+ 
+ 	if( lo->ldo_valid != LDAP_INITIALIZED ) {
+ 		ldap_int_initialize(lo, dbglvl);
++		if ( lo->ldo_valid != LDAP_INITIALIZED )
++			return LDAP_LOCAL_ERROR;
+ 	}
+ 
+ 	if(ld != NULL) {
diff --git a/debian/patches/ITS-8648-init-SASL-library-in-global-init.patch b/debian/patches/ITS-8648-init-SASL-library-in-global-init.patch
new file mode 100644
index 0000000..dd54ecc
--- /dev/null
+++ b/debian/patches/ITS-8648-init-SASL-library-in-global-init.patch
@@ -0,0 +1,76 @@
+Description: init SASL library in global init
+Author: Ryan Tandy <[e-mail]>
+Origin: Upstream, http://www.openldap.org/devel/gitweb.cgi?p=openldap.git;a=commit;h=431c4af526b18abb4a18c2c4c8655690b753cbe5
+Bug: http://www.openldap.org/its/index.cgi/Software%20Bugs?id=8648;selectid=8648
+Bug-Ubuntu: https://launchpad.net/bugs/1688575
+Last-Update: 2018-05-22
+
+---
+ libraries/libldap/cyrus.c | 17 +++--------------
+ libraries/libldap/init.c  |  6 ++++++
+ 2 files changed, 9 insertions(+), 14 deletions(-)
+
+--- a/libraries/libldap/cyrus.c
++++ b/libraries/libldap/cyrus.c
+@@ -69,14 +69,11 @@
+ 	{ SASL_CB_LIST_END, NULL, NULL }
+ };
+ 
++/*
++ * ldap_int_initialize is responsible for calling this only once.
++ */
+ int ldap_int_sasl_init( void )
+ {
+-	/* XXX not threadsafe */
+-	static int sasl_initialized = 0;
+-
+-	if ( sasl_initialized ) {
+-		return 0;
+-	}
+ 
+ /* SASL 2 takes care of its own memory completely internally */
+ #if SASL_VERSION_MAJOR < 2 && !defined(CSRIMALLOC)
+@@ -96,7 +93,6 @@
+ #endif
+ 
+ 	if ( sasl_client_init( NULL ) == SASL_OK ) {
+-		sasl_initialized = 1;
+ 		return 0;
+ 	}
+ 
+@@ -307,11 +303,6 @@
+ 		return ld->ld_errno;
+ 	}
+ 
+-	if ( ldap_int_sasl_init() ) {
+-		ld->ld_errno = LDAP_LOCAL_ERROR;
+-		return ld->ld_errno;
+-	}
+-
+ #if SASL_VERSION_MAJOR >= 2
+ 	rc = sasl_client_new( "ldap", host, NULL, NULL,
+ 		client_callbacks, 0, &ctx );
+@@ -891,8 +882,6 @@
+ ldap_int_sasl_get_option( LDAP *ld, int option, void *arg )
+ {
+ 	if ( option == LDAP_OPT_X_SASL_MECHLIST ) {
+-		if ( ldap_int_sasl_init() )
+-			return -1;
+ 		*(char ***)arg = (char **)sasl_global_listmech();
+ 		return 0;
+ 	}
+--- a/libraries/libldap/init.c
++++ b/libraries/libldap/init.c
+@@ -658,6 +658,12 @@
+ 	if ( ldap_int_tblsize == 0 ) ldap_int_ip_init();
+ #endif
+ 
++#ifdef HAVE_CYRUS_SASL
++	if ( ldap_int_sasl_init() != 0 ) {
++		return;
++	}
++#endif
++
+ 	ldap_int_initialize_global_options(gopts, dbglvl);
+ 
+ 	if( getenv("LDAPNOINIT") != NULL ) {
diff --git a/debian/patches/series b/debian/patches/series
index 5c8c549..5ef63f3 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -29,3 +29,5 @@ ITS8240-remove-obsolete-assert.patch
 fix-ldap-distribution.patch
 ITS8385-fix-use-after-free-with-GnuTLS
 CVE-2017-9287.patch
+ITS-8648-check-result-of-ldap_int_initialize-in-ldap.patch
+ITS-8648-init-SASL-library-in-global-init.patch