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/16 17:28:52 UTC

[geode] branch develop 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 develop
in repository https://gitbox.apache.org/repos/asf/geode.git


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

commit b1107d2e403404337c22830a4964eefc2490ef50
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
---
 .../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 2dedc20..7981d3c 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
@@ -546,10 +546,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;
   }