You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bs...@apache.org on 2020/06/22 15:02:53 UTC

[geode] branch support/1.13 updated: GEODE-8144: setting SNI server name is not needed if endpoint verification is disabled (#5250)

This is an automated email from the ASF dual-hosted git repository.

bschuchardt pushed a commit to branch support/1.13
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/support/1.13 by this push:
     new 85a42d5  GEODE-8144: setting SNI server name is not needed if endpoint verification is disabled (#5250)
85a42d5 is described below

commit 85a42d5213d784373772e6e39a1f273838b049a4
Author: Bruce Schuchardt <bs...@pivotal.io>
AuthorDate: Tue Jun 16 10:27:59 2020 -0700

    GEODE-8144: setting SNI server name is not needed if endpoint verification is disabled (#5250)
    
    * GEODE-8144: endpoint identification in servers is not working
    
    modified the fix for this issue to not set the SNI server name parameter
    if endpoint verification is disabled.  We're doing this because setting
    this parameter appears to decrease performance in large performance
    tests.
    
    * changed test to throw exceptions instead of asserting they don't exist
    
    * replaced check for SNI server name in SSL parameters with a more in-depth check
    
    * SSLParameters.getServerNames() may return a null value
    
    (cherry picked from commit b1107d2e403404337c22830a4964eefc2490ef50)
---
 .../internal/net/SSLSocketIntegrationTest.java     | 29 +++++++++++++++++-----
 .../apache/geode/internal/net/SocketCreator.java   | 10 +++++---
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/geode-core/src/integrationTest/java/org/apache/geode/internal/net/SSLSocketIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/internal/net/SSLSocketIntegrationTest.java
index f1d7397..4800940 100755
--- a/geode-core/src/integrationTest/java/org/apache/geode/internal/net/SSLSocketIntegrationTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/internal/net/SSLSocketIntegrationTest.java
@@ -17,6 +17,7 @@ package org.apache.geode.internal.net;
 import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_CIPHERS;
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_ENABLED_COMPONENTS;
+import static org.apache.geode.distributed.ConfigurationProperties.SSL_ENDPOINT_IDENTIFICATION_ENABLED;
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE;
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_KEYSTORE_PASSWORD;
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_PROTOCOLS;
@@ -32,7 +33,6 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
 import static org.mockito.Mockito.mock;
 
 import java.io.DataInputStream;
@@ -52,13 +52,17 @@ import java.net.URL;
 import java.nio.ByteBuffer;
 import java.nio.channels.ServerSocketChannel;
 import java.nio.channels.SocketChannel;
+import java.util.List;
 import java.util.Properties;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 
+import javax.net.ssl.SNIServerName;
 import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLException;
+import javax.net.ssl.StandardConstants;
 
 import org.apache.commons.io.FileUtils;
 import org.junit.After;
@@ -130,6 +134,7 @@ public class SSLSocketIntegrationTest {
     properties.setProperty(SSL_TRUSTSTORE, keystore.getCanonicalPath());
     properties.setProperty(SSL_TRUSTSTORE_PASSWORD, "password");
     properties.setProperty(SSL_REQUIRE_AUTHENTICATION, "true");
+    properties.setProperty(SSL_ENDPOINT_IDENTIFICATION_ENABLED, "false");
     properties.setProperty(SSL_CIPHERS, "any");
     properties.setProperty(SSL_PROTOCOLS, "TLSv1.2");
 
@@ -176,7 +181,7 @@ public class SSLSocketIntegrationTest {
   }
 
   @Test
-  public void securedSocketTransmissionShouldWork() throws Exception {
+  public void securedSocketTransmissionShouldWork() throws Throwable {
     this.serverSocket = this.socketCreator.forCluster().createServerSocket(0, 0, this.localHost);
     this.serverThread = startServer(this.serverSocket, 15000);
 
@@ -194,12 +199,14 @@ public class SSLSocketIntegrationTest {
     await().until(() -> {
       return !serverThread.isAlive();
     });
-    assertNull(serverException);
+    if (serverException != null) {
+      throw serverException;
+    }
     assertThat(this.messageFromClient.get()).isEqualTo(MESSAGE);
   }
 
   @Test
-  public void testSecuredSocketTransmissionShouldWorkUsingNIO() throws Exception {
+  public void testSecuredSocketTransmissionShouldWorkUsingNIO() throws Throwable {
     ServerSocketChannel serverChannel = ServerSocketChannel.open();
     serverSocket = serverChannel.socket();
 
@@ -232,7 +239,9 @@ public class SSLSocketIntegrationTest {
     await().until(() -> {
       return !serverThread.isAlive();
     });
-    assertNull(serverException);
+    if (serverException != null) {
+      throw serverException;
+    }
     // assertThat(this.messageFromClient.get()).isEqualTo(MESSAGE);
   }
 
@@ -264,12 +273,20 @@ public class SSLSocketIntegrationTest {
 
         socket = serverSocket.accept();
         SocketCreator sc = SocketCreatorFactory.getSocketCreatorForComponent(CLUSTER);
+        final SSLEngine sslEngine = sc.createSSLEngine("localhost", 1234);
         engine =
-            sc.handshakeSSLSocketChannel(socket.getChannel(), sc.createSSLEngine("localhost", 1234),
+            sc.handshakeSSLSocketChannel(socket.getChannel(), sslEngine,
                 timeoutMillis,
                 false,
                 ByteBuffer.allocate(65535),
                 new BufferPool(mock(DMStats.class)));
+        final List<SNIServerName> serverNames = sslEngine.getSSLParameters().getServerNames();
+        if (serverNames != null && serverNames.stream()
+            .mapToInt(SNIServerName::getType)
+            .anyMatch(type -> type == StandardConstants.SNI_HOST_NAME)) {
+          serverException = new AssertionError("found SNI server name in SSL Parameters");
+          return;
+        }
 
         readMessageFromNIOSSLClient(socket, buffer, engine);
         readMessageFromNIOSSLClient(socket, buffer, engine);
diff --git a/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java b/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
index 9b7c54a..1a9a0d3 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
@@ -547,10 +547,12 @@ public class SocketCreator extends TcpSocketCreatorImpl {
    */
   public SSLEngine createSSLEngine(String hostName, int port) {
     SSLEngine engine = getSslContext().createSSLEngine(hostName, port);
-    SSLParameters parameters = engine.getSSLParameters();
-    // set server-names so that endpoint identification algorithms can find what's expected
-    if (setServerNames(parameters, new HostAndPort(hostName, port))) {
-      engine.setSSLParameters(parameters);
+    if (sslConfig.doEndpointIdentification()) {
+      // set server-names so that endpoint identification algorithms can find what's expected
+      SSLParameters parameters = engine.getSSLParameters();
+      if (setServerNames(parameters, new HostAndPort(hostName, port))) {
+        engine.setSSLParameters(parameters);
+      }
     }
     return engine;
   }