geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bschucha...@apache.org
Subject geode git commit: GEODE-2898 A non-responsive SSL client can block a server's "acceptor" thread
Date Wed, 10 May 2017 22:26:19 GMT
Repository: geode
Updated Branches:
  refs/heads/develop 014ad426a -> 810186d4f


GEODE-2898 A non-responsive SSL client can block a server's "acceptor" thread

This moves the SSL handshake from the acceptor thread to the handshake thread
and adds a unit test.


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/810186d4
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/810186d4
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/810186d4

Branch: refs/heads/develop
Commit: 810186d4f01d7d35381f66e00c03e9ab12f0281a
Parents: 014ad42
Author: Bruce Schuchardt <bschuchardt@pivotal.io>
Authored: Wed May 10 15:24:32 2017 -0700
Committer: Bruce Schuchardt <bschuchardt@pivotal.io>
Committed: Wed May 10 15:24:32 2017 -0700

----------------------------------------------------------------------
 .../cache/tier/sockets/AcceptorImpl.java        |  2 +-
 .../CacheServerSSLConnectionDUnitTest.java      | 73 ++++++++++++++++++++
 2 files changed, 74 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/810186d4/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
index ed29472..0fc72d3 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
@@ -1235,7 +1235,6 @@ public class AcceptorImpl extends Acceptor implements Runnable {
             break;
           }
         }
-        this.socketCreator.configureServerSSLSocket(s);
         this.loggedAcceptError = false;
 
         handOffNewClientConnection(s);
@@ -1408,6 +1407,7 @@ public class AcceptorImpl extends Acceptor implements Runnable {
       }
     } else {
       s.setSoTimeout(this.acceptTimeout);
+      this.socketCreator.configureServerSSLSocket(s);
       communicationMode = (byte) s.getInputStream().read();
       if (logger.isTraceEnabled()) {
         logger.trace("read communications mode(2) ", communicationMode);

http://git-wip-us.apache.org/repos/asf/geode/blob/810186d4/geode-core/src/test/java/org/apache/geode/cache/client/internal/CacheServerSSLConnectionDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/cache/client/internal/CacheServerSSLConnectionDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/cache/client/internal/CacheServerSSLConnectionDUnitTest.java
index 3597152..2b93488 100644
--- a/geode-core/src/test/java/org/apache/geode/cache/client/internal/CacheServerSSLConnectionDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/cache/client/internal/CacheServerSSLConnectionDUnitTest.java
@@ -20,7 +20,11 @@ import static org.junit.Assert.*;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.io.StringWriter;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.sql.Time;
 import java.util.Properties;
+import java.util.concurrent.TimeUnit;
 
 import org.apache.geode.cache.Cache;
 import org.apache.geode.cache.CacheFactory;
@@ -34,6 +38,7 @@ import org.apache.geode.cache.client.ClientRegionShortcut;
 import org.apache.geode.cache.server.CacheServer;
 import org.apache.geode.internal.security.SecurableCommunicationChannel;
 import org.apache.geode.security.AuthenticationRequiredException;
+import org.apache.geode.test.dunit.AsyncInvocation;
 import org.apache.geode.test.dunit.Host;
 import org.apache.geode.test.dunit.IgnoredException;
 import org.apache.geode.test.dunit.VM;
@@ -41,6 +46,7 @@ import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
 import org.apache.geode.test.junit.categories.ClientServerTest;
 import org.apache.geode.test.junit.categories.DistributedTest;
 import org.apache.geode.util.test.TestUtil;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
@@ -298,6 +304,73 @@ public class CacheServerSSLConnectionDUnitTest extends JUnit4DistributedTestCase
     serverVM.invoke(() -> doServerRegionTestTask());
   }
 
+  /**
+   * GEODE-2898: A non-responsive SSL client can block a server's "acceptor" thread
+   * <p>
+   * Start a server and then connect to it without completing the SSL handshake
+   * </p>
+   * <p>
+   * Attempt to connect to the server using a real SSL client, demonstrating that the server
is not
+   * blocked and can process the new connection request.
+   * </p>
+   */
+  @Test
+  public void clientSlowToHandshakeDoesNotBlockServer() throws Throwable {
+    final Host host = Host.getHost(0);
+    VM serverVM = host.getVM(1);
+    VM clientVM = host.getVM(2);
+    VM slowClientVM = host.getVM(3);
+    getBlackboard().initBlackboard();
+
+    // a plain-text socket is used to connect to an ssl server & the handshake
+    // is never performed. The server will log this exception & it should be ignored
+    IgnoredException.addIgnoredException("javax.net.ssl.SSLHandshakeException", serverVM);
+
+    boolean cacheServerSslenabled = true;
+    boolean cacheClientSslenabled = true;
+    boolean cacheClientSslRequireAuth = true;
+
+    serverVM.invoke(() -> setUpServerVMTask(cacheServerSslenabled, true));
+    int port = serverVM.invoke(() -> createServerTask());
+
+    String hostName = host.getHostName();
+
+    AsyncInvocation slowAsync = slowClientVM.invokeAsync(() -> connectToServer(hostName,
port));
+    try {
+      getBlackboard().waitForGate("serverIsBlocked", 60, TimeUnit.SECONDS);
+
+      clientVM.invoke(() -> setUpClientVMTask(hostName, port, cacheClientSslenabled,
+          cacheClientSslRequireAuth, CLIENT_KEY_STORE, CLIENT_TRUST_STORE));
+      clientVM.invoke(() -> doClientRegionTestTask());
+      serverVM.invoke(() -> doServerRegionTestTask());
+
+    } finally {
+      getBlackboard().signalGate("testIsCompleted");
+      try {
+        if (slowAsync.isAlive()) {
+          slowAsync.join(60000);
+        }
+        if (slowAsync.exceptionOccurred()) {
+          throw slowAsync.getException();
+        }
+      } finally {
+        assertFalse(slowAsync.isAlive());
+      }
+    }
+
+  }
+
+  private void connectToServer(String hostName, int port) throws Exception {
+    Socket sock = new Socket();
+    sock.connect(new InetSocketAddress(hostName, port));
+    try {
+      getBlackboard().signalGate("serverIsBlocked");
+      getBlackboard().waitForGate("testIsCompleted", 60, TimeUnit.SECONDS);
+    } finally {
+      sock.close();
+    }
+  }
+
   @Test
   public void testNonSSLClient() throws Exception {
     final Host host = Host.getHost(0);


Mime
View raw message