From 07380615b5effc8316f803e715548c563f3ebf34 Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Wed, 11 Jun 2025 09:27:31 -0600 Subject: [PATCH 1/2] JSSE: skip calling freeSSL() inside SSLSocket.close() if InputStream or OutputStream are still active --- .../wolfssl/provider/jsse/WolfSSLSocket.java | 67 +++++++++++++++++-- 1 file changed, 62 insertions(+), 5 deletions(-) diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java b/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java index 0bcb297..1f226d2 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java @@ -1967,6 +1967,30 @@ public class WolfSSLSocket extends SSLSocket { return false; } + /** + * Internal private method to check if WolfSSLInputStream + * and WolfSSLOutputStream are closed. + * + * @return true if both streams are closed (or were not created/null), + * otherwise false if they were created and are still active. + */ + private boolean ioStreamsAreClosed() { + + if (this.inStream == null && this.outStream == null) { + return true; + } + + if (this.inStream != null && !this.inStream.isClosed()) { + return false; + } + + if (this.outStream != null && !this.outStream.isClosed()) { + return false; + } + + return true; + } + /** * Closes this SSLSocket. * @@ -2132,13 +2156,22 @@ public class WolfSSLSocket extends SSLSocket { /* Connection is closed, free native WOLFSSL session * to release native memory earlier than garbage - * collector might with finalize(), if we don't - * have any threads still waiting in poll/select. */ - if (this.ssl.getThreadsBlockedInPoll() == 0) { + * collector might with finalize(), Don't free if we + * have threads still waiting in poll/select or if + * our WolfSSLInputStream or WolfSSLOutputStream are + * still open. */ + if (this.ssl.getThreadsBlockedInPoll() == 0 && + ioStreamsAreClosed()) { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, () -> "calling this.ssl.freeSSL()"); this.ssl.freeSSL(); this.ssl = null; + } else { + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "deferring freeing this.ssl, threads " + + "blocked in poll: " + + this.ssl.getThreadsBlockedInPoll() + + ", or streams not closed"); } /* Reset internal WolfSSLEngineHelper to null */ @@ -2517,7 +2550,7 @@ public class WolfSSLSocket extends SSLSocket { private WolfSSLSession ssl; private WolfSSLSocket socket; - private boolean isClosed = true; + private volatile boolean isClosed = true; /* Atomic boolean to indicate if this InputStream has started to * close. Protects against deadlock between two threads calling @@ -2530,6 +2563,18 @@ public class WolfSSLSocket extends SSLSocket { this.isClosed = false; } + /** + * Non standard method to check if this InputStream has been + * closed. This is used by WolfSSLSocket to check if the associated + * WolfSSLInputStream has been closed before calling freeSSL() + * internally. + * + * @return true if this InputStream has been closed, otherwise false + */ + public boolean isClosed() { + return this.isClosed; + } + /** * Close InputStream, but gives caller option to close underlying * Socket or not. @@ -2741,7 +2786,7 @@ public class WolfSSLSocket extends SSLSocket { private WolfSSLSession ssl; private WolfSSLSocket socket; - private boolean isClosed = true; + private volatile boolean isClosed = true; /* Atomic boolean to indicate if this InputStream has started to * close. Protects against deadlock between two threads calling @@ -2754,6 +2799,18 @@ public class WolfSSLSocket extends SSLSocket { this.isClosed = false; } + /** + * Non standard method to check if this OutputStream has been + * closed. This is used by WolfSSLSocket to check if the associated + * WolfSSLOutputStream has been closed before calling freeSSL() + * internally. + * + * @return true if this OutputStream has been closed, otherwise false + */ + public boolean isClosed() { + return this.isClosed; + } + /** * Close OutputStream, but gives caller option to close underlying * Socket or not. From 8692585e12c84ff1ea2aa611d1201945086f3660 Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Wed, 11 Jun 2025 09:36:27 -0600 Subject: [PATCH 2/2] JNI: set SSLAppData back to NULL inside WOLFSSL struct in freeSSL() --- native/com_wolfssl_WolfSSLSession.c | 36 ++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/native/com_wolfssl_WolfSSLSession.c b/native/com_wolfssl_WolfSSLSession.c index f18e8f5..f6396b5 100644 --- a/native/com_wolfssl_WolfSSLSession.c +++ b/native/com_wolfssl_WolfSSLSession.c @@ -1673,6 +1673,8 @@ JNIEXPORT void JNICALL Java_com_wolfssl_WolfSSLSession_freeSSL jobject* g_cachedVerifyCb; SSLAppData* appData; WOLFSSL* ssl = (WOLFSSL*)(uintptr_t)sslPtr; + wolfSSL_Mutex* sessLock = NULL; + byte lockAvailable = 1; (void)jcl; #if defined(HAVE_PK_CALLBACKS) && (defined(HAVE_ECC) || !defined(NO_RSA)) internCtx* pkCtx = NULL; @@ -1684,9 +1686,41 @@ JNIEXPORT void JNICALL Java_com_wolfssl_WolfSSLSession_freeSSL return; } - /* free session mutex lock */ + /* Get and set WOLFSSL app data back to NULL. Try to lock on jniSessLock + * to prevent race conditions between multiple threads getting + * and using/freeing app data. */ appData = (SSLAppData*)wolfSSL_get_app_data(ssl); if (appData != NULL) { + if (appData->jniSessLock != NULL) { + sessLock = appData->jniSessLock; + if (wc_LockMutex(sessLock) != 0) { + lockAvailable = 0; + } + /* Re-check getting appData after we have mutex */ + appData = (SSLAppData*)wolfSSL_get_app_data(ssl); + if (appData != NULL) { + /* Set SSLAppData back to NULL inside WOLFSSL struct, if other + * threads are trying to use it they will fail with error + * instead of use after free faults. */ + wolfSSL_set_app_data(ssl, NULL); + } + if (lockAvailable) { + /* Unlock on saved mutex, in case appData is now NULL */ + wc_UnLockMutex(sessLock); + sessLock = NULL; + } + } else { + /* If no jniSessLock available, fall back to just + * setting app data to NULL in WOLFSSL. We should always have + * appData->jniSessLock though. */ + wolfSSL_set_app_data(ssl, NULL); + } + } + + /* appData may have been refreshed/NULL retrieved above inside + * mutex lock, check again against NULL before continuing freeing. */ + if (appData != NULL) { + /* free session mutex lock */ if (appData->jniSessLock != NULL) { wc_FreeMutex(appData->jniSessLock); XFREE(appData->jniSessLock, NULL, DYNAMIC_TYPE_TMP_BUFFER);