You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by vi...@apache.org on 2019/01/28 19:12:04 UTC

hive git commit: HIVE-21083 : Remove the requirement to specify the truststore location when TLS to the database is turned on (Morio Ramdenbourg, reviewed by Karthik Manamcheri and Vihang Karajgaonkar)

Repository: hive
Updated Branches:
  refs/heads/master ce654250b -> 698206a29


HIVE-21083 : Remove the requirement to specify the truststore location when TLS to the database is turned on (Morio Ramdenbourg, reviewed by Karthik Manamcheri and Vihang Karajgaonkar)


Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/698206a2
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/698206a2
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/698206a2

Branch: refs/heads/master
Commit: 698206a293cadff81bfe5d9874b3a1ea3accf3bb
Parents: ce65425
Author: Morio Ramdenbourg <mr...@cloudera.com>
Authored: Mon Jan 28 10:55:49 2019 -0800
Committer: Vihang Karajgaonkar <vi...@apache.org>
Committed: Mon Jan 28 11:04:52 2019 -0800

----------------------------------------------------------------------
 .../hive/metastore/conf/MetastoreConf.java      | 15 +++---
 .../hadoop/hive/metastore/ObjectStore.java      | 48 +++++++++--------
 .../hadoop/hive/metastore/TestObjectStore.java  | 54 ++++++++++++--------
 3 files changed, 65 insertions(+), 52 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/698206a2/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
----------------------------------------------------------------------
diff --git a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
index 75f0c0a..313f87b 100644
--- a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
+++ b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
@@ -461,18 +461,21 @@ public class MetastoreConf {
     // If DBACCESS_USE_SSL is false, then all other DBACCESS_SSL_* properties will be ignored
     DBACCESS_SSL_TRUSTSTORE_PASSWORD("metastore.dbaccess.ssl.truststore.password", "hive.metastore.dbaccess.ssl.truststore.password", "",
         "Password for the Java truststore file that is used when encrypting the connection to the database store. \n"
-            + "This directly maps to the javax.net.ssl.trustStorePassword Java system property. \n"
-            + "While Java does allow an empty truststore password, we highly recommend against this. \n"
-            + "An empty password can compromise the integrity of the truststore file."),
+            + "metastore.dbaccess.ssl.use.SSL must be set to true for this property to take effect. \n"
+            + "This directly maps to the javax.net.ssl.trustStorePassword Java system property. Defaults to jssecacerts, if it exists, otherwise uses cacerts. \n"
+            + "It is recommended to specify the password using a credential provider so as to not expose it to discovery by other users. \n"
+            + "One way to do this is by using the Hadoop CredentialProvider API and provisioning credentials for this property. Refer to the Hadoop CredentialProvider API Guide for more details."),
     DBACCESS_SSL_TRUSTSTORE_PATH("metastore.dbaccess.ssl.truststore.path", "hive.metastore.dbaccess.ssl.truststore.path", "",
         "Location on disk of the Java truststore file to use when encrypting the connection to the database store. \n"
-            + "This directly maps to the javax.net.ssl.trustStore Java system property. \n"
-            + "This file consists of a collection of certificates trusted by the metastore server.\n"),
+            + "This file consists of a collection of certificates trusted by the metastore server. \n"
+            + "metastore.dbaccess.ssl.use.SSL must be set to true for this property to take effect. \n"
+            + "This directly maps to the javax.net.ssl.trustStore Java system property. Defaults to the default Java truststore file. \n"),
     DBACCESS_SSL_TRUSTSTORE_TYPE("metastore.dbaccess.ssl.truststore.type", "hive.metastore.dbaccess.ssl.truststore.type", "jks",
         new StringSetValidator("jceks", "jks", "dks", "pkcs11", "pkcs12"),
         "File type for the Java truststore file that is used when encrypting the connection to the database store. \n"
+            + "metastore.dbaccess.ssl.use.SSL must be set to true for this property to take effect. \n"
             + "This directly maps to the javax.net.ssl.trustStoreType Java system property. \n"
-            + "Types jceks, jks, dks, pkcs11, and pkcs12 can be read from Java 8 and beyond. We default to jks. \n"),
+            + "Types jceks, jks, dks, pkcs11, and pkcs12 can be read from Java 8 and beyond. Defaults to jks."),
     DBACCESS_USE_SSL("metastore.dbaccess.ssl.use.SSL", "hive.metastore.dbaccess.ssl.use.SSL", false,
         "Set this to true to use SSL encryption to the database store."),
 

http://git-wip-us.apache.org/repos/asf/hive/blob/698206a2/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
----------------------------------------------------------------------
diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
index 9f72124..0485fe9 100644
--- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
+++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
@@ -130,9 +130,9 @@ public class ObjectStore implements RawStore, Configurable {
   /**
    * Java system properties for configuring SSL to the database store
    */
-  private static final String TRUSTSTORE_PATH_KEY = "javax.net.ssl.trustStore";
-  private static final String TRUSTSTORE_PASSWORD_KEY = "javax.net.ssl.trustStorePassword";
-  private static final String TRUSTSTORE_TYPE_KEY = "javax.net.ssl.trustStoreType";
+  public static final String TRUSTSTORE_PATH_KEY = "javax.net.ssl.trustStore";
+  public static final String TRUSTSTORE_PASSWORD_KEY = "javax.net.ssl.trustStorePassword";
+  public static final String TRUSTSTORE_TYPE_KEY = "javax.net.ssl.trustStoreType";
 
   private static final Map<String, Class<?>> PINCLASSMAP;
   private static final String HOSTNAME;
@@ -333,18 +333,18 @@ public class ObjectStore implements RawStore, Configurable {
    *
    * The following properties must be set correctly to enable encryption:
    *
-   * 1. metastore.dbaccess.ssl.use.SSL
-   * 2. javax.jdo.option.ConnectionURL
-   * 3. metastore.dbaccess.ssl.truststore.path
-   * 4. metastore.dbaccess.ssl.truststore.password
-   * 5. metastore.dbaccess.ssl.truststore.type
+   * 1. {@link MetastoreConf.ConfVars#DBACCESS_USE_SSL}
+   * 2. {@link MetastoreConf.ConfVars#CONNECT_URL_KEY}
+   * 3. {@link MetastoreConf.ConfVars#DBACCESS_SSL_TRUSTSTORE_PATH}
+   * 4. {@link MetastoreConf.ConfVars#DBACCESS_SSL_TRUSTSTORE_PASSWORD}
+   * 5. {@link MetastoreConf.ConfVars#DBACCESS_SSL_TRUSTSTORE_TYPE}
    *
    * The last three properties directly map to JSSE (Java) system properties. The Java layer will handle enabling
    * encryption once these properties are set.
    *
-   * Additionally, javax.jdo.option.ConnectionURL must have the database-specific SSL flag in the connection URL.
+   * Additionally, {@link MetastoreConf.ConfVars#CONNECT_URL_KEY} must have the database-specific SSL flag in the connection URL.
    *
-   * @param conf
+   * @param conf configuration values
    */
   private static void configureSSL(Configuration conf) {
     configureSSLDeprecated(conf); // TODO: Deprecate this method
@@ -355,25 +355,23 @@ public class ObjectStore implements RawStore, Configurable {
       try {
         LOG.info("Setting SSL properties to connect to the database store");
         String trustStorePath = MetastoreConf.getVar(conf, ConfVars.DBACCESS_SSL_TRUSTSTORE_PATH).trim();
-        if (trustStorePath.isEmpty()) {
-          throw new IllegalArgumentException("SSL to the database store has been enabled but " + ConfVars.DBACCESS_SSL_TRUSTSTORE_PATH.toString() + " is empty. "
-              + "Set this property to enable SSL.");
+        // Specifying a truststore path is not necessary. If one is not provided, then the default Java truststore path will be used instead.
+        if (!trustStorePath.isEmpty()) {
+          System.setProperty(TRUSTSTORE_PATH_KEY, trustStorePath);
+        } else {
+          LOG.info(ConfVars.DBACCESS_SSL_TRUSTSTORE_PATH.toString() + " has not been set. Defaulting to jssecacerts, if it exists. Otherwise, cacerts.");
         }
         // If the truststore password has been configured and redacted properly using the Hadoop CredentialProvider API, then
         // MetastoreConf.getPassword() will securely decrypt it. Otherwise, it will default to being read in from the
         // configuration file in plain text.
         String trustStorePassword = MetastoreConf.getPassword(conf, ConfVars.DBACCESS_SSL_TRUSTSTORE_PASSWORD);
-        if (trustStorePassword.isEmpty()) {
-          LOG.warn("SSL has been enabled but " + ConfVars.DBACCESS_SSL_TRUSTSTORE_PASSWORD.toString() + " is empty. "
-              + "It is highly recommended to set this property. An empty truststore password could compromise the integrity of the truststore file. "
-              + "Arbitrary certificates could be placed into the truststore, thereby potentially exposing an attack vector to this application."
-              + "Continuing with SSL enabled.");
+        if (!trustStorePassword.isEmpty()) {
+          System.setProperty(TRUSTSTORE_PASSWORD_KEY, trustStorePassword);
+        } else {
+          LOG.info(ConfVars.DBACCESS_SSL_TRUSTSTORE_PASSWORD.toString() + " has not been set. Using default Java truststore password.");
         }
         // Already validated in MetaStoreConf
         String trustStoreType = MetastoreConf.getVar(conf, ConfVars.DBACCESS_SSL_TRUSTSTORE_TYPE);
-
-        System.setProperty(TRUSTSTORE_PATH_KEY, trustStorePath);
-        System.setProperty(TRUSTSTORE_PASSWORD_KEY, trustStorePassword);
         System.setProperty(TRUSTSTORE_TYPE_KEY, trustStoreType);
       } catch (IOException e) {
         throw new RuntimeException("Failed to set the SSL properties to connect to the database store.", e);
@@ -386,15 +384,15 @@ public class ObjectStore implements RawStore, Configurable {
    *
    * This method was kept for backwards compatibility purposes.
    *
-   * The property metastore.dbaccess.ssl.properties (hive.metastore.dbaccess.ssl.properties) was deprecated in
-   * HIVE-20992 in favor of more transparent and user-friendly properties.
+   * The property {@link MetastoreConf.ConfVars#DBACCESS_SSL_PROPS} was deprecated in HIVE-20992 in favor of more
+   * transparent and user-friendly properties.
    *
-   * Please use the javax.net.ssl.* properties instead. Setting those properties will overwrite the values
+   * Please use the MetastoreConf.ConfVars#DBACCESS_SSL_* instead. Setting those properties will overwrite the values
    * of the deprecated property.
    *
    * The process of completely removing this property and its functionality is being tracked in HIVE-21024.
    *
-   * @param conf Configuration
+   * @param conf configuration values
    */
   @Deprecated
   private static void configureSSLDeprecated(Configuration conf) {

http://git-wip-us.apache.org/repos/asf/hive/blob/698206a2/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java
----------------------------------------------------------------------
diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java
index 29738ba..0e814bc 100644
--- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java
+++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java
@@ -145,9 +145,9 @@ public class TestObjectStore {
   @After
   public void tearDown() throws Exception {
     // Clear the SSL system properties before each test.
-    System.clearProperty("javax.net.ssl.trustStore");
-    System.clearProperty("javax.net.ssl.trustStorePassword");
-    System.clearProperty("javax.net.ssl.trustStoreType");
+    System.clearProperty(ObjectStore.TRUSTSTORE_PATH_KEY);
+    System.clearProperty(ObjectStore.TRUSTSTORE_PASSWORD_KEY);
+    System.clearProperty(ObjectStore.TRUSTSTORE_TYPE_KEY);
   }
 
   @Test
@@ -1050,7 +1050,7 @@ public class TestObjectStore {
   }
 
   /**
-   * Test the property metastore.dbaccess.use.SSL (hive.metastore.dbaccess.use.SSL) to ensure that it correctly
+   * Test the property {@link MetastoreConf.ConfVars#DBACCESS_USE_SSL} to ensure that it correctly
    * toggles whether or not the SSL configuration parameters will be set. Effectively, this is testing whether
    * SSL can be turned on/off correctly.
    */
@@ -1060,32 +1060,36 @@ public class TestObjectStore {
   }
 
   /**
-   * Test that the deprecated property metastore.dbaccess.ssl.properties is overwritten by the javax.net.ssl.* properties
-   * if both are set.
+   * Test that the deprecated property {@link MetastoreConf.ConfVars#DBACCESS_SSL_PROPS} is overwritten by the
+   * MetastoreConf.ConfVars#DBACCESS_SSL_* properties if both are set.
    *
-   * This is not an ideal scenario. It is highly recommend to only set the javax.net.ssl.* properties.
+   * This is not an ideal scenario. It is highly recommend to only set the MetastoreConf#ConfVars.DBACCESS_SSL_* properties.
    */
   @Test
   public void testDeprecatedConfigIsOverwritten() {
     // Different from the values in the safe config
     MetastoreConf.setVar(conf, MetastoreConf.ConfVars.DBACCESS_SSL_PROPS,
-          "javax.net.ssl.trustStore=/tmp/truststore.p12,javax.net.ssl.trustStorePassword=pwd,javax.net.ssl.trustStoreType=pkcs12");
+        ObjectStore.TRUSTSTORE_PATH_KEY + "=/tmp/truststore.p12," + ObjectStore.TRUSTSTORE_PASSWORD_KEY + "=pwd," +
+            ObjectStore.TRUSTSTORE_TYPE_KEY + "=pkcs12");
 
     // Safe config
     setAndCheckSSLProperties(true, "/tmp/truststore.jks", "password", "jks");
   }
 
   /**
-   * Ensure that an empty trustStore path in metastore.dbaccess.ssl.truststore.path (hive.metastore.dbaccess.ssl.truststore.path)
-   * throws an IllegalArgumentException.
+   * Test that providing an empty truststore path and truststore password will not throw an exception.
    */
-  @Test(expected = IllegalArgumentException.class)
-  public void testEmptyTrustStorePath() {
-    setAndCheckSSLProperties(true, "", "password", "jks");
+  @Test
+  public void testEmptyTrustStoreProps() {
+    setAndCheckSSLProperties(true, "", "", "jks");
   }
 
   /**
    * Helper method for setting and checking the SSL configuration parameters.
+   * @param useSSL whether or not SSL is enabled
+   * @param trustStorePath truststore path, corresponding to the value for {@link MetastoreConf.ConfVars#DBACCESS_SSL_TRUSTSTORE_PATH}
+   * @param trustStorePassword truststore password, corresponding to the value for {@link MetastoreConf.ConfVars#DBACCESS_SSL_TRUSTSTORE_PASSWORD}
+   * @param trustStoreType truststore type, corresponding to the value for {@link MetastoreConf.ConfVars#DBACCESS_SSL_TRUSTSTORE_TYPE}
    */
   private void setAndCheckSSLProperties(boolean useSSL, String trustStorePath, String trustStorePassword, String trustStoreType) {
     MetastoreConf.setBoolVar(conf, MetastoreConf.ConfVars.DBACCESS_USE_SSL, useSSL);
@@ -1094,15 +1098,23 @@ public class TestObjectStore {
     MetastoreConf.setVar(conf, MetastoreConf.ConfVars.DBACCESS_SSL_TRUSTSTORE_TYPE, trustStoreType);
     objectStore.setConf(conf); // Calls configureSSL()
 
-    // Check that the Java system values correspond to the values that we set
-    if (useSSL) {
-      Assert.assertEquals(trustStorePath, System.getProperty("javax.net.ssl.trustStore"));
-      Assert.assertEquals(trustStorePassword, System.getProperty("javax.net.ssl.trustStorePassword"));
-      Assert.assertEquals(trustStoreType, System.getProperty("javax.net.ssl.trustStoreType"));
+    // Check that the properties were set correctly
+    checkSSLProperty(useSSL, ObjectStore.TRUSTSTORE_PATH_KEY, trustStorePath);
+    checkSSLProperty(useSSL, ObjectStore.TRUSTSTORE_PASSWORD_KEY, trustStorePassword);
+    checkSSLProperty(useSSL, ObjectStore.TRUSTSTORE_TYPE_KEY, trustStoreType);
+  }
+
+  /**
+   * Helper method to check whether the Java system properties were set correctly in {@link ObjectStore#configureSSL(Configuration)}
+   * @param useSSL whether or not SSL is enabled
+   * @param key Java system property key
+   * @param value Java system property value indicated by the key
+   */
+  private void checkSSLProperty(boolean useSSL, String key, String value) {
+    if (useSSL && !value.isEmpty()) {
+      Assert.assertEquals(value, System.getProperty(key));
     } else {
-      Assert.assertNull(System.getProperty("javax.net.ssl.trustStore"));
-      Assert.assertNull(System.getProperty("javax.net.ssl.trustStorePassword"));
-      Assert.assertNull(System.getProperty("javax.net.ssl.trustStoreType"));
+      Assert.assertNull(System.getProperty(key));
     }
   }