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

OpenSslEngine may lose data if the non-application buffer is small/full

Browse Source 
Motivation:
If an event occurs which generates non-application data (shutdown, handshake failure, alert generation, etc...) and the non-application buffer in the ByteBuffer BIO is full (or sufficiently small) we may not propagate all data to our peer before tearing down the socket.

Modifications:
- when wrap() detects the outbound is closed, but there is more data pending in the non-application buffers, we must also check if OpenSSL will generate more data from calling SSL_shutdown again
- when wrap() detects a handshakeExcpetion during failure we should check if OpenSSL has any pending data (in addition to the non-application buff) before throwing the handshake exception

Result:
OpenSslEngine more reliably transmits data to the peer before closing the socket.
This commit is contained in:
Scott Mitchell 2017-02-14 19:22:13 -08:00
parent 945addc750
commit 2b079498fb
1 changed files with 39 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -447,7 +447,15 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
// There is something left to drain.
// See https://github.com/netty/netty/issues/6260
bytesProduced = SSL.bioFlushByteBuffer(networkBIO);
return newResultMayFinishHandshake(NOT_HANDSHAKING, 0, bytesProduced);
if (bytesProduced <= 0) {
return newResultMayFinishHandshake(NOT_HANDSHAKING, 0, 0);
}
// It is possible when the outbound was closed there was not enough room in the non-application
// buffers to hold the close_notify. We should keep trying to close until we consume all the data
// OpenSSL can give us.
boolean didShutdown = doSSLShutdown();
bytesProduced = bioLengthBefore - SSL.bioLengthByteBuffer(networkBIO);
return newResultMayFinishHandshake(didShutdown ? NEED_WRAP : NOT_HANDSHAKING, 0, bytesProduced);
}
// Flush any data that may be implicitly generated by OpenSSL (handshake, close, etc..).
@ -463,6 +471,11 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
bytesProduced = SSL.bioFlushByteBuffer(networkBIO);
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.
// See https://github.com/netty/netty/issues/6385.
// 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.
@ -995,29 +1008,9 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
outboundClosed = true;
if (handshakeState != HandshakeState.NOT_STARTED && !isDestroyed()) {
if (SSL.isInInit(ssl) != 0) {
// Only try to call SSL_shutdown if we are not in the init state anymore.
// Otherwise we will see 'error:140E0197:SSL routines:SSL_shutdown:shutdown while in init' in our logs.
//
// See also http://hg.nginx.org/nginx/rev/062c189fee20
return;
}
int mode = SSL.getShutdown(ssl);
if ((mode & SSL.SSL_SENT_SHUTDOWN) != SSL.SSL_SENT_SHUTDOWN) {
int err = SSL.shutdownSSL(ssl);
if (err < 0) {
int sslErr = SSL.getError(ssl, err);
if (sslErr == SSL.SSL_ERROR_SYSCALL || sslErr == SSL.SSL_ERROR_SSL) {
if (logger.isDebugEnabled()) {
logger.debug("SSL_shutdown failed: OpenSSL error: {}", SSL.getLastError());
}
// There was an internal error -- shutdown
shutdown();
} else {
SSL.clearError();
}
}
doSSLShutdown();
}
} else {
// engine closing before initial handshake
@ -1025,6 +1018,30 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
}
}
private boolean doSSLShutdown() {
if (SSL.isInInit(ssl) != 0) {
// Only try to call SSL_shutdown if we are not in the init state anymore.
// Otherwise we will see 'error:140E0197:SSL routines:SSL_shutdown:shutdown while in init' in our logs.
//
// See also http://hg.nginx.org/nginx/rev/062c189fee20
return false;
}
int err = SSL.shutdownSSL(ssl);
if (err < 0) {
int sslErr = SSL.getError(ssl, err);
if (sslErr == SSL.SSL_ERROR_SYSCALL || sslErr == SSL.SSL_ERROR_SSL) {
if (logger.isDebugEnabled()) {
logger.debug("SSL_shutdown failed: OpenSSL error: {}", SSL.getLastError());
}
// There was an internal error -- shutdown
shutdown();
return false;
}
SSL.clearError();
}
return true;
}
@Override
public final synchronized boolean isOutboundDone() {
// Check if there is anything left in the outbound buffer.