Move validation of arguments out of synchronized block
Motivation: There is no need already use synchronized when validate the args of the methods. Modifications: First validate arguments and then use synchronized Result: Less code executed in synchronized block.
This commit is contained in:
Norman Maurer
parent
e845670043
commit
9687d77b5a
@ -524,14 +524,8 @@ public final class OpenSslEngine extends SSLEngine {
|
||||
}
|
||||
|
||||
@Override
|
||||
public synchronized SSLEngineResult wrap(
|
||||
public SSLEngineResult wrap(
|
||||
final ByteBuffer[] srcs, final int offset, final int length, final ByteBuffer dst) throws SSLException {
|
||||
|
||||
// Check to make sure the engine has not been closed
|
||||
if (isDestroyed()) {
|
||||
return CLOSED_NOT_HANDSHAKING;
|
||||
}
|
||||
|
||||
// Throw required runtime exceptions
|
||||
if (srcs == null) {
|
||||
throw new IllegalArgumentException("srcs is null");
|
||||
@ -550,95 +544,104 @@ public final class OpenSslEngine extends SSLEngine {
|
||||
throw new ReadOnlyBufferException();
|
||||
}
|
||||
|
||||
HandshakeStatus status = NOT_HANDSHAKING;
|
||||
// Prepare OpenSSL to work in server mode and receive handshake
|
||||
if (handshakeState != HandshakeState.FINISHED) {
|
||||
if (handshakeState != HandshakeState.STARTED_EXPLICITLY) {
|
||||
// Update accepted so we know we triggered the handshake via wrap
|
||||
handshakeState = HandshakeState.STARTED_IMPLICITLY;
|
||||
synchronized (this) {
|
||||
// Check to make sure the engine has not been closed
|
||||
if (isDestroyed()) {
|
||||
return CLOSED_NOT_HANDSHAKING;
|
||||
}
|
||||
|
||||
status = handshake();
|
||||
if (status == NEED_UNWRAP) {
|
||||
return NEED_UNWRAP_OK;
|
||||
HandshakeStatus status = NOT_HANDSHAKING;
|
||||
// Prepare OpenSSL to work in server mode and receive handshake
|
||||
if (handshakeState != HandshakeState.FINISHED) {
|
||||
if (handshakeState != HandshakeState.STARTED_EXPLICITLY) {
|
||||
// Update accepted so we know we triggered the handshake via wrap
|
||||
handshakeState = HandshakeState.STARTED_IMPLICITLY;
|
||||
}
|
||||
|
||||
status = handshake();
|
||||
if (status == NEED_UNWRAP) {
|
||||
return NEED_UNWRAP_OK;
|
||||
}
|
||||
|
||||
if (engineClosed) {
|
||||
return NEED_UNWRAP_CLOSED;
|
||||
}
|
||||
}
|
||||
|
||||
if (engineClosed) {
|
||||
return NEED_UNWRAP_CLOSED;
|
||||
}
|
||||
}
|
||||
// There was no pending data in the network BIO -- encrypt any application data
|
||||
int bytesProduced = 0;
|
||||
int bytesConsumed = 0;
|
||||
int endOffset = offset + length;
|
||||
for (int i = offset; i < endOffset; ++i) {
|
||||
final ByteBuffer src = srcs[i];
|
||||
if (src == null) {
|
||||
throw new IllegalArgumentException("srcs[" + i + "] is null");
|
||||
}
|
||||
while (src.hasRemaining()) {
|
||||
final SSLEngineResult pendingNetResult;
|
||||
// Write plaintext application data to the SSL engine
|
||||
int result = writePlaintextData(src);
|
||||
if (result > 0) {
|
||||
bytesConsumed += result;
|
||||
|
||||
// There was no pending data in the network BIO -- encrypt any application data
|
||||
int bytesProduced = 0;
|
||||
int bytesConsumed = 0;
|
||||
int endOffset = offset + length;
|
||||
for (int i = offset; i < endOffset; ++ i) {
|
||||
final ByteBuffer src = srcs[i];
|
||||
if (src == null) {
|
||||
throw new IllegalArgumentException("srcs[" + i + "] is null");
|
||||
}
|
||||
while (src.hasRemaining()) {
|
||||
final SSLEngineResult pendingNetResult;
|
||||
// Write plaintext application data to the SSL engine
|
||||
int result = writePlaintextData(src);
|
||||
if (result > 0) {
|
||||
bytesConsumed += result;
|
||||
|
||||
pendingNetResult = readPendingBytesFromBIO(dst, bytesConsumed, bytesProduced, status);
|
||||
if (pendingNetResult != null) {
|
||||
if (pendingNetResult.getStatus() != OK) {
|
||||
return pendingNetResult;
|
||||
pendingNetResult = readPendingBytesFromBIO(dst, bytesConsumed, bytesProduced, status);
|
||||
if (pendingNetResult != null) {
|
||||
if (pendingNetResult.getStatus() != OK) {
|
||||
return pendingNetResult;
|
||||
}
|
||||
bytesProduced = pendingNetResult.bytesProduced();
|
||||
}
|
||||
bytesProduced = pendingNetResult.bytesProduced();
|
||||
}
|
||||
} else {
|
||||
int sslError = SSL.getError(ssl, result);
|
||||
switch (sslError) {
|
||||
case SSL.SSL_ERROR_ZERO_RETURN:
|
||||
// This means the connection was shutdown correctly, close inbound and outbound
|
||||
if (!receivedShutdown) {
|
||||
closeAll();
|
||||
} else {
|
||||
int sslError = SSL.getError(ssl, result);
|
||||
switch (sslError) {
|
||||
case SSL.SSL_ERROR_ZERO_RETURN:
|
||||
// This means the connection was shutdown correctly, close inbound and outbound
|
||||
if (!receivedShutdown) {
|
||||
closeAll();
|
||||
}
|
||||
pendingNetResult = readPendingBytesFromBIO(dst, bytesConsumed, bytesProduced, status);
|
||||
return pendingNetResult != null ? pendingNetResult : CLOSED_NOT_HANDSHAKING;
|
||||
case SSL.SSL_ERROR_WANT_READ:
|
||||
// If there is no pending data to read from BIO we should go back to event loop and try
|
||||
// to read more data [1]. It is also possible that event loop will detect the socket
|
||||
// has been closed. [1] https://www.openssl.org/docs/manmaster/ssl/SSL_write.html
|
||||
pendingNetResult = readPendingBytesFromBIO(dst, bytesConsumed, bytesProduced, status);
|
||||
return pendingNetResult != null ? pendingNetResult :
|
||||
new SSLEngineResult(getEngineStatus(),
|
||||
NEED_UNWRAP, bytesConsumed, bytesProduced);
|
||||
case SSL.SSL_ERROR_WANT_WRITE:
|
||||
// SSL_ERROR_WANT_WRITE typically means that the underlying transport is not writable
|
||||
// and we should set the "want write" flag on the selector and try again when the
|
||||
// underlying transport is writable [1]. However we are not directly writing to the
|
||||
// underlying transport and instead writing to a BIO buffer. The OpenSsl documentation
|
||||
// says we should do the following [1]:
|
||||
//
|
||||
// "When using a buffering BIO, like a BIO pair, data must be written into or retrieved
|
||||
// out of the BIO before being able to continue."
|
||||
//
|
||||
// So we attempt to drain the BIO buffer below, but if there is no data this condition
|
||||
// is undefined and we assume their is a fatal error with the openssl engine and close.
|
||||
// [1] https://www.openssl.org/docs/manmaster/ssl/SSL_write.html
|
||||
pendingNetResult = readPendingBytesFromBIO(dst, bytesConsumed, bytesProduced, status);
|
||||
return pendingNetResult != null ? pendingNetResult : NEED_WRAP_CLOSED;
|
||||
default:
|
||||
// Everything else is considered as error
|
||||
throw shutdownWithError("SSL_write");
|
||||
}
|
||||
pendingNetResult = readPendingBytesFromBIO(dst, bytesConsumed, bytesProduced, status);
|
||||
return pendingNetResult != null ? pendingNetResult : CLOSED_NOT_HANDSHAKING;
|
||||
case SSL.SSL_ERROR_WANT_READ:
|
||||
// If there is no pending data to read from BIO we should go back to event loop and try to read
|
||||
// more data [1]. It is also possible that event loop will detect the socket has been closed.
|
||||
// [1] https://www.openssl.org/docs/manmaster/ssl/SSL_write.html
|
||||
pendingNetResult = readPendingBytesFromBIO(dst, bytesConsumed, bytesProduced, status);
|
||||
return pendingNetResult != null ? pendingNetResult :
|
||||
new SSLEngineResult(getEngineStatus(), NEED_UNWRAP, bytesConsumed, bytesProduced);
|
||||
case SSL.SSL_ERROR_WANT_WRITE:
|
||||
// SSL_ERROR_WANT_WRITE typically means that the underlying transport is not writable and we
|
||||
// should set the "want write" flag on the selector and try again when the underlying transport
|
||||
// is writable [1]. However we are not directly writing to the underlying transport and instead
|
||||
// writing to a BIO buffer. The OpenSsl documentation says we should do the following [1]:
|
||||
//
|
||||
// "When using a buffering BIO, like a BIO pair, data must be written into or retrieved out of
|
||||
// the BIO before being able to continue."
|
||||
//
|
||||
// So we attempt to drain the BIO buffer below, but if there is no data this condition is
|
||||
// undefined and we assume their is a fatal error with the openssl engine and close.
|
||||
// [1] https://www.openssl.org/docs/manmaster/ssl/SSL_write.html
|
||||
pendingNetResult = readPendingBytesFromBIO(dst, bytesConsumed, bytesProduced, status);
|
||||
return pendingNetResult != null ? pendingNetResult : NEED_WRAP_CLOSED;
|
||||
default:
|
||||
// Everything else is considered as error
|
||||
throw shutdownWithError("SSL_write");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
// We need to check if pendingWrittenBytesInBIO was checked yet, as we may not checked if the srcs was empty,
|
||||
// or only contained empty buffers.
|
||||
if (bytesConsumed == 0) {
|
||||
SSLEngineResult pendingNetResult = readPendingBytesFromBIO(dst, 0, bytesProduced, status);
|
||||
if (pendingNetResult != null) {
|
||||
return pendingNetResult;
|
||||
// We need to check if pendingWrittenBytesInBIO was checked yet, as we may not checked if the srcs was
|
||||
// empty, or only contained empty buffers.
|
||||
if (bytesConsumed == 0) {
|
||||
SSLEngineResult pendingNetResult = readPendingBytesFromBIO(dst, 0, bytesProduced, status);
|
||||
if (pendingNetResult != null) {
|
||||
return pendingNetResult;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return newResult(bytesConsumed, bytesProduced, status);
|
||||
return newResult(bytesConsumed, bytesProduced, status);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@ -662,15 +665,10 @@ public final class OpenSslEngine extends SSLEngine {
|
||||
return new SSLHandshakeException(err);
|
||||
}
|
||||
|
||||
public synchronized SSLEngineResult unwrap(
|
||||
public SSLEngineResult unwrap(
|
||||
final ByteBuffer[] srcs, int srcsOffset, final int srcsLength,
|
||||
final ByteBuffer[] dsts, final int dstsOffset, final int dstsLength) throws SSLException {
|
||||
|
||||
// Check to make sure the engine has not been closed
|
||||
if (isDestroyed()) {
|
||||
return CLOSED_NOT_HANDSHAKING;
|
||||
}
|
||||
|
||||
// Throw required runtime exceptions
|
||||
if (srcs == null) {
|
||||
throw new NullPointerException("srcs");
|
||||
@ -702,23 +700,6 @@ public final class OpenSslEngine extends SSLEngine {
|
||||
capacity += dst.remaining();
|
||||
}
|
||||
|
||||
HandshakeStatus status = NOT_HANDSHAKING;
|
||||
// Prepare OpenSSL to work in server mode and receive handshake
|
||||
if (handshakeState != HandshakeState.FINISHED) {
|
||||
if (handshakeState != HandshakeState.STARTED_EXPLICITLY) {
|
||||
// Update accepted so we know we triggered the handshake via wrap
|
||||
handshakeState = HandshakeState.STARTED_IMPLICITLY;
|
||||
}
|
||||
|
||||
status = handshake();
|
||||
if (status == NEED_WRAP) {
|
||||
return NEED_WRAP_OK;
|
||||
}
|
||||
if (engineClosed) {
|
||||
return NEED_WRAP_CLOSED;
|
||||
}
|
||||
}
|
||||
|
||||
final int srcsEndOffset = srcsOffset + srcsLength;
|
||||
long len = 0;
|
||||
for (int i = srcsOffset; i < srcsEndOffset; i++) {
|
||||
@ -729,120 +710,144 @@ public final class OpenSslEngine extends SSLEngine {
|
||||
len += src.remaining();
|
||||
}
|
||||
|
||||
// protect against protocol overflow attack vector
|
||||
if (len > MAX_ENCRYPTED_PACKET_LENGTH) {
|
||||
isInboundDone = true;
|
||||
isOutboundDone = true;
|
||||
engineClosed = true;
|
||||
shutdown();
|
||||
throw ENCRYPTED_PACKET_OVERSIZED;
|
||||
}
|
||||
synchronized (this) {
|
||||
// Check to make sure the engine has not been closed
|
||||
if (isDestroyed()) {
|
||||
return CLOSED_NOT_HANDSHAKING;
|
||||
}
|
||||
|
||||
// Write encrypted data to network BIO
|
||||
int bytesConsumed = 0;
|
||||
if (srcsOffset < srcsEndOffset) {
|
||||
do {
|
||||
ByteBuffer src = srcs[srcsOffset];
|
||||
int remaining = src.remaining();
|
||||
if (remaining == 0) {
|
||||
// We must skip empty buffers as BIO_write will return 0 if asked to write something
|
||||
// with length 0.
|
||||
srcsOffset ++;
|
||||
continue;
|
||||
// protect against protocol overflow attack vector
|
||||
if (len > MAX_ENCRYPTED_PACKET_LENGTH) {
|
||||
isInboundDone = true;
|
||||
isOutboundDone = true;
|
||||
engineClosed = true;
|
||||
shutdown();
|
||||
throw ENCRYPTED_PACKET_OVERSIZED;
|
||||
}
|
||||
|
||||
HandshakeStatus status = NOT_HANDSHAKING;
|
||||
// Prepare OpenSSL to work in server mode and receive handshake
|
||||
if (handshakeState != HandshakeState.FINISHED) {
|
||||
if (handshakeState != HandshakeState.STARTED_EXPLICITLY) {
|
||||
// Update accepted so we know we triggered the handshake via wrap
|
||||
handshakeState = HandshakeState.STARTED_IMPLICITLY;
|
||||
}
|
||||
int written = writeEncryptedData(src);
|
||||
if (written > 0) {
|
||||
bytesConsumed += written;
|
||||
|
||||
if (written == remaining) {
|
||||
srcsOffset ++;
|
||||
status = handshake();
|
||||
if (status == NEED_WRAP) {
|
||||
return NEED_WRAP_OK;
|
||||
}
|
||||
if (engineClosed) {
|
||||
return NEED_WRAP_CLOSED;
|
||||
}
|
||||
}
|
||||
|
||||
// Write encrypted data to network BIO
|
||||
int bytesConsumed = 0;
|
||||
if (srcsOffset < srcsEndOffset) {
|
||||
do {
|
||||
ByteBuffer src = srcs[srcsOffset];
|
||||
int remaining = src.remaining();
|
||||
if (remaining == 0) {
|
||||
// We must skip empty buffers as BIO_write will return 0 if asked to write something
|
||||
// with length 0.
|
||||
srcsOffset++;
|
||||
continue;
|
||||
}
|
||||
int written = writeEncryptedData(src);
|
||||
if (written > 0) {
|
||||
bytesConsumed += written;
|
||||
|
||||
if (written == remaining) {
|
||||
srcsOffset++;
|
||||
} else {
|
||||
// We were not able to write everything into the BIO so break the write loop as otherwise
|
||||
// we will produce an error on the next write attempt, which will trigger a SSL.clearError()
|
||||
// later.
|
||||
break;
|
||||
}
|
||||
} else {
|
||||
// We were not able to write everything into the BIO so break the write loop as otherwise
|
||||
// we will produce an error on the next write attempt, which will trigger a SSL.clearError()
|
||||
// later.
|
||||
// BIO_write returned a negative or zero number, this means we could not complete the write
|
||||
// operation and should retry later.
|
||||
// We ignore BIO_* errors here as we use in memory BIO anyway and will do another SSL_* call
|
||||
// later on in which we will produce an exception in case of an error
|
||||
SSL.clearError();
|
||||
break;
|
||||
}
|
||||
} else {
|
||||
// BIO_write returned a negative or zero number, this means we could not complete the write
|
||||
// operation and should retry later.
|
||||
// We ignore BIO_* errors here as we use in memory BIO anyway and will do another SSL_* call later
|
||||
// on in which we will produce an exception in case of an error
|
||||
SSL.clearError();
|
||||
break;
|
||||
}
|
||||
} while (srcsOffset < srcsEndOffset);
|
||||
}
|
||||
} while (srcsOffset < srcsEndOffset);
|
||||
}
|
||||
|
||||
// Number of produced bytes
|
||||
int bytesProduced = 0;
|
||||
|
||||
if (capacity > 0) {
|
||||
// Write decrypted data to dsts buffers
|
||||
int idx = dstsOffset;
|
||||
while (idx < endOffset) {
|
||||
ByteBuffer dst = dsts[idx];
|
||||
if (!dst.hasRemaining()) {
|
||||
idx ++;
|
||||
continue;
|
||||
}
|
||||
|
||||
int bytesRead = readPlaintextData(dst);
|
||||
|
||||
// TODO: We may want to consider if we move this check and only do it in a less often called place at
|
||||
// the price of not being 100% accurate, like for example when calling SSL.getError(...).
|
||||
rejectRemoteInitiatedRenegation();
|
||||
|
||||
if (bytesRead > 0) {
|
||||
bytesProduced += bytesRead;
|
||||
// Number of produced bytes
|
||||
int bytesProduced = 0;
|
||||
|
||||
if (capacity > 0) {
|
||||
// Write decrypted data to dsts buffers
|
||||
int idx = dstsOffset;
|
||||
while (idx < endOffset) {
|
||||
ByteBuffer dst = dsts[idx];
|
||||
if (!dst.hasRemaining()) {
|
||||
idx ++;
|
||||
} else {
|
||||
// We read everything return now.
|
||||
return newResult(bytesConsumed, bytesProduced, status);
|
||||
idx++;
|
||||
continue;
|
||||
}
|
||||
} else {
|
||||
int sslError = SSL.getError(ssl, bytesRead);
|
||||
switch (sslError) {
|
||||
case SSL.SSL_ERROR_ZERO_RETURN:
|
||||
// This means the connection was shutdown correctly, close inbound and outbound
|
||||
if (!receivedShutdown) {
|
||||
closeAll();
|
||||
|
||||
int bytesRead = readPlaintextData(dst);
|
||||
|
||||
// TODO: We may want to consider if we move this check and only do it in a less often called place
|
||||
// at the price of not being 100% accurate, like for example when calling SSL.getError(...).
|
||||
rejectRemoteInitiatedRenegation();
|
||||
|
||||
if (bytesRead > 0) {
|
||||
bytesProduced += bytesRead;
|
||||
|
||||
if (!dst.hasRemaining()) {
|
||||
idx++;
|
||||
} else {
|
||||
// We read everything return now.
|
||||
return newResult(bytesConsumed, bytesProduced, status);
|
||||
}
|
||||
// fall-trough!
|
||||
case SSL.SSL_ERROR_WANT_READ:
|
||||
case SSL.SSL_ERROR_WANT_WRITE:
|
||||
// break to the outer loop
|
||||
return newResult(bytesConsumed, bytesProduced, status);
|
||||
default:
|
||||
return sslReadErrorResult(SSL.getLastErrorNumber(), bytesConsumed, bytesProduced);
|
||||
} else {
|
||||
int sslError = SSL.getError(ssl, bytesRead);
|
||||
switch (sslError) {
|
||||
case SSL.SSL_ERROR_ZERO_RETURN:
|
||||
// This means the connection was shutdown correctly, close inbound and outbound
|
||||
if (!receivedShutdown) {
|
||||
closeAll();
|
||||
}
|
||||
// fall-trough!
|
||||
case SSL.SSL_ERROR_WANT_READ:
|
||||
case SSL.SSL_ERROR_WANT_WRITE:
|
||||
// break to the outer loop
|
||||
return newResult(bytesConsumed, bytesProduced, status);
|
||||
default:
|
||||
return sslReadErrorResult(SSL.getLastErrorNumber(), bytesConsumed, bytesProduced);
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// If the capacity of all destination buffers is 0 we need to trigger a SSL_read anyway to ensure
|
||||
// everything is flushed in the BIO pair and so we can detect it in the pendingAppData() call.
|
||||
if (SSL.readFromSSL(ssl, EMPTY_ADDR, 0) <= 0) {
|
||||
// We do not check SSL_get_error as we are not interested in any error that is not fatal.
|
||||
int err = SSL.getLastErrorNumber();
|
||||
if (OpenSsl.isError(err)) {
|
||||
return sslReadErrorResult(err, bytesConsumed, bytesProduced);
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// If the capacity of all destination buffers is 0 we need to trigger a SSL_read anyway to ensure
|
||||
// everything is flushed in the BIO pair and so we can detect it in the pendingAppData() call.
|
||||
if (SSL.readFromSSL(ssl, EMPTY_ADDR, 0) <= 0) {
|
||||
// We do not check SSL_get_error as we are not interested in any error that is not fatal.
|
||||
int err = SSL.getLastErrorNumber();
|
||||
if (OpenSsl.isError(err)) {
|
||||
return sslReadErrorResult(err, bytesConsumed, bytesProduced);
|
||||
}
|
||||
if (pendingAppData() > 0) {
|
||||
// We filled all buffers but there is still some data pending in the BIO buffer, return BUFFER_OVERFLOW.
|
||||
return new SSLEngineResult(
|
||||
BUFFER_OVERFLOW, mayFinishHandshake(status != FINISHED ? getHandshakeStatus() : status),
|
||||
bytesConsumed, bytesProduced);
|
||||
}
|
||||
}
|
||||
if (pendingAppData() > 0) {
|
||||
// We filled all buffers but there is still some data pending in the BIO buffer, return BUFFER_OVERFLOW.
|
||||
return new SSLEngineResult(
|
||||
BUFFER_OVERFLOW, mayFinishHandshake(status != FINISHED ? getHandshakeStatus(): status),
|
||||
bytesConsumed, bytesProduced);
|
||||
}
|
||||
|
||||
// Check to see if we received a close_notify message from the peer.
|
||||
if (!receivedShutdown && (SSL.getShutdown(ssl) & SSL.SSL_RECEIVED_SHUTDOWN) == SSL.SSL_RECEIVED_SHUTDOWN) {
|
||||
closeAll();
|
||||
}
|
||||
// Check to see if we received a close_notify message from the peer.
|
||||
if (!receivedShutdown && (SSL.getShutdown(ssl) & SSL.SSL_RECEIVED_SHUTDOWN) == SSL.SSL_RECEIVED_SHUTDOWN) {
|
||||
closeAll();
|
||||
}
|
||||
|
||||
return newResult(bytesConsumed, bytesProduced, status);
|
||||
return newResult(bytesConsumed, bytesProduced, status);
|
||||
}
|
||||
}
|
||||
|
||||
private SSLEngineResult sslReadErrorResult(int err, int bytesConsumed, int bytesProduced) throws SSLException {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user