You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by da...@apache.org on 2016/11/08 23:49:52 UTC

hive git commit: HIVE-15120: Storage based auth: allow option to enforce write checks for external tables (Daniel Dai, reviewed by Thejas Nair)

Repository: hive
Updated Branches:
  refs/heads/master f1b41ce30 -> d6748dc34


HIVE-15120: Storage based auth: allow option to enforce write checks for external tables (Daniel Dai, reviewed by Thejas Nair)


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

Branch: refs/heads/master
Commit: d6748dc34de10a4a70d0f3a68afc9f921bc857a4
Parents: f1b41ce
Author: Daniel Dai <da...@hortonworks.com>
Authored: Tue Nov 8 15:49:39 2016 -0800
Committer: Daniel Dai <da...@hortonworks.com>
Committed: Tue Nov 8 15:49:39 2016 -0800

----------------------------------------------------------------------
 .../org/apache/hadoop/hive/conf/HiveConf.java   |  5 +++++
 ...edMetastoreAuthorizationProviderWithACL.java |  6 ++++++
 .../TestMetastoreAuthorizationProvider.java     | 21 +++++++++++++++++++-
 ...rageBasedMetastoreAuthorizationProvider.java |  6 ++++++
 .../StorageBasedAuthorizationProvider.java      |  9 +++++++--
 5 files changed, 44 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/d6748dc3/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
index d287b45..b2ef5f2 100644
--- a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
+++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
@@ -770,6 +770,11 @@ public class HiveConf extends Configuration {
         "for operations like drop-partition (disallow the drop-partition if the user in\n" +
         "question doesn't have permissions to delete the corresponding directory\n" +
         "on the storage)."),
+    METASTORE_AUTHORIZATION_EXTERNALTABLE_DROP_CHECK("hive.metastore.authorization.storage.check.externaltable.drop", true,
+        "Should StorageBasedAuthorization check permission of the storage before dropping external table.\n" +
+        "StorageBasedAuthorization already does this check for managed table. For external table however,\n" +
+        "anyone who has read permission of the directory could drop external table, which is surprising.\n" +
+        "The flag is set to false by default to maintain backward compatibility."),
     METASTORE_EVENT_CLEAN_FREQ("hive.metastore.event.clean.freq", "0s",
         new TimeValidator(TimeUnit.SECONDS),
         "Frequency at which timer task runs to purge expired events in metastore."),

http://git-wip-us.apache.org/repos/asf/hive/blob/d6748dc3/itests/hive-unit-hadoop2/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProviderWithACL.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit-hadoop2/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProviderWithACL.java b/itests/hive-unit-hadoop2/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProviderWithACL.java
index 2c6404b..028c117 100644
--- a/itests/hive-unit-hadoop2/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProviderWithACL.java
+++ b/itests/hive-unit-hadoop2/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProviderWithACL.java
@@ -208,6 +208,12 @@ public class TestStorageBasedMetastoreAuthorizationProviderWithACL
   }
 
   @Override
+  protected void disallowDropOnTable(String tblName, String userName, String location)
+      throws Exception {
+    disallowWriteAccessViaAcl(userName, location);
+  }
+
+  @Override
   protected void allowDropOnDb(String dbName, String userName, String location)
       throws Exception {
     allowWriteAccessViaAcl(userName, location);

http://git-wip-us.apache.org/repos/asf/hive/blob/d6748dc3/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestMetastoreAuthorizationProvider.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestMetastoreAuthorizationProvider.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestMetastoreAuthorizationProvider.java
index c228720..19dc9cf 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestMetastoreAuthorizationProvider.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestMetastoreAuthorizationProvider.java
@@ -19,6 +19,7 @@
 package org.apache.hadoop.hive.ql.security;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 
@@ -300,7 +301,20 @@ public class TestMetastoreAuthorizationProvider extends TestCase {
 
     allowDropOnTable(tblName, userName, tbl.getSd().getLocation());
     allowDropOnDb(dbName,userName,db.getLocationUri());
-    driver.run("drop database if exists "+getTestDbName()+" cascade");
+    ret = driver.run("drop database if exists "+getTestDbName()+" cascade");
+    assertEquals(0,ret.getResponseCode());
+
+    InjectableDummyAuthenticator.injectUserName(userName);
+    InjectableDummyAuthenticator.injectGroupNames(Arrays.asList(ugi.getGroupNames()));
+    InjectableDummyAuthenticator.injectMode(true);
+    allowCreateDatabase(userName);
+    driver.run("create database " + dbName);
+    allowCreateInDb(dbName, userName, dbLocn);
+    tbl.setTableType("EXTERNAL_TABLE");
+    msc.createTable(tbl);
+    disallowDropOnTable(tblName, userName, tbl.getSd().getLocation());
+    ret = driver.run("drop table "+tbl.getTableName());
+    assertEquals(1,ret.getResponseCode());
 
   }
 
@@ -340,6 +354,11 @@ public class TestMetastoreAuthorizationProvider extends TestCase {
     driver.run("grant drop on table "+tblName+" to user "+userName);
   }
 
+  protected void disallowDropOnTable(String tblName, String userName, String location)
+      throws Exception {
+    driver.run("revoke drop on table "+tblName+" from user "+userName);
+  }
+
   protected void allowDropOnDb(String dbName, String userName, String location)
       throws Exception {
     driver.run("grant drop on database "+dbName+" to user "+userName);

http://git-wip-us.apache.org/repos/asf/hive/blob/d6748dc3/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProvider.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProvider.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProvider.java
index 78ff780..272c924 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProvider.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProvider.java
@@ -77,6 +77,12 @@ public class TestStorageBasedMetastoreAuthorizationProvider extends
   }
 
   @Override
+  protected void disallowDropOnTable(String tblName, String userName, String location)
+      throws Exception {
+    setPermissions(location,"-r--r--r--");
+  }
+
+  @Override
   protected void allowDropOnDb(String dbName, String userName, String location)
       throws Exception {
     setPermissions(location,"-rwxr--r-t");

http://git-wip-us.apache.org/repos/asf/hive/blob/d6748dc3/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java
index 7992a70..32d8b3e 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java
@@ -35,6 +35,7 @@ import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.permission.FsAction;
 import org.apache.hadoop.hive.common.FileUtils;
+import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.metastore.HiveMetaStore.HMSHandler;
 import org.apache.hadoop.hive.metastore.TableType;
 import org.apache.hadoop.hive.metastore.Warehouse;
@@ -177,8 +178,12 @@ public class StorageBasedAuthorizationProvider extends HiveAuthorizationProvider
 
     Path path = table.getDataLocation();
     // authorize drops if there was a drop privilege requirement, and
-    // table is not external (external table data is not dropped)
-    if (privExtractor.hasDropPrivilege() && table.getTableType() != TableType.EXTERNAL_TABLE) {
+    // table is not external (external table data is not dropped) or
+    // "hive.metastore.authorization.storage.check.externaltable.drop"
+    // set to true
+    if (privExtractor.hasDropPrivilege() && (table.getTableType() != TableType.EXTERNAL_TABLE ||
+        getConf().getBoolean(HiveConf.ConfVars.METASTORE_AUTHORIZATION_EXTERNALTABLE_DROP_CHECK.varname,
+        HiveConf.ConfVars.METASTORE_AUTHORIZATION_EXTERNALTABLE_DROP_CHECK.defaultBoolVal))) {
       checkDeletePermission(path, getConf(), authenticator.getUserName());
     }