Skip to content

Commit dbd89ba

Browse files
cconloncconlon
authored
Merge pull request #352 from rlm2002/sunJSSE_ServerName
 unwrap() header peek in SSLEngine to fix BUFFER_UNDERFLOW
2 parents 162cf78 + 099a55a commit dbd89ba

File tree

4 files changed

+409
-63
lines changed

4 files changed

+409
-63
lines changed

‎native/com_wolfssl_WolfSSL.h‎

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

‎src/java/com/wolfssl/WolfSSL.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,23 @@ public enum TLS_VERSION {
349349
/** Maximum SSL record size (16KB) as defined by the protocol. */
350350
public static final int MAX_RECORD_SIZE = 16384;
351351

352+
/** TLS record header is: type(1) + version(2) + length(2) */
353+
public static final int TLS_RECORD_HEADER_LEN = 5;
354+
/** TLS record header length high byte offset */
355+
public static final int TLS_RECORD_LEN_HI_OFF = 3;
356+
/** TLS record header length lower byte offset */
357+
public static final int TLS_RECORD_LEN_LO_OFF = 4;
358+
/** TLS record header protocol version offset */
359+
public static final int TLS_RECORD_VERS_OFF = 1;
360+
/** TLS record header protocol version major */
361+
public static final int TLS_RECORD_VERS_MAJOR = 0x03;
362+
/** TLS record header valid content type
363+
* (RFC 8446 B.1, RFC 6520), (change_cipher_spec) */
364+
public static final int TLS_RECORD_CT_MIN = 20;
365+
/** TLS record header valid content type
366+
* (RFC 8446 B.1, RFC 6520), (heartbeat) */
367+
public static final int TLS_RECORD_CT_MAX = 24;
368+
352369
/* ------------------ TLS extension specific ------------------------ */
353370
/** SNI Host name type, for UseSNI() */
354371
public static final int WOLFSSL_SNI_HOST_NAME = 0;

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

Lines changed: 97 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,8 @@ public class WolfSSLEngine extends SSLEngine {
110110

111111
/* session stored (WOLFSSL_SESSION), relevant on client side */
112112
private boolean sessionStored = false;
113+
/* Skip record header peek in next unwrap, continue same TLS record */
114+
private boolean contPartialRecord = false;
113115

114116
/* TLS 1.3 session ticket received (on client side) */
115117
private boolean sessionTicketReceived = false;
@@ -142,11 +144,6 @@ public class WolfSSLEngine extends SSLEngine {
142144
* and expanded only when a larger output window requires it. */
143145
private byte[] recvAppDataBuf = new byte[WolfSSL.MAX_RECORD_SIZE];
144146

145-
/* TLS record header is: type(1) + version(2) + length(2) */
146-
private static final int TLS_RECORD_HEADER_LEN = 5;
147-
private static final int TLS_RECORD_LEN_HI_OFF = 3;
148-
private static final int TLS_RECORD_LEN_LO_OFF = 4;
149-
150147
/* Default size of internalIOSendBuf, 16k to match TLS record size.
151148
* TODO - add upper bound on I/O send buf resize allocations. */
152149
private static final int INTERNAL_IOSEND_BUF_SZ = WolfSSL.MAX_RECORD_SIZE;
@@ -365,7 +362,7 @@ private List<SNIServerName> parseRequestedServerNamesFromNetData() {
365362

366363
synchronized (netDataLock) {
367364
if (this.netData == null ||
368-
this.netData.remaining() < TLS_RECORD_HEADER_LEN) {
365+
this.netData.remaining() < WolfSSL.TLS_RECORD_HEADER_LEN) {
369366
return null;
370367
}
371368
in = this.netData.asReadOnlyBuffer();
@@ -1289,6 +1286,70 @@ private byte[] getRecvAppDataBuf(int minSz) {
12891286
return this.recvAppDataBuf;
12901287
}
12911288

1289+
/**
1290+
* Peek at the record header and return BUFFER_UNDERFLOW before calling
1291+
* into JNI when the full record is not yet present. Without this, native
1292+
* wolfSSL consumes partial record bytes via the I/O callback, violating
1293+
* the JSSE contract that BUFFER_UNDERFLOW must report
1294+
* bytesConsumed() == 0.
1295+
*
1296+
* Skipped if using DTLS.
1297+
*
1298+
* @param in input buffer.
1299+
* @param inRemaining bytes remaining in input buffer.
1300+
*
1301+
* @return true if buffer underflow detected, false otherwise.
1302+
*/
1303+
private boolean peekTlsRecordHeader(ByteBuffer in, int inRemaining) {
1304+
boolean bufferUnderflow = false;
1305+
/* DTLS still relies on the native WANT_READ path. */
1306+
if (inRemaining > 0 && (this.ssl.dtls() == 0)) {
1307+
int pos = in.position();
1308+
if (inRemaining < WolfSSL.TLS_RECORD_HEADER_LEN) {
1309+
/* Not enough for TLS record header */
1310+
bufferUnderflow = true;
1311+
}
1312+
else {
1313+
/* Check if header is plausible. Content type between
1314+
* change_cipher_spec and heartbeat and
1315+
* version major corresponds to TLS. */
1316+
byte headerByte = in.get(pos);
1317+
if ((headerByte & 0xFF) >= WolfSSL.TLS_RECORD_CT_MIN &&
1318+
(headerByte & 0xFF) <= WolfSSL.TLS_RECORD_CT_MAX &&
1319+
(in.get(pos + WolfSSL.TLS_RECORD_VERS_OFF)
1320+
& 0xFF) == WolfSSL.TLS_RECORD_VERS_MAJOR) {
1321+
/* Peek at record length from header
1322+
* bytes 3-4 (big-endian). */
1323+
int recLen =
1324+
((in.get(pos + WolfSSL.TLS_RECORD_LEN_HI_OFF)
1325+
& 0xFF) << 8) |
1326+
(in.get(pos + WolfSSL.TLS_RECORD_LEN_LO_OFF)
1327+
& 0xFF);
1328+
/* Fall through if recLen exceeds max packet buffer size
1329+
* or inRemaining is less than reported record length */
1330+
int packetBufSz = this.getSession().getPacketBufferSize();
1331+
if (recLen <= packetBufSz &&
1332+
inRemaining < WolfSSL.TLS_RECORD_HEADER_LEN + recLen) {
1333+
bufferUnderflow = true;
1334+
}
1335+
else if (recLen > packetBufSz) {
1336+
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
1337+
() -> "recLen: " + recLen +
1338+
" exceeds max packet buffer size, " +
1339+
"skipping underflow check.");
1340+
}
1341+
}
1342+
else {
1343+
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
1344+
() -> "Peeked content-type or version " +
1345+
"major outside TLS range, skipping " +
1346+
"underflow check");
1347+
}
1348+
}
1349+
}
1350+
return bufferUnderflow;
1351+
}
1352+
12921353
@Override
12931354
public synchronized SSLEngineResult unwrap(ByteBuffer in, ByteBuffer out)
12941355
throws SSLException {
@@ -1313,6 +1374,7 @@ public synchronized SSLEngineResult unwrap(ByteBuffer in, ByteBuffer[] out,
13131374
long dtlsPrevDropCount = 0;
13141375
long dtlsCurrDropCount = 0;
13151376
int prevSessionTicketCount = 0;
1377+
boolean bufferUnderflow = false;
13161378
final int tmpRet;
13171379

13181380
/* Set initial status for SSLEngineResult return */
@@ -1463,38 +1525,26 @@ else if (hs == SSLEngineResult.HandshakeStatus.NEED_WRAP &&
14631525
if (this.handshakeFinished == false) {
14641526
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
14651527
() -> "starting or continuing handshake");
1466-
ret = DoHandshake(false);
1528+
if (!this.contPartialRecord) {
1529+
/* Client only: TLS 1.3 ssl_accept() I/O callback
1530+
* patterns conflict with the pre-peek
1531+
* mid-handshake. Post-handshake server unwrap
1532+
* always applies peek. */
1533+
bufferUnderflow =
1534+
this.getUseClientMode() &&
1535+
peekTlsRecordHeader(in, inRemaining);
1536+
}
1537+
if (bufferUnderflow) {
1538+
status = SSLEngineResult.Status.BUFFER_UNDERFLOW;
1539+
}
1540+
else {
1541+
ret = DoHandshake(false);
1542+
}
14671543
}
14681544
else {
1469-
/* TLS-only: peek at the record header and
1470-
* return BUFFER_UNDERFLOW before calling into
1471-
* JNI when the full record is not yet present.
1472-
* Without this, native wolfSSL consumes partial
1473-
* record bytes via the I/O callback, violating
1474-
* the JSSE contract that BUFFER_UNDERFLOW must
1475-
* report bytesConsumed() == 0. DTLS still
1476-
* relies on the native WANT_READ path. */
1477-
boolean bufferUnderflow = false;
1478-
if (inRemaining > 0 && (this.ssl.dtls() == 0)) {
1479-
synchronized (netDataLock) {
1480-
int pos = in.position();
1481-
if (inRemaining < TLS_RECORD_HEADER_LEN) {
1482-
/* Not enough for TLS record header */
1483-
bufferUnderflow = true;
1484-
} else {
1485-
/* Peek at record length from header
1486-
* bytes 3-4 (big-endian) */
1487-
int recLen =
1488-
((in.get(pos + TLS_RECORD_LEN_HI_OFF)
1489-
& 0xFF) << 8) |
1490-
(in.get(pos + TLS_RECORD_LEN_LO_OFF)
1491-
& 0xFF);
1492-
if (inRemaining <
1493-
TLS_RECORD_HEADER_LEN + recLen) {
1494-
bufferUnderflow = true;
1495-
}
1496-
}
1497-
}
1545+
if (!this.contPartialRecord) {
1546+
bufferUnderflow =
1547+
peekTlsRecordHeader(in, inRemaining);
14981548
}
14991549

15001550
/* Serve stashed data from previous
@@ -1697,7 +1747,8 @@ else if (ret < 0 &&
16971747
}
16981748
}
16991749
else if (!this.handshakeFinished && (ret == 0) &&
1700-
(err == 0 || err == WolfSSL.SSL_ERROR_ZERO_RETURN)) {
1750+
(err == 0 || err == WolfSSL.SSL_ERROR_ZERO_RETURN)
1751+
&& !bufferUnderflow) {
17011752

17021753
boolean gotCloseNotify = false;
17031754
synchronized (ioLock) {
@@ -2650,6 +2701,11 @@ protected synchronized int internalRecvCb(ByteBuffer toRead, int sz) {
26502701
if (this.ssl.dtls() == 1 && this.handshakeFinished) {
26512702
this.nativeWantsToRead = 1;
26522703
}
2704+
/* Buffer exhausted mid-record: skip the header peek on the
2705+
* next unwrap() to pass continuation bytes to DoHandshake. */
2706+
if (this.netData != null && this.netData.remaining() == 0) {
2707+
this.contPartialRecord = true;
2708+
}
26532709
return WolfSSL.WOLFSSL_CBIO_ERR_WANT_READ;
26542710
}
26552711

@@ -2676,6 +2732,10 @@ protected synchronized int internalRecvCb(ByteBuffer toRead, int sz) {
26762732
toRead.position(toReadPos);
26772733
}
26782734

2735+
/* Reset to false when all requested bytes were delivered;
2736+
* set to true when the buffer was exhausted mid-record. */
2737+
this.contPartialRecord = (max < sz);
2738+
26792739
return max;
26802740
}
26812741
}

0 commit comments

Comments
 (0)