HDFS-11100. Recursively deleting file protected by sticky bit should fail. Contributed by John Zhuge.

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";
 }

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<INodeDirectory> subINodePath = new ArrayList<>();
+
+    // The stack of levels matches the stack of directory inodes.
+    Stack<Integer> levels = new Stack<>();
+    levels.push(0);    // Level 0 is the root
+
     Stack<INodeDirectory> directories = new Stack<INodeDirectory>();
     for(directories.push(inode.asDirectory()); !directories.isEmpty(); ) {
       INodeDirectory d = directories.pop();
+      int level = levels.pop();
       ReadOnlyList<INode> 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<byte[]> 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, " +
+        FSExceptionMessages.PERMISSION_DENIED_BY_STICKY_BIT +
-        "parent=\"%s\":%s:%s:%s%s", user,
+            ": user=%s, path=\"%s\":%s:%s:%s%s, " +
-        getPath(components, 0, index + 1),
+            "parent=\"%s\":%s:%s:%s%s", user, inodePath, inode.getUserName(),
-        inode.getUserName(), inode.getGroupName(),
+        inode.getGroupName(), inode.isDirectory() ? "d" : "-",
-        inode.isDirectory() ? "d" : "-", inode.getFsPermission().toString(),
+        inode.getFsPermission().toString(), parentPath, parent.getUserName(),
-        getPath(components, 0, index),
+        parent.getGroupName(), parent.isDirectory() ? "d" : "-",
-        parent.getUserName(), parent.getGroupName(),
+        parent.getFsPermission().toString()));
-        parent.isDirectory() ? "d" : "-", parent.getFsPermission().toString()));
   }
 
   /**

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
    */