You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by jb...@apache.org on 2022/02/08 23:19:04 UTC

[geode] branch support/1.15 updated: GEODE-10015: Set java.rmi.server.hostname when SSL enabled. (#7350)

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

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


The following commit(s) were added to refs/heads/support/1.15 by this push:
     new dde5dba  GEODE-10015: Set java.rmi.server.hostname when SSL enabled. (#7350)
dde5dba is described below

commit dde5dbafc7f8e4a830a431997a264709372151ac
Author: Jacob Barrett <jb...@pivotal.io>
AuthorDate: Tue Feb 8 15:15:05 2022 -0800

    GEODE-10015: Set java.rmi.server.hostname when SSL enabled. (#7350)
    
    When SSL is enabled we set the java.rmi.server.hostname System Property
    to force creation of endpoints bound to that address rather than the
    default, which is the local IP address. When RMI clients make
    connections for these endpoints they will use the correct hostname,
    which should match the hostname in the SSL certificate.
    
    (cherry picked from commit 1da903901bf4bb4d339d83a244051eaf58d8b690)
---
 .../src/test/resources/expected-pom.xml            |   5 +
 .../gradle/plugins/DependencyConstraints.groovy    |   1 +
 geode-core/build.gradle                            |   1 +
 .../internal/ManagementAgentIntegrationTest.java   | 161 ++++++++++--------
 .../geode/management/internal/ManagementAgent.java | 117 +++++++++-----
 .../sanctioned-geode-core-serializables.txt        |   1 -
 .../management/internal/ManagementAgentTest.java   |  60 +++++++
 geode-gfsh/build.gradle                            |  13 ++
 .../geode/gfsh/GfshWithSslAcceptanceTest.java      | 153 ++++++++++++++++++
 .../shell/JmxOperationInvokerIntegrationTest.java  | 179 +++++++++++++++++++++
 .../org/apache/geode/cache/ssl/CertStores.java     |   3 +-
 11 files changed, 582 insertions(+), 112 deletions(-)

diff --git a/boms/geode-all-bom/src/test/resources/expected-pom.xml b/boms/geode-all-bom/src/test/resources/expected-pom.xml
index f4e03f7..e354465 100644
--- a/boms/geode-all-bom/src/test/resources/expected-pom.xml
+++ b/boms/geode-all-bom/src/test/resources/expected-pom.xml
@@ -473,6 +473,11 @@
         <version>1.4.01</version>
       </dependency>
       <dependency>
+        <groupId>org.junit-pioneer</groupId>
+        <artifactId>junit-pioneer</artifactId>
+        <version>1.5.0</version>
+      </dependency>
+      <dependency>
         <groupId>com.fasterxml.jackson.core</groupId>
         <artifactId>jackson-annotations</artifactId>
         <version>2.13.1</version>
diff --git a/buildSrc/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy b/buildSrc/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy
index 57af729..27879e2 100644
--- a/buildSrc/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy
+++ b/buildSrc/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy
@@ -175,6 +175,7 @@ class DependencyConstraints implements Plugin<Project> {
         api(group: 'redis.clients', name: 'jedis', version: '3.6.3')
         api(group: 'xerces', name: 'xercesImpl', version: '2.12.0')
         api(group: 'xml-apis', name: 'xml-apis', version: '1.4.01')
+        api(group: 'org.junit-pioneer', name: 'junit-pioneer', version: '1.5.0')
       }
     }
 
diff --git a/geode-core/build.gradle b/geode-core/build.gradle
index 3cfc1cf..1ac2e0b 100755
--- a/geode-core/build.gradle
+++ b/geode-core/build.gradle
@@ -364,6 +364,7 @@ dependencies {
   integrationTestImplementation('org.apache.logging.log4j:log4j-core')
   integrationTestImplementation('pl.pragmatists:JUnitParams')
   integrationTestImplementation('com.tngtech.archunit:archunit-junit4')
+  integrationTestImplementation('org.junit-pioneer:junit-pioneer')
 
   integrationTestRuntimeOnly('org.apache.derby:derby')
   integrationTestRuntimeOnly('xerces:xercesImpl')
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/ManagementAgentIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/ManagementAgentIntegrationTest.java
index 257454d9..9c8e21a 100644
--- a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/ManagementAgentIntegrationTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/ManagementAgentIntegrationTest.java
@@ -12,15 +12,20 @@
  * or implied. See the License for the specific language governing permissions and limitations under
  * the License.
  */
+
 package org.apache.geode.management.internal;
 
+import static org.apache.geode.management.internal.ManagementAgent.JAVA_RMI_SERVER_HOSTNAME;
+import static org.apache.geode.management.internal.ManagementAgent.setRmiServerHostname;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-import java.io.File;
 import java.io.IOException;
 import java.lang.management.ManagementFactory;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -28,96 +33,118 @@ import javax.management.MBeanServer;
 import javax.management.remote.JMXConnectorServer;
 import javax.management.remote.JMXServiceURL;
 
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.junitpioneer.jupiter.ClearSystemProperty;
 
 import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.security.SecurityService;
 import org.apache.geode.internal.serialization.filter.FilterConfiguration;
 
-public class ManagementAgentIntegrationTest {
+class ManagementAgentIntegrationTest {
+
+  private static final String A_HOST_NAME = "a.host.name";
 
-  @Rule
-  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+  @TempDir
+  Path temporaryFolder;
 
   @Test
-  public void testSetMBeanServer() throws IOException {
-    DistributionConfig distributionConfig = mock(DistributionConfig.class);
-    InternalCache internalCache = mock(InternalCache.class);
-    FilterConfiguration filterConfiguration = mock(FilterConfiguration.class);
-    SecurityService securityService = mock(SecurityService.class);
+  void testSetMBeanServer() throws IOException {
+    final DistributionConfig distributionConfig = mock(DistributionConfig.class);
+    final InternalCache internalCache = mock(InternalCache.class);
+    final FilterConfiguration filterConfiguration = mock(FilterConfiguration.class);
+    final SecurityService securityService = mock(SecurityService.class);
     when(internalCache.getSecurityService()).thenReturn(securityService);
     when(securityService.isIntegratedSecurity()).thenReturn(false);
-    File tempFile = temporaryFolder.newFile("testFile");
-    when(distributionConfig.getJmxManagerAccessFile()).thenReturn(tempFile.getCanonicalPath());
-    ManagementAgent managementAgent =
+    final Path tempFile = Files.createFile(temporaryFolder.resolve("testFile")).toAbsolutePath();
+    when(distributionConfig.getJmxManagerAccessFile()).thenReturn(tempFile.toString());
+    final ManagementAgent managementAgent =
         new ManagementAgent(distributionConfig, internalCache, filterConfiguration);
-    MBeanServer mBeanServer = mock(MBeanServer.class);
-    JMXConnectorServer jmxConnectorServerWithMBeanServer = new JMXConnectorServer(mBeanServer) {
-      @Override
-      public void start() throws IOException {
-
-      }
-
-      @Override
-      public void stop() throws IOException {
-
-      }
-
-      @Override
-      public boolean isActive() {
-        return false;
-      }
-
-      @Override
-      public JMXServiceURL getAddress() {
-        return null;
-      }
-
-      @Override
-      public Map<String, ?> getAttributes() {
-        return null;
-      }
-    };
-    managementAgent.setJmxConnectorServer(jmxConnectorServerWithMBeanServer);
+    final MBeanServer mBeanServer = mock(MBeanServer.class);
+
+    managementAgent.setJmxConnectorServer(new JmxConnectorServerWithMBeanServer(mBeanServer));
     assertThatCode(() -> managementAgent
         .setMBeanServerForwarder(ManagementFactory.getPlatformMBeanServer(), new HashMap<>()))
             .doesNotThrowAnyException();
+
     managementAgent.setMBeanServerForwarder(ManagementFactory.getPlatformMBeanServer(),
         new HashMap<>());
 
-    JMXConnectorServer jmxConnectorServerNullMBeanServer = new JMXConnectorServer() {
-      @Override
-      public void start() throws IOException {
+    managementAgent.setJmxConnectorServer(new JmxConnectorServerNullMBeanServer());
+    assertThatCode(() -> managementAgent
+        .setMBeanServerForwarder(ManagementFactory.getPlatformMBeanServer(), new HashMap<>()))
+            .doesNotThrowAnyException();
+  }
 
-      }
+  @Test
+  @ClearSystemProperty(key = JAVA_RMI_SERVER_HOSTNAME)
+  void setRmiServerHostnameSetsSystemPropertyWhenNotBlank() {
+    setRmiServerHostname(A_HOST_NAME);
+    assertThat(System.getProperty(JAVA_RMI_SERVER_HOSTNAME)).isEqualTo(A_HOST_NAME);
+  }
 
-      @Override
-      public void stop() throws IOException {
+  @Test
+  @ClearSystemProperty(key = JAVA_RMI_SERVER_HOSTNAME)
+  void setRmiServerHostnameDoesNotSetSystemPropertyWhenNull() {
+    setRmiServerHostname(null);
+    assertThat(System.getProperty(JAVA_RMI_SERVER_HOSTNAME)).isNull();
+  }
 
-      }
+  @Test
+  @ClearSystemProperty(key = JAVA_RMI_SERVER_HOSTNAME)
+  void setRmiServerHostnameDoesNotSetSystemPropertyWhenEmpty() {
+    setRmiServerHostname("");
+    assertThat(System.getProperty(JAVA_RMI_SERVER_HOSTNAME)).isNull();
+  }
 
-      @Override
-      public boolean isActive() {
-        return false;
-      }
+  private static class JmxConnectorServerWithMBeanServer extends JMXConnectorServer {
+    public JmxConnectorServerWithMBeanServer(final MBeanServer mBeanServer) {
+      super(mBeanServer);
+    }
 
-      @Override
-      public JMXServiceURL getAddress() {
-        return null;
-      }
+    @Override
+    public void start() {}
 
-      @Override
-      public Map<String, ?> getAttributes() {
-        return null;
-      }
-    };
+    @Override
+    public void stop() {}
 
-    managementAgent.setJmxConnectorServer(jmxConnectorServerNullMBeanServer);
-    assertThatCode(() -> managementAgent
-        .setMBeanServerForwarder(ManagementFactory.getPlatformMBeanServer(), new HashMap<>()))
-            .doesNotThrowAnyException();
+    @Override
+    public boolean isActive() {
+      return false;
+    }
+
+    @Override
+    public JMXServiceURL getAddress() {
+      return null;
+    }
+
+    @Override
+    public Map<String, ?> getAttributes() {
+      return null;
+    }
+  }
+
+  private static class JmxConnectorServerNullMBeanServer extends JMXConnectorServer {
+    @Override
+    public void start() {}
+
+    @Override
+    public void stop() {}
+
+    @Override
+    public boolean isActive() {
+      return false;
+    }
+
+    @Override
+    public JMXServiceURL getAddress() {
+      return null;
+    }
+
+    @Override
+    public Map<String, ?> getAttributes() {
+      return null;
+    }
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java b/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
index 22d50bd..83f5925 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
@@ -12,10 +12,12 @@
  * or implied. See the License for the specific language governing permissions and limitations under
  * the License.
  */
+
 package org.apache.geode.management.internal;
 
+import static org.apache.commons.lang3.StringUtils.isNotBlank;
+
 import java.io.IOException;
-import java.io.Serializable;
 import java.lang.management.ManagementFactory;
 import java.net.InetAddress;
 import java.net.ServerSocket;
@@ -50,6 +52,7 @@ import javax.management.remote.rmi.RMIServerImpl;
 import com.healthmarketscience.rmiio.exporter.RemoteStreamExporter;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.Logger;
+import org.jetbrains.annotations.Nullable;
 
 import org.apache.geode.GemFireConfigException;
 import org.apache.geode.annotations.VisibleForTesting;
@@ -92,6 +95,8 @@ public class ManagementAgent {
   private static final Logger logger = LogService.getLogger();
   public static final String SPRING_PROFILES_ACTIVE = "spring.profiles.active";
 
+  static final String JAVA_RMI_SERVER_HOSTNAME = "java.rmi.server.hostname";
+
   /**
    * True if running. Protected by synchronizing on this Manager instance. I used synchronization
    * because I think we'll want to hold the same synchronize while configuring, starting, and
@@ -101,7 +106,6 @@ public class ManagementAgent {
   private Registry registry;
 
   private JMXConnectorServer jmxConnectorServer;
-  private JMXShiroAuthenticator shiroAuthenticator;
   private final DistributionConfig config;
   private final SecurityService securityService;
   private final InternalCache cache;
@@ -126,30 +130,30 @@ public class ManagementAgent {
       FilterConfiguration filterConfiguration) {
     this.config = config;
     this.cache = cache;
-    this.securityService = cache.getSecurityService();
+    securityService = cache.getSecurityService();
     this.filterConfiguration = filterConfiguration;
   }
 
   public synchronized boolean isRunning() {
-    return this.running;
+    return running;
   }
 
   public synchronized void startAgent() {
     filterConfiguration.configure();
     loadWebApplications();
 
-    if (!this.running && this.config.getJmxManagerPort() != 0) {
+    if (!running && config.getJmxManagerPort() != 0) {
       try {
         configureAndStart();
       } catch (IOException e) {
         throw new ManagementException(e);
       }
-      this.running = true;
+      running = true;
     }
   }
 
   public synchronized void stopAgent() {
-    if (!this.running) {
+    if (!running) {
       return;
     }
 
@@ -163,7 +167,7 @@ public class ManagementAgent {
       throw new ManagementException(e);
     }
 
-    this.running = false;
+    running = false;
   }
 
   private final String GEMFIRE_VERSION = GemFireVersion.getGemFireVersion();
@@ -175,7 +179,7 @@ public class ManagementAgent {
 
     final ManagerMXBean managerBean = managementService.getManagerMXBean();
 
-    if (this.config.getHttpServicePort() == 0) {
+    if (config.getHttpServicePort() == 0) {
       setStatusMessage(managerBean,
           "Embedded HTTP server configured not to start (http-service-port=0) or (jmx-manager-http-port=0)");
       return;
@@ -201,7 +205,7 @@ public class ManagementAgent {
         logger.debug(message);
       }
     } else {
-      String pwFile = this.config.getJmxManagerPasswordFile();
+      String pwFile = config.getJmxManagerPasswordFile();
       if (securityService.isIntegratedSecurity()) {
         String[] authTokenEnabledComponents = config.getSecurityAuthTokenEnabledComponents();
         boolean pulseOauth = Arrays.stream(authTokenEnabledComponents)
@@ -211,7 +215,7 @@ public class ManagementAgent {
         } else {
           System.setProperty(SPRING_PROFILES_ACTIVE, "pulse.authentication.gemfire");
         }
-      } else if (StringUtils.isNotBlank(pwFile)) {
+      } else if (isNotBlank(pwFile)) {
         System.setProperty(SPRING_PROFILES_ACTIVE, "pulse.authentication.gemfire");
       }
     }
@@ -220,8 +224,8 @@ public class ManagementAgent {
       HttpService httpService = cache.getService(HttpService.class);
       if (httpService != null && agentUtil.isAnyWarFileAvailable(adminRestWar, pulseWar)) {
 
-        final String bindAddress = this.config.getHttpServiceBindAddress();
-        final int port = this.config.getHttpServicePort();
+        final String bindAddress = config.getHttpServiceBindAddress();
+        final int port = config.getHttpServicePort();
 
         Map<String, Object> serviceAttributes = new HashMap<>();
         serviceAttributes.put(HttpService.SECURITY_SERVICE_SERVLET_CONTEXT_PARAM,
@@ -302,9 +306,9 @@ public class ManagementAgent {
   }
 
   private String getHost(final String bindAddress) throws UnknownHostException {
-    if (StringUtils.isNotBlank(this.config.getJmxManagerHostnameForClients())) {
-      return this.config.getJmxManagerHostnameForClients();
-    } else if (StringUtils.isNotBlank(bindAddress)) {
+    if (isNotBlank(config.getJmxManagerHostnameForClients())) {
+      return config.getJmxManagerHostnameForClients();
+    } else if (isNotBlank(bindAddress)) {
       return InetAddress.getByName(bindAddress).getHostAddress();
     } else {
       return LocalHostUtil.getLocalHost().getHostAddress();
@@ -325,20 +329,15 @@ public class ManagementAgent {
    */
   private void configureAndStart() throws IOException {
     // get the port for RMI Registry and RMI Connector Server
-    port = this.config.getJmxManagerPort();
+    port = config.getJmxManagerPort();
     final String hostname;
-    final InetAddress bindAddr;
-    if (StringUtils.isBlank(this.config.getJmxManagerBindAddress())) {
+    final InetAddress bindAddress;
+    if (StringUtils.isBlank(config.getJmxManagerBindAddress())) {
       hostname = LocalHostUtil.getLocalHostName();
-      bindAddr = null;
+      bindAddress = null;
     } else {
-      hostname = this.config.getJmxManagerBindAddress();
-      bindAddr = InetAddress.getByName(hostname);
-    }
-
-    String jmxManagerHostnameForClients = this.config.getJmxManagerHostnameForClients();
-    if (StringUtils.isNotBlank(jmxManagerHostnameForClients)) {
-      System.setProperty("java.rmi.server.hostname", jmxManagerHostnameForClients);
+      hostname = config.getJmxManagerBindAddress();
+      bindAddress = InetAddress.getByName(hostname);
     }
 
     final SocketCreator socketCreator =
@@ -346,12 +345,15 @@ public class ManagementAgent {
 
     final boolean ssl = socketCreator.forClient().useSSL();
 
+    setRmiServerHostname(
+        getRmiServerHostname(config.getJmxManagerHostnameForClients(), hostname, ssl));
+
     if (logger.isDebugEnabled()) {
       logger.debug("Starting jmx manager agent on port {}{}", port,
-          (bindAddr != null ? (" bound to " + bindAddr) : "") + (ssl ? " using SSL" : ""));
+          (bindAddress != null ? (" bound to " + bindAddress) : "") + (ssl ? " using SSL" : ""));
     }
     rmiClientSocketFactory = ssl ? new ContextAwareSSLRMIClientSocketFactory() : null;
-    rmiServerSocketFactory = new GemFireRMIServerSocketFactory(socketCreator, bindAddr);
+    rmiServerSocketFactory = new GemFireRMIServerSocketFactory(socketCreator, bindAddress);
 
     // Following is done to prevent rmi causing stop the world gcs
     System.setProperty("sun.rmi.dgc.server.gcInterval", Long.toString(Long.MAX_VALUE - 1));
@@ -440,26 +442,59 @@ public class ManagementAgent {
     }
   }
 
+  /**
+   * Sets the {@code java.rmi.server.hostname} System Property.
+   *
+   * @param rmiHostname to set property to if not blank
+   */
+  static void setRmiServerHostname(final @Nullable String rmiHostname) {
+    if (isNotBlank(rmiHostname)) {
+      if (logger.isDebugEnabled()) {
+        logger.debug("Setting RMI hostname to : {}", rmiHostname);
+      }
+      System.setProperty(JAVA_RMI_SERVER_HOSTNAME, rmiHostname);
+    }
+  }
+
+  /**
+   * Gets the hostname that should be set on the RMI server for based on the given values.
+   *
+   * @param hostnameForClients the hostname clients will expect to access.
+   * @param hostnameForServer the hostname that this server is bound to.
+   * @param sslEnabled is SSL enabled.
+   * @return {@code hostnameForClients} if not blank, otherwise {@code hostnameForServer} if not
+   *         blank and {@code sslEnabled}, otherwise {@code null}.
+   */
+  static @Nullable String getRmiServerHostname(final @Nullable String hostnameForClients,
+      final @Nullable String hostnameForServer, final boolean sslEnabled) {
+    if (isNotBlank(hostnameForClients)) {
+      return hostnameForClients;
+    } else if (sslEnabled && isNotBlank(hostnameForServer)) {
+      return hostnameForServer;
+    }
+    return null;
+  }
+
   @VisibleForTesting
   void setMBeanServerForwarder(MBeanServer mbs, HashMap<String, Object> env)
       throws IOException {
     if (securityService.isIntegratedSecurity()) {
-      shiroAuthenticator = new JMXShiroAuthenticator(this.securityService);
+      JMXShiroAuthenticator shiroAuthenticator = new JMXShiroAuthenticator(securityService);
       env.put(JMXConnectorServer.AUTHENTICATOR, shiroAuthenticator);
       jmxConnectorServer.addNotificationListener(shiroAuthenticator, null,
           jmxConnectorServer.getAttributes());
       // always going to assume authorization is needed as well, if no custom AccessControl, then
       // the CustomAuthRealm should take care of that
-      MBeanServerWrapper mBeanServerWrapper = new MBeanServerWrapper(this.securityService);
+      MBeanServerWrapper mBeanServerWrapper = new MBeanServerWrapper(securityService);
       jmxConnectorServer.setMBeanServerForwarder(mBeanServerWrapper);
     } else {
       /* Disable the old authenticator mechanism */
-      String pwFile = this.config.getJmxManagerPasswordFile();
+      String pwFile = config.getJmxManagerPasswordFile();
       if (pwFile != null && pwFile.length() > 0) {
         env.put("jmx.remote.x.password.file", pwFile);
       }
 
-      String accessFile = this.config.getJmxManagerAccessFile();
+      String accessFile = config.getJmxManagerAccessFile();
       if (accessFile != null && accessFile.length() > 0) {
         // Rewire the mbs hierarchy to set accessController
         ReadOpFileAccessController controller = new ReadOpFileAccessController(accessFile);
@@ -473,7 +508,7 @@ public class ManagementAgent {
 
   private void registerAccessControlMBean() {
     try {
-      AccessControlMBean acc = new AccessControlMBean(this.securityService);
+      AccessControlMBean acc = new AccessControlMBean(securityService);
       ObjectName accessControlMBeanON = new ObjectName(ResourceConstants.OBJECT_NAME_ACCESSCONTROL);
       MBeanServer platformMBeanServer = ManagementFactory.getPlatformMBeanServer();
 
@@ -527,21 +562,19 @@ public class ManagementAgent {
     return remoteStreamExporter;
   }
 
-  private static class GemFireRMIServerSocketFactory
-      implements RMIServerSocketFactory, Serializable {
+  private static class GemFireRMIServerSocketFactory implements RMIServerSocketFactory {
 
-    private static final long serialVersionUID = -811909050641332716L;
-    private transient SocketCreator sc;
-    private final InetAddress bindAddr;
+    private final SocketCreator sc;
+    private final InetAddress bindAddress;
 
-    public GemFireRMIServerSocketFactory(SocketCreator sc, InetAddress bindAddr) {
+    public GemFireRMIServerSocketFactory(SocketCreator sc, InetAddress bindAddress) {
       this.sc = sc;
-      this.bindAddr = bindAddr;
+      this.bindAddress = bindAddress;
     }
 
     @Override
     public ServerSocket createServerSocket(int port) throws IOException {
-      return this.sc.forCluster().createServerSocket(port, TCPConduit.getBackLog(), this.bindAddr);
+      return sc.forCluster().createServerSocket(port, TCPConduit.getBackLog(), bindAddress);
     }
   }
 }
diff --git a/geode-core/src/main/resources/org/apache/geode/internal/sanctioned-geode-core-serializables.txt b/geode-core/src/main/resources/org/apache/geode/internal/sanctioned-geode-core-serializables.txt
index 534fca2..3dba828 100644
--- a/geode-core/src/main/resources/org/apache/geode/internal/sanctioned-geode-core-serializables.txt
+++ b/geode-core/src/main/resources/org/apache/geode/internal/sanctioned-geode-core-serializables.txt
@@ -438,7 +438,6 @@ org/apache/geode/management/internal/BackupStatusImpl,true,3704172840296221840,b
 org/apache/geode/management/internal/CacheElementOperation,false
 org/apache/geode/management/internal/ContextAwareSSLRMIClientSocketFactory,true,8159615071011918570
 org/apache/geode/management/internal/JmxManagerLocator$StartJmxManagerFunction,true,-2860286061903069789
-org/apache/geode/management/internal/ManagementAgent$GemFireRMIServerSocketFactory,true,-811909050641332716,bindAddr:java/net/InetAddress
 org/apache/geode/management/internal/ManagementFunction,true,1,mbeanServer:javax/management/MBeanServer,notificationHub:org/apache/geode/management/internal/NotificationHub
 org/apache/geode/management/internal/NotificationKey,false,currentTime:long,objectName:javax/management/ObjectName
 org/apache/geode/management/internal/beans/FileUploader$RemoteFile,false,filename:java/lang/String,outputStream:com/healthmarketscience/rmiio/RemoteOutputStream
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/ManagementAgentTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/ManagementAgentTest.java
new file mode 100644
index 0000000..9254807
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/ManagementAgentTest.java
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal;
+
+
+import static org.apache.geode.management.internal.ManagementAgent.getRmiServerHostname;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.jupiter.api.Test;
+
+class ManagementAgentTest {
+
+  public static final String CLIENT = "client.host.name";
+  public static final String SERVER = "server.host.name";
+  public static final String EMPTY = "";
+
+  @Test
+  void getRmiServerHostnameReturnsHostnameForClientsWhenHostnameForClientsNotBlank() {
+    assertThat(getRmiServerHostname(CLIENT, SERVER, false)).isEqualTo(CLIENT);
+    assertThat(getRmiServerHostname(CLIENT, SERVER, true)).isEqualTo(CLIENT);
+  }
+
+  @Test
+  void getRmiServerHostnameReturnsNullWhenHostnameForClientsIsBlankAndSslDisabled() {
+    assertThat(getRmiServerHostname(null, SERVER, false)).isNull();
+    assertThat(getRmiServerHostname(EMPTY, SERVER, false)).isNull();
+  }
+
+  @Test
+  void getRmiServerHostnameReturnsHostnameForServerWhenHostnameForClientsIsBlankAndSslEnabled() {
+    assertThat(getRmiServerHostname(null, SERVER, true)).isEqualTo(SERVER);
+    assertThat(getRmiServerHostname(EMPTY, SERVER, true)).isEqualTo(SERVER);
+  }
+
+  @Test
+  void getRmiServerHostnameReturnsNullWhenHostnameForClientsIsBlankAndHostameForServerIsBlank() {
+    assertThat(getRmiServerHostname(null, EMPTY, false)).isNull();
+    assertThat(getRmiServerHostname(null, null, false)).isNull();
+    assertThat(getRmiServerHostname(EMPTY, null, false)).isNull();
+    assertThat(getRmiServerHostname(EMPTY, EMPTY, false)).isNull();
+    assertThat(getRmiServerHostname(null, EMPTY, true)).isNull();
+    assertThat(getRmiServerHostname(null, null, true)).isNull();
+    assertThat(getRmiServerHostname(EMPTY, null, true)).isNull();
+    assertThat(getRmiServerHostname(EMPTY, EMPTY, true)).isNull();
+  }
+
+}
diff --git a/geode-gfsh/build.gradle b/geode-gfsh/build.gradle
index f101331..91c6125 100644
--- a/geode-gfsh/build.gradle
+++ b/geode-gfsh/build.gradle
@@ -86,4 +86,17 @@ dependencies {
     exclude module: 'spring-context-support'
     exclude module: 'spring-core'
   }
+
+
+  acceptanceTestImplementation(project(':geode-junit'))
+  acceptanceTestRuntimeOnly(project(':geode-log4j'))
+
+}
+
+configure([
+  acceptanceTest,
+  repeatAcceptanceTest
+]) {
+  environment 'GEODE_HOME', "$buildDir/../../geode-assembly/build/install/apache-geode"
+  dependsOn(':geode-assembly:installDist')
 }
diff --git a/geode-gfsh/src/acceptanceTest/java/org/apache/geode/gfsh/GfshWithSslAcceptanceTest.java b/geode-gfsh/src/acceptanceTest/java/org/apache/geode/gfsh/GfshWithSslAcceptanceTest.java
new file mode 100644
index 0000000..ba0db73
--- /dev/null
+++ b/geode-gfsh/src/acceptanceTest/java/org/apache/geode/gfsh/GfshWithSslAcceptanceTest.java
@@ -0,0 +1,153 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.gfsh;
+
+import static java.lang.String.format;
+import static java.lang.String.valueOf;
+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_KEYSTORE_TYPE;
+import static org.apache.geode.distributed.ConfigurationProperties.SSL_PROTOCOLS;
+import static org.apache.geode.distributed.ConfigurationProperties.SSL_REQUIRE_AUTHENTICATION;
+import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE;
+import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_PASSWORD;
+import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_TYPE;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.net.InetAddress;
+import java.security.GeneralSecurityException;
+import java.util.Properties;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.cache.ssl.CertStores;
+import org.apache.geode.cache.ssl.CertificateBuilder;
+import org.apache.geode.cache.ssl.CertificateMaterial;
+import org.apache.geode.internal.UniquePortSupplier;
+import org.apache.geode.test.junit.rules.gfsh.GfshRule;
+
+public class GfshWithSslAcceptanceTest {
+  private static final String CERTIFICATE_ALGORITHM = "SHA256withRSA";
+  private static final int CERTIFICATE_EXPIRATION_IN_DAYS = 1;
+  private static final String STORE_PASSWORD = "geode";
+  private static final String STORE_TYPE = "jks";
+
+  private final String startLocator;
+  private final String connect;
+
+  @Rule
+  public TemporaryFolder tempFolder = new TemporaryFolder();
+
+  @Rule
+  public final GfshRule gfsh;
+
+  private final File keyStoreFile;
+  private final File trustStoreFile;
+  private final File securityPropertiesFile;
+
+  public GfshWithSslAcceptanceTest() throws IOException,
+      GeneralSecurityException {
+    gfsh = new GfshRule();
+
+    final UniquePortSupplier portSupplier = new UniquePortSupplier();
+    final int port = portSupplier.getAvailablePort();
+
+    tempFolder.create();
+    keyStoreFile = tempFolder.newFile();
+    trustStoreFile = tempFolder.newFile();
+    securityPropertiesFile = tempFolder.newFile();
+
+    final String hostName = InetAddress.getLocalHost().getCanonicalHostName();
+    generateKeyAndTrustStore(hostName, keyStoreFile, trustStoreFile);
+
+    startLocator = format(
+        "start locator --connect=false --http-service-port=0 --name=locator --bind-address=%s --port=%d --J=-Dgemfire.jmx-manager-port=%d --security-properties-file=%s",
+        hostName, port, portSupplier.getAvailablePort(),
+        securityPropertiesFile.getAbsolutePath());
+    connect = format("connect --locator=%s[%d] --security-properties-file=%s", hostName, port,
+        securityPropertiesFile.getAbsolutePath());
+  }
+
+  @Test
+  public void gfshCanConnectViaSslWithEndpointIdentificationEnabled() throws IOException {
+    generateSecurityProperties(true, securityPropertiesFile, keyStoreFile,
+        trustStoreFile);
+
+    gfsh.execute(startLocator);
+    gfsh.execute(connect);
+  }
+
+  // @Test
+  public void gfshCanConnectViaSslWithEndpointIdentificationDisabled() throws IOException {
+    generateSecurityProperties(false, securityPropertiesFile, keyStoreFile,
+        trustStoreFile);
+
+    gfsh.execute(startLocator);
+    gfsh.execute(connect);
+  }
+
+  public static void generateKeyAndTrustStore(final String hostName, final File keyStoreFile,
+      final File trustStoreFile) throws IOException, GeneralSecurityException {
+    final CertificateMaterial ca =
+        new CertificateBuilder(CERTIFICATE_EXPIRATION_IN_DAYS, CERTIFICATE_ALGORITHM)
+            .commonName("Test CA")
+            .isCA()
+            .generate();
+
+    final CertificateMaterial certificate = new CertificateBuilder(CERTIFICATE_EXPIRATION_IN_DAYS,
+        CERTIFICATE_ALGORITHM)
+            .commonName(hostName)
+            .issuedBy(ca)
+            .sanDnsName(hostName)
+            .generate();
+
+    final CertStores store = new CertStores(hostName);
+    store.withCertificate("geode", certificate);
+    store.trust("ca", ca);
+
+    store.createKeyStore(keyStoreFile.getAbsolutePath(), STORE_PASSWORD);
+    store.createTrustStore(trustStoreFile.getAbsolutePath(), STORE_PASSWORD);
+  }
+
+  private static void generateSecurityProperties(final boolean endpointIdentificationEnabled,
+      final File securityPropertiesFile, final File keyStoreFile, final File trustStoreFile)
+      throws IOException {
+    final Properties properties = new Properties();
+
+    properties.setProperty(SSL_REQUIRE_AUTHENTICATION, valueOf(true));
+    properties.setProperty(SSL_ENABLED_COMPONENTS, "all");
+    properties.setProperty(SSL_ENDPOINT_IDENTIFICATION_ENABLED,
+        valueOf(endpointIdentificationEnabled));
+    properties.setProperty(SSL_PROTOCOLS, "any");
+
+    properties.setProperty(SSL_KEYSTORE, keyStoreFile.getAbsolutePath());
+    properties.setProperty(SSL_KEYSTORE_TYPE, STORE_TYPE);
+    properties.setProperty(SSL_KEYSTORE_PASSWORD, STORE_PASSWORD);
+
+    properties.setProperty(SSL_TRUSTSTORE, trustStoreFile.getAbsolutePath());
+    properties.setProperty(SSL_TRUSTSTORE_TYPE, STORE_TYPE);
+    properties.setProperty(SSL_TRUSTSTORE_PASSWORD, STORE_PASSWORD);
+
+    properties.store(new FileWriter(securityPropertiesFile), null);
+  }
+
+}
diff --git a/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvokerIntegrationTest.java b/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvokerIntegrationTest.java
new file mode 100644
index 0000000..33a135e
--- /dev/null
+++ b/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvokerIntegrationTest.java
@@ -0,0 +1,179 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.cli.shell;
+
+import static java.lang.String.valueOf;
+import static org.apache.geode.distributed.ConfigurationProperties.ENABLE_CLUSTER_CONFIGURATION;
+import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT;
+import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_BIND_ADDRESS;
+import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_PORT;
+import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_START;
+import static org.apache.geode.distributed.ConfigurationProperties.LOG_FILE;
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+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_KEYSTORE_TYPE;
+import static org.apache.geode.distributed.ConfigurationProperties.SSL_PROTOCOLS;
+import static org.apache.geode.distributed.ConfigurationProperties.SSL_REQUIRE_AUTHENTICATION;
+import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE;
+import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_PASSWORD;
+import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_TYPE;
+
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.nio.file.Path;
+import java.security.GeneralSecurityException;
+import java.util.Properties;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import org.apache.geode.cache.ssl.CertStores;
+import org.apache.geode.cache.ssl.CertificateBuilder;
+import org.apache.geode.cache.ssl.CertificateMaterial;
+import org.apache.geode.distributed.Locator;
+import org.apache.geode.internal.AvailablePortHelper;
+
+public class JmxOperationInvokerIntegrationTest {
+
+  private static final String CERTIFICATE_ALGORITHM = "SHA256withRSA";
+  private static final int CERTIFICATE_EXPIRATION_IN_DAYS = 1;
+  private static final String STORE_PASSWORD = "geode";
+  private static final String STORE_TYPE = "jks";
+
+  final String hostName = InetAddress.getLocalHost().getCanonicalHostName();
+  final int locatorPort = AvailablePortHelper.getRandomAvailableTCPPort();
+  final int jmxPort = AvailablePortHelper.getRandomAvailableTCPPort();
+  final Properties properties = new Properties();
+
+  @TempDir
+  public Path tempPath;
+
+  private Path keyStoreFile;
+  private Path trustStoreFile;
+
+  public JmxOperationInvokerIntegrationTest() throws UnknownHostException {
+    properties.setProperty(ENABLE_CLUSTER_CONFIGURATION, "false");
+    properties.setProperty(HTTP_SERVICE_PORT, "0");
+    properties.setProperty(LOG_FILE, "");
+    properties.setProperty(MCAST_PORT, "0");
+    properties.setProperty(JMX_MANAGER_PORT, valueOf(jmxPort));
+    properties.setProperty(JMX_MANAGER_START, "true");
+  }
+
+  @BeforeEach
+  public void beforeEach() throws GeneralSecurityException, IOException {
+    keyStoreFile = tempPath.resolve("keystore.jks").toAbsolutePath();
+    trustStoreFile = tempPath.resolve("truststore.jks").toAbsolutePath();
+    generateKeyAndTrustStore(hostName, keyStoreFile, trustStoreFile);
+  }
+
+  @Test
+  public void canConnectToLocatorWithoutSsl() throws Exception {
+    final Locator locator = Locator.startLocatorAndDS(locatorPort, null, properties);
+    try {
+      new JmxOperationInvoker(hostName, jmxPort, properties).stop();
+    } finally {
+      locator.stop();
+    }
+  }
+
+  @Test
+  public void canConnectToLocatorWithSsl() throws Exception {
+    properties.putAll(generateSecurityProperties(false, keyStoreFile, trustStoreFile));
+
+    final Locator locator = Locator.startLocatorAndDS(locatorPort, null, properties);
+    try {
+      new JmxOperationInvoker(hostName, jmxPort, properties).stop();
+    } finally {
+      locator.stop();
+    }
+  }
+
+  @Test
+  public void canConnectToLocatorWithSslAndEndpointValidationEnabled() throws Exception {
+    properties.putAll(generateSecurityProperties(true, keyStoreFile, trustStoreFile));
+
+    final Locator locator = Locator.startLocatorAndDS(locatorPort, null, properties);
+    try {
+      new JmxOperationInvoker(hostName, jmxPort, properties).stop();
+    } finally {
+      locator.stop();
+    }
+  }
+
+  @Test
+  public void canConnectToLocatorWithSslAndEndpointValidationEnabledBindAddress() throws Exception {
+    properties.setProperty(JMX_MANAGER_BIND_ADDRESS, hostName);
+    properties.putAll(generateSecurityProperties(true, keyStoreFile, trustStoreFile));
+
+    final Locator locator = Locator.startLocatorAndDS(locatorPort, null, properties);
+    try {
+      new JmxOperationInvoker(hostName, jmxPort, properties).stop();
+    } finally {
+      locator.stop();
+    }
+  }
+
+  public static void generateKeyAndTrustStore(final String hostName, final Path keyStoreFile,
+      final Path trustStoreFile) throws IOException, GeneralSecurityException {
+    final CertificateMaterial ca =
+        new CertificateBuilder(CERTIFICATE_EXPIRATION_IN_DAYS, CERTIFICATE_ALGORITHM)
+            .commonName("Test CA")
+            .isCA()
+            .generate();
+
+    final CertificateMaterial certificate = new CertificateBuilder(CERTIFICATE_EXPIRATION_IN_DAYS,
+        CERTIFICATE_ALGORITHM)
+            .commonName(hostName)
+            .issuedBy(ca)
+            .sanDnsName(hostName)
+            .generate();
+
+    final CertStores store = new CertStores(hostName);
+    store.withCertificate("geode", certificate);
+    store.trust("ca", ca);
+
+    store.createKeyStore(keyStoreFile.toString(), STORE_PASSWORD);
+    store.createTrustStore(trustStoreFile.toString(), STORE_PASSWORD);
+  }
+
+  private static Properties generateSecurityProperties(final boolean endpointIdentificationEnabled,
+      final Path keyStoreFile, final Path trustStoreFile) {
+    final Properties properties = new Properties();
+
+    properties.setProperty(SSL_REQUIRE_AUTHENTICATION, valueOf(true));
+    properties.setProperty(SSL_ENABLED_COMPONENTS, "all");
+    properties.setProperty(SSL_ENDPOINT_IDENTIFICATION_ENABLED,
+        valueOf(endpointIdentificationEnabled));
+    properties.setProperty(SSL_PROTOCOLS, "any");
+
+    properties.setProperty(SSL_KEYSTORE, keyStoreFile.toString());
+    properties.setProperty(SSL_KEYSTORE_TYPE, STORE_TYPE);
+    properties.setProperty(SSL_KEYSTORE_PASSWORD, STORE_PASSWORD);
+
+    properties.setProperty(SSL_TRUSTSTORE, trustStoreFile.toString());
+    properties.setProperty(SSL_TRUSTSTORE_TYPE, STORE_TYPE);
+    properties.setProperty(SSL_TRUSTSTORE_PASSWORD, STORE_PASSWORD);
+
+    return properties;
+  }
+
+}
diff --git a/geode-junit/src/main/java/org/apache/geode/cache/ssl/CertStores.java b/geode-junit/src/main/java/org/apache/geode/cache/ssl/CertStores.java
index 6cce930..2cb4f40 100644
--- a/geode-junit/src/main/java/org/apache/geode/cache/ssl/CertStores.java
+++ b/geode-junit/src/main/java/org/apache/geode/cache/ssl/CertStores.java
@@ -26,7 +26,6 @@ import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTOR
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_PASSWORD;
 import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_TYPE;
 
-import java.io.EOFException;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
@@ -137,7 +136,7 @@ public class CertStores {
     KeyStore ks = KeyStore.getInstance("JKS");
     try (InputStream in = Files.newInputStream(Paths.get(filename))) {
       ks.load(in, password.toCharArray());
-    } catch (EOFException e) {
+    } catch (IOException e) {
       ks = createEmptyKeyStore();
     }
     for (Map.Entry<String, CertificateMaterial> cert : trustedCerts.entrySet()) {