[patch] Native patch to handle releasing ssl errors on thread exit

0001-ssl-Release-ssl-errors-on-thread-exit.patch (text/plain), 16.25 KB, created by Nate Clark on 2017-01-27 20:31:49 UTC

(hide)

Description: Native patch to handle releasing ssl errors on thread exit

Filename:

MIME Type:

Creator: Nate Clark

Created: 2017-01-27 20:31:49 UTC

Size: 16.25 KB

patch

obsolete

>From c604d69a20b7bb2a5ac31da24bd6baaaefa273b4 Mon Sep 17 00:00:00 2001 >From: Nate Clark <nate@neworld.us> >Date: Thu, 5 Jan 2017 16:21:08 -0500 >Subject: [PATCH] ssl: Release ssl errors on thread exit > >This is an interpretation of how openssl 1.1.0 handles error context >clean up. On windows clean up is invoked on thread detach and on other >platforms a thread local with a destructor is used. Whenever >ERR_clear_error or ERR_get_error is called the thread local is set. When >the thread exits the destructor will be invoked on any thread which does >not have a NULL value for the thread local. >--- > native/include/ssl_private.h | 9 +++++++++ > native/os/win32/system.c | 3 +++ > native/src/ssl.c | 44 +++++++++++++++++++++++++++++++++++++----- > native/src/sslcontext.c | 46 ++++++++++++++++++++++---------------------- > native/src/sslnetwork.c | 8 ++++---- > native/src/sslutils.c | 2 +- > 6 files changed, 79 insertions(+), 33 deletions(-) > >diff --git a/native/include/ssl_private.h b/native/include/ssl_private.h >index ca0da64..633ae06 100644 >--- a/native/include/ssl_private.h >+++ b/native/include/ssl_private.h >@@ -363,4 +363,13 @@ int SSL_callback_select_next_proto(SSL *, unsigned char **, unsigned cha > int SSL_callback_alpn_select_proto(SSL *, const unsigned char **, unsigned char *, const unsigned char *, unsigned int, void *); > > >+void SSL_thread_exit(void); >+#if (OPENSSL_VERSION_NUMBER < 0x10100000L) && ! defined(WIN32) >+unsigned long SSL_ERR_get(void); >+void SSL_ERR_clear(void); >+#else >+#define SSL_ERR_get() ERR_get_error() >+#define SSL_ERR_clear() ERR_clear_error() >+#endif >+ > #endif /* SSL_PRIVATE_H */ >diff --git a/native/os/win32/system.c b/native/os/win32/system.c >index e6f8fdf..a4e99f3 100644 >--- a/native/os/win32/system.c >+++ b/native/os/win32/system.c >@@ -101,6 +101,9 @@ DllMain( > /** The thread of the attached process terminates. > */ > case DLL_THREAD_DETACH: >+#ifdef HAVE_OPENSSL >+ SSL_thread_exit(); >+#endif > break; > > /** DLL unload due to process termination >diff --git a/native/src/ssl.c b/native/src/ssl.c >index d9fd657..e1d4e88 100644 >--- a/native/src/ssl.c >+++ b/native/src/ssl.c >@@ -49,6 +49,8 @@ struct CRYPTO_dynlock_value { > int line; > apr_thread_mutex_t *mutex; > }; >+ >+apr_threadkey_t *thread_exit_key; > #endif > > /* >@@ -435,7 +437,29 @@ static unsigned long ssl_thread_id(void) > #endif > } > >+void SSL_thread_exit(void) { >+#if (OPENSSL_VERSION_NUMBER < 0x10100000L) >+ ERR_remove_thread_state(NULL); >+ apr_threadkey_private_set(NULL, thread_exit_key); >+#endif >+} >+ > #if OPENSSL_VERSION_NUMBER < 0x10100000L >+unsigned long SSL_ERR_get() { >+ apr_threadkey_private_set(thread_exit_key, thread_exit_key); >+ return ERR_get_error(); >+} >+ >+void SSL_ERR_clear() { >+ apr_threadkey_private_set(thread_exit_key, thread_exit_key); >+ ERR_clear_error(); >+} >+ >+static void _ssl_thread_exit(void *data) { >+ UNREFERENCED(data); >+ SSL_thread_exit(); >+} >+ > static void ssl_set_thread_id(CRYPTO_THREADID *id) > { > CRYPTO_THREADID_set_numeric(id, ssl_thread_id()); >@@ -700,6 +724,9 @@ TCN_IMPLEMENT_CALL(jint, SSL, initialize)(TCN_STDARGS, jstring engine) > { > jclass clazz; > jclass sClazz; >+#if !defined(OPENSSL_NO_ENGINE) || OPENSSL_VERSION_NUMBER < 0x10100000L >+ apr_status_t err = APR_SUCCESS; >+#endif > > TCN_ALLOC_CSTRING(engine); > >@@ -729,6 +756,14 @@ TCN_IMPLEMENT_CALL(jint, SSL, initialize)(TCN_STDARGS, jstring engine) > OPENSSL_load_builtin_modules(); > > #if OPENSSL_VERSION_NUMBER < 0x10100000L >+ err = apr_threadkey_private_create(&thread_exit_key, _ssl_thread_exit, >+ tcn_global_pool); >+ if (err != APR_SUCCESS) { >+ ssl_init_cleanup(NULL); >+ tcn_ThrowAPRException(e, err); >+ return (jint)err; >+ } >+ > /* Initialize thread support */ > ssl_thread_setup(tcn_global_pool); > #endif >@@ -736,7 +771,6 @@ TCN_IMPLEMENT_CALL(jint, SSL, initialize)(TCN_STDARGS, jstring engine) > #ifndef OPENSSL_NO_ENGINE > if (J2S(engine)) { > ENGINE *ee = NULL; >- apr_status_t err = APR_SUCCESS; > if(strcmp(J2S(engine), "auto") == 0) { > ENGINE_register_all_complete(); > } >@@ -859,7 +893,7 @@ TCN_IMPLEMENT_CALL(jint, SSL, fipsModeSet)(TCN_STDARGS, jint mode) > #ifdef OPENSSL_FIPS > if(1 != (r = (jint)FIPS_mode_set((int)mode))) { > /* arrange to get a human-readable error message */ >- unsigned long err = ERR_get_error(); >+ unsigned long err = SSL_ERR_get(); > char msg[256]; > > /* ERR_load_crypto_strings() already called in initialize() */ >@@ -1196,7 +1230,7 @@ TCN_IMPLEMENT_CALL(jstring, SSL, getLastError)(TCN_STDARGS) > { > char buf[256]; > UNREFERENCED(o); >- ERR_error_string(ERR_get_error(), buf); >+ ERR_error_string(SSL_ERR_get(), buf); > return tcn_new_string(e, buf); > } > >@@ -1208,7 +1242,7 @@ TCN_IMPLEMENT_CALL(jboolean, SSL, hasOp)(TCN_STDARGS, jint op) > /*** Begin Twitter 1:1 API addition ***/ > TCN_IMPLEMENT_CALL(jint, SSL, getLastErrorNumber)(TCN_STDARGS) { > UNREFERENCED_STDARGS; >- return ERR_get_error(); >+ return SSL_ERR_get(); > } > > static void ssl_info_callback(const SSL *ssl, int where, int ret) { >@@ -1784,7 +1818,7 @@ TCN_IMPLEMENT_CALL(jboolean, SSL, setCipherSuites)(TCN_STDARGS, jlong ssl, > } > if (!SSL_set_cipher_list(ssl_, J2S(ciphers))) { > char err[256]; >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Unable to configure permitted SSL ciphers (%s)", err); > rv = JNI_FALSE; > } >diff --git a/native/src/sslcontext.c b/native/src/sslcontext.c >index 8858ce0..685e379 100644 >--- a/native/src/sslcontext.c >+++ b/native/src/sslcontext.c >@@ -206,7 +206,7 @@ TCN_IMPLEMENT_CALL(jlong, SSLContext, make)(TCN_STDARGS, jlong pool, > > if (!ctx) { > char err[256]; >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Invalid Server SSL Protocol (%s)", err); > goto init_failed; > } >@@ -478,7 +478,7 @@ TCN_IMPLEMENT_CALL(jboolean, SSLContext, setCipherSuite)(TCN_STDARGS, jlong ctx, > if (!SSL_CTX_set_cipher_list(c->ctx, J2S(ciphers))) { > #endif > char err[256]; >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Unable to configure permitted SSL ciphers (%s)", err); > rv = JNI_FALSE; > } >@@ -512,7 +512,7 @@ TCN_IMPLEMENT_CALL(jboolean, SSLContext, setCARevocation)(TCN_STDARGS, jlong ctx > if (J2S(file)) { > lookup = X509_STORE_add_lookup(c->crl, X509_LOOKUP_file()); > if (lookup == NULL) { >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > X509_STORE_free(c->crl); > c->crl = NULL; > tcn_Throw(e, "Lookup failed for file %s (%s)", J2S(file), err); >@@ -523,7 +523,7 @@ TCN_IMPLEMENT_CALL(jboolean, SSLContext, setCARevocation)(TCN_STDARGS, jlong ctx > if (J2S(path)) { > lookup = X509_STORE_add_lookup(c->crl, X509_LOOKUP_hash_dir()); > if (lookup == NULL) { >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > X509_STORE_free(c->crl); > c->crl = NULL; > tcn_Throw(e, "Lookup failed for path %s (%s)", J2S(file), err); >@@ -577,7 +577,7 @@ TCN_IMPLEMENT_CALL(jboolean, SSLContext, setCACertificate)(TCN_STDARGS, > if (!SSL_CTX_load_verify_locations(c->ctx, > J2S(file), J2S(path))) { > char err[256]; >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Unable to configure locations " > "for client authentication (%s)", err); > rv = JNI_FALSE; >@@ -642,7 +642,7 @@ TCN_IMPLEMENT_CALL(void, SSLContext, setTmpDH)(TCN_STDARGS, jlong ctx, > bio = BIO_new_file(J2S(file), "r"); > if (!bio) { > char err[256]; >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Error while configuring DH using %s: %s", J2S(file), err); > TCN_FREE_CSTRING(file); > return; >@@ -652,7 +652,7 @@ TCN_IMPLEMENT_CALL(void, SSLContext, setTmpDH)(TCN_STDARGS, jlong ctx, > BIO_free(bio); > if (!dh) { > char err[256]; >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Error while configuring DH: no DH parameter found in %s (%s)", J2S(file), err); > TCN_FREE_CSTRING(file); > return; >@@ -661,7 +661,7 @@ TCN_IMPLEMENT_CALL(void, SSLContext, setTmpDH)(TCN_STDARGS, jlong ctx, > if (1 != SSL_CTX_set_tmp_dh(c->ctx, dh)) { > char err[256]; > DH_free(dh); >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Error while configuring DH with file %s: %s", J2S(file), err); > TCN_FREE_CSTRING(file); > return; >@@ -702,7 +702,7 @@ TCN_IMPLEMENT_CALL(void, SSLContext, setTmpECDHByCurveName)(TCN_STDARGS, jlong c > if (1 != SSL_CTX_set_tmp_ecdh(c->ctx, ecdh)) { > char err[256]; > EC_KEY_free(ecdh); >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Error while configuring elliptic curve %s: %s", J2S(curveName), err); > TCN_FREE_CSTRING(curveName); > return; >@@ -809,7 +809,7 @@ static X509 *load_pem_cert(tcn_ssl_ctxt_t *c, const char *file) > (void *)cb_data); > if (cert == NULL && > (ERR_GET_REASON(ERR_peek_last_error()) == PEM_R_NO_START_LINE)) { >- ERR_clear_error(); >+ SSL_ERR_clear(); > BIO_ctrl(bio, BIO_CTRL_RESET, 0, NULL); > cert = d2i_X509_bio(bio, NULL); > } >@@ -921,7 +921,7 @@ TCN_IMPLEMENT_CALL(jboolean, SSLContext, setCertificate)(TCN_STDARGS, jlong ctx, > } > if ((p = strrchr(cert_file, '.')) != NULL && strcmp(p, ".pkcs12") == 0) { > if (!ssl_load_pkcs12(c, cert_file, &c->keys[idx], &c->certs[idx], 0)) { >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Unable to load certificate %s (%s)", > cert_file, err); > rv = JNI_FALSE; >@@ -930,14 +930,14 @@ TCN_IMPLEMENT_CALL(jboolean, SSLContext, setCertificate)(TCN_STDARGS, jlong ctx, > } > else { > if ((c->keys[idx] = load_pem_key(c, key_file)) == NULL) { >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Unable to load certificate key %s (%s)", > key_file, err); > rv = JNI_FALSE; > goto cleanup; > } > if ((c->certs[idx] = load_pem_cert(c, cert_file)) == NULL) { >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Unable to load certificate %s (%s)", > cert_file, err); > rv = JNI_FALSE; >@@ -945,19 +945,19 @@ TCN_IMPLEMENT_CALL(jboolean, SSLContext, setCertificate)(TCN_STDARGS, jlong ctx, > } > } > if (SSL_CTX_use_certificate(c->ctx, c->certs[idx]) <= 0) { >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Error setting certificate (%s)", err); > rv = JNI_FALSE; > goto cleanup; > } > if (SSL_CTX_use_PrivateKey(c->ctx, c->keys[idx]) <= 0) { >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Error setting private key (%s)", err); > rv = JNI_FALSE; > goto cleanup; > } > if (SSL_CTX_check_private_key(c->ctx) <= 0) { >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Private key does not match the certificate public key (%s)", > err); > rv = JNI_FALSE; >@@ -1050,7 +1050,7 @@ TCN_IMPLEMENT_CALL(jboolean, SSLContext, setCertificateRaw)(TCN_STDARGS, jlong c > tmp = (const unsigned char *)cert; > certs = d2i_X509(NULL, &tmp, lengthOfCert); > if (certs == NULL) { >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Error reading certificate (%s)", err); > rv = JNI_FALSE; > goto cleanup; >@@ -1066,7 +1066,7 @@ TCN_IMPLEMENT_CALL(jboolean, SSLContext, setCertificateRaw)(TCN_STDARGS, jlong c > evp = PEM_read_bio_PrivateKey(bio, NULL, 0, NULL); > if (evp == NULL) { > BIO_free(bio); >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Error reading private key (%s)", err); > rv = JNI_FALSE; > goto cleanup; >@@ -1078,19 +1078,19 @@ TCN_IMPLEMENT_CALL(jboolean, SSLContext, setCertificateRaw)(TCN_STDARGS, jlong c > c->keys[idx] = evp; > > if (SSL_CTX_use_certificate(c->ctx, c->certs[idx]) <= 0) { >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Error setting certificate (%s)", err); > rv = JNI_FALSE; > goto cleanup; > } > if (SSL_CTX_use_PrivateKey(c->ctx, c->keys[idx]) <= 0) { >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Error setting private key (%s)", err); > rv = JNI_FALSE; > goto cleanup; > } > if (SSL_CTX_check_private_key(c->ctx) <= 0) { >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Private key does not match the certificate public key (%s)", > err); > rv = JNI_FALSE; >@@ -1145,11 +1145,11 @@ TCN_IMPLEMENT_CALL(jboolean, SSLContext, addChainCertificateRaw)(TCN_STDARGS, jl > tmp = (const unsigned char *)cert; > certs = d2i_X509(NULL, &tmp, lengthOfCert); > if (certs == NULL) { >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Error reading certificate (%s)", err); > rv = JNI_FALSE; > } else if (SSL_CTX_add0_chain_cert(c->ctx, certs) <= 0) { >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(e, "Error adding certificate to chain (%s)", err); > rv = JNI_FALSE; > } >diff --git a/native/src/sslnetwork.c b/native/src/sslnetwork.c >index 849510e..768cf0c 100644 >--- a/native/src/sslnetwork.c >+++ b/native/src/sslnetwork.c >@@ -127,7 +127,7 @@ static tcn_ssl_conn_t *ssl_create(JNIEnv *env, tcn_ssl_ctxt_t *ctx, apr_pool_t * > } > if ((ssl = SSL_new(ctx->ctx)) == NULL) { > char err[256]; >- ERR_error_string(ERR_get_error(), err); >+ ERR_error_string(SSL_ERR_get(), err); > tcn_Throw(env, "SSL_new failed (%s)", err); > con = NULL; > return NULL; >@@ -320,7 +320,7 @@ TCN_IMPLEMENT_CALL(jint, SSLSocket, handshake)(TCN_STDARGS, jlong sock) > > apr_socket_timeout_get(con->sock, &timeout); > while (!SSL_is_init_finished(con->ssl)) { >- ERR_clear_error(); >+ SSL_ERR_clear(); > if ((s = SSL_do_handshake(con->ssl)) <= 0) { > if (!con->ssl) > return APR_ENOTSOCK; >@@ -406,7 +406,7 @@ ssl_socket_recv(apr_socket_t *sock, char *buf, apr_size_t *len) > } > apr_socket_timeout_get(con->sock, &timeout); > for (;;) { >- ERR_clear_error(); >+ SSL_ERR_clear(); > if ((s = SSL_read(con->ssl, buf, rd)) <= 0) { > if (!con->ssl) > return APR_ENOTSOCK; >@@ -488,7 +488,7 @@ ssl_socket_send(apr_socket_t *sock, const char *buf, > } > apr_socket_timeout_get(con->sock, &timeout); > for (;;) { >- ERR_clear_error(); >+ SSL_ERR_clear(); > if ((s = SSL_write(con->ssl, buf, wr)) <= 0) { > if (!con->ssl) > return APR_ENOTSOCK; >diff --git a/native/src/sslutils.c b/native/src/sslutils.c >index 4182fb8..bec3155 100644 >--- a/native/src/sslutils.c >+++ b/native/src/sslutils.c >@@ -281,7 +281,7 @@ int SSL_CTX_use_certificate_chain(SSL_CTX *ctx, const char *file, > BIO_free(bio); > return -1; > } >- while (ERR_get_error() > 0) ; >+ while (SSL_ERR_get() > 0) ; > } > BIO_free(bio); > return n; >-- >2.9.3 > View Attachment As Diff View Attachment As Raw

This is ASF Bugzilla: the Apache Software Foundation bug system. In case of problems with the functioning of ASF Bugzilla, please contact bugzilla-admin@apache.org. Please Note: this e-mail address is only for reporting problems with ASF Bugzilla. Mail about any other subject will be silently ignored.

• • Home
  • | New
  • | Browse
  • | Search
  • |
    [?]
  • | Reports
  • | Help
  • | New Account
  • | Log In
    Remember [x]
  • | Forgot Password
    Login: [x]