diff --git a/.github/workflows/fips-ready-dual-provider.yml b/.github/workflows/fips-ready-dual-provider.yml index a94e46bd..5d7106c3 100644 --- a/.github/workflows/fips-ready-dual-provider.yml +++ b/.github/workflows/fips-ready-dual-provider.yml @@ -143,7 +143,7 @@ jobs: working-directory: wolfssljni run: | java -classpath \ - lib/wolfssl.jar:lib/wolfssl-jsse.jar:$GITHUB_WORKSPACE/wolfcryptjni/lib/wolfcrypt-jni.jar:examples/provider \ + examples/provider:lib/wolfssl.jar:lib/wolfssl-jsse.jar:$GITHUB_WORKSPACE/wolfcryptjni/lib/wolfcrypt-jni.jar \ DualProviderFIPSTest - name: Show logs on failure diff --git a/examples/provider/DualProviderFIPSTest.sh b/examples/provider/DualProviderFIPSTest.sh index f1089725..295fbd16 100755 --- a/examples/provider/DualProviderFIPSTest.sh +++ b/examples/provider/DualProviderFIPSTest.sh @@ -19,5 +19,5 @@ WOLFCRYPTJNI_DIR="${WOLFCRYPTJNI_DIR:-../../wolfcryptjni}" java -classpath \ - lib/wolfssl.jar:lib/wolfssl-jsse.jar:"${WOLFCRYPTJNI_DIR}"/lib/wolfcrypt-jni.jar:examples/provider \ + examples/provider:lib/wolfssl.jar:lib/wolfssl-jsse.jar:"${WOLFCRYPTJNI_DIR}"/lib/wolfcrypt-jni.jar \ DualProviderFIPSTest diff --git a/native/com_wolfssl_WolfSSLContext.c b/native/com_wolfssl_WolfSSLContext.c index fd9b21ca..774c0b7a 100644 --- a/native/com_wolfssl_WolfSSLContext.c +++ b/native/com_wolfssl_WolfSSLContext.c @@ -1874,24 +1874,53 @@ JNIEXPORT void JNICALL Java_com_wolfssl_WolfSSLContext_setGenCookie #endif } +/* Delete JNI local references created inside NativeGenCookieCb() */ +static void freeGenCookieCbLocalRefs(JNIEnv* jenv, jclass excClass, + jclass sessClass, jobject ctxRef, jclass innerCtxClass, jbyteArray inData) +{ + if (jenv == NULL) { + return; + } + + if (inData != NULL) { + (*jenv)->DeleteLocalRef(jenv, inData); + } + + if (innerCtxClass != NULL) { + (*jenv)->DeleteLocalRef(jenv, innerCtxClass); + } + + if (ctxRef != NULL) { + (*jenv)->DeleteLocalRef(jenv, ctxRef); + } + + if (sessClass != NULL) { + (*jenv)->DeleteLocalRef(jenv, sessClass); + } + + if (excClass != NULL) { + (*jenv)->DeleteLocalRef(jenv, excClass); + } +} + int NativeGenCookieCb(WOLFSSL *ssl, unsigned char *buf, int sz, void *ctx) { jint retval = 0; jint vmret = 0; JNIEnv* jenv; /* JNI environment */ - jclass excClass; /* WolfSSLJNIException class */ + jclass excClass = NULL; /* WolfSSLJNIException class */ int needsDetach = 0; /* Should we explicitly detach? */ jobject* g_cachedSSLObj; /* WolfSSLSession cached object */ - jclass sessClass; /* WolfSSLSession class */ + jclass sessClass = NULL; /* WolfSSLSession class */ jfieldID ctxFid; /* WolfSSLSession->ctx FieldID */ jmethodID getCtxMethodId; /* WolfSSLSession->getAssCtxPtr() ID */ - jobject ctxRef; /* WolfSSLContext object */ - jclass innerCtxClass; /* WolfSSLContext class */ + jobject ctxRef = NULL; /* WolfSSLContext object */ + jclass innerCtxClass = NULL; /* WolfSSLContext class */ jmethodID cookieCbMethodId; /* internalGenCookieCallback ID */ - jbyteArray inData; /* jbyteArray to hold cookie data */ + jbyteArray inData = NULL; /* jbyteArray to hold cookie data */ (void)ctx; @@ -1921,8 +1950,11 @@ int NativeGenCookieCb(WOLFSSL *ssl, unsigned char *buf, int sz, void *ctx) if ((*jenv)->ExceptionOccurred(jenv)) { (*jenv)->ExceptionDescribe(jenv); (*jenv)->ExceptionClear(jenv); - if (needsDetach) + freeGenCookieCbLocalRefs(jenv, excClass, sessClass, ctxRef, + innerCtxClass, inData); + if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); + } return GEN_COOKIE_E; } @@ -1932,8 +1964,11 @@ int NativeGenCookieCb(WOLFSSL *ssl, unsigned char *buf, int sz, void *ctx) (*jenv)->ThrowNew(jenv, excClass, "Can't get native WolfSSLSession object reference in " "NativeGenCookieCb"); - if (needsDetach) + freeGenCookieCbLocalRefs(jenv, excClass, sessClass, ctxRef, + innerCtxClass, inData); + if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); + } return GEN_COOKIE_E; } @@ -1943,8 +1978,11 @@ int NativeGenCookieCb(WOLFSSL *ssl, unsigned char *buf, int sz, void *ctx) (*jenv)->ThrowNew(jenv, excClass, "Can't get native WolfSSLSession class reference in " "NativeGenCookieCb"); - if (needsDetach) + freeGenCookieCbLocalRefs(jenv, excClass, sessClass, ctxRef, + innerCtxClass, inData); + if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); + } return GEN_COOKIE_E; } @@ -1959,8 +1997,11 @@ int NativeGenCookieCb(WOLFSSL *ssl, unsigned char *buf, int sz, void *ctx) (*jenv)->ThrowNew(jenv, excClass, "Can't get native WolfSSLContext field ID in " "NativeGenCookieCb"); - if (needsDetach) + freeGenCookieCbLocalRefs(jenv, excClass, sessClass, ctxRef, + innerCtxClass, inData); + if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); + } return GEN_COOKIE_E; } @@ -1976,8 +2017,11 @@ int NativeGenCookieCb(WOLFSSL *ssl, unsigned char *buf, int sz, void *ctx) (*jenv)->ThrowNew(jenv, excClass, "Can't get getAssociatedContextPtr() method ID in " "NativeGenCookieCb"); - if (needsDetach) + freeGenCookieCbLocalRefs(jenv, excClass, sessClass, ctxRef, + innerCtxClass, inData); + if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); + } return GEN_COOKIE_E; } @@ -1988,8 +2032,11 @@ int NativeGenCookieCb(WOLFSSL *ssl, unsigned char *buf, int sz, void *ctx) if (!ctxRef) { (*jenv)->ThrowNew(jenv, excClass, "Can't get WolfSSLContext object in NativeGenCookieCb"); - if (needsDetach) + freeGenCookieCbLocalRefs(jenv, excClass, sessClass, ctxRef, + innerCtxClass, inData); + if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); + } return GEN_COOKIE_E; } @@ -1999,9 +2046,11 @@ int NativeGenCookieCb(WOLFSSL *ssl, unsigned char *buf, int sz, void *ctx) (*jenv)->ThrowNew(jenv, excClass, "Can't get native WolfSSLContext class reference in " "NativeGenCookieCb"); - (*jenv)->DeleteLocalRef(jenv, ctxRef); - if (needsDetach) + freeGenCookieCbLocalRefs(jenv, excClass, sessClass, ctxRef, + innerCtxClass, inData); + if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); + } return GEN_COOKIE_E; } @@ -2010,16 +2059,18 @@ int NativeGenCookieCb(WOLFSSL *ssl, unsigned char *buf, int sz, void *ctx) "internalGenCookieCallback", "(Lcom/wolfssl/WolfSSLSession;[BI)I"); - if (!cookieCbMethodId) { + if (!cookieCbMethodId) { if ((*jenv)->ExceptionOccurred(jenv)) { (*jenv)->ExceptionDescribe(jenv); (*jenv)->ExceptionClear(jenv); } (*jenv)->ThrowNew(jenv, excClass, "Error getting internalGenCookieCallback method from JNI"); - (*jenv)->DeleteLocalRef(jenv, ctxRef); - if (needsDetach) + freeGenCookieCbLocalRefs(jenv, excClass, sessClass, ctxRef, + innerCtxClass, inData); + if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); + } return GEN_COOKIE_E; } @@ -2029,10 +2080,12 @@ int NativeGenCookieCb(WOLFSSL *ssl, unsigned char *buf, int sz, void *ctx) inData = (*jenv)->NewByteArray(jenv, sz); if (!inData) { (*jenv)->ThrowNew(jenv, excClass, - "Error getting internalGenCookieCallback method from JNI"); - (*jenv)->DeleteLocalRef(jenv, ctxRef); - if (needsDetach) + "Error creating jbyteArray in NativeGenCookieCb"); + freeGenCookieCbLocalRefs(jenv, excClass, sessClass, ctxRef, + innerCtxClass, inData); + if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); + } return GEN_COOKIE_E; } @@ -2044,10 +2097,11 @@ int NativeGenCookieCb(WOLFSSL *ssl, unsigned char *buf, int sz, void *ctx) if ((*jenv)->ExceptionOccurred(jenv)) { (*jenv)->ExceptionDescribe(jenv); (*jenv)->ExceptionClear(jenv); - (*jenv)->DeleteLocalRef(jenv, ctxRef); - (*jenv)->DeleteLocalRef(jenv, inData); - if (needsDetach) + freeGenCookieCbLocalRefs(jenv, excClass, sessClass, ctxRef, + innerCtxClass, inData); + if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); + } return GEN_COOKIE_E; } @@ -2058,22 +2112,22 @@ int NativeGenCookieCb(WOLFSSL *ssl, unsigned char *buf, int sz, void *ctx) if ((*jenv)->ExceptionOccurred(jenv)) { (*jenv)->ExceptionDescribe(jenv); (*jenv)->ExceptionClear(jenv); - (*jenv)->DeleteLocalRef(jenv, ctxRef); - (*jenv)->DeleteLocalRef(jenv, inData); - if (needsDetach) + freeGenCookieCbLocalRefs(jenv, excClass, sessClass, ctxRef, + innerCtxClass, inData); + if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); + } return GEN_COOKIE_E; } } - - /* delete local refs */ - (*jenv)->DeleteLocalRef(jenv, inData); } /* delete local refs, detach JNIEnv from thread */ - (*jenv)->DeleteLocalRef(jenv, ctxRef); - if (needsDetach) + freeGenCookieCbLocalRefs(jenv, excClass, sessClass, ctxRef, + innerCtxClass, inData); + if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); + } return retval; } @@ -6430,7 +6484,7 @@ unsigned int NativePskClientCb(WOLFSSL* ssl, const char* hint, char* identity, /* Note: since this is called from C, not the JVM, we need to explicitly * free all object refs with DeleteLocalRef() */ - if (!g_vm || !ssl || !hint || !identity || !key) { + if (!g_vm || !ssl || !identity || !key) { /* we can't throw an exception yet, so just return 0 (failure) */ return 0; } @@ -6598,20 +6652,24 @@ unsigned int NativePskClientCb(WOLFSSL* ssl, const char* hint, char* identity, return 0; } - /* create String to wrap 'hint' */ - hintString = (*jenv)->NewStringUTF(jenv, hint); - if (!hintString) { - if ((*jenv)->ExceptionOccurred(jenv)) { - (*jenv)->ExceptionDescribe(jenv); - (*jenv)->ExceptionClear(jenv); - } - (*jenv)->ThrowNew(jenv, excClass, - "Error creating String for PSK client hint"); - (*jenv)->DeleteLocalRef(jenv, ctxRef); - if (needsDetach) { - (*g_vm)->DetachCurrentThread(g_vm); + /* Wrap hint as a String, passing null to Java when hint is NULL. */ + if (hint != NULL) { + hintString = (*jenv)->NewStringUTF(jenv, hint); + if (!hintString) { + if ((*jenv)->ExceptionOccurred(jenv)) { + (*jenv)->ExceptionDescribe(jenv); + (*jenv)->ExceptionClear(jenv); + } + (*jenv)->ThrowNew(jenv, excClass, + "Error creating String for PSK client hint"); + (*jenv)->DeleteLocalRef(jenv, ctxRef); + if (needsDetach) { + (*g_vm)->DetachCurrentThread(g_vm); + } + return 0; } - return 0; + } else { + hintString = NULL; } /* find StringBuffer class to wrap 'identity' */ diff --git a/native/com_wolfssl_WolfSSLSession.c b/native/com_wolfssl_WolfSSLSession.c index d404dea8..376bbf60 100644 --- a/native/com_wolfssl_WolfSSLSession.c +++ b/native/com_wolfssl_WolfSSLSession.c @@ -2435,6 +2435,11 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLSession_setTimeout (void)jenv; (void)jcl; + /* Reject timeouts outside unsigned int range accepted by native */ + if (ssl == NULL || t < 0 || t > (jlong)UINT32_MAX) { + return (jint)BAD_FUNC_ARG; + } + return wolfSSL_set_timeout(ssl, (unsigned int)t); } @@ -3537,7 +3542,10 @@ JNIEXPORT jstring JNICALL Java_com_wolfssl_WolfSSLSession_getPeerX509Issuer } issuer = wolfSSL_X509_NAME_oneline( - wolfSSL_X509_get_issuer_name(x509), 0, 0); + wolfSSL_X509_get_issuer_name(x509), 0, 0); + if (issuer == NULL) { + return NULL; + } retString = (*jenv)->NewStringUTF(jenv, issuer); XFREE(issuer, 0, DYNAMIC_TYPE_OPENSSL); @@ -3569,7 +3577,10 @@ JNIEXPORT jstring JNICALL Java_com_wolfssl_WolfSSLSession_getPeerX509Subject } subject = wolfSSL_X509_NAME_oneline( - wolfSSL_X509_get_subject_name(x509), 0, 0); + wolfSSL_X509_get_subject_name(x509), 0, 0); + if (subject == NULL) { + return NULL; + } retString = (*jenv)->NewStringUTF(jenv, subject); XFREE(subject, 0, DYNAMIC_TYPE_OPENSSL); diff --git a/src/java/com/wolfssl/WolfSSLSession.java b/src/java/com/wolfssl/WolfSSLSession.java index 65ecb484..1bf4215e 100644 --- a/src/java/com/wolfssl/WolfSSLSession.java +++ b/src/java/com/wolfssl/WolfSSLSession.java @@ -2471,9 +2471,11 @@ public long getSessTimeout() throws IllegalStateException { /** * Sets the timeout in seconds in the given SSL object. * - * @param t time in seconds to set + * @param t time in seconds to set, in the range [0, 4294967295]. A + * value of 0 selects the default session timeout. * @throws IllegalStateException WolfSSLSession has been freed - * @return WOLFSSL_SUCCESS on success, negative values on failure. + * @return WOLFSSL_SUCCESS on success, or BAD_FUNC_ARG if t is outside + * the accepted range. * @see #setSession(long) * @see #getSession(long) */ diff --git a/src/test/com/wolfssl/provider/jsse/test/WolfSSLParametersPskTest.java b/src/test/com/wolfssl/provider/jsse/test/WolfSSLParametersPskTest.java index a1135923..b0bbbcc1 100644 --- a/src/test/com/wolfssl/provider/jsse/test/WolfSSLParametersPskTest.java +++ b/src/test/com/wolfssl/provider/jsse/test/WolfSSLParametersPskTest.java @@ -357,6 +357,56 @@ public void testPskEngineHandshake() throws Exception { serverEngine.closeOutbound(); } + @Test + public void testPskEngineHandshakeNoServerHint() throws Exception { + + final String[] recvHint = new String[]{ "__unset__" }; + + SSLContext ctx = SSLContext.getInstance("TLSv1.2", engineProvider); + ctx.init(null, null, null); + + SSLEngine serverEngine = ctx.createSSLEngine(); + serverEngine.setUseClientMode(false); + + /* Server does not set an identity hint, which is optional */ + WolfSSLParameters serverParams = new WolfSSLParameters(); + serverParams.setPskServerCb(testServerCb); + serverParams.setCipherSuites(new String[]{pskCipher}); + serverEngine.setSSLParameters(serverParams); + + SSLEngine clientEngine = ctx.createSSLEngine("localhost", 0); + clientEngine.setUseClientMode(true); + + WolfSSLParameters clientParams = new WolfSSLParameters(); + clientParams.setPskClientCb(new WolfSSLPskClientCallback() { + public long pskClientCallback(WolfSSLSession ssl, String hint, + StringBuffer identity, long idMaxLen, byte[] key, + long keyMaxLen) { + + recvHint[0] = hint; + return testClientCb.pskClientCallback(ssl, hint, identity, + idMaxLen, key, keyMaxLen); + } + }); + clientParams.setCipherSuites(new String[]{pskCipher}); + clientEngine.setSSLParameters(clientParams); + + doInMemoryHandshake(clientEngine, serverEngine); + + assertEquals(HandshakeStatus.NOT_HANDSHAKING, + clientEngine.getHandshakeStatus()); + assertEquals(HandshakeStatus.NOT_HANDSHAKING, + serverEngine.getHandshakeStatus()); + + /* Absent server hint arrives as an empty String, not a failure */ + assertNotNull("PSK client callback did not receive a hint", + recvHint[0]); + assertEquals("", recvHint[0]); + + clientEngine.closeOutbound(); + serverEngine.closeOutbound(); + } + @Test public void testPskEngineKeepArrays() throws Exception { diff --git a/src/test/com/wolfssl/test/WolfSSLSessionTest.java b/src/test/com/wolfssl/test/WolfSSLSessionTest.java index 2cb4eb73..915a27bc 100644 --- a/src/test/com/wolfssl/test/WolfSSLSessionTest.java +++ b/src/test/com/wolfssl/test/WolfSSLSessionTest.java @@ -935,6 +935,44 @@ public void test_WolfSSLSession_timeout() } } + @Test + public void test_WolfSSLSession_setTimeoutInvalid() + throws WolfSSLJNIException, WolfSSLException { + + WolfSSLSession ssl = new WolfSSLSession(ctx); + + try { + /* Values outside the unsigned int range are rejected rather + * than wrapping or truncating */ + if (ssl.setTimeout(-1) >= 0) { + fail("setTimeout accepted a negative value"); + } + if (ssl.setTimeout(0x100000000L) >= 0) { + fail("setTimeout accepted a value above UINT32_MAX"); + } + + /* UINT32_MAX is the inclusive upper bound and is accepted */ + if (ssl.setTimeout(0xFFFFFFFFL) != WolfSSL.SSL_SUCCESS) { + fail("setTimeout rejected UINT32_MAX"); + } + + if (ssl.setTimeout(60) != WolfSSL.SSL_SUCCESS) { + fail("setTimeout rejected a valid value"); + } + + /* A rejected value leaves the previously set timeout intact */ + if (ssl.setTimeout(-1) >= 0) { + fail("setTimeout accepted a negative value"); + } + if (ssl.getTimeout() != 60) { + fail("rejected setTimeout changed the session timeout"); + } + + } finally { + ssl.freeSSL(); + } + } + @Test public void test_WolfSSLSession_status() throws WolfSSLJNIException, WolfSSLException {