Skip to content

Commit

Permalink
Fix bug where SslHandler may stall after TLSv1.3 handshake with deleg…
Browse files Browse the repository at this point in the history
…ate tasks (netty#14411)

Motivation:
When doing a TLSv1.3 handshake with delegate task execution, clients may
complete the handshake in a delegate task wrap that does _not_ any data
written during the handshake. A subsequent unwrap of server response
data is then supposed to let the pending user data flow through to
wrapping - the `wrapLater` flag is tracking this - however, this does
not happen because the unwrap notices that the handshake promise has
already been completed by the earlier wrap call. The unwrap then
assumes, incorrectly, that not only has downstream been notified, but
user data has also been wrapped.

Modification:
The unwrap call should always attempt to wrap later, if the handshake is
finished or not handshaking, and we have pending user data to wrap.

Result:
No more data processing stalls on the client-side, after a TLSv1.3
handshake with task delegation. This also fixes the frequent test
timeouts from `SSLEngineTest.mustCallResumeTrustedOnSessionResumption`,
which was running into this exact scenario.
  • Loading branch information
chrisvest authored Oct 23, 2024
1 parent 8e95a48 commit 047bdfe
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion handler/src/main/java/io/netty/handler/ssl/SslHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -1488,7 +1488,7 @@ private int unwrap(ChannelHandlerContext ctx, ByteBuf packet, int length) throws
if (handshakeStatus == HandshakeStatus.FINISHED || handshakeStatus == HandshakeStatus.NOT_HANDSHAKING) {
wrapLater |= (decodeOut.isReadable() ?
setHandshakeSuccessUnwrapMarkReentry() : setHandshakeSuccess()) ||
handshakeStatus == HandshakeStatus.FINISHED;
handshakeStatus == HandshakeStatus.FINISHED || !pendingUnencryptedWrites.isEmpty();
}

// Dispatch decoded data after we have notified of handshake success. If this method has been invoked
Expand Down

0 comments on commit 047bdfe

Please sign in to comment.