cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ro...@apache.org
Subject [cloudstack] 05/24: CLOUDSTACK-9348: Improve Nio SSH handshake buffers
Date Thu, 21 Dec 2017 12:49:45 GMT
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 204ea6a2cf7de6b80d8d26f74fa29407f1afaa40
Author: Rohit Yadav <rohit.yadav@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 <rohit.yadav@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" <commits@cloudstack.apache.org>.

Mime
View raw message