Allow to offload certificate validation when using BoringSSL (#8974)

Motivation:

BoringSSL supports offloading certificate validation to a different thread. This is useful as it may need to do blocking operations and so may block the EventLoop.

Modification:

- Adjust ReferenceCountedOpenSslEngine to correctly handle offloaded certificate validation (just as we already have code for certificate selection).

Result:

Be able to offload certificate validation when using BoringSSL.
This commit is contained in:
Norman Maurer 2019-03-24 20:03:30 +01:00
parent a817e30d41
commit 1736b0e6a8
5 changed files with 45 additions and 34 deletions

View File

@ -278,6 +278,7 @@ public final class ReferenceCountedOpenSslClientContext extends ReferenceCounted
logger.debug("request of key failed", cause);
SSLHandshakeException e = new SSLHandshakeException("General OpenSslEngine problem");
e.initCause(cause);
assert engine.handshakeException == null;
engine.handshakeException = e;
}
}

View File

@ -664,6 +664,7 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen
logger.debug("verification of certificate failed", cause);
SSLHandshakeException e = new SSLHandshakeException("General OpenSslEngine problem");
e.initCause(cause);
assert engine.handshakeException == null;
engine.handshakeException = e;
// Try to extract the correct error code that should be used.

View File

@ -733,7 +733,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 (bytesProduced > 0 && handshakeException != null) {
if (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.
@ -742,7 +742,9 @@ 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.
return newResult(NEED_WRAP, 0, bytesProduced);
//
// unwrap(...) will then ensure we propagate the handshake error back to the user.
return newResult(NEED_UNWRAP, 0, bytesProduced);
}
status = handshake();
@ -885,7 +887,8 @@ 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) {
} else if (sslError == SSL.SSL_ERROR_WANT_X509_LOOKUP ||
sslError == SSL.SSL_ERROR_WANT_CERTIFICATE_VERIFY) {
return newResult(NEED_TASK, bytesConsumed, bytesProduced);
} else {
// Everything else is considered as error
@ -1172,7 +1175,8 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
}
return newResultMayFinishHandshake(isInboundDone() ? CLOSED : OK, status,
bytesConsumed, bytesProduced);
} else if (sslError == SSL.SSL_ERROR_WANT_X509_LOOKUP) {
} else if (sslError == SSL.SSL_ERROR_WANT_X509_LOOKUP
|| sslError == SSL.SSL_ERROR_WANT_CERTIFICATE_VERIFY) {
return newResult(isInboundDone() ? CLOSED : OK,
NEED_TASK, bytesConsumed, bytesProduced);
} else {
@ -1318,11 +1322,11 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
return new Runnable() {
@Override
public void run() {
try {
if (isDestroyed()) {
// The engine was destroyed in the meantime, just return.
return;
}
try {
task.run();
} finally {
// The task was run, reset needTask to false so getHandshakeStatus() returns the correct value.
@ -1680,29 +1684,33 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
return cert == null || cert.length == 0;
}
private SSLEngineResult.HandshakeStatus handshake() throws SSLException {
if (handshakeState == HandshakeState.FINISHED) {
return FINISHED;
}
checkEngineClosed(HANDSHAKE_ENGINE_CLOSED);
// 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) {
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;
}
// No more data left to send to the remote peer, so null out the exception field, shutdown and throw
// the exception.
SSLHandshakeException exception = handshakeException;
assert exception != null;
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) {
return handshakeException();
}
// Adding the OpenSslEngine to the OpenSslEngineMap so it can be used in the AbstractCertificateVerifier.
engineMap.add(this);
if (lastAccessed == -1) {
@ -1711,24 +1719,21 @@ 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) {
if (sslError == SSL.SSL_ERROR_WANT_X509_LOOKUP || sslError == SSL.SSL_ERROR_WANT_CERTIFICATE_VERIFY) {
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

@ -210,6 +210,7 @@ public final class ReferenceCountedOpenSslServerContext extends ReferenceCounted
logger.debug("Failed to set the server-side key material", cause);
SSLHandshakeException e = new SSLHandshakeException("General OpenSslEngine problem");
e.initCause(cause);
assert engine.handshakeException == null;
engine.handshakeException = e;
}
}

View File

@ -984,6 +984,8 @@ 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;
}
@ -1660,6 +1662,7 @@ 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();