You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ro...@apache.org on 2017/12/13 10:43:19 UTC
[cloudstack] 05/30: CLOUDSTACK-9348: Improve Nio SSH handshake
buffers
This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch debian9-systemvmtemplate
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 6f1c2e07431c7ecc024dfecc74964ae2cb2cf6d4
Author: Rohit Yadav <ro...@shapeblue.com>
AuthorDate: Thu Nov 30 16:51:48 2017 +0530
CLOUDSTACK-9348: Improve Nio SSH handshake buffers
Use a holder class to pass buffers, fixes potential leak.
Signed-off-by: Rohit Yadav <ro...@shapeblue.com>
---
.../agent/manager/ClusteredAgentManagerImpl.java | 2 +-
utils/src/main/java/com/cloud/utils/nio/Link.java | 72 +++++++++++++++++-----
.../main/java/com/cloud/utils/nio/NioClient.java | 2 +-
.../java/com/cloud/utils/nio/NioConnection.java | 2 +-
4 files changed, 58 insertions(+), 20 deletions(-)
diff --git a/engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentManagerImpl.java b/engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentManagerImpl.java
index 2ebfeb5..0b9899e 100644
--- a/engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentManagerImpl.java
+++ b/engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentManagerImpl.java
@@ -519,7 +519,7 @@ public class ClusteredAgentManagerImpl extends AgentManagerImpl implements Clust
sslEngine.setUseClientMode(true);
sslEngine.setEnabledProtocols(SSLUtils.getSupportedProtocols(sslEngine.getEnabledProtocols()));
sslEngine.beginHandshake();
- if (!Link.doHandshake(ch1, sslEngine, true)) {
+ if (!Link.doHandshake(ch1, sslEngine)) {
ch1.close();
throw new IOException(String.format("SSL: Handshake failed with peer management server '%s' on %s:%d ", peerName, ip, port));
}
diff --git a/utils/src/main/java/com/cloud/utils/nio/Link.java b/utils/src/main/java/com/cloud/utils/nio/Link.java
index 8f1b811..35211c8 100644
--- a/utils/src/main/java/com/cloud/utils/nio/Link.java
+++ b/utils/src/main/java/com/cloud/utils/nio/Link.java
@@ -32,6 +32,8 @@ import java.security.GeneralSecurityException;
import java.security.KeyStore;
import java.security.SecureRandom;
import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.Executor;
+import java.util.concurrent.Executors;
import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.SSLContext;
@@ -462,7 +464,7 @@ public class Link {
return buffer;
}
- public static ByteBuffer handleBufferUnderflow(final SSLEngine engine, ByteBuffer buffer) {
+ public static ByteBuffer handleBufferUnderflow(final SSLEngine engine, final ByteBuffer buffer) {
if (engine == null || buffer == null) {
return buffer;
}
@@ -475,14 +477,14 @@ public class Link {
return replaceBuffer;
}
- private static boolean doHandshakeUnwrap(final SocketChannel socketChannel, final SSLEngine sslEngine,
+ private static HandshakeHolder doHandshakeUnwrap(final SocketChannel socketChannel, final SSLEngine sslEngine,
ByteBuffer peerAppData, ByteBuffer peerNetData, final int appBufferSize) throws IOException {
if (socketChannel == null || sslEngine == null || peerAppData == null || peerNetData == null || appBufferSize < 0) {
- return false;
+ return new HandshakeHolder(peerAppData, peerNetData, false);
}
if (socketChannel.read(peerNetData) < 0) {
if (sslEngine.isInboundDone() && sslEngine.isOutboundDone()) {
- return false;
+ return new HandshakeHolder(peerAppData, peerNetData, false);
}
try {
sslEngine.closeInbound();
@@ -492,7 +494,7 @@ public class Link {
sslEngine.closeOutbound();
// After closeOutbound the engine will be set to WRAP state,
// in order to try to send a close message to the client.
- return true;
+ return new HandshakeHolder(peerAppData, peerNetData, true);
}
peerNetData.flip();
SSLEngineResult result = null;
@@ -503,7 +505,10 @@ public class Link {
s_logger.error(String.format("SSL error caught during unwrap data: %s, for local address=%s, remote address=%s. The client may have invalid ca-certificates.",
sslException.getMessage(), socketChannel.getLocalAddress(), socketChannel.getRemoteAddress()));
sslEngine.closeOutbound();
- return false;
+ return new HandshakeHolder(peerAppData, peerNetData, true);
+ }
+ if (result == null) {
+ return new HandshakeHolder(peerAppData, peerNetData, false);
}
switch (result.getStatus()) {
case OK:
@@ -519,23 +524,23 @@ public class Link {
break;
case CLOSED:
if (sslEngine.isOutboundDone()) {
- return false;
+ return new HandshakeHolder(peerAppData, peerNetData, false);
} else {
sslEngine.closeOutbound();
- break;
}
+ break;
default:
throw new IllegalStateException("Invalid SSL status: " + result.getStatus());
}
- return true;
+ return new HandshakeHolder(peerAppData, peerNetData, true);
}
- private static boolean doHandshakeWrap(final SocketChannel socketChannel, final SSLEngine sslEngine,
+ private static HandshakeHolder doHandshakeWrap(final SocketChannel socketChannel, final SSLEngine sslEngine,
ByteBuffer myAppData, ByteBuffer myNetData, ByteBuffer peerNetData,
final int netBufferSize) throws IOException {
if (socketChannel == null || sslEngine == null || myNetData == null || peerNetData == null
|| myAppData == null || netBufferSize < 0) {
- return false;
+ return new HandshakeHolder(myAppData, myNetData, false);
}
myNetData.clear();
SSLEngineResult result = null;
@@ -545,7 +550,10 @@ public class Link {
s_logger.error(String.format("SSL error caught during wrap data: %s, for local address=%s, remote address=%s.",
sslException.getMessage(), socketChannel.getLocalAddress(), socketChannel.getRemoteAddress()));
sslEngine.closeOutbound();
- return false;
+ return new HandshakeHolder(myAppData, myNetData, true);
+ }
+ if (result == null) {
+ return new HandshakeHolder(myAppData, myNetData, false);
}
switch (result.getStatus()) {
case OK :
@@ -579,10 +587,10 @@ public class Link {
default:
throw new IllegalStateException("Invalid SSL status: " + result.getStatus());
}
- return true;
+ return new HandshakeHolder(myAppData, myNetData, true);
}
- public static boolean doHandshake(final SocketChannel socketChannel, final SSLEngine sslEngine, final boolean isClient) throws IOException {
+ public static boolean doHandshake(final SocketChannel socketChannel, final SSLEngine sslEngine) throws IOException {
if (socketChannel == null || sslEngine == null) {
return false;
}
@@ -593,6 +601,7 @@ public class Link {
ByteBuffer myNetData = ByteBuffer.allocate(netBufferSize);
ByteBuffer peerNetData = ByteBuffer.allocate(netBufferSize);
+ final Executor executor = Executors.newSingleThreadExecutor();
final long startTimeMills = System.currentTimeMillis();
HandshakeStatus handshakeStatus = sslEngine.getHandshakeStatus();
@@ -606,12 +615,17 @@ public class Link {
}
switch (handshakeStatus) {
case NEED_UNWRAP:
- if (!doHandshakeUnwrap(socketChannel, sslEngine, peerAppData, peerNetData, appBufferSize)) {
+ final HandshakeHolder unwrapResult = doHandshakeUnwrap(socketChannel, sslEngine, peerAppData, peerNetData, appBufferSize);
+ peerAppData = unwrapResult.getAppDataBuffer();
+ peerNetData = unwrapResult.getNetDataBuffer();
+ if (!unwrapResult.isSuccess()) {
return false;
}
break;
case NEED_WRAP:
- if (!doHandshakeWrap(socketChannel, sslEngine, myAppData, myNetData, peerNetData, netBufferSize)) {
+ final HandshakeHolder wrapResult = doHandshakeWrap(socketChannel, sslEngine, myAppData, myNetData, peerNetData, netBufferSize);
+ myNetData = wrapResult.getNetDataBuffer();
+ if (!wrapResult.isSuccess()) {
return false;
}
break;
@@ -621,7 +635,7 @@ public class Link {
if (s_logger.isTraceEnabled()) {
s_logger.trace("SSL: Running delegated task!");
}
- task.run();
+ executor.execute(task);
}
break;
case FINISHED:
@@ -636,4 +650,28 @@ public class Link {
return true;
}
+ private static class HandshakeHolder {
+ private ByteBuffer appData;
+ private ByteBuffer netData;
+ private boolean success = true;
+
+ HandshakeHolder(ByteBuffer appData, ByteBuffer netData, boolean success) {
+ this.appData = appData;
+ this.netData = netData;
+ this.success = success;
+ }
+
+ ByteBuffer getAppDataBuffer() {
+ return appData;
+ }
+
+ ByteBuffer getNetDataBuffer() {
+ return netData;
+ }
+
+ boolean isSuccess() {
+ return success;
+ }
+ }
+
}
diff --git a/utils/src/main/java/com/cloud/utils/nio/NioClient.java b/utils/src/main/java/com/cloud/utils/nio/NioClient.java
index 1c29b0c..d4a1e02 100644
--- a/utils/src/main/java/com/cloud/utils/nio/NioClient.java
+++ b/utils/src/main/java/com/cloud/utils/nio/NioClient.java
@@ -61,7 +61,7 @@ public class NioClient extends NioConnection {
sslEngine.setUseClientMode(true);
sslEngine.setEnabledProtocols(SSLUtils.getSupportedProtocols(sslEngine.getEnabledProtocols()));
sslEngine.beginHandshake();
- if (!Link.doHandshake(_clientConnection, sslEngine, true)) {
+ if (!Link.doHandshake(_clientConnection, sslEngine)) {
s_logger.error("SSL Handshake failed while connecting to host: " + _host + " port: " + _port);
_selector.close();
throw new IOException("SSL Handshake failed while connecting to host: " + _host + " port: " + _port);
diff --git a/utils/src/main/java/com/cloud/utils/nio/NioConnection.java b/utils/src/main/java/com/cloud/utils/nio/NioConnection.java
index 30000cf..9a5bf7e 100644
--- a/utils/src/main/java/com/cloud/utils/nio/NioConnection.java
+++ b/utils/src/main/java/com/cloud/utils/nio/NioConnection.java
@@ -213,7 +213,7 @@ public abstract class NioConnection implements Callable<Boolean> {
_selector.wakeup();
try {
sslEngine.beginHandshake();
- if (!Link.doHandshake(socketChannel, sslEngine, false)) {
+ if (!Link.doHandshake(socketChannel, sslEngine)) {
throw new IOException("SSL handshake timed out with " + socketChannel.getRemoteAddress());
}
if (s_logger.isTraceEnabled()) {
--
To stop receiving notification emails like this one, please contact
"commits@cloudstack.apache.org" <co...@cloudstack.apache.org>.