Merge pull request #275 from cconlon/jniLockFixes
Fixes to avoid use-after-free issues in WolfSSLSocketmaster
commit
6acd852fb1
|
@ -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);
|
||||
|
|
|
@ -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.
|
||||
|
|
Loading…
Reference in New Issue