diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java index c65fbea484..f47693d4b0 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java @@ -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 {