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

Ensure we preserve the original cause of the SSLException in all case… (#10372)

Browse Source 
Motivation:

We did not correctly preserve the original cause of the SSLException when all the following are true:

 * SSL_read failed
 * handshake was finished
 * some outbound data was produced durigin SSL_read (for example ssl alert) that needs to be picked up by wrap(...)

Modifications:

Ensure we correctly preserve the original cause of the SSLException and rethrow it once we produced all data via wrap(...)

Result:

Be able to understand why we see an error in all cases
This commit is contained in:
Norman Maurer 2020-06-25 22:06:17 +02:00 committed by GitHub
parent eccb87b1f7
commit 662e0b4b81
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 41 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -30,6 +30,7 @@ import io.netty.util.internal.ObjectUtil;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.StringUtil;
import io.netty.util.internal.SuppressJava6Requirement;
import io.netty.util.internal.ThrowableUtil;
import io.netty.util.internal.UnstableApi;
import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory;
@ -214,7 +215,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
private final boolean enableOcsp;
private int maxWrapOverhead;
private int maxWrapBufferSize;
private Throwable handshakeException;
private Throwable pendingException;
/**
* Create a new instance.
@ -756,7 +757,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
// Flush any data that may have been written implicitly during the handshake by OpenSSL.
bytesProduced = SSL.bioFlushByteBuffer(networkBIO);
if (handshakeException != null) {
if (pendingException != null) {
// TODO(scott): It is possible that when the handshake failed there was not enough room in the
// non-application buffers to hold the alert. We should get all the data before progressing on.
// However I'm not aware of a way to do this with the OpenSSL APIs.
@ -840,8 +841,25 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
// There was no pending data in the network BIO -- encrypt any application data
int bytesConsumed = 0;
assert bytesProduced == 0;
// Flush any data that may have been written implicitly by OpenSSL in case a shutdown/alert occurs.
bytesProduced = SSL.bioFlushByteBuffer(networkBIO);
if (bytesProduced > 0) {
return newResultMayFinishHandshake(status, bytesConsumed, bytesProduced);
}
// There was a pending exception that we just delayed because there was something to produce left.
// Throw it now and shutdown the engine.
if (pendingException != null) {
Throwable error = pendingException;
pendingException = null;
shutdown();
// Throw a new exception wrapping the pending exception, so the stacktrace is meaningful and
// contains all the details.
throw new SSLException(error);
}
for (; offset < endOffset; ++offset) {
final ByteBuffer src = srcs[offset];
final int remaining = src.remaining();
@ -1008,9 +1026,9 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
SSLHandshakeException exception = new SSLHandshakeException(errorString);
// If we have a handshakeException stored already we should include it as well to help the user debug things.
if (handshakeException != null) {
exception.initCause(handshakeException);
handshakeException = null;
if (pendingException != null) {
exception.initCause(pendingException);
pendingException = null;
}
return exception;
}
@ -1260,10 +1278,15 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
// This is needed so we ensure close_notify etc is correctly send to the remote peer.
// See https://github.com/netty/netty/issues/3900
if (SSL.bioLengthNonApplication(networkBIO) > 0) {
if (handshakeException == null && handshakeState != HandshakeState.FINISHED) {
// we seems to have data left that needs to be transferred and so the user needs
// call wrap(...). Store the error so we can pick it up later.
handshakeException = new SSLHandshakeException(SSL.getErrorString(stackError));
// we seems to have data left that needs to be transferred and so the user needs
// call wrap(...). Store the error so we can pick it up later.
String message = SSL.getErrorString(stackError);
SSLException exception = handshakeState == HandshakeState.FINISHED ?
new SSLException(message) : new SSLHandshakeException(message);
if (pendingException == null) {
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.
@ -1727,9 +1750,9 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
return NEED_WRAP;
}
Throwable exception = handshakeException;
Throwable exception = pendingException;
assert exception != null;
handshakeException = null;
pendingException = null;
shutdown();
if (exception instanceof SSLHandshakeException) {
throw (SSLHandshakeException) exception;
@ -1744,8 +1767,11 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
* This cause will then be used to give more details as part of the {@link SSLHandshakeException}.
*/
final void initHandshakeException(Throwable cause) {
assert handshakeException == null;
handshakeException = cause;
if (pendingException == null) {
pendingException = cause;
} else {
ThrowableUtil.addSuppressed(pendingException, cause);
}
}
private SSLEngineResult.HandshakeStatus handshake() throws SSLException {
@ -1758,7 +1784,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
checkEngineClosed();
if (handshakeException != null) {
if (pendingException != null) {
// Let's call SSL.doHandshake(...) again in case there is some async operation pending that would fill the
// outbound buffer.
if (SSL.doHandshake(ssl) <= 0) {
@ -1789,7 +1815,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
// Check if we have a pending exception that was created during the handshake and if so throw it after
// shutdown the connection.
if (handshakeException != null) {
if (pendingException != null) {
return handshakeException();
}