Fix possible IOOBE when calling ReferenceCountedSslEngine.unwrap(...) with heap buffers.
Motivation:
fc3c9c9523
introduced a bug which will have ReferenceCountedSslEngine.unwrap(...) produce an IOOBE when be called with an BŷteBuffer as src that contains multiple SSLRecords and has a position != 0.
Modification:
- Correctly set the limit on the ByteBuffer and so fix the IOOBE.
- Add test-case to verify the fix
Result:
Correctly handle heap buffers as well.
This commit is contained in:
parent
b701da8d1c
commit
821501717b
@ -422,12 +422,14 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
|
|||||||
final ByteBuf buf = alloc.directBuffer(len);
|
final ByteBuf buf = alloc.directBuffer(len);
|
||||||
try {
|
try {
|
||||||
final long addr = memoryAddress(buf);
|
final long addr = memoryAddress(buf);
|
||||||
|
|
||||||
|
// Set the limit depending on the length of the allocated buffer and restore the original
|
||||||
|
// after we copied bytes.
|
||||||
|
int limit = src.limit();
|
||||||
int newLimit = pos + len;
|
int newLimit = pos + len;
|
||||||
if (newLimit != src.remaining()) {
|
src.limit(newLimit);
|
||||||
buf.setBytes(0, (ByteBuffer) src.duplicate().position(pos).limit(newLimit));
|
|
||||||
} else {
|
|
||||||
buf.setBytes(0, src);
|
buf.setBytes(0, src);
|
||||||
}
|
src.limit(limit);
|
||||||
|
|
||||||
netWrote = SSL.writeToBIO(networkBIO, addr, len);
|
netWrote = SSL.writeToBIO(networkBIO, addr, len);
|
||||||
if (netWrote >= 0) {
|
if (netWrote >= 0) {
|
||||||
|
@ -1323,4 +1323,79 @@ public abstract class SSLEngineTest {
|
|||||||
cleanupServerSslEngine(server);
|
cleanupServerSslEngine(server);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testMultipleRecordsInOneBufferWithNonZeroPosition() throws Exception {
|
||||||
|
SelfSignedCertificate cert = new SelfSignedCertificate();
|
||||||
|
|
||||||
|
clientSslCtx = SslContextBuilder
|
||||||
|
.forClient()
|
||||||
|
.trustManager(cert.cert())
|
||||||
|
.sslProvider(sslClientProvider())
|
||||||
|
.build();
|
||||||
|
SSLEngine client = clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT);
|
||||||
|
|
||||||
|
serverSslCtx = SslContextBuilder
|
||||||
|
.forServer(cert.certificate(), cert.privateKey())
|
||||||
|
.sslProvider(sslServerProvider())
|
||||||
|
.build();
|
||||||
|
SSLEngine server = serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT);
|
||||||
|
|
||||||
|
try {
|
||||||
|
// Choose buffer size small enough that we can put multiple buffers into one buffer and pass it into the
|
||||||
|
// unwrap call without exceed MAX_ENCRYPTED_PACKET_LENGTH.
|
||||||
|
ByteBuffer plainClientOut = ByteBuffer.allocate(1024);
|
||||||
|
ByteBuffer plainServerOut = ByteBuffer.allocate(server.getSession().getApplicationBufferSize());
|
||||||
|
|
||||||
|
ByteBuffer encClientToServer = ByteBuffer.allocate(client.getSession().getPacketBufferSize());
|
||||||
|
|
||||||
|
int positionOffset = 1;
|
||||||
|
// We need to be able to hold 2 records + positionOffset
|
||||||
|
ByteBuffer combinedEncClientToServer = ByteBuffer.allocate(
|
||||||
|
encClientToServer.capacity() * 2 + positionOffset);
|
||||||
|
combinedEncClientToServer.position(positionOffset);
|
||||||
|
|
||||||
|
handshake(client, server);
|
||||||
|
|
||||||
|
plainClientOut.limit(plainClientOut.capacity());
|
||||||
|
SSLEngineResult result = client.wrap(plainClientOut, encClientToServer);
|
||||||
|
assertEquals(plainClientOut.capacity(), result.bytesConsumed());
|
||||||
|
assertTrue(result.bytesProduced() > 0);
|
||||||
|
|
||||||
|
encClientToServer.flip();
|
||||||
|
|
||||||
|
// Copy the first record into the combined buffer
|
||||||
|
combinedEncClientToServer.put(encClientToServer);
|
||||||
|
|
||||||
|
plainClientOut.clear();
|
||||||
|
encClientToServer.clear();
|
||||||
|
|
||||||
|
result = client.wrap(plainClientOut, encClientToServer);
|
||||||
|
assertEquals(plainClientOut.capacity(), result.bytesConsumed());
|
||||||
|
assertTrue(result.bytesProduced() > 0);
|
||||||
|
|
||||||
|
encClientToServer.flip();
|
||||||
|
|
||||||
|
int encClientToServerLen = encClientToServer.remaining();
|
||||||
|
|
||||||
|
// Copy the first record into the combined buffer
|
||||||
|
combinedEncClientToServer.put(encClientToServer);
|
||||||
|
|
||||||
|
encClientToServer.clear();
|
||||||
|
|
||||||
|
combinedEncClientToServer.flip();
|
||||||
|
combinedEncClientToServer.position(positionOffset);
|
||||||
|
|
||||||
|
// Ensure we have the first record and a tiny amount of the second record in the buffer
|
||||||
|
combinedEncClientToServer.limit(
|
||||||
|
combinedEncClientToServer.limit() - (encClientToServerLen - positionOffset));
|
||||||
|
result = server.unwrap(combinedEncClientToServer, plainServerOut);
|
||||||
|
assertEquals(encClientToServerLen, result.bytesConsumed());
|
||||||
|
assertTrue(result.bytesProduced() > 0);
|
||||||
|
} finally {
|
||||||
|
cert.delete();
|
||||||
|
cleanupClientSslEngine(client);
|
||||||
|
cleanupServerSslEngine(server);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user