Don't share tcn_ssl_verify_config_t between tcn_ssl_state_t and tcn_s… (

#850)

…sl_ctxt_t

Motivation:

Let's not share the same memory between tcn_ssl_state_t and
tcn_ssl_ctxt_t as it makes lifetime handling of the memory and prone to
segfaults

Modifications:

- Let tcn_ssl_state_t contain tcn_ssl_verify_config_t and copy over the
config from the tcn_ssl_ctxt_t if needed.
- Remove free calls that are not needed anymore

Result:

Fix possible lifetime issue

openssl-dynamic/src/main/c/ssl.c

@@ -919,8 +919,8 @@ static tcn_ssl_state_t* new_ssl_state(tcn_ssl_ctxt_t* ctx) {
     memset(state, 0, sizeof(tcn_ssl_state_t));
     state->ctx = ctx;
 
-     // Initially we will share the configuration from the SSLContext.
-    state->verify_config = &ctx->verify_config;
+     // Initially we will copy the configuration from the SSLContext.
+    memcpy(&state->verify_config, &ctx->verify_config, sizeof(tcn_ssl_verify_config_t));
     return state;
 }
 
@@ -929,12 +929,6 @@ static void free_ssl_state(JNIEnv* e, tcn_ssl_state_t* state) {
         return;
     }
 
-    // Only free the verify_config if it is not shared with the SSLContext.
-    if (state->verify_config != NULL && state->verify_config != &state->ctx->verify_config) {
-        OPENSSL_free(state->verify_config);
-        state->verify_config = NULL;
-    }
-
     tcn_ssl_task_free(e, state->ssl_task);
     state->ssl_task = NULL;
 
@@ -1526,24 +1520,13 @@ TCN_IMPLEMENT_CALL(void, SSL, setVerify)(TCN_STDARGS, jlong ssl, jint level, jin
     state = tcn_SSL_get_app_state(ssl_);
     TCN_ASSERT(state != NULL);
     TCN_ASSERT(state->ctx != NULL);
-    TCN_ASSERT(state->verify_config != NULL);
-
-    // If we are sharing the configuration from the SSLContext we now need to create a new configuration just for this SSL.
-    if (state->verify_config == &state->ctx->verify_config) {
-       if ((state->verify_config = (tcn_ssl_verify_config_t*) OPENSSL_malloc(sizeof(tcn_ssl_verify_config_t))) == NULL) {
-           tcn_ThrowException(e, "failed to allocate tcn_ssl_verify_config_t");
-           return;
-       }
-       // Copy the verify depth form the context in case depth is <0.
-       state->verify_config->verify_depth = state->ctx->verify_config.verify_depth;
-    }
 
 #ifdef OPENSSL_IS_BORINGSSL
-    SSL_set_custom_verify(ssl_, tcn_set_verify_config(state->verify_config, level, depth), tcn_SSL_cert_custom_verify);
+    SSL_set_custom_verify(ssl_, tcn_set_verify_config(&state->verify_config, level, depth), tcn_SSL_cert_custom_verify);
 #else
     // No need to specify a callback for SSL_set_verify because we override the default certificate verification via SSL_CTX_set_cert_verify_callback.
-    SSL_set_verify(ssl_, tcn_set_verify_config(state->verify_config, level, depth), NULL);
-    SSL_set_verify_depth(ssl_, state->verify_config->verify_depth);
+    SSL_set_verify(ssl_, tcn_set_verify_config(&state->verify_config, level, depth), NULL);
+    SSL_set_verify_depth(ssl_, state->verify_config.verify_depth);
 #endif // OPENSSL_IS_BORINGSSL
 }
 

openssl-dynamic/src/main/c/ssl_private.h

@@ -398,7 +398,7 @@ struct tcn_ssl_state_t {
     int handshakeCount;
     tcn_ssl_ctxt_t *ctx;
     tcn_ssl_task_t* ssl_task;
-    tcn_ssl_verify_config_t* verify_config;
+    tcn_ssl_verify_config_t verify_config;
 };
 
 #define TCN_GET_SSL_CTX(ssl, C)                             \

openssl-dynamic/src/main/c/sslcontext.c

@@ -1426,14 +1426,13 @@ static jbyteArray get_certs(JNIEnv *e, SSL* ssl, STACK_OF(X509)* chain) {
 
     tcn_ssl_state_t* state = tcn_SSL_get_app_state(ssl);
     TCN_ASSERT(state != NULL);
-    TCN_ASSERT(state->verify_config != NULL);
 
     // SSL_CTX_set_verify_depth() and SSL_set_verify_depth() set the limit up to which depth certificates in a chain are
     // used during the verification procedure. If the certificate chain is longer than allowed, the certificates above
     // the limit are ignored. Error messages are generated as if these certificates would not be present,
     // most likely a X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY will be issued.
     // https://www.openssl.org/docs/man1.0.2/ssl/SSL_set_verify.html
-    int len = TCN_MIN(state->verify_config->verify_depth, totalQueuedLength);
+    int len = TCN_MIN(state->verify_config.verify_depth, totalQueuedLength);
     unsigned i;
     int length;
 
@@ -1562,7 +1561,7 @@ static int SSL_cert_verify(X509_STORE_CTX *ctx, void *arg) {
 #endif // X509_V_ERR_UNSPECIFIED
 
 
-    // TODO(scott): if verify_config->verify_depth == SSL_CVERIFY_OPTIONAL we have the option to let the handshake
+    // TODO(scott): if verify_config.verify_depth == SSL_CVERIFY_OPTIONAL we have the option to let the handshake
     // succeed for some of the "informational" error messages (e.g. X509_V_ERR_EMAIL_MISMATCH ?)
 
 complete:
@@ -1674,7 +1673,7 @@ enum ssl_verify_result_t tcn_SSL_cert_custom_verify(SSL* ssl, uint8_t *out_alert
             result = X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY;
         }
 
-        // TODO(scott): if verify_config->verify_depth == SSL_CVERIFY_OPTIONAL we have the option to let the handshake
+        // TODO(scott): if verify_config.verify_depth == SSL_CVERIFY_OPTIONAL we have the option to let the handshake
         // succeed for some of the "informational" error messages (e.g. X509_V_ERR_EMAIL_MISMATCH ?)
     }
 complete: