diff --git a/common/src/java/org/apache/hadoop/hive/common/FileUtils.java b/common/src/java/org/apache/hadoop/hive/common/FileUtils.java index d755798..7df17a5 100644 --- a/common/src/java/org/apache/hadoop/hive/common/FileUtils.java +++ b/common/src/java/org/apache/hadoop/hive/common/FileUtils.java @@ -413,7 +413,12 @@ public Object run() throws Exception { * @throws IOException */ public static boolean isActionPermittedForFileHierarchy(FileSystem fs, FileStatus fileStatus, - String userName, FsAction action) throws Exception { + String userName, FsAction action) throws Exception { + return isActionPermittedForFileHierarchy(fs,fileStatus,userName, action, true); + } + + public static boolean isActionPermittedForFileHierarchy(FileSystem fs, FileStatus fileStatus, + String userName, FsAction action, boolean recurse) throws Exception { boolean isDir = fileStatus.isDir(); FsAction dirActionNeeded = action; @@ -429,15 +434,15 @@ public static boolean isActionPermittedForFileHierarchy(FileSystem fs, FileStatu return false; } - if (!isDir) { + if ((!isDir) || (!recurse)) { // no sub dirs to be checked return true; } // check all children FileStatus[] childStatuses = fs.listStatus(fileStatus.getPath()); for (FileStatus childStatus : childStatuses) { - // check children recursively - if (!isActionPermittedForFileHierarchy(fs, childStatus, userName, action)) { + // check children recursively - recurse is true if we're here. + if (!isActionPermittedForFileHierarchy(fs, childStatus, userName, action, true)) { return false; } } @@ -476,22 +481,27 @@ public static boolean isLocalFile(HiveConf conf, URI fileUri) { } return false; } - public static boolean isOwnerOfFileHierarchy(FileSystem fs, FileStatus fileStatus, String userName) throws IOException { + return isOwnerOfFileHierarchy(fs, fileStatus, userName, true); + } + + public static boolean isOwnerOfFileHierarchy(FileSystem fs, FileStatus fileStatus, + String userName, boolean recurse) + throws IOException { if (!fileStatus.getOwner().equals(userName)) { return false; } - if (!fileStatus.isDir()) { + if ((!fileStatus.isDir()) || (!recurse)) { // no sub dirs to be checked return true; } // check all children FileStatus[] childStatuses = fs.listStatus(fileStatus.getPath()); for (FileStatus childStatus : childStatuses) { - // check children recursively - if (!isOwnerOfFileHierarchy(fs, childStatus, userName)) { + // check children recursively - recurse is true if we're here. + if (!isOwnerOfFileHierarchy(fs, childStatus, userName, true)) { return false; } } diff --git a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLAuthorizationUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLAuthorizationUtils.java index b6b2699..94e059b 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLAuthorizationUtils.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLAuthorizationUtils.java @@ -379,22 +379,43 @@ public static RequiredPrivileges getPrivilegesFromFS(Path filePath, HiveConf con String userName) throws HiveAuthzPluginException { // get the 'available privileges' from file system - RequiredPrivileges availPrivs = new RequiredPrivileges(); // check file system permission FileSystem fs; try { fs = FileSystem.get(filePath.toUri(), conf); - FileStatus fileStatus = FileUtils.getPathOrParentThatExists(fs, filePath); - if (FileUtils.isOwnerOfFileHierarchy(fs, fileStatus, userName)) { - availPrivs.addPrivilege(SQLPrivTypeGrant.OWNER_PRIV); - } - if (FileUtils.isActionPermittedForFileHierarchy(fs, fileStatus, userName, FsAction.WRITE)) { - availPrivs.addPrivilege(SQLPrivTypeGrant.INSERT_NOGRANT); - availPrivs.addPrivilege(SQLPrivTypeGrant.DELETE_NOGRANT); - } - if (FileUtils.isActionPermittedForFileHierarchy(fs, fileStatus, userName, FsAction.READ)) { - availPrivs.addPrivilege(SQLPrivTypeGrant.SELECT_NOGRANT); + FileStatus[] fileMatches = fs.globStatus(filePath); + + // There are a couple of possibilities to consider here to see if we should recurse or not. + // a) Path is a regex, and may match multiple entries - if so, this is likely a load and + // we should listStatus for all relevant matches, and recurse-check each of those. + // Simply passing the filestatus on as recurse=true makes sense for this. + // b) Path is a singular directory/file and exists + // recurse=true to check all its children if applicable + // c) Path is a singular entity that does not exist + // recurse=false to check its parent - this is likely a case of + // needing to create a dir that does not exist yet. + + if ((fileMatches != null ) && (fileMatches.length > 1)){ + LOG.debug("Checking fs privileges for multiple files that matched {}", + filePath.toString()); + addPrivilegesFromFS(userName, availPrivs, fs, fileMatches, true); + } else { + FileStatus fileStatus = FileUtils.getFileStatusOrNull(fs, filePath); + boolean pickParent = (fileStatus == null); // did we find the file/dir itself? + if (pickParent){ + fileStatus = FileUtils.getPathOrParentThatExists(fs, filePath.getParent()); + } + Path path = fileStatus.getPath(); + if (pickParent){ + LOG.debug("Checking fs privileges for parent path {} for nonexistent {}", + path.toString(), filePath.toString()); + addPrivilegesFromFS(userName, availPrivs, fs, fileStatus, false); + } else { + LOG.debug("Checking fs privileges for path itself {}, originally specified as {}", + path.toString(), filePath.toString()); + addPrivilegesFromFS(userName, availPrivs, fs, fileStatus, true); + } } } catch (Exception e) { String msg = "Error getting permissions for " + filePath + ": " + e.getMessage(); @@ -403,6 +424,48 @@ public static RequiredPrivileges getPrivilegesFromFS(Path filePath, HiveConf con return availPrivs; } + private static void addPrivilegesFromFS( + String userName, RequiredPrivileges availPrivs, FileSystem fs, + FileStatus[] fileStatuses, boolean recurse) throws Exception { + // We need to obtain an intersection of all the privileges + if (fileStatuses.length > 0){ + Set privs = getPrivilegesFromFS(userName, fs, fileStatuses[0], recurse); + + for (int i = 1; (i < fileStatuses.length) && (privs.size() > 0); i++){ + privs.retainAll(getPrivilegesFromFS(userName, fs, fileStatuses[i], recurse)); + } + availPrivs.addAll(privs.toArray(new SQLPrivTypeGrant[privs.size()])); + } + } + + private static void addPrivilegesFromFS( + String userName, RequiredPrivileges availPrivs, FileSystem fs, + FileStatus fileStatus, boolean recurse) throws Exception { + Set privs = getPrivilegesFromFS(userName, fs, fileStatus, recurse); + availPrivs.addAll(privs.toArray(new SQLPrivTypeGrant[privs.size()])); + } + + private static Set getPrivilegesFromFS( + String userName, FileSystem fs, + FileStatus fileStatus, boolean recurse) throws Exception { + Set privs = new HashSet(); + LOG.debug("Checking fs privileges for {} {}", + fileStatus.toString(), (recurse? "recursively":"without recursion")); + if (FileUtils.isOwnerOfFileHierarchy(fs, fileStatus, userName, recurse)) { + privs.add(SQLPrivTypeGrant.OWNER_PRIV); + } + if (FileUtils.isActionPermittedForFileHierarchy(fs, fileStatus, userName, FsAction.WRITE, recurse)) { + privs.add(SQLPrivTypeGrant.INSERT_NOGRANT); + privs.add(SQLPrivTypeGrant.DELETE_NOGRANT); + } + if (FileUtils.isActionPermittedForFileHierarchy(fs, fileStatus, userName, FsAction.READ, recurse)) { + privs.add(SQLPrivTypeGrant.SELECT_NOGRANT); + } + LOG.debug("addPrivilegesFromFS:[{}] asked for privileges on [{}] with recurse={} and obtained:[{}]", + userName, fileStatus, recurse, privs); + return privs; + } + public static void assertNoDeniedPermissions(HivePrincipal hivePrincipal, HiveOperationType hiveOpType, List deniedMessages) throws HiveAccessControlException { if (deniedMessages.size() != 0) { diff --git a/ql/src/test/queries/clientpositive/authorization_load.q b/ql/src/test/queries/clientpositive/authorization_load.q new file mode 100644 index 0000000..92ae316 --- /dev/null +++ b/ql/src/test/queries/clientpositive/authorization_load.q @@ -0,0 +1,30 @@ +set hive.security.authorization.enabled=true; +set hive.security.authorization.manager=org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactory; +set hive.security.authenticator.manager=org.apache.hadoop.hive.ql.security.SessionStateConfigUserAuthenticator; + +-- the main goal of these tests is to run a simple load and a load with regex, while being in the scope of SQLStdHiveAuthorizer + +create table t_auth_load(key string, value string) stored as textfile; +create table t_auth_load2(key string, value string) stored as textfile; + +GRANT ALL on TABLE t_auth_load to ROLE public; +GRANT ALL on TABLE t_auth_load2 to ROLE public; + +load data local inpath '../../data/files/kv1.txt' into table t_auth_load; +load data local inpath '../../data/files/kv2.txt' into table t_auth_load; +load data local inpath '../../data/files/kv3.txt' into table t_auth_load; + +show table extended like t_auth_load; +desc extended t_auth_load; + +load data local inpath '../../data/files/kv[123].tx*' into table t_auth_load2; + +show table extended like t_auth_load2; +desc extended t_auth_load2; + +-- the following two selects should be identical +select count(*) from t_auth_load; +select count(*) from t_auth_load2; + + + diff --git a/ql/src/test/queries/clientpositive/exim_25_export_parentpath_has_inaccessible_children.q b/ql/src/test/queries/clientpositive/exim_25_export_parentpath_has_inaccessible_children.q new file mode 100644 index 0000000..af33cbe --- /dev/null +++ b/ql/src/test/queries/clientpositive/exim_25_export_parentpath_has_inaccessible_children.q @@ -0,0 +1,27 @@ +set hive.security.authorization.manager=org.apache.hadoop.hive.ql.security.authorization.DefaultHiveAuthorizationProvider; +set hive.security.authorization.manager=org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactory; +set hive.security.authenticator.manager=org.apache.hadoop.hive.ql.security.SessionStateConfigUserAuthenticator; + +set hive.test.mode=true; +set hive.test.mode.prefix=; + +create table t_exppath ( dep_id int) stored as textfile; +-- this test tests HIVE-10022, by showing that we can output to a directory that does not yet exist +-- even if we do not have permissions to other subdirs of the parent dir + +load data local inpath "../../data/files/test.dat" into table t_exppath; + +dfs ${system:test.dfs.mkdir} ${system:test.tmp.dir}/t_exppath/nope; +dfs -rmr ${system:test.tmp.dir}/t_exppath/; +dfs ${system:test.dfs.mkdir} ${system:test.tmp.dir}/t_exppath/; +dfs ${system:test.dfs.mkdir} ${system:test.tmp.dir}/t_exppath/nope; +dfs -chmod -R 400 ${system:test.tmp.dir}/t_exppath/nope; + +set hive.security.authorization.enabled=true; + +GRANT ALL on TABLE t_exppath to ROLE public; +export table t_exppath to '${system:test.tmp.dir}/t_exppath/yup'; + +set hive.security.authorization.enabled=false; +dfs -chmod -R 777 ${system:test.tmp.dir}/t_exppath/nope; +drop table t_exppath; diff --git a/ql/src/test/results/clientnegative/authorization_disallow_transform.q.out b/ql/src/test/results/clientnegative/authorization_disallow_transform.q.out index 75203f9..fcc35bd 100644 --- a/ql/src/test/results/clientnegative/authorization_disallow_transform.q.out +++ b/ql/src/test/results/clientnegative/authorization_disallow_transform.q.out @@ -10,4 +10,12 @@ POSTHOOK: query: create table t1(i int) POSTHOOK: type: CREATETABLE POSTHOOK: Output: database:default POSTHOOK: Output: default@t1 -FAILED: HiveAccessControlException Permission denied: Principal [name=hive_test_user, type=USER] does not have following privileges for operation QUERY [[SELECT] on Object [type=LOCAL_URI, name=cat]] +PREHOOK: query: SELECT TRANSFORM (*) USING 'cat' AS (key, value) FROM t1 +PREHOOK: type: QUERY +PREHOOK: Input: cat +PREHOOK: Input: default@t1 +#### A masked pattern was here #### +FAILED: Hive Internal Error: org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAccessControlException(Query with transform clause is disallowed in current configuration.) +org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAccessControlException: Query with transform clause is disallowed in current configuration. +#### A masked pattern was here #### + diff --git a/ql/src/test/results/clientpositive/authorization_load.q.out b/ql/src/test/results/clientpositive/authorization_load.q.out new file mode 100644 index 0000000..513ac53 --- /dev/null +++ b/ql/src/test/results/clientpositive/authorization_load.q.out @@ -0,0 +1,138 @@ +PREHOOK: query: -- the main goal of these tests is to run a simple load and a load with regex, while being in the scope of SQLStdHiveAuthorizer + +create table t_auth_load(key string, value string) stored as textfile +PREHOOK: type: CREATETABLE +PREHOOK: Output: database:default +PREHOOK: Output: default@t_auth_load +POSTHOOK: query: -- the main goal of these tests is to run a simple load and a load with regex, while being in the scope of SQLStdHiveAuthorizer + +create table t_auth_load(key string, value string) stored as textfile +POSTHOOK: type: CREATETABLE +POSTHOOK: Output: database:default +POSTHOOK: Output: default@t_auth_load +PREHOOK: query: create table t_auth_load2(key string, value string) stored as textfile +PREHOOK: type: CREATETABLE +PREHOOK: Output: database:default +PREHOOK: Output: default@t_auth_load2 +POSTHOOK: query: create table t_auth_load2(key string, value string) stored as textfile +POSTHOOK: type: CREATETABLE +POSTHOOK: Output: database:default +POSTHOOK: Output: default@t_auth_load2 +PREHOOK: query: GRANT ALL on TABLE t_auth_load to ROLE public +PREHOOK: type: GRANT_PRIVILEGE +PREHOOK: Output: default@t_auth_load +POSTHOOK: query: GRANT ALL on TABLE t_auth_load to ROLE public +POSTHOOK: type: GRANT_PRIVILEGE +POSTHOOK: Output: default@t_auth_load +PREHOOK: query: GRANT ALL on TABLE t_auth_load2 to ROLE public +PREHOOK: type: GRANT_PRIVILEGE +PREHOOK: Output: default@t_auth_load2 +POSTHOOK: query: GRANT ALL on TABLE t_auth_load2 to ROLE public +POSTHOOK: type: GRANT_PRIVILEGE +POSTHOOK: Output: default@t_auth_load2 +PREHOOK: query: load data local inpath '../../data/files/kv1.txt' into table t_auth_load +PREHOOK: type: LOAD +#### A masked pattern was here #### +PREHOOK: Output: default@t_auth_load +POSTHOOK: query: load data local inpath '../../data/files/kv1.txt' into table t_auth_load +POSTHOOK: type: LOAD +#### A masked pattern was here #### +POSTHOOK: Output: default@t_auth_load +PREHOOK: query: load data local inpath '../../data/files/kv2.txt' into table t_auth_load +PREHOOK: type: LOAD +#### A masked pattern was here #### +PREHOOK: Output: default@t_auth_load +POSTHOOK: query: load data local inpath '../../data/files/kv2.txt' into table t_auth_load +POSTHOOK: type: LOAD +#### A masked pattern was here #### +POSTHOOK: Output: default@t_auth_load +PREHOOK: query: load data local inpath '../../data/files/kv3.txt' into table t_auth_load +PREHOOK: type: LOAD +#### A masked pattern was here #### +PREHOOK: Output: default@t_auth_load +POSTHOOK: query: load data local inpath '../../data/files/kv3.txt' into table t_auth_load +POSTHOOK: type: LOAD +#### A masked pattern was here #### +POSTHOOK: Output: default@t_auth_load +PREHOOK: query: show table extended like t_auth_load +PREHOOK: type: SHOW_TABLESTATUS +POSTHOOK: query: show table extended like t_auth_load +POSTHOOK: type: SHOW_TABLESTATUS +tableName:t_auth_load +#### A masked pattern was here #### +inputformat:org.apache.hadoop.mapred.TextInputFormat +outputformat:org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat +columns:struct columns { string key, string value} +partitioned:false +partitionColumns: +totalNumberFiles:3 +totalFileSize:11819 +maxFileSize:5812 +minFileSize:216 +#### A masked pattern was here #### + +PREHOOK: query: desc extended t_auth_load +PREHOOK: type: DESCTABLE +PREHOOK: Input: default@t_auth_load +POSTHOOK: query: desc extended t_auth_load +POSTHOOK: type: DESCTABLE +POSTHOOK: Input: default@t_auth_load +key string +value string + +#### A masked pattern was here #### +PREHOOK: query: load data local inpath '../../data/files/kv[123].tx*' into table t_auth_load2 +PREHOOK: type: LOAD +#### A masked pattern was here #### +PREHOOK: Output: default@t_auth_load2 +POSTHOOK: query: load data local inpath '../../data/files/kv[123].tx*' into table t_auth_load2 +POSTHOOK: type: LOAD +#### A masked pattern was here #### +POSTHOOK: Output: default@t_auth_load2 +PREHOOK: query: show table extended like t_auth_load2 +PREHOOK: type: SHOW_TABLESTATUS +POSTHOOK: query: show table extended like t_auth_load2 +POSTHOOK: type: SHOW_TABLESTATUS +tableName:t_auth_load2 +#### A masked pattern was here #### +inputformat:org.apache.hadoop.mapred.TextInputFormat +outputformat:org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat +columns:struct columns { string key, string value} +partitioned:false +partitionColumns: +totalNumberFiles:3 +totalFileSize:11819 +maxFileSize:5812 +minFileSize:216 +#### A masked pattern was here #### + +PREHOOK: query: desc extended t_auth_load2 +PREHOOK: type: DESCTABLE +PREHOOK: Input: default@t_auth_load2 +POSTHOOK: query: desc extended t_auth_load2 +POSTHOOK: type: DESCTABLE +POSTHOOK: Input: default@t_auth_load2 +key string +value string + +#### A masked pattern was here #### +PREHOOK: query: -- the following two selects should be identical +select count(*) from t_auth_load +PREHOOK: type: QUERY +PREHOOK: Input: default@t_auth_load +#### A masked pattern was here #### +POSTHOOK: query: -- the following two selects should be identical +select count(*) from t_auth_load +POSTHOOK: type: QUERY +POSTHOOK: Input: default@t_auth_load +#### A masked pattern was here #### +1025 +PREHOOK: query: select count(*) from t_auth_load2 +PREHOOK: type: QUERY +PREHOOK: Input: default@t_auth_load2 +#### A masked pattern was here #### +POSTHOOK: query: select count(*) from t_auth_load2 +POSTHOOK: type: QUERY +POSTHOOK: Input: default@t_auth_load2 +#### A masked pattern was here #### +1025 diff --git a/ql/src/test/results/clientpositive/exim_25_export_parentpath_has_inaccessible_children.q.out b/ql/src/test/results/clientpositive/exim_25_export_parentpath_has_inaccessible_children.q.out new file mode 100644 index 0000000..4d0aa66 --- /dev/null +++ b/ql/src/test/results/clientpositive/exim_25_export_parentpath_has_inaccessible_children.q.out @@ -0,0 +1,44 @@ +PREHOOK: query: create table t_exppath ( dep_id int) stored as textfile +PREHOOK: type: CREATETABLE +PREHOOK: Output: database:default +PREHOOK: Output: default@t_exppath +POSTHOOK: query: create table t_exppath ( dep_id int) stored as textfile +POSTHOOK: type: CREATETABLE +POSTHOOK: Output: database:default +POSTHOOK: Output: default@t_exppath +PREHOOK: query: -- this test tests HIVE-10022, by showing that we can output to a directory that does not yet exist +-- even if we do not have permissions to other subdirs of the parent dir + +load data local inpath "../../data/files/test.dat" into table t_exppath +PREHOOK: type: LOAD +#### A masked pattern was here #### +PREHOOK: Output: default@t_exppath +POSTHOOK: query: -- this test tests HIVE-10022, by showing that we can output to a directory that does not yet exist +-- even if we do not have permissions to other subdirs of the parent dir + +load data local inpath "../../data/files/test.dat" into table t_exppath +POSTHOOK: type: LOAD +#### A masked pattern was here #### +POSTHOOK: Output: default@t_exppath +#### A masked pattern was here #### +PREHOOK: query: GRANT ALL on TABLE t_exppath to ROLE public +PREHOOK: type: GRANT_PRIVILEGE +PREHOOK: Output: default@t_exppath +POSTHOOK: query: GRANT ALL on TABLE t_exppath to ROLE public +POSTHOOK: type: GRANT_PRIVILEGE +POSTHOOK: Output: default@t_exppath +#### A masked pattern was here #### +PREHOOK: type: EXPORT +PREHOOK: Input: default@t_exppath +#### A masked pattern was here #### +POSTHOOK: type: EXPORT +POSTHOOK: Input: default@t_exppath +#### A masked pattern was here #### +PREHOOK: query: drop table t_exppath +PREHOOK: type: DROPTABLE +PREHOOK: Input: default@t_exppath +PREHOOK: Output: default@t_exppath +POSTHOOK: query: drop table t_exppath +POSTHOOK: type: DROPTABLE +POSTHOOK: Input: default@t_exppath +POSTHOOK: Output: default@t_exppath