diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java b/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java index be6cee75..523442f4 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java @@ -110,6 +110,8 @@ public class WolfSSLEngine extends SSLEngine { /* session stored (WOLFSSL_SESSION), relevant on client side */ private boolean sessionStored = false; + /* Skip record header peek in next unwrap, continue same TLS record */ + private boolean contPartialRecord = false; /* TLS 1.3 session ticket received (on client side) */ private boolean sessionTicketReceived = false; @@ -1289,6 +1291,47 @@ private byte[] getRecvAppDataBuf(int minSz) { return this.recvAppDataBuf; } + /** + * TLS-only: peek at the record header and + * return BUFFER_UNDERFLOW before calling into + * JNI when the full record is not yet present. + * Without this, native wolfSSL consumes partial + * record bytes via the I/O callback, violating + * the JSSE contract that BUFFER_UNDERFLOW must + * report bytesConsumed() == 0. DTLS still + * relies on the native WANT_READ path. + * + * @param in input buffer + * @param inRemaining bytes remaining in input buffer. + * @param handshaking whether the function is being called during + * handshake. + * + * @return true if buffer underflow detected, false otherwise + */ + private boolean peekTlsRecordHeader(ByteBuffer in, int inRemaining) { + boolean bufferUnderflow = false; + if (inRemaining > 0 && (this.ssl.dtls() == 0)) { + int pos = in.position(); + if (inRemaining < TLS_RECORD_HEADER_LEN) { + /* Not enough for TLS record header */ + bufferUnderflow = true; + } else { + /* Peek at record length from header + * bytes 3-4 (big-endian) */ + int recLen = + ((in.get(pos + TLS_RECORD_LEN_HI_OFF) + & 0xFF) << 8) | + (in.get(pos + TLS_RECORD_LEN_LO_OFF) + & 0xFF); + if (recLen <= WolfSSL.MAX_RECORD_SIZE && + inRemaining < TLS_RECORD_HEADER_LEN + recLen) { + bufferUnderflow = true; + } + } + } + return bufferUnderflow; + } + @Override public synchronized SSLEngineResult unwrap(ByteBuffer in, ByteBuffer out) throws SSLException { @@ -1313,6 +1356,7 @@ public synchronized SSLEngineResult unwrap(ByteBuffer in, ByteBuffer[] out, long dtlsPrevDropCount = 0; long dtlsCurrDropCount = 0; int prevSessionTicketCount = 0; + boolean bufferUnderflow = false; final int tmpRet; /* Set initial status for SSLEngineResult return */ @@ -1463,38 +1507,22 @@ else if (hs == SSLEngineResult.HandshakeStatus.NEED_WRAP && if (this.handshakeFinished == false) { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, () -> "starting or continuing handshake"); - ret = DoHandshake(false); + if (!this.contPartialRecord) { + bufferUnderflow = + this.getUseClientMode() && + peekTlsRecordHeader(in, inRemaining); + } + if (bufferUnderflow) { + status = SSLEngineResult.Status.BUFFER_UNDERFLOW; + } + else { + ret = DoHandshake(false); + } } else { - /* TLS-only: peek at the record header and - * return BUFFER_UNDERFLOW before calling into - * JNI when the full record is not yet present. - * Without this, native wolfSSL consumes partial - * record bytes via the I/O callback, violating - * the JSSE contract that BUFFER_UNDERFLOW must - * report bytesConsumed() == 0. DTLS still - * relies on the native WANT_READ path. */ - boolean bufferUnderflow = false; - if (inRemaining > 0 && (this.ssl.dtls() == 0)) { - synchronized (netDataLock) { - int pos = in.position(); - if (inRemaining < TLS_RECORD_HEADER_LEN) { - /* Not enough for TLS record header */ - bufferUnderflow = true; - } else { - /* Peek at record length from header - * bytes 3-4 (big-endian) */ - int recLen = - ((in.get(pos + TLS_RECORD_LEN_HI_OFF) - & 0xFF) << 8) | - (in.get(pos + TLS_RECORD_LEN_LO_OFF) - & 0xFF); - if (inRemaining < - TLS_RECORD_HEADER_LEN + recLen) { - bufferUnderflow = true; - } - } - } + if (!this.contPartialRecord) { + bufferUnderflow = + peekTlsRecordHeader(in, inRemaining); } /* Serve stashed data from previous @@ -1629,8 +1657,8 @@ else if (inRemaining > 0 && } } /* end DoHandshake() / RecvAppData() */ - if (outBoundOpen == false || this.closeNotifySent || - this.closeNotifyReceived) { + if ((outBoundOpen == false || this.closeNotifySent || + this.closeNotifyReceived) && !bufferUnderflow) { /* Mark SSLEngine status as CLOSED */ status = SSLEngineResult.Status.CLOSED; /* Handshake has finished and SSLEngine is closed, @@ -1697,7 +1725,8 @@ else if (ret < 0 && } } else if (!this.handshakeFinished && (ret == 0) && - (err == 0 || err == WolfSSL.SSL_ERROR_ZERO_RETURN)) { + (err == 0 || err == WolfSSL.SSL_ERROR_ZERO_RETURN) + && !bufferUnderflow) { boolean gotCloseNotify = false; synchronized (ioLock) { @@ -2650,6 +2679,16 @@ protected synchronized int internalRecvCb(ByteBuffer toRead, int sz) { if (this.ssl.dtls() == 1 && this.handshakeFinished) { this.nativeWantsToRead = 1; } + /* wolfSSL asked for bytes but we have none remaining + * (netData is valid but empty). Mark as mid-record so + * the next unwrap() skips the TLS header peek and + * passes the continuation bytes directly to DoHandshake. + * Only set when netData is non-null (netData==null means + * we are inside wrap(), not a mid-record scenario). + * Prevents stale state from being used. */ + if (this.netData != null) { + this.contPartialRecord = true; + } return WolfSSL.WOLFSSL_CBIO_ERR_WANT_READ; } @@ -2676,6 +2715,8 @@ protected synchronized int internalRecvCb(ByteBuffer toRead, int sz) { toRead.position(toReadPos); } + this.contPartialRecord = (max < sz); + return max; } } diff --git a/src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java b/src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java index fdc99a2c..95d9764d 100644 --- a/src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java +++ b/src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java @@ -3047,13 +3047,13 @@ public void testBufferUnderflowPartialRecord() } @Test - public void testHandshakeUnwrapConsumedNotBufferUnderflow() + public void testHandshakeUnwrapIncrementalStatus() throws NoSuchProviderException, NoSuchAlgorithmException, KeyManagementException, KeyStoreException, CertificateException, IOException, UnrecoverableKeyException { - System.out.print("\tTest unwrap consumed != BUFFER_UNDERFLOW"); + System.out.print("\tTest unwrap incremental status"); if (!enabledProtocols.contains("TLSv1.3")) { System.out.println("\t... skipped"); @@ -3127,42 +3127,108 @@ public void testHandshakeUnwrapConsumedNotBufferUnderflow() srvFlight.flip(); int total = srvFlight.remaining(); - if (total < 2) { + /* Parse first TLS record header to find exact boundary: + * type[1] + version[2] + length[2] = 5 bytes. */ + if (total < 5) { error("\t... failed"); - fail("server flight too small to split: " + total); + fail("server flight too small: " + total); + } + int firstRecPayload = + ((srvFlight.get(srvFlight.position() + 3) & 0xFF) << 8) + | (srvFlight.get(srvFlight.position() + 4) & 0xFF); + int firstRecordLen = 5 + firstRecPayload; + if (total < firstRecordLen) { + error("\t... failed"); + fail("first TLS record exceeds server flight"); } - /* Feed the first half of the server flight to - * client.unwrap(). wolfSSL will consume these bytes through - * the I/O callback, exhaust the buffer, and return - * SSL_ERROR_WANT_READ. With the fix, BUFFER_UNDERFLOW must - * NOT be set because inRemaining > 0 (data was provided). */ - int half = total / 2; - byte[] halfBytes = new byte[half]; - srvFlight.get(halfBytes); - ByteBuffer firstHalf = ByteBuffer.wrap(halfBytes); + /* Feed exactly the first TLS record to client.unwrap(). + * Peek confirms it is complete, DoHandshake() runs, + * wolfSSL consumes all bytes and returns WANT_READ. */ + byte[] firstRecBytes = new byte[firstRecordLen]; + srvFlight.get(firstRecBytes); + ByteBuffer firstRecord = ByteBuffer.wrap(firstRecBytes); ByteBuffer cliAppBuf = ByteBuffer.allocateDirect(appBufSize); - result = client.unwrap(firstHalf, cliAppBuf); + result = client.unwrap(firstRecord, cliAppBuf); - /* BUFFER_UNDERFLOW must not be returned when input was - * consumed - regression for inRemaining == 0 guard fix */ - if (result.getStatus() == - SSLEngineResult.Status.BUFFER_UNDERFLOW) { + /* wolfSSL processed first record but needs more data: + * status must be OK, not BUFFER_UNDERFLOW. */ + if (result.getStatus() != SSLEngineResult.Status.OK) { error("\t... failed"); - fail("unwrap() with consumed handshake data must not " + - "return BUFFER_UNDERFLOW (regression: inRemaining" + - " == 0 guard), bytesConsumed=" + - result.bytesConsumed()); + fail("expected Status.OK, got: " + + result.getStatus()); } - /* Input was non-empty so at least some bytes must have - * been consumed */ - if (result.bytesConsumed() == 0) { + /* Handshake needs more data to continue. */ + if (result.getHandshakeStatus() != + SSLEngineResult.HandshakeStatus.NEED_UNWRAP) { error("\t... failed"); - fail("unwrap() consumed 0 bytes from a non-empty " + - "handshake buffer"); + fail("expected NEED_UNWRAP, got: " + + result.getHandshakeStatus()); + } + + /* All bytes consumed via I/O callback. */ + if (result.bytesConsumed() != firstRecordLen) { + error("\t... failed"); + fail("expected " + firstRecordLen + " bytes " + + "consumed, got: " + result.bytesConsumed()); + } + + /* No application data produced during handshake. */ + if (result.bytesProduced() != 0) { + error("\t... failed"); + fail("expected 0 bytes produced, got: " + + result.bytesProduced()); + } + + /* + * The first unwrap set contPartialRecord=true because wolfSSL + * exhausted firstRecord and returned WANT_READ. On this call, + * peek is skipped and DoHandshake() receives raw continuation + * bytes. inRemaining > 0 must suppress BUFFER_UNDERFLOW. */ + int contLen = Math.min(3, srvFlight.remaining()); + if (contLen > 0) { + byte[] contBytes = new byte[contLen]; + srvFlight.get(contBytes); + ByteBuffer contBuf = ByteBuffer.wrap(contBytes); + ByteBuffer cliAppBuf2 = + ByteBuffer.allocateDirect(appBufSize); + result = client.unwrap(contBuf, cliAppBuf2); + + /* wolfSSL consumed continuation bytes, needs more: + * must be Status.OK, not BUFFER_UNDERFLOW. */ + if (result.getStatus() != + SSLEngineResult.Status.OK) { + error("\t... failed"); + fail("expected Status.OK for continuation " + + "bytes, got: " + result.getStatus()); + } + + /* Handshake still needs more data. */ + if (result.getHandshakeStatus() != + SSLEngineResult.HandshakeStatus.NEED_UNWRAP) { + error("\t... failed"); + fail("expected NEED_UNWRAP for continuation " + + "bytes, got: " + + result.getHandshakeStatus()); + } + + /* All continuation bytes consumed. */ + if (result.bytesConsumed() != contLen) { + error("\t... failed"); + fail("expected " + contLen + " continuation " + + "bytes consumed, got: " + + result.bytesConsumed()); + } + + /* No application data produced. */ + if (result.bytesProduced() != 0) { + error("\t... failed"); + fail("expected 0 bytes produced, got: " + + result.bytesProduced()); + } } pass("\t... passed");