andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Correctly use HandshakeStatus.NEED_WRAP when a handshake failed and a alert was produced (#11412)

Browse Source 
Motivation:

We need to ensure we always "consumed" all alerts etc via SSLEngine.wrap(...) before we teardown the engine. Failing to do so may lead to a situation where the remote peer will not be able to see the actual cause of the handshake failure but just see the connection being closed.

Modifications:

Correctly return HandshakeStatus.NEED_WRAP when we need to wrap some data first before we shutdown the engine because of a handshake failure.

Result:

Fixes https://github.com/netty/netty/issues/11388
This commit is contained in:
Norman Maurer 2021-06-24 10:05:47 +02:00 committed by GitHub
parent 6edb231bf5
commit a71ec15fc4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 17 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java
View File

@ -1321,8 +1321,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
}
}
private SSLEngineResult sslReadErrorResult(int error, int stackError, int bytesConsumed, int bytesProduced)
throws SSLException {
private boolean needWrapAgain(int stackError) {
// Check if we have a pending handshakeException and if so see if we need to consume all pending data from the
// BIO first or can just shutdown and throw it now.
// This is needed so we ensure close_notify etc is correctly send to the remote peer.
@ -1334,13 +1333,23 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
SSLException exception = handshakeState == HandshakeState.FINISHED ?
new SSLException(message) : new SSLHandshakeException(message);
if (pendingException == null) {
pendingException = exception;
pendingException = exception;
} else {
ThrowableUtil.addSuppressed(pendingException, exception);
}
// We need to clear all errors so we not pick up anything that was left on the stack on the next
// operation. Note that shutdownWithError(...) will cleanup the stack as well so its only needed here.
SSL.clearError();
return true;
}
return false;
}
private SSLEngineResult sslReadErrorResult(int error, int stackError, int bytesConsumed, int bytesProduced)
throws SSLException {
if (needWrapAgain(stackError)) {
// There is something that needs to be send to the remote peer before we can teardown.
// This is most likely some alert.
return new SSLEngineResult(OK, NEED_WRAP, bytesConsumed, bytesProduced);
}
throw shutdownWithError("SSL_read", error, stackError);
@ -1904,6 +1913,11 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
return NEED_TASK;
}
if (needWrapAgain(SSL.getLastErrorNumber())) {
// There is something that needs to be send to the remote peer before we can teardown.
// This is most likely some alert.
return NEED_WRAP;
}
// Check if we have a pending exception that was created during the handshake and if so throw it after
// shutdown the connection.
if (pendingException != null) {