From d8f22e5047b12e3d6b55e7ea93b560a274f456fd Mon Sep 17 00:00:00 2001 From: Ruby Martin Date: Mon, 6 Apr 2026 17:14:42 -0600 Subject: [PATCH 1/2] move peek logic higher in unwrap, do not consume data if BUFFER_UNDERFLOW encountered edit comment --- .../wolfssl/provider/jsse/WolfSSLEngine.java | 266 ++++++++++-------- 1 file changed, 156 insertions(+), 110 deletions(-) diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java b/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java index 45a908d8..596d1ea1 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java @@ -1459,10 +1459,46 @@ else if (hs == SSLEngineResult.HandshakeStatus.NEED_WRAP && prevSessionTicketCount = this.sessionTicketCount; } + boolean hsUnderflow = false; if (this.handshakeFinished == false) { - WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "starting or continuing handshake"); - ret = DoHandshake(false); + /* Client mode: pre-check record header to return + * BUFFER_UNDERFLOW with bytesConsumed == 0 per + * the JSSE spec. Bogus headers fall through to + * wolfSSL. Server mode is excluded: wolfSSL buffers + * partial records via the I/O callback and returns + * WANT_READ, mapping to OK + NEED_UNWRAP. Returning + * BUFFER_UNDERFLOW server-side would break the + * SSLEngineExplorer pattern, whose BUFFER_UNDERFLOW + * handler reads from the socket, not the pre-read + * ClientHello buffer. */ + if (this.getUseClientMode() && + inRemaining > 0 && + (this.ssl.dtls() == 0)) { + synchronized (netDataLock) { + int pos = in.position(); + if (inRemaining < TLS_RECORD_HEADER_LEN) { + hsUnderflow = true; + } else { + 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) { + hsUnderflow = true; + } + } + } + } + if (hsUnderflow) { + status = SSLEngineResult.Status.BUFFER_UNDERFLOW; + } else { + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "starting or continuing handshake"); + ret = DoHandshake(false); + } } else { /* TLS-only: peek at the record header and @@ -1488,8 +1524,9 @@ else if (hs == SSLEngineResult.HandshakeStatus.NEED_WRAP && & 0xFF) << 8) | (in.get(pos + TLS_RECORD_LEN_LO_OFF) & 0xFF); - if (inRemaining < - TLS_RECORD_HEADER_LEN + recLen) { + if (recLen <= WolfSSL.MAX_RECORD_SIZE + && inRemaining < + TLS_RECORD_HEADER_LEN + recLen) { bufferUnderflow = true; } } @@ -1527,11 +1564,13 @@ else if (hs == SSLEngineResult.HandshakeStatus.NEED_WRAP && } else { /* Still not enough output space */ - status = SSLEngineResult.Status.BUFFER_OVERFLOW; + status = + SSLEngineResult.Status.BUFFER_OVERFLOW; } } else if (bufferUnderflow) { - status = SSLEngineResult.Status.BUFFER_UNDERFLOW; + status = + SSLEngineResult.Status.BUFFER_UNDERFLOW; } /* If we have input data, make sure output buffer * length is greater than zero, otherwise ask app to @@ -1616,7 +1655,8 @@ else if (inRemaining > 0 && (ssl.getError(0) == 0) && !this.sessionStored) { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "calling engineHelper.saveSession()"); + () -> + "calling engineHelper.saveSession()"); int ret2 = this.engineHelper.saveSession(); WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, () -> "return from saveSession(), ret = " + @@ -1628,123 +1668,129 @@ else if (inRemaining > 0 && } } /* end DoHandshake() / RecvAppData() */ - if (outBoundOpen == false || this.closeNotifySent || - this.closeNotifyReceived) { - /* Mark SSLEngine status as CLOSED */ - status = SSLEngineResult.Status.CLOSED; - /* Handshake has finished and SSLEngine is closed, - * release, global JNI verify callback pointer */ - this.engineHelper.unsetVerifyCallback(); - } + if (!hsUnderflow) { + if (outBoundOpen == false || this.closeNotifySent || + this.closeNotifyReceived) { + /* Mark SSLEngine status as CLOSED */ + status = SSLEngineResult.Status.CLOSED; + /* Handshake has finished and SSLEngine is closed, + * release, global JNI verify callback pointer */ + this.engineHelper.unsetVerifyCallback(); + } - synchronized (ioLock) { - err = ssl.getError(ret); - } - if (ret < 0 && (this.ssl.dtls() == 1) && - (err == WolfSSL.OUT_OF_ORDER_E)) { - /* Received out of order DTLS message. Ignore and set - * our status to NEED_UNWRAP again to wait for correct - * data */ - WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "out of order message, dropping and " + - "putting state back to NEED_UNWRAP"); - } - else if (ret < 0 && - (err != WolfSSL.SSL_ERROR_WANT_READ) && - (err != WolfSSL.SSL_ERROR_WANT_WRITE)) { - - if (err == WolfSSL.UNKNOWN_ALPN_PROTOCOL_NAME_E) { - /* Native wolfSSL could not negotiate a common ALPN - * protocol */ - this.inBoundOpen = false; - throw new SSLHandshakeException( - "Unrecognized protocol name error, ret:err = " + - ret + " : " + err); + synchronized (ioLock) { + err = ssl.getError(ret); } - else { - /* Native wolfSSL threw an exception when unwrapping - * data, close inbound since we can't receive more - * data */ - this.inBoundOpen = false; - if (err == WolfSSL.FATAL_ERROR) { - /* If client side and we received fatal alert, - * close outbound since we won't be receiving - * any more data */ - this.outBoundOpen = false; + if (ret < 0 && (this.ssl.dtls() == 1) && + (err == WolfSSL.OUT_OF_ORDER_E)) { + /* Received out of order DTLS message. Ignore and + * set our status to NEED_UNWRAP again to wait for + * correct data */ + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "out of order message, dropping and " + + "putting state back to NEED_UNWRAP"); + } + else if (ret < 0 && + (err != WolfSSL.SSL_ERROR_WANT_READ) && + (err != WolfSSL.SSL_ERROR_WANT_WRITE)) { + + if (err == WolfSSL.UNKNOWN_ALPN_PROTOCOL_NAME_E) { + /* Native wolfSSL could not negotiate a common + * ALPN protocol */ + this.inBoundOpen = false; + throw new SSLHandshakeException( + "Unrecognized protocol name error, " + + "ret:err = " + ret + " : " + err); } - /* Throw SSLHandshakeException if handshake not - * finished, otherwise throw SSLException for - * post-handshake errors */ - if (!this.handshakeFinished) { - SSLHandshakeException hse2 = - new SSLHandshakeException( - "SSL/TLS handshake error, ret:err = " + - ret + " : " + err); - if (this.engineHelper != null) { - Exception verifyEx2 = this.engineHelper - .getLastVerifyException(); - if (verifyEx2 != null) { - hse2.initCause(verifyEx2); + else { + /* Native wolfSSL threw an exception when + * unwrapping data, close inbound since we + * can't receive more data */ + this.inBoundOpen = false; + if (err == WolfSSL.FATAL_ERROR) { + /* If client side and we received fatal + * alert, close outbound since we won't be + * receiving any more data */ + this.outBoundOpen = false; + } + /* Throw SSLHandshakeException if handshake not + * finished, otherwise throw SSLException for + * post-handshake errors */ + if (!this.handshakeFinished) { + SSLHandshakeException hse2 = + new SSLHandshakeException( + "SSL/TLS handshake error, ret:err = " + + ret + " : " + err); + if (this.engineHelper != null) { + Exception verifyEx2 = this.engineHelper + .getLastVerifyException(); + if (verifyEx2 != null) { + hse2.initCause(verifyEx2); + } } + throw hse2; } - throw hse2; + throw new SSLException( + "wolfSSL error, ret:err = " + ret + " : " + + err); } - throw new SSLException( - "wolfSSL error, ret:err = " + ret + " : " + - err); } - } - else if (!this.handshakeFinished && (ret == 0) && - (err == 0 || err == WolfSSL.SSL_ERROR_ZERO_RETURN)) { + else if (!this.handshakeFinished && (ret == 0) && + (err == 0 || + err == WolfSSL.SSL_ERROR_ZERO_RETURN)) { - boolean gotCloseNotify = false; - synchronized (ioLock) { - gotCloseNotify = this.ssl.gotCloseNotify(); - } + boolean gotCloseNotify = false; + synchronized (ioLock) { + gotCloseNotify = this.ssl.gotCloseNotify(); + } - if (!gotCloseNotify && !this.closeNotifyReceived) { - this.inBoundOpen = false; - this.outBoundOpen = false; - throw new SSLHandshakeException( - "Peer closed connection during handshake"); + if (!gotCloseNotify && !this.closeNotifyReceived) { + this.inBoundOpen = false; + this.outBoundOpen = false; + throw new SSLHandshakeException( + "Peer closed connection during handshake"); + } } - } - /* Get DTLS drop count after data has been processed. To be - * used below when setting BUFFER_UNDERFLOW status. */ - synchronized (ioLock) { - dtlsCurrDropCount = ssl.getDtlsMacDropCount(); - } + /* Get DTLS drop count after data has been processed. + * To be used below when setting BUFFER_UNDERFLOW + * status. */ + synchronized (ioLock) { + dtlsCurrDropCount = ssl.getDtlsMacDropCount(); + } - /* Detect if we need to set BUFFER_UNDERFLOW. - * If we consume data in unwrap() but it's just a session - * ticket, we don't set BUFFER_UNDERFLOW and just continue - * on to set status as OK. */ - synchronized (toSendLock) { - synchronized (netDataLock) { - if (ret <= 0 && - err == WolfSSL.SSL_ERROR_WANT_READ && - in.remaining() == 0 && - inRemaining == 0 && - (this.internalIOSendBufOffset == 0) && - (prevSessionTicketCount == - this.sessionTicketCount)) { - - if ((this.ssl.dtls() == 0) || - (this.handshakeFinished && - (dtlsPrevDropCount == - dtlsCurrDropCount))) { - /* Need more data. For DTLS only set - * after handshake has completed, since - * apps expect to switch on NEED_UNWRAP - * HandshakeStatus at that point */ - status = - SSLEngineResult.Status.BUFFER_UNDERFLOW; + /* Detect if we need to set BUFFER_UNDERFLOW. + * If we consume data in unwrap() but it's just a + * session ticket, we don't set BUFFER_UNDERFLOW and + * just continue on to set status as OK. */ + synchronized (toSendLock) { + synchronized (netDataLock) { + if (ret <= 0 && + err == WolfSSL.SSL_ERROR_WANT_READ && + in.remaining() == 0 && + inRemaining == 0 && + (this.internalIOSendBufOffset == 0) && + (prevSessionTicketCount == + this.sessionTicketCount)) { + + if ((this.ssl.dtls() == 0) || + (this.handshakeFinished && + (dtlsPrevDropCount == + dtlsCurrDropCount))) { + /* Need more data. For DTLS only set + * after handshake has completed, since + * apps expect to switch on NEED_UNWRAP + * HandshakeStatus at that point */ + status = + SSLEngineResult.Status + .BUFFER_UNDERFLOW; + } } } } - } - } + } /* end if (!hsUnderflow) */ + + } /* end else (outBoundOpen == true) */ synchronized (netDataLock) { consumed += in.position() - inPosition; From 34e923dbca738723aa9453f72fb3d3bb603792b5 Mon Sep 17 00:00:00 2001 From: Ruby Martin Date: Mon, 6 Apr 2026 17:15:18 -0600 Subject: [PATCH 2/2] update test to catch expected statuses --- .../wolfssl/provider/jsse/test/WolfSSLEngineTest.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java b/src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java index fdc99a2c..e0501c92 100644 --- a/src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java +++ b/src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java @@ -3147,9 +3147,12 @@ public void testHandshakeUnwrapConsumedNotBufferUnderflow() result = client.unwrap(firstHalf, cliAppBuf); /* BUFFER_UNDERFLOW must not be returned when input was - * consumed - regression for inRemaining == 0 guard fix */ + * consumed - regression for inRemaining == 0 guard fix. + * If BUFFER_UNDERFLOW is returned, bytesConsumed() must be 0 + * per the JSSE spec ("No data was consumed"). */ if (result.getStatus() == - SSLEngineResult.Status.BUFFER_UNDERFLOW) { + SSLEngineResult.Status.BUFFER_UNDERFLOW && + result.bytesConsumed() > 0) { error("\t... failed"); fail("unwrap() with consumed handshake data must not " + "return BUFFER_UNDERFLOW (regression: inRemaining" + @@ -3159,7 +3162,8 @@ public void testHandshakeUnwrapConsumedNotBufferUnderflow() /* Input was non-empty so at least some bytes must have * been consumed */ - if (result.bytesConsumed() == 0) { + if (result.getStatus() != SSLEngineResult.Status.BUFFER_UNDERFLOW + && result.bytesConsumed() == 0) { error("\t... failed"); fail("unwrap() consumed 0 bytes from a non-empty " + "handshake buffer");