Index: common/src/java/org/apache/hadoop/hive/common/FileUtils.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- common/src/java/org/apache/hadoop/hive/common/FileUtils.java (date 1424399922000) +++ common/src/java/org/apache/hadoop/hive/common/FileUtils.java (revision ) @@ -25,12 +25,16 @@ import java.security.AccessControlException; import java.security.PrivilegedExceptionAction; import java.util.BitSet; +import java.util.EnumSet; +import java.util.Iterator; import java.util.List; +import com.google.common.base.Function; +import com.google.common.collect.Iterators; +import com.google.common.collect.Lists; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.fs.DefaultFileAccess; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FileUtil; @@ -46,7 +50,6 @@ import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.util.Shell; - /** * Collection of file manipulation utilities common across Hive. */ @@ -369,27 +372,55 @@ public static void checkFileAccessWithImpersonation(final FileSystem fs, final FileStatus stat, final FsAction action, final String user) throws IOException, AccessControlException, InterruptedException, Exception { + checkFileAccessWithImpersonation(fs, + Iterators.singletonIterator(stat), + EnumSet.of(action), + user); + } + + /** + * Perform a check to determine if the user is able to access the file passed in. + * If the user name passed in is different from the current user, this method will + * attempt to do impersonate the user to do the check; the current user should be + * able to create proxy users in this case. + * @param fs FileSystem of the path to check + * @param statuses FileStatus instances representing the files being checked + * @param actions The FsActions that will be checked + * @param user User name of the user that will be checked for access. If the user name + * is null or the same as the current user, no user impersonation will be done + * and the check will be done as the current user. Otherwise the file access + * check will be performed within a doAs() block to use the access privileges + * of this user. In this case the user must be configured to impersonate other + * users, otherwise this check will fail with error. + * @throws IOException + * @throws AccessControlException + * @throws InterruptedException + * @throws Exception + */ + public static void checkFileAccessWithImpersonation(final FileSystem fs, + final Iterator statuses, final EnumSet actions, final String user) + throws IOException, AccessControlException, InterruptedException, Exception { UserGroupInformation ugi = Utils.getUGI(); String currentUser = ugi.getShortUserName(); if (user == null || currentUser.equals(user)) { // No need to impersonate user, do the checks as the currently configured user. - ShimLoader.getHadoopShims().checkFileAccess(fs, stat, action); - return; + ShimLoader.getHadoopShims().checkFileAccess(fs, statuses, actions); } - + else { - // Otherwise, try user impersonation. Current user must be configured to do user impersonation. - UserGroupInformation proxyUser = UserGroupInformation.createProxyUser( - user, UserGroupInformation.getLoginUser()); - proxyUser.doAs(new PrivilegedExceptionAction() { - @Override - public Object run() throws Exception { - FileSystem fsAsUser = FileSystem.get(fs.getUri(), fs.getConf()); + // Otherwise, try user impersonation. Current user must be configured to do user impersonation. + UserGroupInformation proxyUser = UserGroupInformation.createProxyUser( + user, UserGroupInformation.getLoginUser()); + proxyUser.doAs(new PrivilegedExceptionAction() { + @Override + public Object run() throws Exception { + FileSystem fsAsUser = FileSystem.get(fs.getUri(), fs.getConf()); - ShimLoader.getHadoopShims().checkFileAccess(fsAsUser, stat, action); + ShimLoader.getHadoopShims().checkFileAccess(fsAsUser, statuses, actions); - return null; - } - }); - } + return null; + } + }); + } + } /** * Check if user userName has permissions to perform the given FsAction action @@ -711,36 +742,65 @@ // no file/dir to be deleted return; } - FileUtils.checkFileAccessWithImpersonation(fs, stat, FsAction.WRITE, user); + checkDeletePermission(path.getFileSystem(conf), Lists.newArrayList(stat), conf, user); + + } + /** + * Checks if delete can be performed for the specified FileStatus instances. + * If file does not exist it just returns without throwing an Exception + * @param fs The FileSystem instance + * @param fileStatuses The FileStatus instances for the paths being checked. + * @param conf Configuration, corresponding to the FileSystem. + * @param user The user, whose permission is to be checked. + * @throws AccessControlException + * @throws InterruptedException + * @throws Exception + */ + public static void checkDeletePermission(FileSystem fs, Iterable fileStatuses, + Configuration conf, String user) throws Exception { + // This requires ability to delete the given path. + // The following 2 conditions should be satisfied for this- + // 1. Write permissions on parent dir + // 2. If sticky bit is set on parent dir then one of following should be + // true + // a. User is owner of the current dir/file + // b. User is owner of the parent dir + FileUtils.checkFileAccessWithImpersonation(fs, fileStatuses.iterator(), EnumSet.of(FsAction.WRITE), user); HadoopShims shims = ShimLoader.getHadoopShims(); if (!shims.supportStickyBit()) { - // not supports sticky bit + // No support for sticky-bit. return; } - - // check if sticky bit is set on the parent dir - FileStatus parStatus = fs.getFileStatus(path.getParent()); - if (!shims.hasStickyBit(parStatus.getPermission())) { - // no sticky bit, so write permission on parent dir is sufficient - // no further checks needed - return; + List allParentPaths = + Lists.newArrayList( + Iterators.transform(fileStatuses.iterator(), new Function() { + @Override + public Path apply(FileStatus input) { + return input.getPath().getParent(); - } + } - - // check if user is owner of parent dir - if (parStatus.getOwner().equals(user)) { - return; + }) + ); + Iterator childStatusIterator = fileStatuses.iterator(); + for (List parentPaths : Lists.partition(allParentPaths, getListStatusBatchSize(conf)) ) { + for (FileStatus parentFileStatus : fs.listStatus(parentPaths.toArray(new Path[parentPaths.size()]))) { + assert childStatusIterator.hasNext() : "Number of parent-file-statuses doesn't match children."; + FileStatus childFileStatus = childStatusIterator.next(); + // Check sticky-bits on parent-dirs. + if (shims.hasStickyBit(parentFileStatus.getPermission()) + && !parentFileStatus.getOwner().equals(user) + && !childFileStatus.getOwner().equals(user)) { + throw new IOException(String.format("Permission Denied: User %s can't delete %s because sticky bit is\"" + + " set on the parent dir and user does not own this file or its parent\"", user, childFileStatus.getPath())); - } + } + } // for_each( parentPath ); - // check if user is owner of current dir/file - FileStatus childStatus = fs.getFileStatus(path); - if (childStatus.getOwner().equals(user)) { - return; - } - String msg = String.format("Permission Denied: User %s can't delete %s because sticky bit is" - + " set on the parent dir and user does not own this file or its parent", user, path); - throw new IOException(msg); + } // for_each( batch-of-parentPaths ); - } + } // static void checkDeletePermission(); + private static int getListStatusBatchSize(Configuration configuration) { + return HiveConf.getIntVar(configuration, + HiveConf.ConfVars.HIVE_AUTHORIZATION_HDFS_LIST_STATUS_BATCH_SIZE); + } } Index: shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java (date 1424399922000) +++ shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java (revision ) @@ -28,11 +28,11 @@ import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Comparator; +import java.util.EnumSet; import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.TreeMap; import org.apache.commons.lang.StringUtils; @@ -40,7 +40,6 @@ import org.apache.hadoop.crypto.key.KeyProvider; import org.apache.hadoop.crypto.key.KeyProvider.Options; import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension; -import org.apache.hadoop.crypto.key.KeyProviderFactory; import org.apache.hadoop.fs.BlockLocation; import org.apache.hadoop.fs.DefaultFileAccess; import org.apache.hadoop.fs.FSDataInputStream; @@ -951,6 +950,27 @@ } else { accessMethod.invoke(fs, stat.getPath(), action); } + } catch (Exception err) { + throw wrapAccessException(err); + } + } + + @Override + public void checkFileAccess(FileSystem fs, Iterator statuses, EnumSet actions) + throws IOException, AccessControlException, Exception { + try { + if (accessMethod == null) { + // Have to rely on Hive implementation of filesystem permission checks. + DefaultFileAccess.checkFileAccess(fs, statuses, actions); + } + else { + for (FsAction action : actions) { + while (statuses.hasNext()) { + accessMethod.invoke(fs, statuses.next(), action); + } + } + } + } catch (Exception err) { throw wrapAccessException(err); } Index: shims/common/src/main/java/org/apache/hadoop/fs/DefaultFileAccess.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- shims/common/src/main/java/org/apache/hadoop/fs/DefaultFileAccess.java (date 1424399922000) +++ shims/common/src/main/java/org/apache/hadoop/fs/DefaultFileAccess.java (revision ) @@ -18,23 +18,22 @@ package org.apache.hadoop.fs; -import java.io.FileNotFoundException; import java.io.IOException; import java.security.AccessControlException; -import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.EnumSet; +import java.util.Iterator; import java.util.List; import javax.security.auth.login.LoginException; +import com.google.common.collect.Iterators; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.permission.FsAction; import org.apache.hadoop.fs.permission.FsPermission; -import org.apache.hadoop.hive.shims.ShimLoader; import org.apache.hadoop.hive.shims.Utils; import org.apache.hadoop.security.UserGroupInformation; @@ -47,7 +46,7 @@ private static Log LOG = LogFactory.getLog(DefaultFileAccess.class); - private static List emptyGroups = new ArrayList(0); + private static List emptyGroups = Collections.emptyList(); public static void checkFileAccess(FileSystem fs, FileStatus stat, FsAction action) throws IOException, AccessControlException, LoginException { @@ -60,34 +59,57 @@ public static void checkFileAccess(FileSystem fs, FileStatus stat, FsAction action, String user, List groups) throws IOException, AccessControlException { + checkFileAccess(fs, Iterators.singletonIterator(stat), EnumSet.of(action), user, groups); + } + public static void checkFileAccess(FileSystem fs, Iterator statuses, EnumSet actions, + String user, List groups) + throws IOException, AccessControlException { + if (groups == null) { groups = emptyGroups; } + // Short-circuit for super-users. String superGroupName = getSuperGroupName(fs.getConf()); if (userBelongsToSuperGroup(superGroupName, groups)) { LOG.info("User \"" + user + "\" belongs to super-group \"" + superGroupName + "\". " + - "Permission granted for action: " + action + "."); + "Permission granted for actions: " + actions + "."); return; } + while (statuses.hasNext()) { + + FileStatus stat = statuses.next(); - final FsPermission dirPerms = stat.getPermission(); - final String grp = stat.getGroup(); + final FsPermission dirPerms = stat.getPermission(); + final String grp = stat.getGroup(); + for (FsAction action : actions) { + - if (user.equals(stat.getOwner())) { - if (dirPerms.getUserAction().implies(action)) { + if (user.equals(stat.getOwner())) { + if (dirPerms.getUserAction().implies(action)) { - return; + continue; - } - } else if (groups.contains(grp)) { - if (dirPerms.getGroupAction().implies(action)) { + } + } else if (groups.contains(grp)) { + if (dirPerms.getGroupAction().implies(action)) { - return; + continue; - } - } else if (dirPerms.getOtherAction().implies(action)) { + } + } else if (dirPerms.getOtherAction().implies(action)) { - return; + continue; - } + } + - throw new AccessControlException("action " + action + " not permitted on path " - + stat.getPath() + " for user " + user); + throw new AccessControlException("action " + action + " not permitted on path " + + stat.getPath() + " for user " + user); + + } // for_each(action); + + } // for_each(fileStatus); + } + + public static void checkFileAccess(FileSystem fs, Iterator statuses, EnumSet actions) + throws IOException, AccessControlException, LoginException { + UserGroupInformation ugi = Utils.getUGI(); + checkFileAccess(fs, statuses, actions, ugi.getShortUserName(), Arrays.asList(ugi.getGroupNames())); } private static String getSuperGroupName(Configuration configuration) { Index: shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java (date 1424399922000) +++ shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java (revision ) @@ -25,7 +25,9 @@ import java.security.AccessControlException; import java.util.ArrayList; import java.util.Collections; +import java.util.EnumSet; import java.util.HashSet; +import java.util.Iterator; import java.util.Set; import org.apache.commons.lang.ArrayUtils; @@ -387,5 +389,11 @@ public void checkFileAccess(FileSystem fs, FileStatus stat, FsAction action) throws IOException, AccessControlException, Exception { DefaultFileAccess.checkFileAccess(fs, stat, action); + } + + @Override + public void checkFileAccess(FileSystem fs, Iterator statuses, EnumSet action) + throws IOException, AccessControlException, Exception { + DefaultFileAccess.checkFileAccess(fs, statuses, action); } } Index: ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java (date 1424399922000) +++ ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java (revision ) @@ -22,11 +22,17 @@ import java.io.IOException; import java.security.AccessControlException; import java.util.ArrayList; +import java.util.Arrays; import java.util.EnumSet; +import java.util.Iterator; import java.util.List; import javax.security.auth.login.LoginException; +import com.google.common.base.Function; +import com.google.common.base.Predicate; +import com.google.common.collect.Iterators; +import com.google.common.collect.Lists; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; @@ -35,6 +41,7 @@ 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; @@ -63,7 +70,7 @@ * out to the parent directory recursively to determine its permissions till * it finds a parent that does exist. */ -public class StorageBasedAuthorizationProvider extends HiveAuthorizationProviderBase +public class StorageBasedAuthorizationProvider extends HiveMultiPartitionAuthorizationProviderBase implements HiveMetastoreAuthorizationProvider { private Warehouse wh; @@ -242,6 +249,88 @@ } } + @Override + public void authorize(Table table, Iterable partitions, + Privilege[] requiredReadPrivileges, Privilege[] requiredWritePrivileges) + throws HiveException, AuthorizationException { + + try { + class MustCheckTablePermissions { // For closure. + public boolean value = false; + } + + final MustCheckTablePermissions mustCheckTablePermissions = new MustCheckTablePermissions(); + final FileSystem fs = table.getDataLocation().getFileSystem(getConf()); + + // Get partition paths. Filter out null-partitions, and partitions without data-locations. + Iterator nonNullPartitions + = Iterators.filter(partitions.iterator(), new Predicate() { + @Override + public boolean apply(Partition partition) { + try { + boolean isValidPartitionPath = partition != null + && partition.getDataLocation() != null + && fs.exists(partition.getDataLocation()); + mustCheckTablePermissions.value |= isValidPartitionPath; + return isValidPartitionPath; + } + catch (IOException exception) { + throw new RuntimeException("Could not find location for partition: " + partition, exception); + } + } + }); + + if (mustCheckTablePermissions.value) { + // At least one partition was null, or had a non-existent path. So check table-permissions, once. + // Partition path can be null in the case of a new create partition - in this case, + // we try to default to checking the permissions of the parent table. + // Partition itself can also be null, in cases where this gets called as a generic + // catch-all call in cases like those with CTAS onto an unpartitioned table (see HIVE-1887) + + // this should be the case only if this is a create partition. + // The privilege needed on the table should be ALTER_DATA, and not CREATE + authorize(table, new Privilege[]{}, new Privilege[]{Privilege.ALTER_DATA}); + } + + // authorize drops if there was a drop privilege requirement + // extract drop privileges + DropPrivilegeExtractor privExtractor = new DropPrivilegeExtractor(requiredReadPrivileges, requiredWritePrivileges); + requiredReadPrivileges = privExtractor.getReadReqPriv(); + requiredWritePrivileges = privExtractor.getWriteReqPriv(); + EnumSet actions = getFsActions(requiredReadPrivileges); + actions.addAll(getFsActions(requiredWritePrivileges)); + + ArrayList allPartitionPaths + = Lists.newArrayList(Iterators.transform(nonNullPartitions, new Function() { + @Override + public Path apply(Partition input) { + return input.getDataLocation(); + } + })); + + for (List partitionPaths : Lists.partition(allPartitionPaths, getListStatusBatchSize(getConf()))) { + + List fileStatuses = Arrays.asList( + fs.listStatus(partitionPaths.toArray(new Path[partitionPaths.size()]))); + + if (privExtractor.hasDropPrivilege) { + FileUtils.checkDeletePermission(fs, fileStatuses, getConf(), authenticator.getUserName()); + } + + checkPermissions(fs, fileStatuses.iterator(), actions, authenticator.getUserName()); + } + + } + catch (Exception exception) { + throw hiveException(exception); + } + } + + private static int getListStatusBatchSize(Configuration configuration) { + return HiveConf.getIntVar(configuration, + HiveConf.ConfVars.HIVE_AUTHORIZATION_HDFS_LIST_STATUS_BATCH_SIZE); + } + private void checkDeletePermission(Path dataLocation, Configuration conf, String userName) throws HiveException { try { @@ -385,15 +474,17 @@ protected static void checkPermissions(final FileSystem fs, final Path path, final EnumSet actions, String user) throws IOException, AccessControlException, HiveException { + checkPermissions(fs, Iterators.singletonIterator(fs.getFileStatus(path)), actions, user); + } + @SuppressWarnings("deprecation") + protected static void checkPermissions(final FileSystem fs, Iterator fileStatuses, + final EnumSet actions, String user) + throws IOException, AccessControlException, HiveException { try { - FileStatus stat = fs.getFileStatus(path); - for (FsAction action : actions) { - FileUtils.checkFileAccessWithImpersonation(fs, stat, action, user); - } + FileUtils.checkFileAccessWithImpersonation(fs, fileStatuses, actions, user); } catch (FileNotFoundException fnfe) { // File named by path doesn't exist; nothing to validate. - return; } catch (org.apache.hadoop.fs.permission.AccessControlException ace) { // Older hadoop version will throw this @deprecated Exception. throw accessControlException(ace); Index: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (date 1424399922000) +++ common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (revision ) @@ -1544,6 +1544,11 @@ "of updating the original list means that you can append to the defaults\n" + "set by SQL standard authorization instead of replacing it entirely."), + HIVE_AUTHORIZATION_HDFS_LIST_STATUS_BATCH_SIZE( + "hive.authprovider.hdfs.liststatus.batch.size", 1000, + "Number of FileStatus objects to be queried for when listing files, for HDFS-based authorization." + ), + HIVE_CLI_PRINT_HEADER("hive.cli.print.header", false, "Whether to print the names of the columns in query output."), HIVE_ERROR_ON_EMPTY_PARTITION("hive.error.on.empty.partition", false, Index: shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java (date 1424399922000) +++ shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java (revision ) @@ -24,18 +24,15 @@ import java.nio.ByteBuffer; import java.security.AccessControlException; import java.security.NoSuchAlgorithmException; -import java.security.PrivilegedExceptionAction; import java.util.Comparator; +import java.util.EnumSet; +import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.TreeMap; -import javax.security.auth.login.LoginException; import com.google.common.annotations.VisibleForTesting; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.BlockLocation; import org.apache.hadoop.fs.FSDataInputStream; @@ -46,7 +43,6 @@ import org.apache.hadoop.fs.PathFilter; import org.apache.hadoop.fs.permission.FsAction; import org.apache.hadoop.fs.permission.FsPermission; -import org.apache.hadoop.hive.shims.HadoopShims.StoragePolicyValue; import org.apache.hadoop.io.LongWritable; import org.apache.hadoop.mapred.ClusterStatus; import org.apache.hadoop.mapred.JobConf; @@ -524,6 +520,21 @@ */ public void checkFileAccess(FileSystem fs, FileStatus status, FsAction action) throws IOException, AccessControlException, Exception; + + /** + * Check if the configured UGI has access to the path for the given file system action. + * Method will return successfully if action is permitted. AccessControlExceptoin will + * be thrown if user does not have access to perform the action. Other exceptions may + * be thrown for non-access related errors. + * @param fs The FileSystem instance + * @param statuses The FileStatuses for the paths being checked + * @param actions The FsActions being checked + * @throws IOException + * @throws AccessControlException + * @throws Exception + */ + public void checkFileAccess(FileSystem fs, Iterator statuses, EnumSet actions) + throws Exception; /** * Use password API (if available) to fetch credentials/password Index: shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java (date 1424399922000) +++ shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java (revision ) @@ -27,7 +27,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Comparator; +import java.util.EnumSet; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -44,6 +46,7 @@ import org.apache.hadoop.fs.PathFilter; import org.apache.hadoop.fs.ProxyFileSystem; import org.apache.hadoop.fs.Trash; +import org.apache.hadoop.fs.permission.FsAction; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hdfs.MiniDFSCluster; import org.apache.hadoop.io.LongWritable;