Skip to content

Commit

Permalink
[#4235] Ensure OpenSslEngine.unwrap(...) / wrap(...) correctly return…
Browse files Browse the repository at this point in the history 
… HandshakeStatus.FINISHED

Motivation:

OpenSslEngine.unwrap(...) / wrap(...) must return HandhsakeStatus.FINISHED if an unwrap or wrap finishes a handshake to behave like descripted in the SSLEngine docs.

Modifications:

- Ensure we return HandshakeStatus.FINISHED

Result:

Behave correctly.
  • Loading branch information
@normanmaurer
normanmaurer committed Sep 24, 2015
1 parent 042cb5f commit fc6a9e0
Showing 1 changed file with 22 additions and 15 deletions.
37 changes: 22 additions & 15 deletions handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java
View file
Expand Up @@ -401,16 +401,17 @@ private int readEncryptedData(final ByteBuffer dst, final int pending) {
return bioRead;
}

private SSLEngineResult readPendingBytesFromBIO(ByteBuffer dst, int bytesConsumed, int bytesProduced)
throws SSLException {
private SSLEngineResult readPendingBytesFromBIO(
ByteBuffer dst, int bytesConsumed, int bytesProduced, HandshakeStatus status) throws SSLException {
// Check to see if the engine wrote data into the network BIO
int pendingNet = SSL.pendingWrittenBytesInBIO(networkBIO);
if (pendingNet > 0) {

// Do we have enough room in dst to write encrypted data?
int capacity = dst.remaining();
if (capacity < pendingNet) {
return new SSLEngineResult(BUFFER_OVERFLOW, mayFinishHandshake(getHandshakeStatus(pendingNet)),
return new SSLEngineResult(BUFFER_OVERFLOW,
mayFinishHandshake(status != FINISHED ? getHandshakeStatus(pendingNet) : status),
bytesConsumed, bytesProduced);
}

Expand All @@ -432,7 +433,8 @@ private SSLEngineResult readPendingBytesFromBIO(ByteBuffer dst, int bytesConsume
shutdown();
}

return new SSLEngineResult(getEngineStatus(), mayFinishHandshake(getHandshakeStatus(pendingNet)),
return new SSLEngineResult(getEngineStatus(),
mayFinishHandshake(status != FINISHED ? getHandshakeStatus(pendingNet) : status),
bytesConsumed, bytesProduced);
}
return null;
Expand Down Expand Up @@ -465,14 +467,15 @@ public synchronized SSLEngineResult wrap(
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;
}

HandshakeStatus status = handshake();
status = handshake();
if (status == NEED_UNWRAP) {
return NEED_UNWRAP_OK;
}
Expand Down Expand Up @@ -517,7 +520,7 @@ public synchronized SSLEngineResult wrap(
}
}

SSLEngineResult pendingNetResult = readPendingBytesFromBIO(dst, bytesConsumed, bytesProduced);
SSLEngineResult pendingNetResult = readPendingBytesFromBIO(dst, bytesConsumed, bytesProduced, status);
if (pendingNetResult != null) {
return pendingNetResult;
}
Expand All @@ -526,13 +529,13 @@ public synchronized SSLEngineResult wrap(
// 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);
SSLEngineResult pendingNetResult = readPendingBytesFromBIO(dst, 0, bytesProduced, status);
if (pendingNetResult != null) {
return pendingNetResult;
}
}

return newResult(bytesConsumed, bytesProduced);
return newResult(bytesConsumed, bytesProduced, status);
}

private void checkPendingHandshakeException() throws SSLHandshakeException {
Expand Down Expand Up @@ -605,14 +608,15 @@ public synchronized SSLEngineResult unwrap(
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;
}

HandshakeStatus status = handshake();
status = handshake();
if (status == NEED_WRAP) {
return NEED_WRAP_OK;
}
Expand Down Expand Up @@ -703,7 +707,7 @@ public synchronized SSLEngineResult unwrap(
idx ++;
} else {
// We read everything return now.
return newResult(bytesConsumed, bytesProduced);
return newResult(bytesConsumed, bytesProduced, status);
}
} else {
int sslError = SSL.getError(ssl, bytesRead);
Expand All @@ -717,7 +721,7 @@ public synchronized SSLEngineResult unwrap(
case SSL.SSL_ERROR_WANT_READ:
case SSL.SSL_ERROR_WANT_WRITE:
// break to the outer loop
return newResult(bytesConsumed, bytesProduced);
return newResult(bytesConsumed, bytesProduced, status);
default:
// Everything else is considered as error so shutdown and throw an exceptions
shutdownWithError("SSL_read");
Expand All @@ -738,15 +742,16 @@ public synchronized SSLEngineResult unwrap(
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(getHandshakeStatus()), bytesConsumed, bytesProduced);
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();
}

return newResult(bytesConsumed, bytesProduced);
return newResult(bytesConsumed, bytesProduced, status);
}

private int pendingAppData() {
Expand All @@ -755,9 +760,11 @@ private int pendingAppData() {
return handshakeState == HandshakeState.FINISHED ? SSL.pendingReadableBytesInSSL(ssl) : 0;
}

private SSLEngineResult newResult(int bytesConsumed, int bytesProduced) throws SSLException {
private SSLEngineResult newResult(
int bytesConsumed, int bytesProduced, HandshakeStatus status) throws SSLException {
return new SSLEngineResult(
getEngineStatus(), mayFinishHandshake(getHandshakeStatus()), bytesConsumed, bytesProduced);
getEngineStatus(), mayFinishHandshake(status != FINISHED ? getHandshakeStatus() : status)
, bytesConsumed, bytesProduced);
}

private void closeAll() throws SSLException {
Expand Down

0 comments on commit fc6a9e0

Please sign in to comment.