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..8e3ce02 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 @@ -29,6 +29,7 @@ import java.util.Map; import java.util.Set; +import com.google.common.base.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.fs.FileStatus; @@ -379,22 +380,38 @@ 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.length > 1){ + LOG.debug("Checking fs privileges for multiple files that matched "+filePath); + for (FileStatus matchedStatus : fileMatches){ + addPrivilegesFromFs(userName, availPrivs, fs, matchedStatus, true); + } + } else { + FileStatus fileStatus = FileUtils.getPathOrParentThatExists(fs, filePath); + Path path = fileStatus.getPath(); + if (!Strings.isNullOrEmpty(path.toString()) && !path.equals(filePath)) { + LOG.debug("Checking fs privileges for parent path " + + path + " for nonexistent "+filePath); + addPrivilegesFromFs(userName, availPrivs, fs, fileStatus, false); + } else { + addPrivilegesFromFs(userName, availPrivs, fs, fileStatus, true); + } } } catch (Exception e) { String msg = "Error getting permissions for " + filePath + ": " + e.getMessage(); @@ -403,6 +420,25 @@ public static RequiredPrivileges getPrivilegesFromFS(Path filePath, HiveConf con return availPrivs; } + private static void addPrivilegesFromFs( + String userName, RequiredPrivileges availPrivs, FileSystem fs, + FileStatus fileStatus, boolean recurse) throws Exception { + if (LOG.isDebugEnabled()){ + LOG.debug("Checking fs privileges for "+ fileStatus + + (recurse? " recursively":" without recursion")); + } + if (FileUtils.isOwnerOfFileHierarchy(fs, fileStatus, userName, recurse)) { + availPrivs.addPrivilege(SQLPrivTypeGrant.OWNER_PRIV); + } + if (FileUtils.isActionPermittedForFileHierarchy(fs, fileStatus, userName, FsAction.WRITE, recurse)) { + availPrivs.addPrivilege(SQLPrivTypeGrant.INSERT_NOGRANT); + availPrivs.addPrivilege(SQLPrivTypeGrant.DELETE_NOGRANT); + } + if (FileUtils.isActionPermittedForFileHierarchy(fs, fileStatus, userName, FsAction.READ, recurse)) { + availPrivs.addPrivilege(SQLPrivTypeGrant.SELECT_NOGRANT); + } + } + 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/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