Repository: hive
Updated Branches:
  refs/heads/master 4e415609c -> 4d9df0fdf


HIVE-20992 : Split the config hive.metastore.dbaccess.ssl.properties into more 
meaningful configs (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/4d9df0fd
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/4d9df0fd
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/4d9df0fd

Branch: refs/heads/master
Commit: 4d9df0fdf4f31adceacb67195497deddbf624103
Parents: 4e41560
Author: Morio Ramdenbourg <mramdenbo...@cloudera.com>
Authored: Mon Dec 17 13:25:35 2018 -0800
Committer: Vihang Karajgaonkar <vihan...@apache.org>
Committed: Mon Dec 17 13:58:16 2018 -0800

----------------------------------------------------------------------
 .../hive/metastore/conf/MetastoreConf.java      | 35 ++++++++-
 .../hadoop/hive/metastore/ObjectStore.java      | 78 +++++++++++++++++++-
 .../hadoop/hive/metastore/TestObjectStore.java  | 74 +++++++++++++++++++
 3 files changed, 181 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/4d9df0fd/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 e25a8cf..400097b 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
@@ -247,7 +247,9 @@ public class MetastoreConf {
       ConfVars.SSL_KEYSTORE_PASSWORD.varname,
       ConfVars.SSL_KEYSTORE_PASSWORD.hiveName,
       ConfVars.SSL_TRUSTSTORE_PASSWORD.varname,
-      ConfVars.SSL_TRUSTSTORE_PASSWORD.hiveName
+      ConfVars.SSL_TRUSTSTORE_PASSWORD.hiveName,
+      ConfVars.DBACCESS_SSL_TRUSTSTORE_PASSWORD.varname,
+      ConfVars.DBACCESS_SSL_TRUSTSTORE_PASSWORD.hiveName
   );
 
   public static ConfVars getMetaConf(String name) {
@@ -452,9 +454,26 @@ public class MetastoreConf {
         "Default transaction isolation level for identity generation."),
     
DATANUCLEUS_USE_LEGACY_VALUE_STRATEGY("datanucleus.rdbms.useLegacyNativeValueStrategy",
         "datanucleus.rdbms.useLegacyNativeValueStrategy", true, ""),
-    DBACCESS_SSL_PROPS("metastore.dbaccess.ssl.properties", 
"hive.metastore.dbaccess.ssl.properties", "",
-        "Comma-separated SSL properties for metastore to access database when 
JDO connection URL\n" +
-            "enables SSL access. e.g. 
javax.net.ssl.trustStore=/tmp/truststore,javax.net.ssl.trustStorePassword=pwd."),
+
+    // Parameters for configuring SSL encryption to the database store
+    // 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."),
+    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"),
+    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"
+            + "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"),
+    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."),
+
     DEFAULTPARTITIONNAME("metastore.default.partition.name",
         "hive.exec.default.partition.name", "__HIVE_DEFAULT_PARTITION__",
         "The default partition name in case the dynamic partition column value 
is null/empty string or any other values that cannot be escaped. \n" +
@@ -1070,6 +1089,14 @@ public class MetastoreConf {
             "Deprecated, use METRICS_REPORTERS instead. This configuraiton 
will be"
             + " overridden by HIVE_CODAHALE_METRICS_REPORTER_CLASSES and 
METRICS_REPORTERS if " +
             "present. Comma separated list of JMX, CONSOLE, JSON_FILE, 
HADOOP2"),
+    // Planned to be removed in HIVE-21024
+    @Deprecated
+    DBACCESS_SSL_PROPS("metastore.dbaccess.ssl.properties", 
"hive.metastore.dbaccess.ssl.properties", "",
+        "Deprecated. Use the metastore.dbaccess.ssl.* properties instead. 
Comma-separated SSL properties for " +
+            "metastore to access database when JDO connection URL enables SSL 
access. \n"
+            + "e.g. 
javax.net.ssl.trustStore=/tmp/truststore,javax.net.ssl.trustStorePassword=pwd.\n
 " +
+            "If both this and the metastore.dbaccess.ssl.* properties are set, 
then the latter properties \n" +
+            "will overwrite what was set in the deprecated property."),
 
     // These are all values that we put here just for testing
     STR_TEST_ENTRY("test.str", "hive.test.str", "defaultval", "comment"),

http://git-wip-us.apache.org/repos/asf/hive/blob/4d9df0fd/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 3fa21b7..0324a19 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
@@ -127,6 +127,13 @@ public class ObjectStore implements RawStore, Configurable 
{
     NO_STATE, OPEN, COMMITED, ROLLBACK
   }
 
+  /**
+   * 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";
+
   private static final Map<String, Class<?>> PINCLASSMAP;
   private static final String HOSTNAME;
   private static final String USER;
@@ -322,14 +329,81 @@ public class ObjectStore implements RawStore, 
Configurable {
   }
 
   /**
-   * Configure the SSL properties of the connection from provided config
+   * Configure SSL encryption to the database store.
+   *
+   * 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
+   *
+   * 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.
+   *
    * @param conf
    */
   private static void configureSSL(Configuration conf) {
+    configureSSLDeprecated(conf); // TODO: Deprecate this method
+
+    boolean useSSL = MetastoreConf.getBoolVar(conf, ConfVars.DBACCESS_USE_SSL);
+
+    if (useSSL) {
+      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.");
+        }
+        // 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.");
+        }
+        // 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);
+      }
+    }
+  }
+
+  /**
+   * Configure the SSL properties of the connection from provided config
+   *
+   * 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.
+   *
+   * Please use the javax.net.ssl.* properties 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
+   */
+  @Deprecated
+  private static void configureSSLDeprecated(Configuration conf) {
     // SSL support
     String sslPropString = MetastoreConf.getVar(conf, 
ConfVars.DBACCESS_SSL_PROPS);
     if (org.apache.commons.lang.StringUtils.isNotEmpty(sslPropString)) {
-      LOG.info("Metastore setting SSL properties of the connection to backed 
DB");
+      LOG.warn("Configuring SSL using a deprecated key " + 
ConfVars.DBACCESS_SSL_PROPS.toString() +
+              ". This may be removed in the future. See HIVE-20992 for more 
details.");
+      LOG.info("Metastore setting SSL properties of the connection to backend 
DB");
       for (String sslProp : sslPropString.split(",")) {
         String[] pair = sslProp.trim().split("=");
         if (pair != null && pair.length == 2) {

http://git-wip-us.apache.org/repos/asf/hive/blob/4d9df0fd/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 0cf113c..29738ba 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
@@ -68,6 +68,7 @@ import 
org.apache.hadoop.hive.metastore.model.MNotificationNextId;
 import org.junit.Assert;
 import org.junit.Assume;
 import org.junit.Before;
+import org.junit.After;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -141,6 +142,14 @@ public class TestObjectStore {
     HiveMetaStore.HMSHandler.createDefaultCatalog(objectStore, new 
Warehouse(conf));
   }
 
+  @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");
+  }
+
   @Test
   public void catalogs() throws MetaException, NoSuchObjectException {
     final String names[] = {"cat1", "cat2"};
@@ -1032,6 +1041,71 @@ public class TestObjectStore {
         counter.get());
   }
 
+  /**
+   * Test the SSL configuration parameters to ensure that they modify the Java 
system properties correctly.
+   */
+  @Test
+  public void testSSLPropertiesAreSet() {
+    setAndCheckSSLProperties(true, "/tmp/truststore.p12", "password", 
"pkcs12");
+  }
+
+  /**
+   * Test the property metastore.dbaccess.use.SSL 
(hive.metastore.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.
+   */
+  @Test
+  public void testUseSSLProperty() {
+    setAndCheckSSLProperties(false, "/tmp/truststore.jks", "password", "jks");
+  }
+
+  /**
+   * Test that the deprecated property metastore.dbaccess.ssl.properties is 
overwritten by the javax.net.ssl.* properties
+   * if both are set.
+   *
+   * This is not an ideal scenario. It is highly recommend to only set the 
javax.net.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");
+
+    // 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(expected = IllegalArgumentException.class)
+  public void testEmptyTrustStorePath() {
+    setAndCheckSSLProperties(true, "", "password", "jks");
+  }
+
+  /**
+   * Helper method for setting and checking the SSL configuration parameters.
+   */
+  private void setAndCheckSSLProperties(boolean useSSL, String trustStorePath, 
String trustStorePassword, String trustStoreType) {
+    MetastoreConf.setBoolVar(conf, MetastoreConf.ConfVars.DBACCESS_USE_SSL, 
useSSL);
+    MetastoreConf.setVar(conf, 
MetastoreConf.ConfVars.DBACCESS_SSL_TRUSTSTORE_PATH, trustStorePath);
+    MetastoreConf.setVar(conf, 
MetastoreConf.ConfVars.DBACCESS_SSL_TRUSTSTORE_PASSWORD, trustStorePassword);
+    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"));
+    } 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"));
+    }
+  }
+
   private void createTestCatalog(String catName) throws MetaException {
     Catalog cat = new CatalogBuilder()
         .setName(catName)

Reply via email to