Revert "Allow to offload certificate validation when using BoringSSL (#8908)"

This reverts commit 316dd98284.
This commit is contained in:
Norman Maurer 2019-03-24 09:33:42 +01:00
parent 316dd98284
commit 33e2f5609d
2 changed files with 34 additions and 46 deletions

View File

@ -737,7 +737,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 (bytesProduced > 0 && handshakeException != 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.
@ -746,9 +746,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
// We produced / consumed some data during the handshake, signal back to the caller.
// If there is a handshake exception and we have produced data, we should send the data before
// we allow handshake() to throw the handshake exception.
//
// unwrap(...) will then ensure we propagate the handshake error back to the user.
return newResult(NEED_UNWRAP, 0, bytesProduced);
return newResult(NEED_WRAP, 0, bytesProduced);
}
status = handshake();
@ -891,8 +889,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
// to write encrypted data to. This is an OVERFLOW condition.
// [1] https://www.openssl.org/docs/manmaster/ssl/SSL_write.html
return newResult(BUFFER_OVERFLOW, status, bytesConsumed, bytesProduced);
} else if (sslError == SSL.SSL_ERROR_WANT_X509_LOOKUP ||
sslError == SSL.SSL_ERROR_WANT_CERTIFICATE_VERIFY) {
} else if (sslError == SSL.SSL_ERROR_WANT_X509_LOOKUP) {
return newResult(NEED_TASK, bytesConsumed, bytesProduced);
} else {
// Everything else is considered as error
@ -1181,8 +1178,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
}
return newResultMayFinishHandshake(isInboundDone() ? CLOSED : OK, status,
bytesConsumed, bytesProduced);
} else if (sslError == SSL.SSL_ERROR_WANT_X509_LOOKUP
|| sslError == SSL.SSL_ERROR_WANT_CERTIFICATE_VERIFY) {
} else if (sslError == SSL.SSL_ERROR_WANT_X509_LOOKUP) {
return newResult(isInboundDone() ? CLOSED : OK,
NEED_TASK, bytesConsumed, bytesProduced);
} else {
@ -1328,11 +1324,11 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
return new Runnable() {
@Override
public void run() {
if (isDestroyed()) {
// The engine was destroyed in the meantime, just return.
return;
}
try {
if (isDestroyed()) {
// The engine was destroyed in the meantime, just return.
return;
}
task.run();
} finally {
// The task was run, reset needTask to false so getHandshakeStatus() returns the correct value.
@ -1683,35 +1679,27 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
return cert == null || cert.length == 0;
}
private SSLEngineResult.HandshakeStatus handshakeException() throws SSLException {
if (SSL.bioLengthNonApplication(networkBIO) > 0) {
// There is something pending, we need to consume it first via a WRAP so we don't loose anything.
return NEED_WRAP;
}
SSLHandshakeException exception = handshakeException;
handshakeException = null;
shutdown();
throw exception;
}
private SSLEngineResult.HandshakeStatus handshake() throws SSLException {
if (needTask) {
return NEED_TASK;
}
if (handshakeState == HandshakeState.FINISHED) {
return FINISHED;
}
checkEngineClosed(HANDSHAKE_ENGINE_CLOSED);
if (handshakeException != null) {
// If we there was an handshake exception we need to call SSL.doHandshake(...) again as it may produce
// some alert or advance the internal state machine. Also clear the error queue as this call may put
// something on the queue.
SSL.doHandshake(ssl);
SSL.clearError();
return handshakeException();
// 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.
// See https://github.com/netty/netty/issues/3900
SSLHandshakeException exception = handshakeException;
if (exception != null) {
if (SSL.bioLengthNonApplication(networkBIO) > 0) {
// There is something pending, we need to consume it first via a WRAP so we don't loose anything.
return NEED_WRAP;
}
// No more data left to send to the remote peer, so null out the exception field, shutdown and throw
// the exception.
handshakeException = null;
shutdown();
throw exception;
}
// Adding the OpenSslEngine to the OpenSslEngineMap so it can be used in the AbstractCertificateVerifier.
@ -1722,21 +1710,24 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
int code = SSL.doHandshake(ssl);
if (code <= 0) {
// 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) {
exception = handshakeException;
handshakeException = null;
shutdown();
throw exception;
}
int sslError = SSL.getError(ssl, code);
if (sslError == SSL.SSL_ERROR_WANT_READ || sslError == SSL.SSL_ERROR_WANT_WRITE) {
return pendingStatus(SSL.bioLengthNonApplication(networkBIO));
}
if (sslError == SSL.SSL_ERROR_WANT_X509_LOOKUP || sslError == SSL.SSL_ERROR_WANT_CERTIFICATE_VERIFY) {
if (sslError == SSL.SSL_ERROR_WANT_X509_LOOKUP) {
return NEED_TASK;
}
// 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) {
return handshakeException();
}
// Everything else is considered as error
throw shutdownWithError("SSL_do_handshake", sslError);
}

View File

@ -989,8 +989,6 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
throw new IllegalStateException("Unknown handshake status: " + result.getHandshakeStatus());
}
// Check if did not produce any bytes and if so break out of the loop, but only if we did not process
// a task as last action. It's fine to not produce any data as part of executing a task.
if (result.bytesProduced() == 0 && status != HandshakeStatus.NEED_TASK) {
break;
}
@ -1659,7 +1657,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
}
// Flush now as we may have written some data as part of the wrap call.
forceFlush(ctx);
forceFlush(ctx);
} catch (Throwable e) {
taskError(e);
return;
@ -1668,7 +1666,6 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
// Now try to feed in more data that we have buffered.
tryDecodeAgain();
break;
default:
// Should never reach here as we handle all cases.
throw new AssertionError();