Skip to content

Commit 96d8d5c

Browse files
committed
Address PR wolfSSL#334 review feedback
- Add native JNI wrapper for wolfSSL_SNI_GetFromBuffer(), switch SSLEngine SNI extraction to use it with Java fallback for NOT_COMPILED_IN builds - Add clearPendingAppData() helper, clear stale pending app data on unwrap() exception paths, closeInbound(), and closeOutbound() - Reset pendingNetConsumed when stashing new decrypted data - Move getBytes() length validation before allocation - Add TLS-only clarifying comment on record-header underflow check - Detect peer closing connection during handshake (SSLHandshakeException) - Store verifyException on hostname verification failure paths - Add debug logging in setNativeTimeout catch block - Move word32 dropCount declarations before (void) casts for C89 - Re-interrupt thread on InterruptedException in session context tests - Add restoreClientSessionCacheProperty() test helper
1 parent dd6aab3 commit 96d8d5c

8 files changed

Lines changed: 196 additions & 33 deletions

File tree

native/com_wolfssl_WolfSSL.c

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2625,3 +2625,80 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSL_getErrno
26252625
return 0;
26262626
#endif
26272627
}
2628+
2629+
JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSL_getSNIFromBuffer
2630+
(JNIEnv* jenv, jclass jcl, jbyteArray clientHello, jint helloSz,
2631+
jbyte type, jbyteArray sni, jintArray inOutSz)
2632+
{
2633+
int ret = BAD_FUNC_ARG;
2634+
#if defined(HAVE_SNI) && !defined(NO_WOLFSSL_SERVER)
2635+
jbyte* clientHelloBuf = NULL;
2636+
jbyte* sniBuf = NULL;
2637+
jint* sizeBuf = NULL;
2638+
jsize clientHelloLen = 0;
2639+
jsize sniLen = 0;
2640+
jsize sizeLen = 0;
2641+
unsigned int sniOutSz = 0;
2642+
2643+
(void)jcl;
2644+
2645+
if (jenv == NULL || clientHello == NULL || sni == NULL ||
2646+
inOutSz == NULL || helloSz < 0) {
2647+
return ret;
2648+
}
2649+
2650+
clientHelloLen = (*jenv)->GetArrayLength(jenv, clientHello);
2651+
sniLen = (*jenv)->GetArrayLength(jenv, sni);
2652+
sizeLen = (*jenv)->GetArrayLength(jenv, inOutSz);
2653+
if (helloSz > clientHelloLen || sizeLen < 1) {
2654+
return ret;
2655+
}
2656+
2657+
sizeBuf = (*jenv)->GetIntArrayElements(jenv, inOutSz, NULL);
2658+
if (sizeBuf == NULL) {
2659+
return MEMORY_E;
2660+
}
2661+
2662+
if (sizeBuf[0] < 0 || sizeBuf[0] > sniLen) {
2663+
(*jenv)->ReleaseIntArrayElements(jenv, inOutSz, sizeBuf, JNI_ABORT);
2664+
return ret;
2665+
}
2666+
sniOutSz = (unsigned int)sizeBuf[0];
2667+
2668+
clientHelloBuf = (*jenv)->GetByteArrayElements(jenv, clientHello, NULL);
2669+
if (clientHelloBuf == NULL) {
2670+
(*jenv)->ReleaseIntArrayElements(jenv, inOutSz, sizeBuf, JNI_ABORT);
2671+
return MEMORY_E;
2672+
}
2673+
2674+
sniBuf = (*jenv)->GetByteArrayElements(jenv, sni, NULL);
2675+
if (sniBuf == NULL) {
2676+
(*jenv)->ReleaseByteArrayElements(jenv, clientHello, clientHelloBuf,
2677+
JNI_ABORT);
2678+
(*jenv)->ReleaseIntArrayElements(jenv, inOutSz, sizeBuf, JNI_ABORT);
2679+
return MEMORY_E;
2680+
}
2681+
2682+
ret = wolfSSL_SNI_GetFromBuffer((const unsigned char*)clientHelloBuf,
2683+
(unsigned int)helloSz,
2684+
(unsigned char)type,
2685+
(unsigned char*)sniBuf, &sniOutSz);
2686+
2687+
sizeBuf[0] = (jint)sniOutSz;
2688+
(*jenv)->ReleaseByteArrayElements(jenv, sni, sniBuf, 0);
2689+
(*jenv)->ReleaseByteArrayElements(jenv, clientHello, clientHelloBuf,
2690+
JNI_ABORT);
2691+
(*jenv)->ReleaseIntArrayElements(jenv, inOutSz, sizeBuf, 0);
2692+
#else
2693+
(void)jenv;
2694+
(void)jcl;
2695+
(void)clientHello;
2696+
(void)helloSz;
2697+
(void)type;
2698+
(void)sni;
2699+
(void)inOutSz;
2700+
ret = NOT_COMPILED_IN;
2701+
#endif
2702+
2703+
return ret;
2704+
}

native/com_wolfssl_WolfSSL.h

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

native/com_wolfssl_WolfSSLSession.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2761,11 +2761,11 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLSession_sendHrrCookie
27612761
JNIEXPORT jlong JNICALL Java_com_wolfssl_WolfSSLSession_getDtlsMacDropCount
27622762
(JNIEnv* jenv, jobject jcl, jlong sslPtr)
27632763
{
2764+
word32 dropCount = 0;
2765+
27642766
(void)jenv;
27652767
(void)jcl;
27662768
(void)sslPtr;
2767-
2768-
word32 dropCount = 0;
27692769
#if defined(WOLFSSL_DTLS) && defined(WOLFSSL_DTLS_DROP_STATS)
27702770
int ret = 0;
27712771
WOLFSSL* ssl = (WOLFSSL*)(uintptr_t)sslPtr;
@@ -2782,11 +2782,11 @@ JNIEXPORT jlong JNICALL Java_com_wolfssl_WolfSSLSession_getDtlsMacDropCount
27822782
JNIEXPORT jlong JNICALL Java_com_wolfssl_WolfSSLSession_getDtlsReplayDropCount
27832783
(JNIEnv* jenv, jobject jcl, jlong sslPtr)
27842784
{
2785+
word32 dropCount = 0;
2786+
27852787
(void)jenv;
27862788
(void)jcl;
27872789
(void)sslPtr;
2788-
2789-
word32 dropCount = 0;
27902790
#if defined(WOLFSSL_DTLS) && defined(WOLFSSL_DTLS_DROP_STATS)
27912791
int ret = 0;
27922792
WOLFSSL* ssl = (WOLFSSL*)(uintptr_t)sslPtr;

src/java/com/wolfssl/WolfSSL.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1850,6 +1850,25 @@ public static int cryptoCbUnRegisterDevice(int devId) {
18501850
*/
18511851
public static native int getErrno();
18521852

1853+
/**
1854+
* Extract a requested SNI value from a raw ClientHello buffer.
1855+
*
1856+
* This delegates parsing to native wolfSSL, which already handles the
1857+
* protocol framing and bounds checks.
1858+
*
1859+
* @param clientHello raw ClientHello/TLS record bytes
1860+
* @param helloSz number of bytes in {@code clientHello} to parse
1861+
* @param type SNI type to fetch, for example {@code WOLFSSL_SNI_HOST_NAME}
1862+
* @param sni output buffer for the SNI value
1863+
* @param inOutSz input/output size array of length 1. On input this is the
1864+
* available size of {@code sni}; on success it is updated with the
1865+
* number of bytes written.
1866+
* @return {@code SSL_SUCCESS} on success, otherwise a native wolfSSL error
1867+
* code or {@code NOT_COMPILED_IN} when unavailable
1868+
*/
1869+
public static native int getSNIFromBuffer(byte[] clientHello, int helloSz,
1870+
byte type, byte[] sni, int[] inOutSz);
1871+
18531872
/**
18541873
* Gets the internal wolfSSL named group enum matching provided string.
18551874
*

src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -771,6 +771,10 @@ protected void updateTimeouts(int in, int side) {
771771
} catch (IllegalStateException e) {
772772
/* Native WolfSSLSession has been freed,
773773
* invalidate this session entry */
774+
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
775+
() -> "Native session freed while updating " +
776+
"timeout, invalidating cache entry: " +
777+
e.getMessage());
774778
current.invalidate();
775779
}
776780
}

src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -338,16 +338,38 @@ protected synchronized void cacheRequestedServerNamesFromNetData() {
338338

339339
private List<SNIServerName> parseRequestedServerNamesFromNetData() {
340340
ByteBuffer in;
341+
byte[] clientHello;
342+
byte[] sni;
343+
int[] sniSz;
344+
int ret;
345+
List<SNIServerName> names;
341346

342347
synchronized (netDataLock) {
343348
if (this.netData == null || this.netData.remaining() < 5) {
344349
return null;
345350
}
346351
in = this.netData.asReadOnlyBuffer();
352+
clientHello = new byte[in.remaining()];
353+
in.get(clientHello);
347354
}
348355

349356
try {
350-
return parseRequestedServerNamesFromTlsRecord(in);
357+
sni = new byte[clientHello.length];
358+
sniSz = new int[] { sni.length };
359+
ret = WolfSSL.getSNIFromBuffer(clientHello, clientHello.length,
360+
(byte)WolfSSL.WOLFSSL_SNI_HOST_NAME, sni, sniSz);
361+
if (ret == WolfSSL.SSL_SUCCESS && sniSz[0] > 0 &&
362+
sniSz[0] <= sni.length) {
363+
364+
names = new ArrayList<SNIServerName>(1);
365+
names.add(new SNIHostName(Arrays.copyOf(sni, sniSz[0])));
366+
return names;
367+
}
368+
369+
if (ret == WolfSSL.NOT_COMPILED_IN) {
370+
return parseRequestedServerNamesFromTlsRecord(
371+
ByteBuffer.wrap(clientHello));
372+
}
351373
} catch (IllegalArgumentException e) {
352374
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
353375
() -> "Unable to parse SNI from pending input: " +
@@ -526,16 +548,24 @@ private static ByteBuffer sliceBytes(ByteBuffer in, int len) {
526548
}
527549

528550
private static byte[] getBytes(ByteBuffer in, int len) {
529-
byte[] out = new byte[len];
530-
531551
if (len < 0 || len > in.remaining()) {
532552
throw new IllegalArgumentException("invalid TLS length");
533553
}
534554

555+
byte[] out = new byte[len];
535556
in.get(out);
536557
return out;
537558
}
538559

560+
private void clearPendingAppData() {
561+
if (this.pendingAppData != null) {
562+
Arrays.fill(this.pendingAppData, (byte)0);
563+
this.pendingAppData = null;
564+
}
565+
this.pendingAppDataLen = 0;
566+
this.pendingNetConsumed = 0;
567+
}
568+
539569
/**
540570
* Register I/O callbacks and contexts with WolfSSLSession and native
541571
* wolfSSL. Uses singleton pattern on callbacks.
@@ -1345,6 +1375,7 @@ private synchronized int RecvAppData(ByteBuffer[] out, int ofst, int length)
13451375
this.pendingAppData = new byte[ret];
13461376
System.arraycopy(tmp, 0, this.pendingAppData, 0, ret);
13471377
this.pendingAppDataLen = ret;
1378+
this.pendingNetConsumed = 0;
13481379
return 0; /* 0 bytes written to output */
13491380
}
13501381

@@ -1546,8 +1577,10 @@ else if (hs == SSLEngineResult.HandshakeStatus.NEED_WRAP &&
15461577
ret = DoHandshake(false);
15471578
}
15481579
else {
1549-
/* Check TLS record header for complete
1550-
* record, return BUFFER_UNDERFLOW if not */
1580+
/* TLS-only fast path: peek at the record header and
1581+
* return BUFFER_UNDERFLOW before a JNI read when the
1582+
* full record is not present. DTLS still relies on the
1583+
* native WANT_READ path as the correctness fallback. */
15511584
boolean bufferUnderflow = false;
15521585
if (inRemaining > 0 && (this.ssl.dtls() == 0)) {
15531586
synchronized (netDataLock) {
@@ -1599,10 +1632,7 @@ else if (hs == SSLEngineResult.HandshakeStatus.NEED_WRAP &&
15991632
in.position(in.position() +
16001633
this.pendingNetConsumed);
16011634
}
1602-
Arrays.fill(this.pendingAppData, (byte) 0);
1603-
this.pendingAppData = null;
1604-
this.pendingAppDataLen = 0;
1605-
this.pendingNetConsumed = 0;
1635+
clearPendingAppData();
16061636
}
16071637
else {
16081638
/* Still not enough output space */
@@ -1776,6 +1806,21 @@ else if (ret < 0 &&
17761806
err);
17771807
}
17781808
}
1809+
else if (!this.handshakeFinished && (ret == 0) &&
1810+
(err == 0 || err == WolfSSL.SSL_ERROR_ZERO_RETURN)) {
1811+
1812+
boolean gotCloseNotify = false;
1813+
synchronized (ioLock) {
1814+
gotCloseNotify = this.ssl.gotCloseNotify();
1815+
}
1816+
1817+
if (!gotCloseNotify && !this.closeNotifyReceived) {
1818+
this.inBoundOpen = false;
1819+
this.outBoundOpen = false;
1820+
throw new SSLHandshakeException(
1821+
"Peer closed connection during handshake");
1822+
}
1823+
}
17791824

17801825
/* Get DTLS drop count after data has been processed. To be
17811826
* used below when setting BUFFER_UNDERFLOW status. */
@@ -1908,6 +1953,9 @@ else if (ret < 0 &&
19081953

19091954
return new SSLEngineResult(status, hs, consumed, produced);
19101955

1956+
} catch (SSLException | RuntimeException e) {
1957+
clearPendingAppData();
1958+
throw e;
19111959
} finally {
19121960
try {
19131961
/* Don't hold references to this SSLEngine object in
@@ -2060,6 +2108,8 @@ public synchronized void closeInbound() throws SSLException {
20602108
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
20612109
() -> "entered closeInbound");
20622110

2111+
clearPendingAppData();
2112+
20632113
if (!inBoundOpen)
20642114
return;
20652115

@@ -2084,6 +2134,8 @@ public synchronized boolean isInboundDone() {
20842134
public synchronized void closeOutbound() {
20852135
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
20862136
() -> "entered closeOutbound, outBoundOpen = false");
2137+
2138+
clearPendingAppData();
20872139
outBoundOpen = false;
20882140

20892141
/* If handshake has not started yet, close inBound as well */

src/java/com/wolfssl/provider/jsse/WolfSSLInternalVerifyCb.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ else if (this.callingEngine != null &&
172172
"passed, no endpoint identification configured)");
173173
}
174174
} catch (CertificateException e) {
175+
this.verifyException = e;
175176
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
176177
() -> "X509ExtendedTrustManager hostname verification " +
177178
"failed: " + e.getMessage());
@@ -269,6 +270,8 @@ else if (this.callingSocket != null &&
269270
}
270271

271272
if (peerHost == null || peerHost.isEmpty()) {
273+
this.verifyException = new CertificateException(
274+
"No peer host available for hostname verification");
272275
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
273276
() -> "No peer host available for hostname verification");
274277
/* No peer host to verify against, fail verification since

0 commit comments

Comments
 (0)