KNOX-3311: Fix X509CertificateUtil.fetchPublicCertsFromServer issue w…

gateway-discovery-cm/pom.xml

@@ -63,6 +63,10 @@
             <groupId>org.apache.knox</groupId>
             <artifactId>gateway-util-configinjector</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.knox</groupId>
+            <artifactId>gateway-util-common</artifactId>
+        </dependency>
         <dependency>
             <groupId>com.cloudera.api.swagger</groupId>
             <artifactId>cloudera-manager-api-swagger</artifactId>

gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ApiClientFactory.java

@@ -21,28 +21,26 @@
 import org.apache.knox.gateway.services.security.AliasService;
 import org.apache.knox.gateway.services.security.AliasServiceException;
 import org.apache.knox.gateway.topology.discovery.ServiceDiscoveryConfig;
+import org.apache.knox.gateway.util.TruststorePasswordSetter;
 
 import java.security.KeyStore;
 
 public class ApiClientFactory {
 
     private static final ClouderaManagerServiceDiscoveryMessages LOG = MessagesFactory.get(ClouderaManagerServiceDiscoveryMessages.class);
-    static final String TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY = "javax.net.ssl.trustStorePassword";
-    public static final String TRUSTSTORE_PASSWORD_ALIAS = "cm.discovery.trustStorePassword";
 
     public static DiscoveryApiClient getApiClient(final GatewayConfig gatewayConfig, final ServiceDiscoveryConfig discoveryConfig,
                                                   final AliasService aliasService, final KeyStore truststore) {
+        final char[] trustStorePassword;
         try {
-            final char[] trustStorePassword = aliasService.getPasswordFromAliasForGateway(TRUSTSTORE_PASSWORD_ALIAS);
-            if (trustStorePassword != null && trustStorePassword.length > 0) {
-                System.setProperty(TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY, new String(trustStorePassword));
-            }
-            return new DiscoveryApiClient(gatewayConfig, discoveryConfig, aliasService, truststore);
+            trustStorePassword = aliasService.getPasswordFromAliasForGateway(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_ALIAS);
         } catch (AliasServiceException e) {
             LOG.clouderaManagerApiClientBuildError(e);
             throw new ServiceDiscoveryException("Unable to retrieve CM service discovery truststore password", e);
-        } finally {
-            System.clearProperty(TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY);
+        }
+
+        try (TruststorePasswordSetter ignored = new TruststorePasswordSetter(trustStorePassword)) {
+            return new DiscoveryApiClient(gatewayConfig, discoveryConfig, aliasService, truststore);
         }
     }
 }

gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/ApiClientFactoryTest.java

@@ -21,6 +21,7 @@
 import org.apache.knox.gateway.services.security.AliasService;
 import org.apache.knox.gateway.services.security.AliasServiceException;
 import org.apache.knox.gateway.topology.discovery.ServiceDiscoveryConfig;
+import org.apache.knox.gateway.util.TruststorePasswordSetter;
 import org.easymock.EasyMock;
 import org.junit.After;
 import org.junit.Assert;
@@ -77,9 +78,9 @@ private void testGetApiClient(final boolean shouldSetSystemProperty, String trus
         EasyMock.expect(serviceDiscoveryConfig.getPasswordAlias()).andReturn("myCmPasswordAlias").anyTimes();
         final AliasService aliasService = EasyMock.createMock(AliasService.class);
         if (shouldSetSystemProperty) {
-            EasyMock.expect(aliasService.getPasswordFromAliasForGateway(ApiClientFactory.TRUSTSTORE_PASSWORD_ALIAS)).andReturn(trustStorePassword.toCharArray()).anyTimes();
+            EasyMock.expect(aliasService.getPasswordFromAliasForGateway(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_ALIAS)).andReturn(trustStorePassword.toCharArray()).anyTimes();
         } else {
-            EasyMock.expect(aliasService.getPasswordFromAliasForGateway(ApiClientFactory.TRUSTSTORE_PASSWORD_ALIAS)).andReturn(null).anyTimes();
+            EasyMock.expect(aliasService.getPasswordFromAliasForGateway(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_ALIAS)).andReturn(null).anyTimes();
         }
         EasyMock.expect(aliasService.getPasswordFromAliasForGateway("myCmPasswordAlias")).andReturn("myCmPassword".toCharArray()).anyTimes();
         final KeyStore trustStore = EasyMock.createMock(KeyStore.class);
@@ -88,13 +89,13 @@ private void testGetApiClient(final boolean shouldSetSystemProperty, String trus
         ApiClientFactory.getApiClient(gatewayConfig, serviceDiscoveryConfig, aliasService, trustStore);
 
         if (shouldSetSystemProperty && StringUtils.isNotBlank(trustStorePassword)) {
-            Assert.assertEquals(ApiClientFactory.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY, testProps.lastSetKey);
+            Assert.assertEquals(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY, testProps.lastSetKey);
             Assert.assertEquals(trustStorePassword, testProps.lastSetValue);
-            Assert.assertEquals(ApiClientFactory.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY, testProps.lastRemovedKey);
+            Assert.assertEquals(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY, testProps.lastRemovedKey);
         } else {
             Assert.assertNull(testProps.lastSetKey);
             Assert.assertNull(testProps.lastSetValue);
-            Assert.assertEquals(ApiClientFactory.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY, testProps.lastRemovedKey);
+            Assert.assertEquals(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY, testProps.lastRemovedKey);
         }
     }
 

gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/monitor/PollingConfigurationAnalyzerTest.java

@@ -32,9 +32,9 @@
 import org.apache.knox.gateway.services.topology.impl.GatewayStatusService;
 import org.apache.knox.gateway.topology.ClusterConfigurationMonitorService;
 import org.apache.knox.gateway.topology.discovery.ServiceDiscoveryConfig;
-import org.apache.knox.gateway.topology.discovery.cm.ApiClientFactory;
 import org.apache.knox.gateway.topology.discovery.cm.model.hdfs.NameNodeServiceModelGenerator;
 import org.apache.knox.gateway.topology.discovery.cm.model.hive.HiveOnTezServiceModelGenerator;
+import org.apache.knox.gateway.util.TruststorePasswordSetter;
 import org.easymock.EasyMock;
 import org.junit.After;
 import org.junit.Test;
@@ -363,7 +363,7 @@ public void testClusterConfigMonitorTerminationForNoLongerReferencedClusters() t
     EasyMock.replay(ts, ccms, gatewayStatusService, gws);
 
     AliasService aliasService = EasyMock.createNiceMock(AliasService.class);
-    EasyMock.expect(aliasService.getPasswordFromAliasForGateway(ApiClientFactory.TRUSTSTORE_PASSWORD_ALIAS)).andReturn(null).anyTimes();
+    EasyMock.expect(aliasService.getPasswordFromAliasForGateway(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_ALIAS)).andReturn(null).anyTimes();
     EasyMock.replay(aliasService);
 
     try {
@@ -540,7 +540,7 @@ private TestablePollingConfigAnalyzer buildPollingConfigAnalyzer(final String ad
     EasyMock.replay(configCache);
 
     AliasService aliasService = EasyMock.createNiceMock(AliasService.class);
-    EasyMock.expect(aliasService.getPasswordFromAliasForGateway(ApiClientFactory.TRUSTSTORE_PASSWORD_ALIAS)).andReturn(null).anyTimes();
+    EasyMock.expect(aliasService.getPasswordFromAliasForGateway(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_ALIAS)).andReturn(null).anyTimes();
     EasyMock.replay(aliasService);
 
     if (isKnoxGatewayReady) {

gateway-service-metadata/src/main/java/org/apache/knox/gateway/service/metadata/KnoxMetadataResource.java

@@ -69,6 +69,7 @@
 import org.apache.knox.gateway.topology.Service;
 import org.apache.knox.gateway.topology.Topology;
 import org.apache.knox.gateway.util.JsonUtils;
+import org.apache.knox.gateway.util.TruststorePasswordSetter;
 import org.apache.knox.gateway.util.X509CertificateUtil;
 
 import com.kstruct.gethostname4j.Hostname;
@@ -173,11 +174,16 @@ private Response generateFailureFileDownloadResponse(Status status, String error
 
   private Certificate[] getPublicCertificates() {
     try {
-      return X509CertificateUtil.fetchPublicCertsFromServer(request.getRequestURL().toString(), true, null);
+      final GatewayServices gatewayServices = (GatewayServices) request.getServletContext().getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
+      if (gatewayServices != null) {
+        final AliasService aliasService = gatewayServices.getService(ServiceType.ALIAS_SERVICE);
+        char[] trustStorePassword = aliasService.getPasswordFromAliasForGateway(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_ALIAS);
+        return X509CertificateUtil.fetchPublicCertsFromServer(request.getRequestURL().toString(), trustStorePassword, true, null);
+      }
     } catch (Exception e) {
       LOG.failedToFetchPublicCert(e.getMessage(), e);
-      return null;
     }
+    return null;
   }
 
   private void generateCertificatePem(Certificate[] certificateChain, GatewayConfig gatewayConfig) {

gateway-shell/src/main/java/org/apache/knox/gateway/shell/KnoxSh.java

@@ -181,7 +181,8 @@ class KnoxBuildTrustStore extends Command {
     public void execute() throws Exception {
       String result = GATEWAY_CERT_NOT_EXPORTED;
       try {
-        final X509Certificate[] gatewayServerPublicCerts = X509CertificateUtil.fetchPublicCertsFromServer(gateway, false, out);
+        final X509Certificate[] gatewayServerPublicCerts = X509CertificateUtil.fetchPublicCertsFromServer(
+                gateway, ClientTrustStoreHelper.getClientTrustStoreFilePassword().toCharArray(), false, out);
         if (gatewayServerPublicCerts != null) {
           final File trustStoreFile = ClientTrustStoreHelper.getClientTrustStoreFile();
           final String trustStorePassword = ClientTrustStoreHelper.getClientTrustStoreFilePassword();

gateway-util-common/src/main/java/org/apache/knox/gateway/util/TruststorePasswordSetter.java

@@ -0,0 +1,37 @@
+/*
+ * 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.knox.gateway.util;
+
+/**
+ * A utility class that sets and clears the javax.net.ssl.trustStorePassword system property.
+ */
+public class TruststorePasswordSetter implements AutoCloseable {
+    public static final String TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY = "javax.net.ssl.trustStorePassword";
+    public static final String TRUSTSTORE_PASSWORD_ALIAS = "cm.discovery.trustStorePassword";
+
+    public TruststorePasswordSetter(char[] password) {
+        if (password != null && password.length > 0) {
+            System.setProperty(TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY, new String(password));
+        }
+    }
+
+    @Override
+    public void close() {
+        System.clearProperty(TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY);
+    }
+}

gateway-util-common/src/main/java/org/apache/knox/gateway/util/X509CertificateUtil.java

@@ -232,11 +232,14 @@ public static boolean isSelfSignedCertificate(Certificate certificate) {
     }
   }
 
-  public static X509Certificate[] fetchPublicCertsFromServer(String serverUrl, boolean forceReturnCert, PrintStream out) throws Exception {
-      final TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
-      trustManagerFactory.init((KeyStore) null);
-      final X509TrustManager defaultTrustManager = (X509TrustManager) trustManagerFactory.getTrustManagers()[0];
-      final CertificateChainAwareTrustManager trustManagerWithCertificateChain = new CertificateChainAwareTrustManager(defaultTrustManager);
+    public static X509Certificate[] fetchPublicCertsFromServer(String serverUrl, char[] trustStorePassword, boolean forceReturnCert, PrintStream out) throws Exception {
+      CertificateChainAwareTrustManager trustManagerWithCertificateChain;
+      try (TruststorePasswordSetter ignored = new TruststorePasswordSetter(trustStorePassword)) {
+          final TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
+          trustManagerFactory.init((KeyStore) null);
+          final X509TrustManager defaultTrustManager = (X509TrustManager) trustManagerFactory.getTrustManagers()[0];
+          trustManagerWithCertificateChain = new CertificateChainAwareTrustManager(defaultTrustManager);
+      }
       final SSLContext sslContext = SSLContext.getInstance("TLS");
       sslContext.init(null, new TrustManager[] { trustManagerWithCertificateChain }, null);
 

gateway-util-common/src/test/java/org/apache/knox/gateway/util/TruststorePasswordSetterTest.java

@@ -0,0 +1,71 @@
+/*
+ * 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.knox.gateway.util;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TruststorePasswordSetterTest {
+  private String originalPassword;
+
+  @Before
+  public void setUp() {
+    originalPassword = System.getProperty(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY);
+    System.clearProperty(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY);
+  }
+
+  @After
+  public void tearDown() {
+    if (originalPassword != null) {
+      System.setProperty(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY, originalPassword);
+    } else {
+      System.clearProperty(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY);
+    }
+  }
+
+  @Test
+  public void testSetPassword() {
+    final String password = "test-password";
+    try (TruststorePasswordSetter setter = new TruststorePasswordSetter(password.toCharArray())) {
+      Assert.assertEquals("The system property should be set to the provided password.",
+                          password, System.getProperty(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY));
+    }
+    Assert.assertNull("The system property should be cleared after closing the setter.",
+                      System.getProperty(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY));
+  }
+
+  @Test
+  public void testNullPassword() {
+    try (TruststorePasswordSetter setter = new TruststorePasswordSetter(null)) {
+      Assert.assertNull("The system property should not be set if the password is null.",
+                        System.getProperty(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY));
+    }
+    Assert.assertNull(System.getProperty(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY));
+  }
+
+  @Test
+  public void testEmptyPassword() {
+    try (TruststorePasswordSetter setter = new TruststorePasswordSetter(new char[0])) {
+      Assert.assertNull("The system property should not be set if the password array is empty.",
+                        System.getProperty(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY));
+    }
+    Assert.assertNull(System.getProperty(TruststorePasswordSetter.TRUSTSTORE_PASSWORD_SYSTEM_PROPERTY));
+  }
+}

gateway-util-common/src/test/java/org/apache/knox/gateway/util/X509CertificateUtilTest.java

@@ -0,0 +1,94 @@
+/*
+ * 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.knox.gateway.util;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLServerSocket;
+import javax.net.ssl.SSLServerSocketFactory;
+import javax.net.ssl.SSLSocket;
+import java.io.IOException;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.KeyStore;
+import java.security.SecureRandom;
+import java.security.cert.X509Certificate;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+public class X509CertificateUtilTest {
+
+  @Test
+  public void testFetchPublicCertsFromServer() throws Exception {
+    // 1. Generate a self-signed certificate
+    KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance("RSA");
+    keyPairGenerator.initialize(2048);
+    KeyPair keyPair = keyPairGenerator.generateKeyPair();
+    X509Certificate cert = X509CertificateUtil.generateCertificate("CN=localhost", keyPair, 30, "SHA256withRSA");
+
+    // 2. Set up a KeyStore with the certificate
+    KeyStore keyStore = KeyStore.getInstance("JKS");
+    keyStore.load(null, null);
+    keyStore.setKeyEntry("alias", keyPair.getPrivate(), "password".toCharArray(), new java.security.cert.Certificate[]{cert});
+
+    KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+    kmf.init(keyStore, "password".toCharArray());
+
+    SSLContext serverSslContext = SSLContext.getInstance("TLS");
+    serverSslContext.init(kmf.getKeyManagers(), null, new SecureRandom());
+
+    SSLServerSocketFactory ssf = serverSslContext.getServerSocketFactory();
+    try (SSLServerSocket serverSocket = (SSLServerSocket) ssf.createServerSocket(0)) {
+      int port = serverSocket.getLocalPort();
+
+      ExecutorService executor = Executors.newSingleThreadExecutor();
+      Future<Void> serverFuture = executor.submit(new Callable<Void>() {
+        @Override
+        public Void call() throws Exception {
+          try (SSLSocket clientSocket = (SSLSocket) serverSocket.accept()) {
+            clientSocket.startHandshake();
+          } catch (IOException e) {
+            // Expected if client closes connection early
+          }
+          return null;
+        }
+      });
+
+      // 3. Fetch the certificate from the server
+      try {
+        String serverUrl = "https://localhost:" + port;
+        X509Certificate[] certs = X509CertificateUtil.fetchPublicCertsFromServer(serverUrl, null, true, null);
+
+        // 4. Verify
+        Assert.assertNotNull(certs);
+        Assert.assertTrue(certs.length > 0);
+        Assert.assertEquals(cert.getSubjectX500Principal(), certs[0].getSubjectX500Principal());
+        Assert.assertEquals(cert.getPublicKey(), certs[0].getPublicKey());
+      } finally {
+        serverFuture.get(5, TimeUnit.SECONDS);
+        executor.shutdown();
+      }
+    }
+  }
+}