[Date Prev][Date Next] [Chronological] [Thread] [Top]

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




Am 03.01.2017 um 20:12 schrieb Howard Chu:
> 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.

Hello Howard,

looks like you're much more familiar with open[ssl|ldap] insides. My intension was to show a way to build openldap with openssl-1.1.0.
I'm aware that is not ideal and you prove that now.
Thanks. But I cannot contribute better code. Sorry.

Andreas