Skip to content

Commit 57f35cf

Browse files
authored
Merge pull request wolfSSL#360 from JeremiahM37/fenrir-fixes-3
WolfSSLEngine ByteBuffer offset/bounds fixes and JNI arrayOffset honoring
2 parents 2b49287 + 56810a8 commit 57f35cf

6 files changed

Lines changed: 216 additions & 13 deletions

File tree

native/com_wolfssl_WolfSSL.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ jmethodID g_bufferPositionMethodId = NULL;
6767
jmethodID g_bufferLimitMethodId = NULL;
6868
jmethodID g_bufferHasArrayMethodId = NULL;
6969
jmethodID g_bufferArrayMethodId = NULL;
70+
jmethodID g_bufferArrayOffsetMethodId = NULL;
7071
jmethodID g_bufferSetPositionMethodId = NULL;
7172
jmethodID g_verifyCallbackMethodId = NULL;
7273

@@ -185,6 +186,12 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void* reserved)
185186
return JNI_ERR;
186187
}
187188

189+
g_bufferArrayOffsetMethodId = (*env)->GetMethodID(env, byteBufferClass,
190+
"arrayOffset", "()I");
191+
if (g_bufferArrayOffsetMethodId == NULL) {
192+
return JNI_ERR;
193+
}
194+
188195
g_bufferSetPositionMethodId = (*env)->GetMethodID(env, byteBufferClass,
189196
"position", "(I)Ljava/nio/Buffer;");
190197
if (g_bufferSetPositionMethodId == NULL) {
@@ -236,6 +243,7 @@ JNIEXPORT void JNICALL JNI_OnUnload(JavaVM* vm, void* reserved)
236243
g_bufferLimitMethodId = NULL;
237244
g_bufferHasArrayMethodId = NULL;
238245
g_bufferArrayMethodId = NULL;
246+
g_bufferArrayOffsetMethodId = NULL;
239247
g_bufferSetPositionMethodId = NULL;
240248
g_verifyCallbackMethodId = NULL;
241249
}

native/com_wolfssl_WolfSSLSession.c

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1303,6 +1303,7 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLSession_write__JLjava_nio_ByteBuf
13031303
int ret = BAD_FUNC_ARG;
13041304
int maxInputSz;
13051305
int inSz = length;
1306+
int arrayOffset = 0;
13061307
byte* data = NULL;
13071308
WOLFSSL* ssl = (WOLFSSL*)(uintptr_t)sslPtr;
13081309
jbyteArray bufArr = NULL;
@@ -1333,6 +1334,15 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLSession_write__JLjava_nio_ByteBuf
13331334
return SSL_FAILURE;
13341335
}
13351336

1337+
/* Honor arrayOffset() for sliced/duplicated array-backed
1338+
* ByteBuffers, where logical position 0 maps to backing
1339+
* array index arrayOffset() */
1340+
arrayOffset = (int)(*jenv)->CallIntMethod(jenv, buf,
1341+
g_bufferArrayOffsetMethodId);
1342+
if ((*jenv)->ExceptionCheck(jenv)) {
1343+
return SSL_FAILURE;
1344+
}
1345+
13361346
/* Get array elements */
13371347
data = (byte *)(*jenv)->GetByteArrayElements(jenv, bufArr, NULL);
13381348
if (data == NULL) {
@@ -1356,8 +1366,8 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLSession_write__JLjava_nio_ByteBuf
13561366
}
13571367
}
13581368

1359-
ret = SSLWriteNonblockingWithSelectPoll(ssl, data + position,
1360-
(int)inSz, (int)timeout);
1369+
ret = SSLWriteNonblockingWithSelectPoll(ssl,
1370+
data + arrayOffset + position, (int)inSz, (int)timeout);
13611371

13621372
/* release memory if using array mode */
13631373
if (hasArray) {
@@ -1530,6 +1540,7 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLSession_read__JLjava_nio_ByteBuff
15301540
int size = 0;
15311541
int maxOutputSz;
15321542
int outSz = length;
1543+
int arrayOffset = 0;
15331544
byte* data = NULL;
15341545
WOLFSSL* ssl = (WOLFSSL*)(uintptr_t)sslPtr;
15351546
jbyteArray bufArr = NULL;
@@ -1560,6 +1571,15 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLSession_read__JLjava_nio_ByteBuff
15601571
return SSL_FAILURE;
15611572
}
15621573

1574+
/* Honor arrayOffset() for sliced/duplicated array-backed
1575+
* ByteBuffers, where logical position 0 maps to backing
1576+
* array index arrayOffset() */
1577+
arrayOffset = (int)(*jenv)->CallIntMethod(jenv, buf,
1578+
g_bufferArrayOffsetMethodId);
1579+
if ((*jenv)->ExceptionCheck(jenv)) {
1580+
return SSL_FAILURE;
1581+
}
1582+
15631583
/* Get array elements */
15641584
data = (byte *)(*jenv)->GetByteArrayElements(jenv, bufArr, NULL);
15651585
if (data == NULL) {
@@ -1583,8 +1603,8 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLSession_read__JLjava_nio_ByteBuff
15831603
}
15841604
}
15851605

1586-
size = SSLReadNonblockingWithSelectPoll(ssl, data + position,
1587-
outSz, (int)timeout);
1606+
size = SSLReadNonblockingWithSelectPoll(ssl,
1607+
data + arrayOffset + position, outSz, (int)timeout);
15881608

15891609
/* Release array elements if using array-backed buffer.
15901610
* Note: DirectByteBuffer doesn't need releasing data */

native/com_wolfssl_globals.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ extern jmethodID g_bufferPositionMethodId; /* ByteBuffer.position() */
4242
extern jmethodID g_bufferLimitMethodId; /* ByteBuffer.limit() */
4343
extern jmethodID g_bufferHasArrayMethodId; /* ByteBuffer.hasArray() */
4444
extern jmethodID g_bufferArrayMethodId; /* ByteBuffer.array() */
45+
extern jmethodID g_bufferArrayOffsetMethodId; /* ByteBuffer.arrayOffset() */
4546
extern jmethodID g_bufferSetPositionMethodId; /* ByteBuffer.position(int) */
4647
extern jmethodID g_verifyCallbackMethodId; /* WolfSSLVerifyCallback.verifyCallback */
4748

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -742,8 +742,8 @@ private synchronized int SendAppData(ByteBuffer[] in, int ofst, int len)
742742
/* Get total input data size, store input array positions */
743743
for (i = ofst; i < ofst + len; i++) {
744744
totalIn += in[i].remaining();
745-
pos[i] = in[i].position();
746-
limit[i] = in[i].limit();
745+
pos[i - ofst] = in[i].position();
746+
limit[i - ofst] = in[i].limit();
747747
}
748748

749749
/* Allocate static buffer for application data, clear before use */
@@ -765,7 +765,7 @@ private synchronized int SendAppData(ByteBuffer[] in, int ofst, int len)
765765
in[i].limit(in[i].position() + bufChunk); /* set limit */
766766
this.staticAppDataBuf.put(in[i]); /* get data */
767767
inputLeft -= bufChunk;
768-
in[i].limit(limit[i]); /* reset limit */
768+
in[i].limit(limit[i - ofst]); /* reset limit */
769769

770770
if (inputLeft == 0) {
771771
break; /* reached data size needed, stop reading */
@@ -786,7 +786,7 @@ private synchronized int SendAppData(ByteBuffer[] in, int ofst, int len)
786786
if (ret <= 0) {
787787
/* error, reset in[] positions for next call */
788788
for (i = ofst; i < ofst + len; i++) {
789-
in[i].position(pos[i]);
789+
in[i].position(pos[i - ofst]);
790790
}
791791
}
792792

@@ -828,7 +828,7 @@ public synchronized SSLEngineResult wrap(ByteBuffer[] in, int ofst, int len,
828828
throw new IndexOutOfBoundsException();
829829
}
830830

831-
for (i = ofst; i < len; ++i) {
831+
for (i = ofst; i < ofst + len; ++i) {
832832
if (in[i] == null) {
833833
throw new SSLException("SSLEngine.wrap() bad arguments");
834834
}
@@ -850,7 +850,7 @@ public synchronized SSLEngineResult wrap(ByteBuffer[] in, int ofst, int len,
850850
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
851851
() -> "setUseClientMode: " +
852852
this.engineHelper.getUseClientMode());
853-
for (i = 0; i < len; i++) {
853+
for (i = ofst; i < ofst + len; i++) {
854854
final int idx = i;
855855
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
856856
() -> "ByteBuffer in["+idx+"].remaining(): " +
@@ -1392,7 +1392,7 @@ public synchronized SSLEngineResult unwrap(ByteBuffer in, ByteBuffer[] out,
13921392
throw new IndexOutOfBoundsException();
13931393
}
13941394

1395-
for (i = ofst; i < length; ++i) {
1395+
for (i = ofst; i < ofst + length; ++i) {
13961396
if (out[i] == null) {
13971397
throw new IllegalArgumentException(
13981398
"SSLEngine.unwrap() bad arguments");
@@ -1434,7 +1434,7 @@ public synchronized SSLEngineResult unwrap(ByteBuffer in, ByteBuffer[] out,
14341434
() -> "in.position(): " + in.position());
14351435
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
14361436
() -> "in.limit(): " + in.limit());
1437-
for (i = 0; i < length; i++) {
1437+
for (i = ofst; i < ofst + length; i++) {
14381438
final int idx = i;
14391439
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
14401440
() -> "out["+idx+"].remaining(): " +
@@ -2093,7 +2093,7 @@ public synchronized void closeOutbound() {
20932093

20942094
/* If handshake has not started yet, close inBound as well */
20952095
if (needInit) {
2096-
inBoundOpen = true;
2096+
inBoundOpen = false;
20972097
}
20982098

20992099
/* Update status based on internal state. Some calling applications

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
import java.util.concurrent.CyclicBarrier;
7171
import java.util.concurrent.BrokenBarrierException;
7272
import java.util.concurrent.atomic.AtomicIntegerArray;
73+
import static org.junit.Assert.assertArrayEquals;
7374
import static org.junit.Assert.assertEquals;
7475
import static org.junit.Assert.assertNotNull;
7576
import static org.junit.Assert.fail;
@@ -3347,5 +3348,76 @@ public void testWrapPartialDrainOffsetUpdate()
33473348
fail("drained output does not match injected queue");
33483349
}
33493350
}
3351+
3352+
/* Regression: closeOutbound() before handshake must also close
3353+
* inbound, otherwise isInboundDone() never returns true. */
3354+
@Test
3355+
public void testCloseOutboundBeforeHandshake() throws Exception {
3356+
this.ctx = tf.createSSLContext("TLS", engineProvider);
3357+
SSLEngine e = this.ctx.createSSLEngine();
3358+
e.setUseClientMode(true);
3359+
e.closeOutbound();
3360+
assertTrue(e.isOutboundDone());
3361+
assertTrue(e.isInboundDone());
3362+
}
3363+
3364+
/* Regression for wrap(ByteBuffer[], ofst, len, out) when ofst > 0:
3365+
* pos[]/limit[] OOB and null-check loop bound. */
3366+
@Test
3367+
public void testWrapWithBufferArrayOffset() throws Exception {
3368+
this.ctx = tf.createSSLContext("TLS", engineProvider);
3369+
SSLEngine server = this.ctx.createSSLEngine();
3370+
SSLEngine client = this.ctx.createSSLEngine("wolfSSL test", 11111);
3371+
server.setUseClientMode(false);
3372+
client.setUseClientMode(true);
3373+
server.beginHandshake();
3374+
client.beginHandshake();
3375+
assertEquals(0, tf.testConnection(server, client, null, null, "x"));
3376+
3377+
byte[] payload = "real-payload".getBytes();
3378+
ByteBuffer[] in = {ByteBuffer.wrap("DECOY".getBytes()),
3379+
ByteBuffer.wrap(payload)};
3380+
ByteBuffer net = ByteBuffer.allocateDirect(
3381+
client.getSession().getPacketBufferSize());
3382+
3383+
SSLEngineResult r = client.wrap(in, 1, 1, net);
3384+
assertEquals(SSLEngineResult.Status.OK, r.getStatus());
3385+
assertEquals(0, in[0].position());
3386+
assertEquals(payload.length, in[1].position());
3387+
3388+
net.flip();
3389+
ByteBuffer plain = ByteBuffer.allocate(
3390+
server.getSession().getApplicationBufferSize());
3391+
assertEquals(SSLEngineResult.Status.OK,
3392+
server.unwrap(net, plain).getStatus());
3393+
plain.flip();
3394+
byte[] got = new byte[plain.remaining()];
3395+
plain.get(got);
3396+
assertArrayEquals(payload, got);
3397+
}
3398+
3399+
/* Direct regression: wrap() null-check must reach in[ofst+len-1]. */
3400+
@Test(expected = SSLException.class)
3401+
public void testWrapRejectsNullAtOffset() throws Exception {
3402+
this.ctx = tf.createSSLContext("TLS", engineProvider);
3403+
SSLEngine c = this.ctx.createSSLEngine("wolfSSL test", 11111);
3404+
c.setUseClientMode(true);
3405+
ByteBuffer[] in = {ByteBuffer.wrap("x".getBytes()), null};
3406+
c.wrap(in, 1, 1, ByteBuffer.allocateDirect(
3407+
c.getSession().getPacketBufferSize()));
3408+
}
3409+
3410+
/* Direct regression: unwrap() readOnly-check must reach
3411+
* out[ofst+length-1]. */
3412+
@Test(expected = java.nio.ReadOnlyBufferException.class)
3413+
public void testUnwrapRejectsReadOnlyAtOffset() throws Exception {
3414+
this.ctx = tf.createSSLContext("TLS", engineProvider);
3415+
SSLEngine s = this.ctx.createSSLEngine();
3416+
s.setUseClientMode(false);
3417+
ByteBuffer[] out = {ByteBuffer.allocate(64),
3418+
ByteBuffer.allocate(64).asReadOnlyBuffer()};
3419+
s.unwrap(ByteBuffer.allocateDirect(
3420+
s.getSession().getPacketBufferSize()), out, 1, 1);
3421+
}
33503422
}
33513423

src/test/com/wolfssl/test/WolfSSLSessionTest.java

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4210,5 +4210,107 @@ public void test_WolfSSLSession_dtlsCidDataExchangeAfterHandshake()
42104210
}
42114211
}
42124212
}
4213+
4214+
/* Regression: read(ByteBuffer) must honor arrayOffset() so a
4215+
* sliced array-backed buffer reads into backing[arrayOffset+pos),
4216+
* not backing[pos). */
4217+
@Test
4218+
public void test_WolfSSLSession_readSlicedByteBuffer() throws Exception {
4219+
final ServerSocket srvSocket = new ServerSocket(0);
4220+
final WolfSSLContext srvCtx = createAndSetupWolfSSLContext(
4221+
srvCert, srvKey, WolfSSL.SSL_FILETYPE_PEM, cliCert,
4222+
WolfSSL.SSLv23_ServerMethod());
4223+
WolfSSLContext cliCtx = createAndSetupWolfSSLContext(
4224+
cliCert, cliKey, WolfSSL.SSL_FILETYPE_PEM, caCert,
4225+
WolfSSL.SSLv23_ClientMethod());
4226+
final byte[] payload = "sliced-buf-payload".getBytes();
4227+
4228+
ExecutorService es = Executors.newSingleThreadExecutor();
4229+
Future<Void> srv = es.submit(() -> {
4230+
try (Socket s = srvSocket.accept()) {
4231+
WolfSSLSession ss = new WolfSSLSession(srvCtx);
4232+
ss.setFd(s);
4233+
int r;
4234+
int e;
4235+
do {
4236+
r = ss.accept();
4237+
e = ss.getError(r);
4238+
} while (r != WolfSSL.SSL_SUCCESS &&
4239+
(e == WolfSSL.SSL_ERROR_WANT_READ ||
4240+
e == WolfSSL.SSL_ERROR_WANT_WRITE));
4241+
ss.write(payload, payload.length, 0);
4242+
ss.shutdownSSL();
4243+
ss.freeSSL();
4244+
}
4245+
return null;
4246+
});
4247+
4248+
Socket cliSock = null;
4249+
WolfSSLSession cliSes = null;
4250+
try {
4251+
cliSock = new Socket(InetAddress.getLoopbackAddress(),
4252+
srvSocket.getLocalPort());
4253+
cliSes = new WolfSSLSession(cliCtx);
4254+
cliSes.setFd(cliSock);
4255+
int r;
4256+
int e;
4257+
do {
4258+
r = cliSes.connect();
4259+
e = cliSes.getError(r);
4260+
} while (r != WolfSSL.SSL_SUCCESS &&
4261+
(e == WolfSSL.SSL_ERROR_WANT_READ ||
4262+
e == WolfSSL.SSL_ERROR_WANT_WRITE));
4263+
4264+
int prefix = 64;
4265+
ByteBuffer parent = ByteBuffer.allocate(256);
4266+
byte[] backing = parent.array();
4267+
byte sentinel = (byte) 0xA5;
4268+
Arrays.fill(backing, sentinel);
4269+
parent.position(prefix);
4270+
ByteBuffer slice = parent.slice();
4271+
assertEquals(prefix, slice.arrayOffset());
4272+
4273+
int total = 0;
4274+
while (total < payload.length) {
4275+
int n = cliSes.read(slice, payload.length - total, 5000);
4276+
if (n > 0) {
4277+
total += n;
4278+
continue;
4279+
}
4280+
int err = cliSes.getError(n);
4281+
if (err == WolfSSL.SSL_ERROR_WANT_READ ||
4282+
err == WolfSSL.SSL_ERROR_WANT_WRITE) {
4283+
continue;
4284+
}
4285+
fail("cliSes.read() failed: ret=" + n + " err=" + err +
4286+
" total=" + total + "/" + payload.length);
4287+
}
4288+
4289+
for (int i = 0; i < prefix; i++) {
4290+
assertEquals("backing[" + i + "] corrupted",
4291+
sentinel, backing[i]);
4292+
}
4293+
assertArrayEquals(payload, Arrays.copyOfRange(backing,
4294+
prefix, prefix + payload.length));
4295+
assertEquals(payload.length, slice.position());
4296+
4297+
cliSes.shutdownSSL();
4298+
} finally {
4299+
try {
4300+
srv.get(10, TimeUnit.SECONDS);
4301+
} finally {
4302+
es.shutdownNow();
4303+
if (cliSes != null) {
4304+
cliSes.freeSSL();
4305+
}
4306+
if (cliSock != null) {
4307+
cliSock.close();
4308+
}
4309+
srvSocket.close();
4310+
cliCtx.free();
4311+
srvCtx.free();
4312+
}
4313+
}
4314+
}
42134315
}
42144316

0 commit comments

Comments
 (0)