Skip to content

Commit 1bea4be

Browse files
authored
Merge pull request #365 from JeremiahM37/fenrir-fixes-4
unwrap bounds, CBIO error returns, key zeroization
2 parents 0c92ec0 + 10f4df4 commit 1bea4be

10 files changed

Lines changed: 196 additions & 23 deletions

native/com_wolfssl_WolfSSLCRL.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,14 +368,15 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLCRL_X509_1CRL_1sign
368368
const WOLFSSL_EVP_MD* md = NULL;
369369
unsigned char* rsaPrivBuf = NULL;
370370
const char* mdName = NULL;
371+
jboolean isCopy = JNI_FALSE;
371372
int ret = WOLFSSL_SUCCESS;
372373
(void)jcl;
373374

374375
if (jenv == NULL || crl == NULL || keyBytes == NULL) {
375376
return WOLFSSL_FAILURE;
376377
}
377378

378-
keyBuf = (byte*)(*jenv)->GetByteArrayElements(jenv, keyBytes, NULL);
379+
keyBuf = (byte*)(*jenv)->GetByteArrayElements(jenv, keyBytes, &isCopy);
379380
if ((*jenv)->ExceptionOccurred(jenv)) {
380381
(*jenv)->ExceptionClear(jenv);
381382
return WOLFSSL_FAILURE;
@@ -471,6 +472,15 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLCRL_X509_1CRL_1sign
471472
(*jenv)->ReleaseStringUTFChars(jenv, digestAlg, mdName);
472473
}
473474
if (keyBuf != NULL) {
475+
/* Only zero if JVM made a copy */
476+
if (isCopy == JNI_TRUE) {
477+
#if (LIBWOLFSSL_VERSION_HEX >= 0x05008004) && \
478+
!defined(WOLFSSL_NO_FORCE_ZERO)
479+
wc_ForceZero(keyBuf, keySz);
480+
#else
481+
XMEMSET(keyBuf, 0, keySz);
482+
#endif
483+
}
474484
(*jenv)->ReleaseByteArrayElements(jenv, keyBytes, (jbyte*)keyBuf,
475485
JNI_ABORT);
476486
}

native/com_wolfssl_WolfSSLCertRequest.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLCertRequest_X509_1REQ_1sign
247247
const WOLFSSL_EVP_MD* md = NULL;
248248
unsigned char* rsaPrivBuf = NULL;
249249
const char* mdName = NULL;
250+
jboolean isCopy = JNI_FALSE;
250251
int ret = WOLFSSL_SUCCESS;
251252
(void)jcl;
252253

@@ -255,7 +256,7 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLCertRequest_X509_1REQ_1sign
255256
return WOLFSSL_FAILURE;
256257
}
257258

258-
keyBuf = (byte*)(*jenv)->GetByteArrayElements(jenv, keyBytes, NULL);
259+
keyBuf = (byte*)(*jenv)->GetByteArrayElements(jenv, keyBytes, &isCopy);
259260
keySz = (*jenv)->GetArrayLength(jenv, keyBytes);
260261

261262
if (keyBuf == NULL || keySz == 0) {
@@ -343,6 +344,15 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLCertRequest_X509_1REQ_1sign
343344
XFREE(derBuf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
344345
}
345346
if (keyBuf != NULL) {
347+
/* Only zero if JVM made a copy */
348+
if (isCopy == JNI_TRUE) {
349+
#if (LIBWOLFSSL_VERSION_HEX >= 0x05008004) && \
350+
!defined(WOLFSSL_NO_FORCE_ZERO)
351+
wc_ForceZero(keyBuf, keySz);
352+
#else
353+
XMEMSET(keyBuf, 0, keySz);
354+
#endif
355+
}
346356
(*jenv)->ReleaseByteArrayElements(jenv, keyBytes,
347357
(jbyte*)keyBuf, JNI_ABORT);
348358
}

native/com_wolfssl_WolfSSLCertificate.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -888,6 +888,7 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLCertificate_X509_1sign
888888
const WOLFSSL_EVP_MD* md = NULL;
889889
unsigned char* rsaPrivBuf = NULL;
890890
const char* mdName = NULL;
891+
jboolean isCopy = JNI_FALSE;
891892

892893
int ret = WOLFSSL_SUCCESS;
893894
(void)jcl;
@@ -897,7 +898,7 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLCertificate_X509_1sign
897898
return WOLFSSL_FAILURE;
898899
}
899900

900-
fileBuf = (byte*)(*jenv)->GetByteArrayElements(jenv, fileBytes, NULL);
901+
fileBuf = (byte*)(*jenv)->GetByteArrayElements(jenv, fileBytes, &isCopy);
901902
fileSz = (*jenv)->GetArrayLength(jenv, fileBytes);
902903

903904
if (fileBuf == NULL || fileSz == 0) {
@@ -990,6 +991,15 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLCertificate_X509_1sign
990991
XFREE(derBuf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
991992
}
992993
if (fileBuf != NULL) {
994+
/* Only zero if JVM made a copy */
995+
if (isCopy == JNI_TRUE) {
996+
#if (LIBWOLFSSL_VERSION_HEX >= 0x05008004) && \
997+
!defined(WOLFSSL_NO_FORCE_ZERO)
998+
wc_ForceZero(fileBuf, fileSz);
999+
#else
1000+
XMEMSET(fileBuf, 0, fileSz);
1001+
#endif
1002+
}
9931003
(*jenv)->ReleaseByteArrayElements(jenv, fileBytes,
9941004
(jbyte*)fileBuf, JNI_ABORT);
9951005
}

native/com_wolfssl_WolfSSLContext.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,7 +1117,7 @@ int NativeIORecvCb(WOLFSSL *ssl, char *buf, int sz, void *ctx)
11171117
"NativeIORecvCb");
11181118
if (needsDetach)
11191119
(*g_vm)->DetachCurrentThread(g_vm);
1120-
return 0;
1120+
return WOLFSSL_CBIO_ERR_GENERAL;
11211121
}
11221122

11231123
/* lookup WolfSSLSession class from object */
@@ -1329,7 +1329,7 @@ int NativeIOSendCb(WOLFSSL *ssl, char *buf, int sz, void *ctx)
13291329
"NativeIOSendCb");
13301330
if (needsDetach)
13311331
(*g_vm)->DetachCurrentThread(g_vm);
1332-
return 0;
1332+
return WOLFSSL_CBIO_ERR_GENERAL;
13331333
}
13341334

13351335
/* lookup WolfSSLSession class from object */

native/com_wolfssl_WolfSSLSession.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6613,7 +6613,7 @@ int NativeSSLIORecvCb(WOLFSSL *ssl, char *buf, int sz, void *ctx)
66136613
if (needsDetach) {
66146614
(*g_vm)->DetachCurrentThread(g_vm);
66156615
}
6616-
return 0;
6616+
return WOLFSSL_CBIO_ERR_GENERAL;
66176617
}
66186618

66196619
/* Detect if we should use ByteBuffer or byte[] I/O callbacks */
@@ -6789,7 +6789,7 @@ int NativeSSLIOSendCb(WOLFSSL *ssl, char *buf, int sz, void *ctx)
67896789
if (needsDetach) {
67906790
(*g_vm)->DetachCurrentThread(g_vm);
67916791
}
6792-
return 0;
6792+
return WOLFSSL_CBIO_ERR_GENERAL;
67936793
}
67946794

67956795
/* Detect if we should use ByteBuffer or byte[] I/O callbacks */

src/java/com/wolfssl/WolfSSLCRL.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.nio.charset.StandardCharsets;
2929
import java.text.ParseException;
3030
import java.text.SimpleDateFormat;
31+
import java.util.Arrays;
3132
import java.util.Calendar;
3233
import java.util.Date;
3334
import java.util.Locale;
@@ -561,9 +562,13 @@ else if (key instanceof ECPrivateKey) {
561562
"PrivateKey does not support encoding");
562563
}
563564

564-
synchronized (crlLock) {
565-
ret = X509_CRL_sign(this.crlPtr, evpKeyType, encodedKey,
566-
WolfSSL.SSL_FILETYPE_ASN1, digestAlg);
565+
try {
566+
synchronized (crlLock) {
567+
ret = X509_CRL_sign(this.crlPtr, evpKeyType, encodedKey,
568+
WolfSSL.SSL_FILETYPE_ASN1, digestAlg);
569+
}
570+
} finally {
571+
Arrays.fill(encodedKey, (byte)0);
567572
}
568573

569574
return ret;

src/java/com/wolfssl/WolfSSLCertRequest.java

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.io.File;
2424
import java.io.IOException;
2525
import java.nio.charset.Charset;
26+
import java.util.Arrays;
2627
import java.security.PublicKey;
2728
import java.security.PrivateKey;
2829
import java.security.interfaces.RSAPublicKey;
@@ -634,7 +635,11 @@ public void signRequest(String filePath, int keyType, int format,
634635
"Failed to read bytes from file: " + filePath);
635636
}
636637

637-
signRequest(fileBytes, keyType, format, digestAlg);
638+
try {
639+
signRequest(fileBytes, keyType, format, digestAlg);
640+
} finally {
641+
Arrays.fill(fileBytes, (byte)0);
642+
}
638643
}
639644

640645
/**
@@ -751,9 +756,13 @@ else if (key instanceof ECPrivateKey) {
751756
throw new WolfSSLException("PrivateKey does not support encoding");
752757
}
753758

754-
synchronized (x509ReqLock) {
755-
ret = X509_REQ_sign(this.x509ReqPtr, evpKeyType, encodedKey,
756-
WolfSSL.SSL_FILETYPE_ASN1, digestAlg);
759+
try {
760+
synchronized (x509ReqLock) {
761+
ret = X509_REQ_sign(this.x509ReqPtr, evpKeyType, encodedKey,
762+
WolfSSL.SSL_FILETYPE_ASN1, digestAlg);
763+
}
764+
} finally {
765+
Arrays.fill(encodedKey, (byte)0);
757766
}
758767

759768
if (ret != WolfSSL.SSL_SUCCESS) {

src/java/com/wolfssl/WolfSSLCertificate.java

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,7 +1345,11 @@ public void signCert(String filePath, int keyType, int format,
13451345
"Failed to read bytes from file: " + filePath);
13461346
}
13471347

1348-
signCert(fileBytes, keyType, format, digestAlg);
1348+
try {
1349+
signCert(fileBytes, keyType, format, digestAlg);
1350+
} finally {
1351+
Arrays.fill(fileBytes, (byte)0);
1352+
}
13491353
}
13501354

13511355
/**
@@ -1461,9 +1465,13 @@ else if (key instanceof ECPrivateKey) {
14611465
throw new WolfSSLException("PrivateKey does not support encoding");
14621466
}
14631467

1464-
synchronized (x509Lock) {
1465-
ret = X509_sign(this.x509Ptr, evpKeyType, encodedKey,
1466-
WolfSSL.SSL_FILETYPE_ASN1, digestAlg);
1468+
try {
1469+
synchronized (x509Lock) {
1470+
ret = X509_sign(this.x509Ptr, evpKeyType, encodedKey,
1471+
WolfSSL.SSL_FILETYPE_ASN1, digestAlg);
1472+
}
1473+
} finally {
1474+
Arrays.fill(encodedKey, (byte)0);
14671475
}
14681476

14691477
if (ret != WolfSSL.SSL_SUCCESS) {

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,7 +1276,7 @@ private synchronized int RecvAppData(ByteBuffer[] out, int ofst, int length)
12761276

12771277
/* Copy from intermediate buffer to output bufs */
12781278
for (i = 0; i < ret;) {
1279-
if (idx + ofst >= length) {
1279+
if (idx >= length) {
12801280
/* no more output buffers left */
12811281
break;
12821282
}
@@ -1585,9 +1585,9 @@ else if (hs == SSLEngineResult.HandshakeStatus.NEED_WRAP &&
15851585
if (outputSpace >= this.pendingAppDataLen) {
15861586
/* Serve stashed data to output buffers */
15871587
int idx2 = 0;
1588-
for (int pos = 0;
1589-
pos < this.pendingAppDataLen;) {
1590-
if (idx2 + ofst >= length) break;
1588+
int pos = 0;
1589+
for (; pos < this.pendingAppDataLen;) {
1590+
if (idx2 >= length) break;
15911591
int space = out[idx2 + ofst].remaining();
15921592
if (space == 0) { idx2++; continue; }
15931593
int sz2 = Math.min(space,
@@ -1597,7 +1597,7 @@ else if (hs == SSLEngineResult.HandshakeStatus.NEED_WRAP &&
15971597
pos += sz2;
15981598
if (pos < this.pendingAppDataLen) idx2++;
15991599
}
1600-
produced += this.pendingAppDataLen;
1600+
produced += pos;
16011601
/* Advance past previously consumed bytes */
16021602
synchronized (netDataLock) {
16031603
in.position(in.position() +

src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3420,5 +3420,126 @@ public void testUnwrapRejectsReadOnlyAtOffset() throws Exception {
34203420
s.unwrap(ByteBuffer.allocateDirect(
34213421
s.getSession().getPacketBufferSize()), out, 1, 1);
34223422
}
3423+
3424+
/* Verify unwrap() with ofst > 0 placed all `data` bytes into
3425+
* out[2] and out[3], and that bytesProduced matches. Regression
3426+
* for the `idx + ofst >= length` copy-loop bound in unwrap. */
3427+
private void assertOffsetUnwrapOk(SSLEngineResult result,
3428+
ByteBuffer[] outArr, byte[] data) {
3429+
3430+
assertEquals(data.length, result.bytesProduced());
3431+
assertEquals(0, outArr[0].position());
3432+
assertEquals(0, outArr[1].position());
3433+
3434+
outArr[2].flip();
3435+
outArr[3].flip();
3436+
int n2 = outArr[2].remaining();
3437+
int n3 = outArr[3].remaining();
3438+
assertEquals(data.length, n2 + n3);
3439+
3440+
byte[] got = new byte[n2 + n3];
3441+
outArr[2].get(got, 0, n2);
3442+
outArr[3].get(got, n2, n3);
3443+
assertTrue(java.util.Arrays.equals(got, data));
3444+
}
3445+
3446+
/* Wrap `data` from client and return server's network buffer
3447+
* positioned for unwrap(). */
3448+
private ByteBuffer wrapForServer(SSLEngine client, byte[] data)
3449+
throws SSLException {
3450+
3451+
ByteBuffer netBuf = ByteBuffer.allocateDirect(
3452+
client.getSession().getPacketBufferSize());
3453+
SSLEngineResult r = client.wrap(ByteBuffer.wrap(data), netBuf);
3454+
assertEquals(SSLEngineResult.Status.OK, r.getStatus());
3455+
netBuf.flip();
3456+
return netBuf;
3457+
}
3458+
3459+
@Test
3460+
public void testUnwrapOffsetMultipleBuffers() throws Exception {
3461+
3462+
this.ctx = tf.createSSLContext("TLS", engineProvider);
3463+
SSLEngine server = this.ctx.createSSLEngine();
3464+
SSLEngine client = this.ctx.createSSLEngine("wolfSSL test", 11111);
3465+
server.setUseClientMode(false);
3466+
client.setUseClientMode(true);
3467+
assertEquals(0, tf.testConnection(server, client, null, null, "x"));
3468+
3469+
byte[] data = new byte[1024];
3470+
new Random().nextBytes(data);
3471+
ByteBuffer netBuf = wrapForServer(client, data);
3472+
3473+
ByteBuffer[] outArr = new ByteBuffer[] {
3474+
ByteBuffer.allocate(600), ByteBuffer.allocate(600),
3475+
ByteBuffer.allocate(600), ByteBuffer.allocate(600)
3476+
};
3477+
SSLEngineResult r = server.unwrap(netBuf, outArr, 2, 2);
3478+
assertEquals(SSLEngineResult.Status.OK, r.getStatus());
3479+
assertOffsetUnwrapOk(r, outArr, data);
3480+
}
3481+
3482+
@Test
3483+
public void testUnwrapPendingAppDataWithOffset() throws Exception {
3484+
3485+
this.ctx = tf.createSSLContext("TLS", engineProvider);
3486+
SSLEngine server = this.ctx.createSSLEngine();
3487+
SSLEngine client = this.ctx.createSSLEngine("wolfSSL test", 11111);
3488+
server.setUseClientMode(false);
3489+
client.setUseClientMode(true);
3490+
assertEquals(0, tf.testConnection(server, client, null, null, "x"));
3491+
3492+
byte[] data = new byte[1024];
3493+
new Random().nextBytes(data);
3494+
ByteBuffer netBuf = wrapForServer(client, data);
3495+
3496+
/* First unwrap into too-small output stashes pendingAppData */
3497+
SSLEngineResult r = server.unwrap(netBuf, ByteBuffer.allocate(64));
3498+
assertEquals(SSLEngineResult.Status.BUFFER_OVERFLOW, r.getStatus());
3499+
3500+
ByteBuffer[] outArr = new ByteBuffer[] {
3501+
ByteBuffer.allocate(600), ByteBuffer.allocate(600),
3502+
ByteBuffer.allocate(600), ByteBuffer.allocate(600)
3503+
};
3504+
r = server.unwrap(netBuf, outArr, 2, 2);
3505+
assertOffsetUnwrapOk(r, outArr, data);
3506+
}
3507+
3508+
@Test
3509+
public void testUnwrapPendingAppDataReStashWithOffset()
3510+
throws Exception {
3511+
3512+
this.ctx = tf.createSSLContext("TLS", engineProvider);
3513+
SSLEngine server = this.ctx.createSSLEngine();
3514+
SSLEngine client = this.ctx.createSSLEngine("wolfSSL test", 11111);
3515+
server.setUseClientMode(false);
3516+
client.setUseClientMode(true);
3517+
assertEquals(0, tf.testConnection(server, client, null, null, "x"));
3518+
3519+
byte[] data = new byte[1024];
3520+
new Random().nextBytes(data);
3521+
ByteBuffer netBuf = wrapForServer(client, data);
3522+
3523+
/* First unwrap stashes pendingAppData */
3524+
SSLEngineResult r = server.unwrap(netBuf, ByteBuffer.allocate(64));
3525+
assertEquals(SSLEngineResult.Status.BUFFER_OVERFLOW, r.getStatus());
3526+
3527+
/* Second unwrap: ofst > 0 but total still too small;
3528+
* pendingAppData must survive intact for a later call */
3529+
ByteBuffer[] tooSmall = new ByteBuffer[] {
3530+
ByteBuffer.allocate(200), ByteBuffer.allocate(200),
3531+
ByteBuffer.allocate(200), ByteBuffer.allocate(200)
3532+
};
3533+
r = server.unwrap(netBuf, tooSmall, 2, 2);
3534+
assertEquals(SSLEngineResult.Status.BUFFER_OVERFLOW, r.getStatus());
3535+
3536+
/* Third unwrap: ofst > 0 with room drains the stash */
3537+
ByteBuffer[] outArr = new ByteBuffer[] {
3538+
ByteBuffer.allocate(600), ByteBuffer.allocate(600),
3539+
ByteBuffer.allocate(600), ByteBuffer.allocate(600)
3540+
};
3541+
r = server.unwrap(netBuf, outArr, 2, 2);
3542+
assertOffsetUnwrapOk(r, outArr, data);
3543+
}
34233544
}
34243545

0 commit comments

Comments
 (0)