Re: (ITS#8533) Support OpenSSL-1.1.0c

[hyc@symas.com]

sca@andreasschulze.de wrote:
> Am 28.11.2016 um 17:42 schrieb Quanah Gibson-Mount:
>> I have a few additional comments:
>> ...
>
> I updated the patch as suggested: ftp://ftp.openldap.org/incoming/andreas-schulze-161204.patch

+/* REVIEW: CRYPTO_num_locks is defined as "(1)" openssl-1.1.0 */
+static ldap_pvt_thread_mutex_t	tlso_mutexes[CRYPTO_num_locks()];

This is not acceptable. The fact that the macro is defined as a function macro 
implies that it may be turned into a genuine function at some point in the 
future. Either we get a commitment from the OpenSSL developers that this macro 
will always be a constant, or the code must change to dynamically allocate the 
mutexes array.

@@ -882,6 +924,9 @@ static BIO_METHOD tlso_bio_method =
  	tlso_bio_create,
  	tlso_bio_destroy
  };
+#else
+static BIO_METHOD *tlso_bio_method = NULL;
+#endif

  static int
  tlso_sb_setup( Sockbuf_IO_Desc *sbiod, void *arg )
@@ -898,8 +943,29 @@ tlso_sb_setup( Sockbuf_IO_Desc *sbiod, void *arg )
  	
  	p->session = arg;
  	p->sbiod = sbiod;
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
  	bio = BIO_new( &tlso_bio_method );
  	bio->ptr = (void *)p;
+#else
+	if ( tlso_bio_method == NULL ) {
+		tlso_bio_method = BIO_meth_new(( 100 | 0x400 ), "sockbuf glue");
+		if ( tlso_bio_method == NULL
+		     || !BIO_meth_set_write(tlso_bio_method, tlso_bio_write)
+		     || !BIO_meth_set_read(tlso_bio_method, tlso_bio_read)
+		     || !BIO_meth_set_puts(tlso_bio_method, tlso_bio_puts)
+		     || !BIO_meth_set_gets(tlso_bio_method, tlso_bio_gets)
+		     || !BIO_meth_set_ctrl(tlso_bio_method, tlso_bio_ctrl)
+		     || !BIO_meth_set_create(tlso_bio_method, tlso_bio_create)
+		     || !BIO_meth_set_destroy(tlso_bio_method, tlso_bio_destroy)) {
+			return -1;
+		}
+	}
+	bio = BIO_new(tlso_bio_method);
+	if ( bio == NULL ) {
+		return -1;
+	}
+	BIO_set_data(bio, (void *)p);
+#endif

This is no good, the tlso_bio_method must be initialized once and only once. 
The structure can be referenced simultaneously from multiple threads, and 
initing on each use will be a performance hit as well as a waste of memory.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/