HADOOP-17259. Allow SSLFactory fallback to input config if ssl-*.xml … (#2301)

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ssl/SSLFactory.java

@@ -165,6 +165,13 @@ public static Configuration readSSLConfiguration(Configuration conf,
           SSL_SERVER_CONF_DEFAULT);
     }
     sslConf.addResource(sslConfResource);
+    // Only fallback to input config if classpath SSL config does not load for
+    // backward compatibility.
+    if (sslConf.getResource(sslConfResource) == null) {
+      LOG.debug("{} can't be loaded form classpath, fallback using SSL" +
+          " config from input configuration.", sslConfResource);
+      sslConf = conf;
+    }
     return sslConf;
   }
 

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/ssl/TestSSLFactory.java

@@ -17,7 +17,14 @@
  */
 package org.apache.hadoop.security.ssl;
 
+import static org.apache.hadoop.security.ssl.FileBasedKeyStoresFactory.SSL_TRUSTSTORE_LOCATION_TPL_KEY;
 import static org.apache.hadoop.security.ssl.KeyStoreTestUtil.TRUST_STORE_PASSWORD_DEFAULT;
+import static org.apache.hadoop.security.ssl.SSLFactory.Mode.CLIENT;
+import static org.apache.hadoop.security.ssl.SSLFactory.SSL_CLIENT_CONF_KEY;
+import static org.apache.hadoop.security.ssl.SSLFactory.SSL_REQUIRE_CLIENT_CERT_KEY;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import org.apache.hadoop.conf.Configuration;
@@ -66,6 +73,10 @@ public class TestSSLFactory {
       + "SSL_RSA_EXPORT_WITH_DES40_CBC_SHA,"
       + "SSL_RSA_WITH_RC4_128_MD5,"
       + "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA";
+  private static final Configuration FAKE_SSL_CONFIG =
+      KeyStoreTestUtil.createClientSSLConfig("clientKeyStoreLocation",
+          "keystorePassword", "keyPassword",
+          "trustStoreLocation", "trustStorePassword");
 
   @BeforeClass
   public static void setUp() throws Exception {
@@ -90,6 +101,55 @@ public void cleanUp() throws Exception {
     KeyStoreTestUtil.cleanupSSLConfig(KEYSTORES_DIR, sslConfsDir);
   }
 
+  private String getClientTrustStoreKeyName() {
+    return FileBasedKeyStoresFactory.resolvePropertyName(
+        CLIENT, SSL_TRUSTSTORE_LOCATION_TPL_KEY);
+  }
+
+  @Test
+  public void testNonExistSslClientXml() throws Exception{
+    Configuration conf = new Configuration(false);
+    conf.setBoolean(SSL_REQUIRE_CLIENT_CERT_KEY, false);
+    conf.set(SSL_CLIENT_CONF_KEY, "non-exist-ssl-client.xml");
+    Configuration sslConf =
+        SSLFactory.readSSLConfiguration(conf, SSLFactory.Mode.CLIENT);
+    assertNull(sslConf.getResource("non-exist-ssl-client.xml"));
+    assertNull(sslConf.get("ssl.client.truststore.location"));
+  }
+
+  @Test
+  public void testSslConfFallback() throws Exception {
+    Configuration conf = new Configuration(FAKE_SSL_CONFIG);
+
+    // Set non-exist-ssl-client.xml that fails to load.
+    // This triggers fallback to SSL config from input conf.
+    conf.set(SSL_CLIENT_CONF_KEY, "non-exist-ssl-client.xml");
+    Configuration sslConf = SSLFactory.readSSLConfiguration(conf, CLIENT);
+
+    // Verify fallback to input conf when ssl conf can't be loaded from
+    // classpath.
+    String clientTsLoc = sslConf.get(getClientTrustStoreKeyName());
+    assertEquals("trustStoreLocation", clientTsLoc);
+    assertEquals(sslConf, conf);
+  }
+
+  @Test
+  public void testSslConfClassPathFirst() throws Exception {
+    // Generate a valid ssl-client.xml into classpath.
+    // This will be the preferred approach.
+    Configuration conf = createConfiguration(false, true);
+
+    // Injecting fake ssl config into input conf.
+    conf.addResource(FAKE_SSL_CONFIG);
+
+    // Classpath SSL config will be preferred if both input conf and
+    // the classpath SSL config exist for backward compatibility.
+    Configuration sslConfLoaded = SSLFactory.readSSLConfiguration(conf, CLIENT);
+    String clientTsLoc = sslConfLoaded.get(getClientTrustStoreKeyName());
+    assertNotEquals("trustStoreLocation", clientTsLoc);
+    assertNotEquals(conf, sslConfLoaded);
+  }
+
   @Test(expected = IllegalStateException.class)
   public void clientMode() throws Exception {
     Configuration conf = createConfiguration(false, true);
@@ -452,7 +512,7 @@ private void checkSSLFactoryInitWithPasswords(SSLFactory.Mode mode,
     // Create the master configuration for use by the SSLFactory, which by
     // default refers to the ssl-server.xml or ssl-client.xml created above.
     Configuration conf = new Configuration();
-    conf.setBoolean(SSLFactory.SSL_REQUIRE_CLIENT_CERT_KEY, true);
+    conf.setBoolean(SSL_REQUIRE_CLIENT_CERT_KEY, true);
 
     // Try initializing an SSLFactory.
     SSLFactory sslFactory = new SSLFactory(mode, conf);
@@ -466,7 +526,7 @@ private void checkSSLFactoryInitWithPasswords(SSLFactory.Mode mode,
   @Test
   public void testNoClientCertsInitialization() throws Exception {
     Configuration conf = createConfiguration(false, true);
-    conf.unset(SSLFactory.SSL_REQUIRE_CLIENT_CERT_KEY);
+    conf.unset(SSL_REQUIRE_CLIENT_CERT_KEY);
     SSLFactory sslFactory = new SSLFactory(SSLFactory.Mode.CLIENT, conf);
     try {
       sslFactory.init();
@@ -478,7 +538,7 @@ public void testNoClientCertsInitialization() throws Exception {
   @Test
   public void testNoTrustStore() throws Exception {
     Configuration conf = createConfiguration(false, false);
-    conf.unset(SSLFactory.SSL_REQUIRE_CLIENT_CERT_KEY);
+    conf.unset(SSL_REQUIRE_CLIENT_CERT_KEY);
     SSLFactory sslFactory = new SSLFactory(SSLFactory.Mode.SERVER, conf);
     try {
       sslFactory.init();