diff --git a/native/com_wolfssl_WolfSSLContext.c b/native/com_wolfssl_WolfSSLContext.c index b0a3faf..75f5f7f 100644 --- a/native/com_wolfssl_WolfSSLContext.c +++ b/native/com_wolfssl_WolfSSLContext.c @@ -804,7 +804,7 @@ int NativeVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) { JNIEnv* jenv; jint vmret = 0; - jint retval = -1; + jint retval = 0; int needsDetach = 0; jclass excClass = NULL; jclass verifyClass = NULL; @@ -828,11 +828,11 @@ int NativeVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) vmret = (*g_vm)->AttachCurrentThread(g_vm, (void**) &jenv, NULL); #endif if (vmret) { - return -101; /* failed to attach JNIEnv to thread */ + return 0; /* failed to attach JNIEnv to thread */ } needsDetach = 1; } else if (vmret != JNI_OK) { - return -102; /* unable to get JNIEnv from JavaVM */ + return 0; /* unable to get JNIEnv from JavaVM */ } /* find exception class */ @@ -843,7 +843,7 @@ int NativeVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) freeNativeVerifyCbLocalRefs(jenv, excClass, verifyClass, verifyCbObj); if (needsDetach) (*g_vm)->DetachCurrentThread(g_vm); - return -103; + return 0; } /* Locate the per-context verify callback jobject via @@ -896,7 +896,7 @@ int NativeVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) verifyCbObj); if (needsDetach) (*g_vm)->DetachCurrentThread(g_vm); - return -104; + return 0; } verifyMethod = (*jenv)->GetMethodID(jenv, verifyClass, @@ -913,7 +913,7 @@ int NativeVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) verifyCbObj); if (needsDetach) (*g_vm)->DetachCurrentThread(g_vm); - return -105; + return 0; } retval = (*jenv)->CallIntMethod(jenv, verifyCbObj, @@ -927,7 +927,7 @@ int NativeVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) verifyCbObj); if (needsDetach) (*g_vm)->DetachCurrentThread(g_vm); - return -106; + return 0; } } else { @@ -941,14 +941,19 @@ int NativeVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) freeNativeVerifyCbLocalRefs(jenv, excClass, verifyClass, verifyCbObj); if (needsDetach) (*g_vm)->DetachCurrentThread(g_vm); - return -1; + return 0; } freeNativeVerifyCbLocalRefs(jenv, excClass, verifyClass, verifyCbObj); if (needsDetach) (*g_vm)->DetachCurrentThread(g_vm); - return retval; + /* Accept only on an explicit callback success (1), reject otherwise. */ + if (retval == 1) { + return 1; + } + + return 0; } JNIEXPORT jlong JNICALL Java_com_wolfssl_WolfSSLContext_setOptions diff --git a/native/com_wolfssl_WolfSSLSession.c b/native/com_wolfssl_WolfSSLSession.c index 8d26e12..322723e 100644 --- a/native/com_wolfssl_WolfSSLSession.c +++ b/native/com_wolfssl_WolfSSLSession.c @@ -107,7 +107,7 @@ int NativeSSLVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) { JNIEnv* jenv; jint vmret = 0; - jint retval = -1; + jint retval = 0; int needsDetach = 0; jobjectRefType refcheck; SSLAppData* appData; /* WOLFSSL app data, stored verify cb obj */ @@ -128,11 +128,11 @@ int NativeSSLVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) vmret = (*g_vm)->AttachCurrentThread(g_vm, (void**) &jenv, NULL); #endif if (vmret) { - return -101; /* failed to attach JNIEnv to thread */ + return 0; /* failed to attach JNIEnv to thread */ } needsDetach = 1; } else if (vmret != JNI_OK) { - return -102; /* unable to get JNIEnv from JavaVM */ + return 0; /* unable to get JNIEnv from JavaVM */ } /* get app data to retrieve stored Java jobject callback object */ @@ -143,7 +143,7 @@ int NativeSSLVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); } - return -105; + return 0; } /* Promote stored global callback ref to a local ref under g_verifyCbMutex @@ -161,7 +161,7 @@ int NativeSSLVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); } - return -106; + return 0; } /* valid ref check: non-zero type covers local/global/weak, and verifyCbObj @@ -180,7 +180,7 @@ int NativeSSLVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) if (needsDetach) { (*g_vm)->DetachCurrentThread(g_vm); } - return -107; + return 0; } retval = (*jenv)->CallIntMethod(jenv, verifyCbObj, @@ -194,7 +194,7 @@ int NativeSSLVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) (*jenv)->DeleteLocalRef(jenv, verifyCbObj); if (needsDetach) (*g_vm)->DetachCurrentThread(g_vm); - return -109; + return 0; } } else { @@ -208,7 +208,7 @@ int NativeSSLVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) (*jenv)->DeleteLocalRef(jenv, verifyCbObj); if (needsDetach) (*g_vm)->DetachCurrentThread(g_vm); - return -1; + return 0; } (*jenv)->DeleteLocalRef(jenv, verifyCbObj); @@ -216,7 +216,12 @@ int NativeSSLVerifyCallback(int preverify_ok, WOLFSSL_X509_STORE_CTX* store) if (needsDetach) (*g_vm)->DetachCurrentThread(g_vm); - return retval; + /* Accept only on an explicit callback success (1), reject otherwise. */ + if (retval == 1) { + return 1; + } + + return 0; } #ifndef USE_WINDOWS_API diff --git a/src/java/com/wolfssl/WolfSSLVerifyCallback.java b/src/java/com/wolfssl/WolfSSLVerifyCallback.java index fd247b2..2c00f7e 100644 --- a/src/java/com/wolfssl/WolfSSLVerifyCallback.java +++ b/src/java/com/wolfssl/WolfSSLVerifyCallback.java @@ -27,7 +27,7 @@ package com.wolfssl; * callback class to be used by wolfSSL during the handshake process. *

* After implementing this interface, it should be passed as a parameter - * to the {@link WolfSSLContext#setVerify(long, int, WolfSSLVerifyCallback) + * to the {@link WolfSSLContext#setVerify(int, WolfSSLVerifyCallback) * WolfSSLContext.setVerify()} method to be registered with the native wolfSSL * library. * @@ -45,10 +45,9 @@ public interface WolfSSLVerifyCallback { * already passed. 0 if failed, 1 if passed. * @param x509StorePtr pointer to the context used for certificate * chain verification. - * @return 0 if the verification process should - * stop immediately with an error. 1 if - * the verification process should continue with the - * rest of the handshake. + * @return 1 to accept and continue the handshake. + * Any other return value or exception thrown is + * treated as verification failure and aborts handshake. */ public int verifyCallback(int preverify_ok, long x509StorePtr); diff --git a/src/test/com/wolfssl/test/WolfSSLSessionTest.java b/src/test/com/wolfssl/test/WolfSSLSessionTest.java index 2cb4eb7..fcad33a 100644 --- a/src/test/com/wolfssl/test/WolfSSLSessionTest.java +++ b/src/test/com/wolfssl/test/WolfSSLSessionTest.java @@ -721,6 +721,202 @@ public class WolfSSLSessionTest { } } + /* Accept one TLS connection on a background thread. A server-side + * handshake failure is expected once the client rejects the cert. */ + private Future runOneShotTlsServer(final ServerSocket srvSocket, + final WolfSSLContext srvCtx, ExecutorService es) { + return es.submit(new Callable() { + @Override + public Void call() { + Socket server = null; + WolfSSLSession srvSes = null; + try { + server = srvSocket.accept(); + server.setSoTimeout(10000); + srvSes = new WolfSSLSession(srvCtx); + srvSes.setFd(server); + int ret, err; + do { + ret = srvSes.accept(); + err = srvSes.getError(ret); + } while (ret != WolfSSL.SSL_SUCCESS && + (err == WolfSSL.SSL_ERROR_WANT_READ || + err == WolfSSL.SSL_ERROR_WANT_WRITE)); + } catch (Exception e) { + /* expected once the client rejects the certificate */ + } finally { + if (srvSes != null) { + try { + srvSes.freeSSL(); + } catch (Exception e) { } + } + if (server != null) { + try { + server.close(); + } catch (Exception e) { } + } + } + return null; + } + }); + } + + /* Client handshake against a one-shot server, trusting no CA so the + * server cert cannot verify, with the verify callback on the context + * or session. Assert the handshake fails closed. */ + private void assertHandshakeFailsWithVerifyCallback( + WolfSSLVerifyCallback cb, boolean registerOnCtx) throws Exception { + + ServerSocket srvSocket = null; + WolfSSLContext srvCtx = null; + WolfSSLContext cliCtx = null; + ExecutorService es = null; + Future srvFuture = null; + Socket cliSock = null; + WolfSSLSession ssl = null; + + /* Initialize wolfSSL so the verify callback ex_data slot is + * allocated before setVerify() is called. */ + new WolfSSL(); + + try { + srvSocket = new ServerSocket(0); + srvSocket.setSoTimeout(10000); + int port = srvSocket.getLocalPort(); + + srvCtx = createAndSetupWolfSSLContext( + srvCert, srvKey, WolfSSL.SSL_FILETYPE_PEM, cliCert, + WolfSSL.SSLv23_ServerMethod()); + + cliCtx = new WolfSSLContext(WolfSSL.SSLv23_ClientMethod()); + if (registerOnCtx) { + cliCtx.setVerify(WolfSSL.SSL_VERIFY_PEER, cb); + } + + es = Executors.newSingleThreadExecutor(); + srvFuture = runOneShotTlsServer(srvSocket, srvCtx, es); + + cliSock = new Socket("localhost", port); + cliSock.setSoTimeout(10000); + ssl = new WolfSSLSession(cliCtx); + if (!registerOnCtx) { + ssl.setVerify(WolfSSL.SSL_VERIFY_PEER, cb); + } + + int ret = ssl.setFd(cliSock); + assertEquals(WolfSSL.SSL_SUCCESS, ret); + + int err; + do { + ret = ssl.connect(); + err = ssl.getError(ret); + } while (ret != WolfSSL.SSL_SUCCESS && + (err == WolfSSL.SSL_ERROR_WANT_READ || + err == WolfSSL.SSL_ERROR_WANT_WRITE)); + + assertTrue("handshake must fail closed when the verify callback " + + "throws or returns non-success on an untrusted certificate", + ret != WolfSSL.SSL_SUCCESS); + } finally { + if (ssl != null) { + try { + ssl.freeSSL(); + } catch (Exception e) { + /* ignore on purpose */ + } + } + if (cliSock != null) { + try { + cliSock.close(); + } catch (Exception e) { + /* ignore on purpose */ + } + } + if (srvFuture != null) { + try { + srvFuture.get(10, TimeUnit.SECONDS); + } catch (Exception e) { + /* ignore on purpose */ + } + } + if (es != null) { + es.shutdown(); + } + if (srvSocket != null) { + try { + srvSocket.close(); + } catch (Exception e) { + /* ignore on purpose */ + } + } + if (srvCtx != null) { + try { + srvCtx.free(); + } catch (Exception e) { + /* ignore on purpose */ + } + } + if (cliCtx != null) { + try { + cliCtx.free(); + } catch (Exception e) { + /* ignore on purpose */ + } + } + } + } + + @Test + public void test_WolfSSLSession_verifyCallbackExceptionFailsClosed() + throws Exception { + /* Throwing callback on the context must fail the handshake closed. */ + assertHandshakeFailsWithVerifyCallback(new WolfSSLVerifyCallback() { + @Override + public int verifyCallback(int preverify_ok, long storePtr) { + throw new RuntimeException("verify callback failure"); + } + }, true); + } + + @Test + public void test_WolfSSLSession_sslVerifyCallbackExceptionFailsClosed() + throws Exception { + /* Throwing callback on the session (the wolfJSSE path) must fail + * closed. */ + assertHandshakeFailsWithVerifyCallback(new WolfSSLVerifyCallback() { + @Override + public int verifyCallback(int preverify_ok, long storePtr) { + throw new RuntimeException("verify callback failure"); + } + }, false); + } + + @Test + public void test_WolfSSLSession_verifyCallbackNegativeReturnFailsClosed() + throws Exception { + /* Negative return on the context must fail closed, exercising the + * return normalization rather than the exception path. */ + assertHandshakeFailsWithVerifyCallback(new WolfSSLVerifyCallback() { + @Override + public int verifyCallback(int preverify_ok, long storePtr) { + return -1; + } + }, true); + } + + @Test + public void test_WolfSSLSession_sslVerifyCallbackNegativeReturnFailsClosed() + throws Exception { + /* Negative return on the session must fail closed, exercising the + * return normalization rather than the exception path. */ + assertHandshakeFailsWithVerifyCallback(new WolfSSLVerifyCallback() { + @Override + public int verifyCallback(int preverify_ok, long storePtr) { + return -1; + } + }, false); + } + @Test public void test_WolfSSLSession_getDhKeySizeBeforeHandshakeAndAfterFree() throws WolfSSLJNIException, WolfSSLException {