unwrap() Client-side pre-check in SSLEngine to fix BUFFER_UNDERFLOW by rlm2002 · Pull Request #352 · wolfSSL/wolfssljni

rlm2002 · 2026-04-06T23:25:43Z

Adds a client-mode pre-check in unwrap() that inspects the TLS record header before calling into wolfSSL, returning BUFFER_UNDERFLOW with bytesConsumed() == 0 when the buffer holds a partial record: "no data was consumed" true on underflow.

Also corrects the testHandshakeUnwrapConsumedNotBufferUnderflow regression test: rather than rejecting all BUFFER_UNDERFLOW results, now fails if BUFFER_UNDERFLOW is returned when bytesConsumed() > 0.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adjusts SSLEngine.unwrap() handshake behavior to better comply with the JSSE spec on BUFFER_UNDERFLOW, and updates a regression test to match the expected bytesConsumed() semantics.

Changes:

Added a client-mode pre-check for partial TLS records during handshake to return BUFFER_UNDERFLOW with bytesConsumed() == 0.
Guarded post-handshake/close-status logic to avoid running when the handshake underflow pre-check triggers.
Updated the handshake regression test to only fail when BUFFER_UNDERFLOW is returned with bytesConsumed() > 0.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java	Adds client-side TLS record header pre-check to drive spec-compliant `BUFFER_UNDERFLOW` behavior during handshake.
src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java	Adjusts regression test assertions to align with JSSE “no data consumed on underflow” requirement.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

BUFFER_UNDERFLOW encountered edit comment

rlm2002 · 2026-04-09T15:04:04Z

retest this please Jenkins

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T23:20:34Z

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

                        prevSessionTicketCount = this.sessionTicketCount;
                    }

+                    boolean hsUnderflow = false;


When hsUnderflow is detected, the method sets status = BUFFER_UNDERFLOW but (from the shown hunk) does not update the handshake status to reflect “need more inbound data”. If the returned SSLEngineResult ends up carrying the prior HandshakeStatus (notably NEED_WRAP in this surrounding branch), callers that prioritize HandshakeStatus may incorrectly try to wrap() instead of reading more network data. Consider explicitly ensuring the result’s HandshakeStatus is NEED_UNWRAP (or otherwise consistent with BUFFER_UNDERFLOW) in the hsUnderflow path.

Copilot · 2026-04-09T23:20:34Z

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

+                        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);
+                        }


When hsUnderflow is detected, the method sets status = BUFFER_UNDERFLOW but (from the shown hunk) does not update the handshake status to reflect “need more inbound data”. If the returned SSLEngineResult ends up carrying the prior HandshakeStatus (notably NEED_WRAP in this surrounding branch), callers that prioritize HandshakeStatus may incorrectly try to wrap() instead of reading more network data. Consider explicitly ensuring the result’s HandshakeStatus is NEED_UNWRAP (or otherwise consistent with BUFFER_UNDERFLOW) in the hsUnderflow path.

Copilot · 2026-04-09T23:20:35Z

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

+                    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();
+                        }



Gating the CLOSED-state propagation behind if (!hsUnderflow) can return BUFFER_UNDERFLOW even when the engine is already logically closed (outBoundOpen == false, closeNotifySent, or closeNotifyReceived). CLOSED typically should take precedence over underflow to avoid prompting callers to read more data on a closed engine. Consider checking and applying the CLOSED state before the underflow early-return path (or applying it regardless of hsUnderflow).

Suggested change

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();

}

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) {

Copilot · 2026-04-09T23:20:35Z

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

+                                    if (recLen <= WolfSSL.MAX_RECORD_SIZE
+                                            && inRemaining <
+                                            TLS_RECORD_HEADER_LEN + recLen) {
                                        bufferUnderflow = true;
                                    }


The “partial TLS record” detection logic (read header, compute recLen, compare against TLS_RECORD_HEADER_LEN + recLen, and cap with MAX_RECORD_SIZE) now appears in multiple places (hsUnderflow and bufferUnderflow). This duplication increases the risk of future drift (e.g., changing the cap/conditions in one place but not the other). Consider extracting a small private helper (e.g., isPartialTlsRecord(ByteBuffer in, int inRemaining)) and reusing it for both branches.

Copilot · 2026-04-09T23:20:35Z

src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java

        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) {


This test now permits BUFFER_UNDERFLOW as long as bytesConsumed() == 0, but it doesn’t assert the newly introduced behavior that specifically motivated the change in WolfSSLEngine.unwrap() (client-mode underflow on a partial TLS record header/record before calling into wolfSSL). Consider adding a targeted test case that feeds (a) <5 bytes (partial header) and (b) a full header with an incomplete payload, and asserts Status.BUFFER_UNDERFLOW with bytesConsumed() == 0 for both.

Copilot · 2026-04-09T23:20:35Z

src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java

+        if (result.getStatus() != SSLEngineResult.Status.BUFFER_UNDERFLOW
+                && result.bytesConsumed() == 0) {


This test now permits BUFFER_UNDERFLOW as long as bytesConsumed() == 0, but it doesn’t assert the newly introduced behavior that specifically motivated the change in WolfSSLEngine.unwrap() (client-mode underflow on a partial TLS record header/record before calling into wolfSSL). Consider adding a targeted test case that feeds (a) <5 bytes (partial header) and (b) a full header with an incomplete payload, and asserts Status.BUFFER_UNDERFLOW with bytesConsumed() == 0 for both.

rlm2002 self-assigned this Apr 6, 2026

cconlon requested a review from Copilot April 6, 2026 23:26

Copilot AI reviewed Apr 6, 2026

View reviewed changes

src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java Show resolved Hide resolved

src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java Outdated Show resolved Hide resolved

src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java Outdated Show resolved Hide resolved

Copilot started reviewing on behalf of cconlon April 7, 2026 00:49 View session

rlm2002 force-pushed the sunJSSE_ServerName branch from 8322b35 to e593e63 Compare April 7, 2026 22:24

rlm2002 added 2 commits April 7, 2026 16:44

move peek logic higher in unwrap, do not consume data if

d8f22e5

BUFFER_UNDERFLOW encountered edit comment

update test to catch expected statuses

34e923d

rlm2002 force-pushed the sunJSSE_ServerName branch from e593e63 to 34e923d Compare April 7, 2026 22:44

rlm2002 marked this pull request as ready for review April 8, 2026 21:45

rlm2002 assigned cconlon and unassigned rlm2002 Apr 9, 2026

cconlon requested a review from Copilot April 9, 2026 23:13

Copilot AI reviewed Apr 9, 2026

View reviewed changes

cconlon assigned rlm2002 and unassigned cconlon Apr 9, 2026

Copilot started reviewing on behalf of cconlon April 10, 2026 02:03 View session

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

unwrap() Client-side pre-check in SSLEngine to fix BUFFER_UNDERFLOW#352

unwrap() Client-side pre-check in SSLEngine to fix BUFFER_UNDERFLOW#352
rlm2002 wants to merge 2 commits intowolfSSL:masterfrom
rlm2002:sunJSSE_ServerName

rlm2002 commented Apr 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rlm2002 commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		if (result.getStatus() != SSLEngineResult.Status.BUFFER_UNDERFLOW
		&& result.bytesConsumed() == 0) {

Conversation

rlm2002 commented Apr 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rlm2002 commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants