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/feature/GEODE-2900 Commit: 810186d4f01d7d35381f66e00c03e9ab12f0281a Parents: 014ad42 Author: Bruce Schuchardt <bschucha...@pivotal.io> Authored: Wed May 10 15:24:32 2017 -0700 Committer: Bruce Schuchardt <bschucha...@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);
