Author: umamahesh
Date: Sat Jun 28 11:40:02 2014
New Revision: 1606320
URL: http://svn.apache.org/r1606320
Log:
HDFS-6556. Refine XAttr permissions. Contributed by Uma Maheswara Rao G.
Modified:
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/fs/XAttr.java
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrPermissionFilter.java
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java
Modified: hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt?rev=1606320&r1=1606319&r2=1606320&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt (original)
+++ hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt Sat Jun 28
11:40:02 2014
@@ -728,6 +728,8 @@ Release 2.5.0 - UNRELEASED
HADOOP-10701. NFS should not validate the access premission only based on
the user's primary group (Harsh J via atm)
+ HDFS-6556. Refine XAttr permissions (umamahesh)
+
BREAKDOWN OF HDFS-2006 SUBTASKS AND RELATED JIRAS
HDFS-6299. Protobuf for XAttr and client-side implementation. (Yi Liu via
umamahesh)
Modified:
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/fs/XAttr.java
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/fs/XAttr.java?rev=1606320&r1=1606319&r2=1606320&view=diff
==============================================================================
---
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/fs/XAttr.java
(original)
+++
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/fs/XAttr.java
Sat Jun 28 11:40:02 2014
@@ -30,11 +30,11 @@ import org.apache.hadoop.classification.
* namespaces are defined: user, trusted, security and system.
* 1) USER namespace attributes may be used by any user to store
* arbitrary information. Access permissions in this namespace are
- * defined by a file directory's permission bits.
+ * defined by a file directory's permission bits. For sticky directories,
+ * only the owner and privileged user can write attributes.
* <br>
* 2) TRUSTED namespace attributes are only visible and accessible to
- * privileged users (a file or directory's owner or the fs
- * admin). This namespace is available from both user space
+ * privileged users. This namespace is available from both user space
* (filesystem API) and fs kernel.
* <br>
* 3) SYSTEM namespace attributes are used by the fs kernel to store
Modified:
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java?rev=1606320&r1=1606319&r2=1606320&view=diff
==============================================================================
---
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
(original)
+++
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
Sat Jun 28 11:40:02 2014
@@ -8196,10 +8196,7 @@ public class FSNamesystem implements Nam
checkOperation(OperationCategory.WRITE);
checkNameNodeSafeMode("Cannot set XAttr on " + src);
src = FSDirectory.resolvePath(src, pathComponents, dir);
- if (isPermissionEnabled) {
- checkOwner(pc, src);
- checkPathAccess(pc, src, FsAction.WRITE);
- }
+ checkXAttrChangeAccess(src, xAttr, pc);
List<XAttr> xAttrs = Lists.newArrayListWithCapacity(1);
xAttrs.add(xAttr);
dir.setXAttrs(src, xAttrs, flag);
@@ -8319,10 +8316,7 @@ public class FSNamesystem implements Nam
checkOperation(OperationCategory.WRITE);
checkNameNodeSafeMode("Cannot remove XAttr entry on " + src);
src = FSDirectory.resolvePath(src, pathComponents, dir);
- if (isPermissionEnabled) {
- checkOwner(pc, src);
- checkPathAccess(pc, src, FsAction.WRITE);
- }
+ checkXAttrChangeAccess(src, xAttr, pc);
List<XAttr> xAttrs = Lists.newArrayListWithCapacity(1);
xAttrs.add(xAttr);
@@ -8341,6 +8335,21 @@ public class FSNamesystem implements Nam
logAuditEvent(true, "removeXAttr", src, null, resultingStat);
}
+ private void checkXAttrChangeAccess(String src, XAttr xAttr,
+ FSPermissionChecker pc) throws UnresolvedLinkException,
+ AccessControlException {
+ if (isPermissionEnabled && xAttr.getNameSpace() == XAttr.NameSpace.USER) {
+ final INode inode = dir.getINode(src);
+ if (inode.isDirectory() && inode.getFsPermission().getStickyBit()) {
+ if (!pc.isSuperUser()) {
+ checkOwner(pc, src);
+ }
+ } else {
+ checkPathAccess(pc, src, FsAction.WRITE);
+ }
+ }
+ }
+
/**
* Default AuditLogger implementation; used when no access logger is
* defined in the config file. It can also be explicitly listed in the
Modified:
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrPermissionFilter.java
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrPermissionFilter.java?rev=1606320&r1=1606319&r2=1606320&view=diff
==============================================================================
---
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrPermissionFilter.java
(original)
+++
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrPermissionFilter.java
Sat Jun 28 11:40:02 2014
@@ -34,7 +34,8 @@ import com.google.common.collect.Lists;
* USER - extended user attributes: these can be assigned to files and
* directories to store arbitrary additional information. The access
* permissions for user attributes are defined by the file permission
- * bits.
+ * bits. For sticky directories, only the owner and privileged user can
+ * write attributes.
* <br>
* TRUSTED - trusted extended attributes: these are visible/accessible
* only to/by the super user.
Modified:
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java?rev=1606320&r1=1606319&r2=1606320&view=diff
==============================================================================
---
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java
(original)
+++
hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java
Sat Jun 28 11:40:02 2014
@@ -2456,38 +2456,39 @@ public class TestDFSShell {
}
/**
- * HDFS-6374 setXAttr should require the user to be the owner of the file
- * or directory.
- *
- * Test to make sure that only the owner of a file or directory can set
- * or remove the xattrs.
- *
- * As user1:
- * Create a directory (/foo) as user1, chown it to user1 (and user1's group),
- * grant rwx to "other".
- *
- * As user2:
- * Set an xattr (should fail).
- *
- * As user1:
- * Set an xattr (should pass).
- *
- * As user2:
- * Read the xattr (should pass).
- * Remove the xattr (should fail).
- *
- * As user1:
- * Read the xattr (should pass).
- * Remove the xattr (should pass).
+ *
+ * Test to make sure that user namespace xattrs can be set only if path has
+ * access and for sticky directorries, only owner/privileged user can write.
+ * Trusted namespace xattrs can be set only with privileged users.
+ *
+ * As user1: Create a directory (/foo) as user1, chown it to user1 (and
+ * user1's group), grant rwx to "other".
+ *
+ * As user2: Set an xattr (should pass with path access).
+ *
+ * As user1: Set an xattr (should pass).
+ *
+ * As user2: Read the xattr (should pass). Remove the xattr (should pass with
+ * path access).
+ *
+ * As user1: Read the xattr (should pass). Remove the xattr (should pass).
+ *
+ * As user1: Change permissions only to owner
+ *
+ * As User2: Set an Xattr (Should fail set with no path access) Remove an
+ * Xattr (Should fail with no path access)
+ *
+ * As SuperUser: Set an Xattr with Trusted (Should pass)
*/
@Test (timeout = 30000)
public void testSetXAttrPermissionAsDifferentOwner() throws Exception {
final String USER1 = "user1";
- final String GROUP1 = "mygroup1";
+ final String GROUP1 = "supergroup";
final UserGroupInformation user1 = UserGroupInformation.
createUserForTesting(USER1, new String[] {GROUP1});
final UserGroupInformation user2 = UserGroupInformation.
createUserForTesting("user2", new String[] {"mygroup2"});
+ final UserGroupInformation SUPERUSER =
UserGroupInformation.getCurrentUser();
MiniDFSCluster cluster = null;
PrintStream bak = null;
try {
@@ -2503,7 +2504,7 @@ public class TestDFSShell {
final ByteArrayOutputStream out = new ByteArrayOutputStream();
System.setErr(new PrintStream(out));
- // mkdir foo as user1
+ //Test 1. Let user1 be owner for /foo
user1.doAs(new PrivilegedExceptionAction<Object>() {
@Override
public Object run() throws Exception {
@@ -2514,7 +2515,8 @@ public class TestDFSShell {
return null;
}
});
-
+
+ //Test 2. Give access to others
user1.doAs(new PrivilegedExceptionAction<Object>() {
@Override
public Object run() throws Exception {
@@ -2527,23 +2529,21 @@ public class TestDFSShell {
}
});
- // No permission to write xattr for non-owning user (user2).
+ // Test 3. Should be allowed to write xattr if there is a path access to
+ // user (user2).
user2.doAs(new PrivilegedExceptionAction<Object>() {
@Override
public Object run() throws Exception {
final int ret = ToolRunner.run(fshell, new String[]{
"-setfattr", "-n", "user.a1", "-v", "1234", "/foo"});
- assertEquals("Returned should be 1", 1, ret);
- final String str = out.toString();
- assertTrue("Permission denied printed",
- str.indexOf("Permission denied") != -1);
+ assertEquals("Returned should be 0", 0, ret);
out.reset();
return null;
}
});
- // But there should be permission to write xattr for
- // the owning user.
+ //Test 4. There should be permission to write xattr for
+ // the owning user with write permissions.
user1.doAs(new PrivilegedExceptionAction<Object>() {
@Override
public Object run() throws Exception {
@@ -2555,19 +2555,55 @@ public class TestDFSShell {
}
});
- // There should be permission to read,but not to remove for
- // non-owning user (user2).
+ // Test 5. There should be permission to read non-owning user (user2) if
+ // there is path access to that user and also can remove.
user2.doAs(new PrivilegedExceptionAction<Object>() {
@Override
public Object run() throws Exception {
// Read
- int ret = ToolRunner.run(fshell, new String[]{
- "-getfattr", "-n", "user.a1", "/foo"});
+ int ret = ToolRunner.run(fshell, new String[] { "-getfattr", "-n",
+ "user.a1", "/foo" });
assertEquals("Returned should be 0", 0, ret);
out.reset();
// Remove
- ret = ToolRunner.run(fshell, new String[]{
- "-setfattr", "-x", "user.a1", "/foo"});
+ ret = ToolRunner.run(fshell, new String[] { "-setfattr", "-x",
+ "user.a1", "/foo" });
+ assertEquals("Returned should be 0", 0, ret);
+ out.reset();
+ return null;
+ }
+ });
+
+ // Test 6. There should be permission to read/remove for
+ // the owning user with path access.
+ user1.doAs(new PrivilegedExceptionAction<Object>() {
+ @Override
+ public Object run() throws Exception {
+ return null;
+ }
+ });
+
+ // Test 7. Change permission to have path access only to owner(user1)
+ user1.doAs(new PrivilegedExceptionAction<Object>() {
+ @Override
+ public Object run() throws Exception {
+ // Give access to "other"
+ final int ret = ToolRunner.run(fshell, new String[]{
+ "-chmod", "700", "/foo"});
+ assertEquals("Return should be 0", 0, ret);
+ out.reset();
+ return null;
+ }
+ });
+
+ // Test 8. There should be no permissions to set for
+ // the non-owning user with no path access.
+ user2.doAs(new PrivilegedExceptionAction<Object>() {
+ @Override
+ public Object run() throws Exception {
+ // set
+ int ret = ToolRunner.run(fshell, new String[] { "-setfattr", "-n",
+ "user.a2", "/foo" });
assertEquals("Returned should be 1", 1, ret);
final String str = out.toString();
assertTrue("Permission denied printed",
@@ -2576,20 +2612,31 @@ public class TestDFSShell {
return null;
}
});
-
- // But there should be permission to read/remove for
- // the owning user.
- user1.doAs(new PrivilegedExceptionAction<Object>() {
+
+ // Test 9. There should be no permissions to remove for
+ // the non-owning user with no path access.
+ user2.doAs(new PrivilegedExceptionAction<Object>() {
@Override
public Object run() throws Exception {
- // Read
- int ret = ToolRunner.run(fshell, new String[]{
- "-getfattr", "-n", "user.a1", "/foo"});
- assertEquals("Returned should be 0", 0, ret);
+ // set
+ int ret = ToolRunner.run(fshell, new String[] { "-setfattr", "-x",
+ "user.a2", "/foo" });
+ assertEquals("Returned should be 1", 1, ret);
+ final String str = out.toString();
+ assertTrue("Permission denied printed",
+ str.indexOf("Permission denied") != -1);
out.reset();
- // Remove
- ret = ToolRunner.run(fshell, new String[]{
- "-setfattr", "-x", "user.a1", "/foo"});
+ return null;
+ }
+ });
+
+ // Test 10. Superuser should be allowed to set with trusted namespace
+ SUPERUSER.doAs(new PrivilegedExceptionAction<Object>() {
+ @Override
+ public Object run() throws Exception {
+ // set
+ int ret = ToolRunner.run(fshell, new String[] { "-setfattr", "-n",
+ "trusted.a3", "/foo" });
assertEquals("Returned should be 0", 0, ret);
out.reset();
return null;
