From 5690b51ef7c708c0a71162ddaff04466bc71cdcc Mon Sep 17 00:00:00 2001 From: Wei-Chiu Chuang Date: Thu, 16 Feb 2017 05:39:37 -0800 Subject: [PATCH] HDFS-11100. Recursively deleting file protected by sticky bit should fail. Contributed by John Zhuge. --- .../apache/hadoop/fs/FSExceptionMessages.java | 3 + .../server/namenode/FSPermissionChecker.java | 87 ++++++++++++++++--- .../hadoop/fs/permission/TestStickyBit.java | 63 ++++++++++++++ 3 files changed, 142 insertions(+), 11 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FSExceptionMessages.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FSExceptionMessages.java index 1511bb0a480..a8e7b71bb11 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FSExceptionMessages.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FSExceptionMessages.java @@ -48,4 +48,7 @@ public class FSExceptionMessages { = "Requested more bytes than destination buffer size"; public static final String PERMISSION_DENIED = "Permission denied"; + + public static final String PERMISSION_DENIED_BY_STICKY_BIT = + "Permission denied by sticky bit"; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java index 107d5630532..f1250dd277d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java @@ -18,11 +18,15 @@ package org.apache.hadoop.hdfs.server.namenode; import java.io.IOException; +import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.Stack; +import com.google.common.base.Preconditions; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.fs.FSExceptionMessages; import org.apache.hadoop.fs.ParentNotDirectoryException; import org.apache.hadoop.fs.permission.AclEntryScope; import org.apache.hadoop.fs.permission.AclEntryType; @@ -280,9 +284,20 @@ class FSPermissionChecker implements AccessControlEnforcer { return; } + // Each inode in the subtree has a level. The root inode has level 0. + // List subINodePath tracks the inode path in the subtree during + // traversal. The root inode is not stored because it is already in array + // components. The list index is (level - 1). + ArrayList subINodePath = new ArrayList<>(); + + // The stack of levels matches the stack of directory inodes. + Stack levels = new Stack<>(); + levels.push(0); // Level 0 is the root + Stack directories = new Stack(); for(directories.push(inode.asDirectory()); !directories.isEmpty(); ) { INodeDirectory d = directories.pop(); + int level = levels.pop(); ReadOnlyList cList = d.getChildrenList(snapshotId); if (!(cList.isEmpty() && ignoreEmptyDir)) { //TODO have to figure this out with inodeattribute provider @@ -292,11 +307,44 @@ class FSPermissionChecker implements AccessControlEnforcer { throw new AccessControlException( toAccessControlString(inodeAttr, d.getFullPathName(), access)); } + + if (level > 0) { + if (level - 1 < subINodePath.size()) { + subINodePath.set(level - 1, d); + } else { + Preconditions.checkState(level - 1 == subINodePath.size()); + subINodePath.add(d); + } + } + + if (inodeAttr.getFsPermission().getStickyBit()) { + for (INode child : cList) { + INodeAttributes childInodeAttr = + getINodeAttrs(components, pathIdx, child, snapshotId); + if (isStickyBitViolated(inodeAttr, childInodeAttr)) { + List allComponentList = new ArrayList<>(); + for (int i = 0; i <= pathIdx; ++i) { + allComponentList.add(components[i]); + } + for (int i = 0; i < level; ++i) { + allComponentList.add(subINodePath.get(i).getLocalNameBytes()); + } + allComponentList.add(child.getLocalNameBytes()); + int index = pathIdx + level; + byte[][] allComponents = + allComponentList.toArray(new byte[][]{}); + throwStickyBitException( + getPath(allComponents, 0, index + 1), child, + getPath(allComponents, 0, index), inode); + } + } + } } for(INode child : cList) { if (child.isDirectory()) { directories.push(child.asDirectory()); + levels.push(level + 1); } } } @@ -425,26 +473,43 @@ class FSPermissionChecker implements AccessControlEnforcer { return; } + INodeAttributes inode = inodes[index + 1]; + if (!isStickyBitViolated(parent, inode)) { + return; + } + + throwStickyBitException(getPath(components, 0, index + 1), inode, + getPath(components, 0, index), parent); + } + + /** Return true when sticky bit is violated. */ + private boolean isStickyBitViolated(INodeAttributes parent, + INodeAttributes inode) { // If this user is the directory owner, return if (parent.getUserName().equals(getUser())) { - return; + return false; } - INodeAttributes inode = inodes[index + 1]; // if this user is the file owner, return if (inode.getUserName().equals(getUser())) { - return; + return false; } + return true; + } + + private void throwStickyBitException( + String inodePath, INodeAttributes inode, + String parentPath, INodeAttributes parent) + throws AccessControlException { throw new AccessControlException(String.format( - "Permission denied by sticky bit: user=%s, path=\"%s\":%s:%s:%s%s, " + - "parent=\"%s\":%s:%s:%s%s", user, - getPath(components, 0, index + 1), - inode.getUserName(), inode.getGroupName(), - inode.isDirectory() ? "d" : "-", inode.getFsPermission().toString(), - getPath(components, 0, index), - parent.getUserName(), parent.getGroupName(), - parent.isDirectory() ? "d" : "-", parent.getFsPermission().toString())); + FSExceptionMessages.PERMISSION_DENIED_BY_STICKY_BIT + + ": user=%s, path=\"%s\":%s:%s:%s%s, " + + "parent=\"%s\":%s:%s:%s%s", user, inodePath, inode.getUserName(), + inode.getGroupName(), inode.isDirectory() ? "d" : "-", + inode.getFsPermission().toString(), parentPath, parent.getUserName(), + parent.getGroupName(), parent.isDirectory() ? "d" : "-", + parent.getFsPermission().toString())); } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/permission/TestStickyBit.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/permission/TestStickyBit.java index 99f1b1b2f7b..a6409fde6cd 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/permission/TestStickyBit.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/permission/TestStickyBit.java @@ -32,6 +32,7 @@ import java.util.Arrays; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FSDataOutputStream; +import org.apache.hadoop.fs.FSExceptionMessages; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; @@ -43,6 +44,7 @@ import org.apache.hadoop.hdfs.MiniDFSCluster; import org.apache.hadoop.io.IOUtils; import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.security.UserGroupInformation; +import org.apache.hadoop.test.GenericTestUtils; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; @@ -426,6 +428,67 @@ public class TestStickyBit { assertFalse(hdfs.getFileStatus(sbSetOff).getPermission().getStickyBit()); } + @Test + public void testStickyBitRecursiveDeleteFile() throws Exception { + Path root = new Path("/" + GenericTestUtils.getMethodName()); + Path tmp = new Path(root, "tmp"); + Path file = new Path(tmp, "file"); + + // Create a tmp directory with wide-open permissions and sticky bit + hdfs.mkdirs(tmp); + hdfs.setPermission(root, new FsPermission((short) 0777)); + hdfs.setPermission(tmp, new FsPermission((short) 01777)); + + // Create a file protected by sticky bit + writeFile(hdfsAsUser1, file); + hdfs.setPermission(file, new FsPermission((short) 0666)); + + try { + hdfsAsUser2.delete(tmp, true); + fail("Non-owner can not delete a file protected by sticky bit" + + " recursively"); + } catch (AccessControlException e) { + GenericTestUtils.assertExceptionContains( + FSExceptionMessages.PERMISSION_DENIED_BY_STICKY_BIT, e); + } + + // Owner can delete a file protected by sticky bit recursively + hdfsAsUser1.delete(tmp, true); + } + + @Test + public void testStickyBitRecursiveDeleteDir() throws Exception { + Path root = new Path("/" + GenericTestUtils.getMethodName()); + Path tmp = new Path(root, "tmp"); + Path dir = new Path(tmp, "dir"); + Path file = new Path(dir, "file"); + + // Create a tmp directory with wide-open permissions and sticky bit + hdfs.mkdirs(tmp); + hdfs.setPermission(root, new FsPermission((short) 0777)); + hdfs.setPermission(tmp, new FsPermission((short) 01777)); + + // Create a dir protected by sticky bit + hdfsAsUser1.mkdirs(dir); + hdfsAsUser1.setPermission(dir, new FsPermission((short) 0777)); + + // Create a file in dir + writeFile(hdfsAsUser1, file); + hdfs.setPermission(file, new FsPermission((short) 0666)); + + try { + hdfsAsUser2.delete(tmp, true); + fail("Non-owner can not delete a directory protected by sticky bit" + + " recursively"); + } catch (AccessControlException e) { + GenericTestUtils.assertExceptionContains( + FSExceptionMessages.PERMISSION_DENIED_BY_STICKY_BIT, e); + } + + // Owner can delete a directory protected by sticky bit recursively + hdfsAsUser1.delete(tmp, true); + } + /*** * Write a quick file to the specified file system at specified path */