Bug 64429

Summary: Commit b8649e81458194d70667952d9e26df82a79c773f in 1.1.24 breaks compilation with LibreSSL
Product: Tomcat Native Reporter: Michael Osipov <michaelo>
Component: LibraryAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: regression    
Priority: P2    
Version: unspecified   
Target Milestone: ---   
Hardware: All   
OS: All   

Description Michael Osipov 2020-05-13 13:44:33 UTC
As documented in BZ 64419, commit b8649e81458194d70667952d9e26df82a79c773f unfortunately breaks compilation with LibreSSL:

> src/ssl.c:789:41: error: use of undeclared identifier 'thread_exit_key'; did you mean 'pthread_exit'?
>     err = apr_threadkey_private_create(&thread_exit_key, _ssl_thread_exit,
>                                         ^~~~~~~~~~~~~~~
>                                         pthread_exit
> /usr/include/pthread.h:215:7: note: 'pthread_exit' declared here
> void            pthread_exit(void *) __dead2;
>                 ^
> src/ssl.c:789:58: error: use of undeclared identifier '_ssl_thread_exit'
>     err = apr_threadkey_private_create(&thread_exit_key, _ssl_thread_exit,
>                                                          ^
> src/ssl.c:796:5: error: use of undeclared identifier 'threadkey_initialized'
>     threadkey_initialized = 1;
>     ^
> src/ssl.c:799:5: warning: implicit declaration of function 'ssl_thread_setup' is invalid in C99
>       [-Wimplicit-function-declaration]
>     ssl_thread_setup(tcn_global_pool);
>     ^

It seems like the change is incomplete for LibreSSL. We should either provide a complete solution or revert the change until a complete solution has been developed.
Comment 1 Christopher Schultz 2020-05-13 14:50:23 UTC
I think a solution is desired, but there is no need to revert.

Support for LibreSSL is a goal, not a requirement.
Comment 2 Michael Osipov 2020-05-13 16:25:45 UTC
(In reply to Christopher Schultz from comment #1)
> I think a solution is desired, but there is no need to revert.
> 
> Support for LibreSSL is a goal, not a requirement.

I agree, but if it worked before we shouldn't break it w/o any further announcement. Are you skilled enough to work on a working solution?
Comment 3 Mark Thomas 2020-08-20 10:11:12 UTC
The fix looks to be fairly simple and I have this committed locally. The various #if preprocessor directives are not consistent.

#if OPENSSL_VERSION_NUMBER < 0x10100000L

vs

#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)

Making them consistent fixes the compilation issue. However, I am not currently able to confirm the fix because LibreSSL portable on Linux appears to enter a tight loop (never exits, 100% CPU on one thread) when SSL_CTX_use_PrivateKey is called.

I've tested 2.9.1, 2.9.2, 3.1.4 and 3.2.0 and the behaviour is the same.

I want to investigate the "tight loop" further before I push the commit that fixes this issue.
Comment 4 Michael Osipov 2020-08-20 11:16:27 UTC
(In reply to Mark Thomas from comment #3)
> The fix looks to be fairly simple and I have this committed locally. The
> various #if preprocessor directives are not consistent.
> 
> #if OPENSSL_VERSION_NUMBER < 0x10100000L
> 
> vs
> 
> #if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
> 
> Making them consistent fixes the compilation issue. However, I am not
> currently able to confirm the fix because LibreSSL portable on Linux appears
> to enter a tight loop (never exits, 100% CPU on one thread) when
> SSL_CTX_use_PrivateKey is called.
> 
> I've tested 2.9.1, 2.9.2, 3.1.4 and 3.2.0 and the behaviour is the same.
> 
> I want to investigate the "tight loop" further before I push the commit that
> fixes this issue.

Can this be reproduced with the tests?
Comment 5 Mark Thomas 2020-08-20 13:19:49 UTC
It was another inconsistent directive.
Comment 6 Michael Osipov 2020-08-20 13:25:15 UTC
(In reply to Mark Thomas from comment #5)
> It was another inconsistent directive.

Does your fix apply to every version of LibreSSL? While working on other LibreSSL related issues I used this pattern: https://github.com/apache/tomcat-native/commit/51f949dc6e0b6e4e27972b8ba2d0a2626fc3c1c5#diff-d5ecebaa2939a925164d1e10b8ab0f35R1265
Comment 7 Mark Thomas 2020-08-20 14:43:20 UTC
The directives I added needed to be consistent with the directives already in place to avoid the errors.