Skip to content

Commit

Permalink
Merge pull request #236 from cconlon/get1Session
Browse files Browse the repository at this point in the history 
Always call wolfSSL_get1_session() inside native JNI WolfSSLSession.getSession()
  • Loading branch information
@JacobBarthelmeh
JacobBarthelmeh authored Dec 6, 2024
2 parents 87b6c6a + 4dbbef9 commit 06feb77
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 38 deletions.
19 changes: 19 additions & 0 deletions .github/workflows/main.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,25 @@ jobs:
jdk_version: ${{ matrix.jdk_version }}
wolfssl_configure: ${{ matrix.wolfssl_configure }}

# -------------------- session cache variant sanity checks -----------------------
# Only check one Linux and Mac JDK version as a sanity check.
# Using Zulu, but this can be expanded if needed.
linux-zulu-sesscache:
strategy:
matrix:
os: [ 'ubuntu-latest', 'macos-latest' ]
jdk_version: [ '11' ]
wolfssl_configure: [
'--enable-jni CFLAGS=-DNO_SESSION_CACHE_REF',
]
name: ${{ matrix.os }} (Zulu JDK ${{ matrix.jdk_version }}, ${{ matrix.wolfssl_configure}})
uses: ./.github/workflows/linux-common.yml
with:
os: ${{ matrix.os }}
jdk_distro: "zulu"
jdk_version: ${{ matrix.jdk_version }}
wolfssl_configure: ${{ matrix.wolfssl_configure }}

# ------------------ Facebook Infer static analysis -------------------
# Run Facebook infer over PR code, only running on Linux with one
# JDK/version for now.
Expand Down
25 changes: 10 additions & 15 deletions native/com_wolfssl_WolfSSLSession.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1644,23 +1644,18 @@ JNIEXPORT jlong JNICALL Java_com_wolfssl_WolfSSLSession_get1Session
}
}
} while (err == SSL_ERROR_WANT_READ);

sess = wolfSSL_get_session(ssl);
hasTicket = wolfSSL_SESSION_has_ticket((const WOLFSSL_SESSION*)sess);
}

/* Only duplicate / save session if not TLS 1.3 (will be using normal
* session IDs), or is TLS 1.3 and we have a session ticket */
if (version != TLS1_3_VERSION || hasTicket == 1) {

/* wolfSSL checks ssl for NULL, returns pointer to new WOLFSSL_SESSION,
* Returns new duplicated WOLFSSL_SESSION. Needs to be freed with
* wolfSSL_SESSION_free() when finished with pointer. */
if (sess != NULL) {
/* Guarantee that we own the WOLFSSL_SESSION, make a copy */
dup = wolfSSL_SESSION_dup(sess);
}
}
/* Call wolfSSL_get1_session() to increase the ref count of the internal
* WOLFSSL_SESSION struct. This is needed in all build option cases,
* since Java callers of this function expect to explicitly free this
* pointer when finished with use. In some build cases, for example
* NO_CLIENT_CACHE or NO_SESSION_CACHE_REF, the poiner returned by
* wolfSSL_get_session() will be a pointer into the WOLFSSL struct, which
* will be freed with wolfSSL_free(). This can cause issues if the Java
* app expects to hold a valid session pointer for resumption and free
* later on. */
dup = wolfSSL_get1_session(ssl);

if (wc_UnLockMutex(jniSessLock) != 0) {
printf("Failed to unlock jniSessLock in get1Session()");
Expand Down
72 changes: 49 additions & 23 deletions src/java/com/wolfssl/WolfSSLSession.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1358,15 +1358,25 @@ public int getError(int ret) throws IllegalStateException {
}

/**
* Sets the session to be used when the SSL object is used to create
* a SSL/TLS connection.
* For session resumption, before calling <code>shutdownSSL()</code>
* with your session object, an application should save the session ID
* from the object with a call to <code>getSession()</code>, which returns
* a pointer to the session. Later, the application should create a new
* SSL session object and assign the saved session with <code>
* setSession()</code>. At this point, the application may call <code>
* connect()</code> and wolfSSL will try to resume the session.
* Sets the session (native WOLFSSL_SESSION) to be used with this object
* for session resumption.
*
* The native WOLFSSL_SESSION pointed to contains all the necessary
* information required to perform a session resumption and reestablishment
* of the connection without a new handshake.
* <p>
* To do session resumption, before calling <code>shutdownSSL()</code>
* with your WolfSSLSession object, save the internal session state by
* calling <code>getSession()</code>, which returns a pointer to the
* native WOLFSSL_SESSION session structure. Later, when the application
* is ready to resume a session, it should create a new WolfSSLSession
* object and assign the previously-saved session pointer by passing it
* to the <code>setSession(long session)</code> method. This should be
* done before the handshake is started for the second/resumed time. After
* calling <code>setSession(long session)</code>, the application may call
* <code>connect()</code> and wolfSSL will try to resume the session. If
* the session cannot be resumed, a new fresh handshake will be
* established.
*
* @param session pointer to the native WOLFSSL_SESSION structure used
* to set the session for the SSL session object.
Expand Down Expand Up @@ -1411,25 +1421,35 @@ public int setSession(long session) throws IllegalStateException {
}

/**
* Returns a pointer to the current session used in the given SSL object.
* Returns a pointer to the current session (native WOLFSSL_SESSION)
* associated with this object, or null if not available.
*
* The native WOLFSSL_SESSION pointed to contains all the necessary
* information required to perform a session resumption and reestablishment
* the connection without a new handshake.
* of the connection without a new handshake.
* <p>
* To do session resumption, before calling <code>shutdownSSL()</code>
* with your WolfSSLSession object, save the internal session state by
* calling <code>getSession()</code>, which returns a pointer to the
* native WOLFSSL_SESSION session structure. Later, when the application
* is ready to resume a session, it should create a new WolfSSLSession
* object and assign the previously-saved session pointer by passing it
* to the <code>setSession(long session)</code> method. This should be
* done before the handshake is started for the second/resumed time. After
* calling <code>setSession(long session)</code>, the application may call
* <code>connect()</code> and wolfSSL will try to resume the session. If
* the session cannot be resumed, a new fresh handshake will be
* established.
* <p>
* <b>IMPORTANT:</b>
* <p>
* For session resumption, before calling <code>shutdownSSL()</code>
* with your session object, an application should save the session ID
* from the object with a call to <code>getSession()</code>, which returns
* a pointer to the session. Later, the application should create a new
* SSL object and assign the saved session with <code>setSession</code>.
* At this point, the application may call <code>connect()</code> and
* wolfSSL will try to resume the session.
*
* The pointer (WOLFSSL_SESSION) returned by this method needs to be freed
* when the application is finished with it, by calling
* <code>freeSession(long)</code>. This will release the underlying
* native memory associated with this WOLFSSL_SESSION.
* when the application is finished with it by calling
* <code>freeSession(long session)</code>. This will release the underlying
* native memory associated with this WOLFSSL_SESSION. Failing to free
* the session will result in a memory leak.
*
* @throws IllegalStateException WolfSSLContext has been freed
* @throws IllegalStateException this WolfSSLSession has been freed
* @return a pointer to the current SSL session object on success.
* <code>null</code> if <b>ssl</b> is <code>null</code>,
* the SSL session cache is disabled, wolfSSL doesn't have
Expand All @@ -1446,6 +1466,12 @@ public long getSession() throws IllegalStateException {
WolfSSLDebug.log(getClass(), WolfSSLDebug.Component.JNI,
WolfSSLDebug.INFO, this.sslPtr, "entered getSession()");

/* Calling get1Session() here as an indication that the native
* JNI level should always return a session pointer that needs
* to be freed by the application. This behavior can change in
* native wolfSSL depending on build options
* (ex: NO_SESSION_CACHE_REF), so JNI layer here will make that
* behavior consistent to the JNI/JSSE callers. */
sessPtr = get1Session(this.sslPtr);

WolfSSLDebug.log(getClass(), WolfSSLDebug.Component.JNI,
Expand Down

0 comments on commit 06feb77

Please sign in to comment.