Skip to content

Commit 615ab75

Browse files
peterdettmanpeterdettman
committed
Rewrite ProvSSLEngine.wrap to be record-aligned for app data
- avoids any internal buffering of app data b/w calls - never produce any output when returning BUFFER_OVERFLOW
1 parent f3fd8cf commit 615ab75

File tree

5 files changed

+142
-29
lines changed

5 files changed

+142
-29
lines changed

‎tls/src/main/java/org/bouncycastle/jsse/provider/ProvSSLEngine.java‎

Lines changed: 57 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ public synchronized SSLEngineResult unwrap(ByteBuffer src, ByteBuffer[] dsts, in
342342
{
343343
resultStatus = Status.BUFFER_UNDERFLOW;
344344
}
345-
else if (getTotalRemaining(dsts) < (long)preview.getApplicationDataLimit())
345+
else if (hasInsufficientSpace(dsts, offset, length, preview.getApplicationDataLimit()))
346346
{
347347
resultStatus = Status.BUFFER_OVERFLOW;
348348
}
@@ -357,7 +357,7 @@ else if (getTotalRemaining(dsts) < (long)preview.getApplicationDataLimit())
357357
int appDataAvailable = protocol.getAvailableInputBytes();
358358
for (int dstIndex = 0; dstIndex < length && appDataAvailable > 0; ++dstIndex)
359359
{
360-
ByteBuffer dst = dsts[dstIndex];
360+
ByteBuffer dst = dsts[offset + dstIndex];
361361
int count = Math.min(dst.remaining(), appDataAvailable);
362362
if (count > 0)
363363
{
@@ -469,36 +469,56 @@ public synchronized SSLEngineResult wrap(ByteBuffer[] srcs, int offset, int leng
469469
{
470470
resultStatus = Status.CLOSED;
471471
}
472-
else
472+
else if (protocol.getAvailableOutputBytes() > 0)
473473
{
474474
/*
475-
* Limit the app data that we will process in one call
475+
* Handle any buffered handshake data fully before sending application data.
476476
*/
477-
int srcLimit = ProvSSLSessionImpl.NULL_SESSION.getApplicationBufferSize();
478-
479-
for (int srcIndex = 0; srcIndex < length && srcLimit > 0; ++srcIndex)
477+
}
478+
else
479+
{
480+
try
480481
{
481-
ByteBuffer src = srcs[srcIndex];
482-
int count = Math.min(src.remaining(), srcLimit);
483-
if (count > 0)
482+
/*
483+
* Generate at most one maximum-sized application data record per call.
484+
*/
485+
int srcRemaining = getTotalRemaining(srcs, offset, length, protocol.getApplicationDataLimit());
486+
if (srcRemaining > 0)
484487
{
485-
byte[] input = new byte[count];
486-
src.get(input);
487-
488-
try
488+
RecordPreview preview = protocol.previewOutputRecord(srcRemaining);
489+
490+
int srcLimit = preview.getApplicationDataLimit();
491+
int dstLimit = preview.getRecordSize();
492+
493+
if (dst.remaining() < dstLimit)
489494
{
490-
protocol.writeApplicationData(input, 0, count);
495+
resultStatus = Status.BUFFER_OVERFLOW;
491496
}
492-
catch (IOException e)
497+
else
493498
{
494-
// TODO[jsse] Throw a subclass of SSLException?
495-
throw new SSLException(e);
499+
for (int srcIndex = 0; srcIndex < length && srcLimit > 0; ++srcIndex)
500+
{
501+
ByteBuffer src = srcs[offset + srcIndex];
502+
int count = Math.min(src.remaining(), srcLimit);
503+
if (count > 0)
504+
{
505+
byte[] input = new byte[count];
506+
src.get(input);
507+
508+
protocol.writeApplicationData(input, 0, count);
509+
510+
bytesConsumed += count;
511+
srcLimit -= count;
512+
}
513+
}
496514
}
497-
498-
bytesConsumed += count;
499-
srcLimit -= count;
500515
}
501516
}
517+
catch (IOException e)
518+
{
519+
// TODO[jsse] Throw a subclass of SSLException?
520+
throw new SSLException(e);
521+
}
502522
}
503523
}
504524

@@ -520,8 +540,7 @@ public synchronized SSLEngineResult wrap(ByteBuffer[] srcs, int offset, int leng
520540
bytesProduced += count;
521541
outputAvailable -= count;
522542
}
523-
524-
if (outputAvailable > 0)
543+
else
525544
{
526545
resultStatus = Status.BUFFER_OVERFLOW;
527546
}
@@ -628,13 +647,24 @@ private RecordPreview getRecordPreview(ByteBuffer src)
628647
return protocol.previewInputRecord(recordHeader);
629648
}
630649

631-
private long getTotalRemaining(ByteBuffer[] buffers)
650+
private int getTotalRemaining(ByteBuffer[] bufs, int off, int len, int limit)
632651
{
633-
long result = 0;
634-
for (ByteBuffer buffer : buffers)
652+
int result = 0;
653+
for (int i = 0; i < len; ++i)
635654
{
636-
result += buffer.remaining();
655+
ByteBuffer buf = bufs[off + i];
656+
int next = buf.remaining();
657+
if (next >= (limit - result))
658+
{
659+
return limit;
660+
}
661+
result += next;
637662
}
638663
return result;
639664
}
665+
666+
private boolean hasInsufficientSpace(ByteBuffer[] dsts, int off, int len, int amount)
667+
{
668+
return getTotalRemaining(dsts, off, len, amount) < amount;
669+
}
640670
}

‎tls/src/main/java/org/bouncycastle/jsse/provider/ProvSSLSessionImpl.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,12 @@ public int getPacketBufferSize()
179179
* when the max_fragment_length extension has been negotiated, or when no compression was negotiated).
180180
*/
181181
// Header size + Fragment length limit + Compression expansion + Cipher expansion
182-
return RecordFormat.FRAGMENT_OFFSET + (1 << 14) + 1024 + 1024;
182+
// return RecordFormat.FRAGMENT_OFFSET + (1 << 14) + 1024 + 1024;
183+
184+
/*
185+
* Worst case accounts for possible application data splitting (before TLS 1.1)
186+
*/
187+
return (1 << 14) + 1 + 2 * (RecordFormat.FRAGMENT_OFFSET + 1024 + 1024);
183188
}
184189

185190
public javax.security.cert.X509Certificate[] getPeerCertificateChain() throws SSLPeerUnverifiedException

‎tls/src/main/java/org/bouncycastle/tls/RecordPreview.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ public final class RecordPreview
55
private final int recordSize;
66
private final int applicationDataLimit;
77

8+
static RecordPreview combine(RecordPreview a, RecordPreview b)
9+
{
10+
return new RecordPreview(
11+
a.getRecordSize() + b.getRecordSize(),
12+
a.getApplicationDataLimit() + b.getApplicationDataLimit());
13+
}
14+
815
RecordPreview(int recordSize, int applicationDataLimit)
916
{
1017
this.recordSize = recordSize;

‎tls/src/main/java/org/bouncycastle/tls/RecordStream.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,21 @@ else if (!version.equals(readVersion))
192192
return new RecordPreview(recordSize, applicationDataLimit);
193193
}
194194

195+
RecordPreview previewOutputRecord(int applicationDataSize)
196+
{
197+
int applicationDataLimit = Math.max(0, Math.min(getPlaintextLimit(), applicationDataSize));
198+
199+
int recordSize = applicationDataLimit;
200+
if (writeCompression.getClass() != TlsNullCompression.class)
201+
{
202+
recordSize += 1024;
203+
}
204+
205+
recordSize = writeCipher.getCiphertextLimit(recordSize) + RecordFormat.FRAGMENT_OFFSET;
206+
207+
return new RecordPreview(recordSize, applicationDataLimit);
208+
}
209+
195210
boolean readFullRecord(byte[] record)
196211
throws IOException
197212
{

‎tls/src/main/java/org/bouncycastle/tls/TlsProtocol.java‎

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -840,7 +840,7 @@ public RecordPreview previewInputRecord(byte[] recordHeader) throws IOException
840840
{
841841
if (blocking)
842842
{
843-
throw new IllegalStateException("Cannot use previewInputRecord() in blocking mode! Use getInputStream() instead.");
843+
throw new IllegalStateException("Cannot use previewInputRecord() in blocking mode!");
844844
}
845845
if (inputBuffers.available() != 0)
846846
{
@@ -855,6 +855,57 @@ public RecordPreview previewInputRecord(byte[] recordHeader) throws IOException
855855
return safePreviewRecordHeader(recordHeader);
856856
}
857857

858+
public RecordPreview previewOutputRecord(int applicationDataSize) throws IOException
859+
{
860+
if (blocking)
861+
{
862+
throw new IllegalStateException("Cannot use previewOutputRecord() in blocking mode!");
863+
}
864+
if (outputBuffer.getBuffer().available() != 0)
865+
{
866+
throw new IllegalStateException("Can only use previewOutputRecord() for record-aligned output.");
867+
}
868+
869+
if (closed)
870+
{
871+
throw new IOException("Connection is closed, cannot produce any more output");
872+
}
873+
874+
if (applicationDataSize < 1)
875+
{
876+
return new RecordPreview(0, 0);
877+
}
878+
879+
if (this.appDataSplitEnabled)
880+
{
881+
switch (appDataSplitMode)
882+
{
883+
case ADS_MODE_0_N_FIRSTONLY:
884+
case ADS_MODE_0_N:
885+
{
886+
RecordPreview a = recordStream.previewOutputRecord(0);
887+
RecordPreview b = recordStream.previewOutputRecord(applicationDataSize);
888+
return RecordPreview.combine(a, b);
889+
}
890+
case ADS_MODE_1_Nsub1:
891+
default:
892+
{
893+
RecordPreview a = recordStream.previewOutputRecord(1);
894+
if (applicationDataSize > 1)
895+
{
896+
RecordPreview b = recordStream.previewOutputRecord(applicationDataSize - 1);
897+
a = RecordPreview.combine(a, b);
898+
}
899+
return a;
900+
}
901+
}
902+
}
903+
else
904+
{
905+
return recordStream.previewOutputRecord(applicationDataSize);
906+
}
907+
}
908+
858909
/**
859910
* Offer input from an arbitrary source. Only allowed in non-blocking mode.<br>
860911
* <br>
@@ -928,6 +979,11 @@ public void offerInput(byte[] input) throws IOException
928979
}
929980
}
930981

982+
public int getApplicationDataLimit()
983+
{
984+
return recordStream.getPlaintextLimit();
985+
}
986+
931987
/**
932988
* Gets the amount of received application data. A call to {@link #readInput(byte[], int, int)}
933989
* is guaranteed to be able to return at least this much data.<br>

0 commit comments

Comments
 (0)