Merge from trunk. HDFS-6556. Refine XAttr permissions. Contributed by Uma Maheswara Rao G.
git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1606322 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
parent
705a5cede9
commit
e41a821997
|
@ -476,6 +476,8 @@ Release 2.5.0 - UNRELEASED
|
||||||
HADOOP-10701. NFS should not validate the access premission only based on
|
HADOOP-10701. NFS should not validate the access premission only based on
|
||||||
the user's primary group (Harsh J via atm)
|
the user's primary group (Harsh J via atm)
|
||||||
|
|
||||||
|
HDFS-6556. Refine XAttr permissions (umamahesh)
|
||||||
|
|
||||||
BREAKDOWN OF HDFS-2006 SUBTASKS AND RELATED JIRAS
|
BREAKDOWN OF HDFS-2006 SUBTASKS AND RELATED JIRAS
|
||||||
|
|
||||||
HDFS-6299. Protobuf for XAttr and client-side implementation. (Yi Liu via umamahesh)
|
HDFS-6299. Protobuf for XAttr and client-side implementation. (Yi Liu via umamahesh)
|
||||||
|
|
|
@ -30,11 +30,11 @@
|
||||||
* namespaces are defined: user, trusted, security and system.
|
* namespaces are defined: user, trusted, security and system.
|
||||||
* 1) USER namespace attributes may be used by any user to store
|
* 1) USER namespace attributes may be used by any user to store
|
||||||
* arbitrary information. Access permissions in this namespace are
|
* 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>
|
* <br>
|
||||||
* 2) TRUSTED namespace attributes are only visible and accessible to
|
* 2) TRUSTED namespace attributes are only visible and accessible to
|
||||||
* privileged users (a file or directory's owner or the fs
|
* privileged users. This namespace is available from both user space
|
||||||
* admin). This namespace is available from both user space
|
|
||||||
* (filesystem API) and fs kernel.
|
* (filesystem API) and fs kernel.
|
||||||
* <br>
|
* <br>
|
||||||
* 3) SYSTEM namespace attributes are used by the fs kernel to store
|
* 3) SYSTEM namespace attributes are used by the fs kernel to store
|
||||||
|
|
|
@ -8195,10 +8195,7 @@ private void setXAttrInt(String src, XAttr xAttr, EnumSet<XAttrSetFlag> flag,
|
||||||
checkOperation(OperationCategory.WRITE);
|
checkOperation(OperationCategory.WRITE);
|
||||||
checkNameNodeSafeMode("Cannot set XAttr on " + src);
|
checkNameNodeSafeMode("Cannot set XAttr on " + src);
|
||||||
src = FSDirectory.resolvePath(src, pathComponents, dir);
|
src = FSDirectory.resolvePath(src, pathComponents, dir);
|
||||||
if (isPermissionEnabled) {
|
checkXAttrChangeAccess(src, xAttr, pc);
|
||||||
checkOwner(pc, src);
|
|
||||||
checkPathAccess(pc, src, FsAction.WRITE);
|
|
||||||
}
|
|
||||||
List<XAttr> xAttrs = Lists.newArrayListWithCapacity(1);
|
List<XAttr> xAttrs = Lists.newArrayListWithCapacity(1);
|
||||||
xAttrs.add(xAttr);
|
xAttrs.add(xAttr);
|
||||||
dir.setXAttrs(src, xAttrs, flag);
|
dir.setXAttrs(src, xAttrs, flag);
|
||||||
|
@ -8318,10 +8315,7 @@ void removeXAttr(String src, XAttr xAttr) throws IOException {
|
||||||
checkOperation(OperationCategory.WRITE);
|
checkOperation(OperationCategory.WRITE);
|
||||||
checkNameNodeSafeMode("Cannot remove XAttr entry on " + src);
|
checkNameNodeSafeMode("Cannot remove XAttr entry on " + src);
|
||||||
src = FSDirectory.resolvePath(src, pathComponents, dir);
|
src = FSDirectory.resolvePath(src, pathComponents, dir);
|
||||||
if (isPermissionEnabled) {
|
checkXAttrChangeAccess(src, xAttr, pc);
|
||||||
checkOwner(pc, src);
|
|
||||||
checkPathAccess(pc, src, FsAction.WRITE);
|
|
||||||
}
|
|
||||||
|
|
||||||
List<XAttr> xAttrs = Lists.newArrayListWithCapacity(1);
|
List<XAttr> xAttrs = Lists.newArrayListWithCapacity(1);
|
||||||
xAttrs.add(xAttr);
|
xAttrs.add(xAttr);
|
||||||
|
@ -8340,6 +8334,21 @@ void removeXAttr(String src, XAttr xAttr) throws IOException {
|
||||||
logAuditEvent(true, "removeXAttr", src, null, resultingStat);
|
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
|
* Default AuditLogger implementation; used when no access logger is
|
||||||
* defined in the config file. It can also be explicitly listed in the
|
* defined in the config file. It can also be explicitly listed in the
|
||||||
|
|
|
@ -34,7 +34,8 @@
|
||||||
* USER - extended user attributes: these can be assigned to files and
|
* USER - extended user attributes: these can be assigned to files and
|
||||||
* directories to store arbitrary additional information. The access
|
* directories to store arbitrary additional information. The access
|
||||||
* permissions for user attributes are defined by the file permission
|
* 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>
|
* <br>
|
||||||
* TRUSTED - trusted extended attributes: these are visible/accessible
|
* TRUSTED - trusted extended attributes: these are visible/accessible
|
||||||
* only to/by the super user.
|
* only to/by the super user.
|
||||||
|
|
|
@ -2440,38 +2440,39 @@ private void doSetXattr(ByteArrayOutputStream out, FsShell fshell,
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* 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
|
* Test to make sure that user namespace xattrs can be set only if path has
|
||||||
* or remove the xattrs.
|
* access and for sticky directorries, only owner/privileged user can write.
|
||||||
|
* Trusted namespace xattrs can be set only with privileged users.
|
||||||
*
|
*
|
||||||
* As user1:
|
* As user1: Create a directory (/foo) as user1, chown it to user1 (and
|
||||||
* Create a directory (/foo) as user1, chown it to user1 (and user1's group),
|
* user1's group), grant rwx to "other".
|
||||||
* grant rwx to "other".
|
|
||||||
*
|
*
|
||||||
* As user2:
|
* As user2: Set an xattr (should pass with path access).
|
||||||
* Set an xattr (should fail).
|
|
||||||
*
|
*
|
||||||
* As user1:
|
* As user1: Set an xattr (should pass).
|
||||||
* Set an xattr (should pass).
|
|
||||||
*
|
*
|
||||||
* As user2:
|
* As user2: Read the xattr (should pass). Remove the xattr (should pass with
|
||||||
* Read the xattr (should pass).
|
* path access).
|
||||||
* Remove the xattr (should fail).
|
|
||||||
*
|
*
|
||||||
* As user1:
|
* As user1: Read the xattr (should pass). Remove the xattr (should pass).
|
||||||
* 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)
|
@Test (timeout = 30000)
|
||||||
public void testSetXAttrPermissionAsDifferentOwner() throws Exception {
|
public void testSetXAttrPermissionAsDifferentOwner() throws Exception {
|
||||||
final String USER1 = "user1";
|
final String USER1 = "user1";
|
||||||
final String GROUP1 = "mygroup1";
|
final String GROUP1 = "supergroup";
|
||||||
final UserGroupInformation user1 = UserGroupInformation.
|
final UserGroupInformation user1 = UserGroupInformation.
|
||||||
createUserForTesting(USER1, new String[] {GROUP1});
|
createUserForTesting(USER1, new String[] {GROUP1});
|
||||||
final UserGroupInformation user2 = UserGroupInformation.
|
final UserGroupInformation user2 = UserGroupInformation.
|
||||||
createUserForTesting("user2", new String[] {"mygroup2"});
|
createUserForTesting("user2", new String[] {"mygroup2"});
|
||||||
|
final UserGroupInformation SUPERUSER = UserGroupInformation.getCurrentUser();
|
||||||
MiniDFSCluster cluster = null;
|
MiniDFSCluster cluster = null;
|
||||||
PrintStream bak = null;
|
PrintStream bak = null;
|
||||||
try {
|
try {
|
||||||
|
@ -2487,7 +2488,7 @@ public void testSetXAttrPermissionAsDifferentOwner() throws Exception {
|
||||||
final ByteArrayOutputStream out = new ByteArrayOutputStream();
|
final ByteArrayOutputStream out = new ByteArrayOutputStream();
|
||||||
System.setErr(new PrintStream(out));
|
System.setErr(new PrintStream(out));
|
||||||
|
|
||||||
// mkdir foo as user1
|
//Test 1. Let user1 be owner for /foo
|
||||||
user1.doAs(new PrivilegedExceptionAction<Object>() {
|
user1.doAs(new PrivilegedExceptionAction<Object>() {
|
||||||
@Override
|
@Override
|
||||||
public Object run() throws Exception {
|
public Object run() throws Exception {
|
||||||
|
@ -2499,6 +2500,7 @@ public Object run() throws Exception {
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
//Test 2. Give access to others
|
||||||
user1.doAs(new PrivilegedExceptionAction<Object>() {
|
user1.doAs(new PrivilegedExceptionAction<Object>() {
|
||||||
@Override
|
@Override
|
||||||
public Object run() throws Exception {
|
public Object run() throws Exception {
|
||||||
|
@ -2511,23 +2513,21 @@ public Object run() throws Exception {
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
// 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>() {
|
user2.doAs(new PrivilegedExceptionAction<Object>() {
|
||||||
@Override
|
@Override
|
||||||
public Object run() throws Exception {
|
public Object run() throws Exception {
|
||||||
final int ret = ToolRunner.run(fshell, new String[]{
|
final int ret = ToolRunner.run(fshell, new String[]{
|
||||||
"-setfattr", "-n", "user.a1", "-v", "1234", "/foo"});
|
"-setfattr", "-n", "user.a1", "-v", "1234", "/foo"});
|
||||||
assertEquals("Returned should be 1", 1, ret);
|
assertEquals("Returned should be 0", 0, ret);
|
||||||
final String str = out.toString();
|
|
||||||
assertTrue("Permission denied printed",
|
|
||||||
str.indexOf("Permission denied") != -1);
|
|
||||||
out.reset();
|
out.reset();
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
// But there should be permission to write xattr for
|
//Test 4. There should be permission to write xattr for
|
||||||
// the owning user.
|
// the owning user with write permissions.
|
||||||
user1.doAs(new PrivilegedExceptionAction<Object>() {
|
user1.doAs(new PrivilegedExceptionAction<Object>() {
|
||||||
@Override
|
@Override
|
||||||
public Object run() throws Exception {
|
public Object run() throws Exception {
|
||||||
|
@ -2539,19 +2539,55 @@ public Object run() throws Exception {
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
// There should be permission to read,but not to remove for
|
// Test 5. There should be permission to read non-owning user (user2) if
|
||||||
// non-owning user (user2).
|
// there is path access to that user and also can remove.
|
||||||
user2.doAs(new PrivilegedExceptionAction<Object>() {
|
user2.doAs(new PrivilegedExceptionAction<Object>() {
|
||||||
@Override
|
@Override
|
||||||
public Object run() throws Exception {
|
public Object run() throws Exception {
|
||||||
// Read
|
// Read
|
||||||
int ret = ToolRunner.run(fshell, new String[]{
|
int ret = ToolRunner.run(fshell, new String[] { "-getfattr", "-n",
|
||||||
"-getfattr", "-n", "user.a1", "/foo"});
|
"user.a1", "/foo" });
|
||||||
assertEquals("Returned should be 0", 0, ret);
|
assertEquals("Returned should be 0", 0, ret);
|
||||||
out.reset();
|
out.reset();
|
||||||
// Remove
|
// Remove
|
||||||
ret = ToolRunner.run(fshell, new String[]{
|
ret = ToolRunner.run(fshell, new String[] { "-setfattr", "-x",
|
||||||
"-setfattr", "-x", "user.a1", "/foo"});
|
"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);
|
assertEquals("Returned should be 1", 1, ret);
|
||||||
final String str = out.toString();
|
final String str = out.toString();
|
||||||
assertTrue("Permission denied printed",
|
assertTrue("Permission denied printed",
|
||||||
|
@ -2561,19 +2597,30 @@ public Object run() throws Exception {
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
// But there should be permission to read/remove for
|
// Test 9. There should be no permissions to remove for
|
||||||
// the owning user.
|
// the non-owning user with no path access.
|
||||||
user1.doAs(new PrivilegedExceptionAction<Object>() {
|
user2.doAs(new PrivilegedExceptionAction<Object>() {
|
||||||
@Override
|
@Override
|
||||||
public Object run() throws Exception {
|
public Object run() throws Exception {
|
||||||
// Read
|
// set
|
||||||
int ret = ToolRunner.run(fshell, new String[]{
|
int ret = ToolRunner.run(fshell, new String[] { "-setfattr", "-x",
|
||||||
"-getfattr", "-n", "user.a1", "/foo"});
|
"user.a2", "/foo" });
|
||||||
assertEquals("Returned should be 0", 0, ret);
|
assertEquals("Returned should be 1", 1, ret);
|
||||||
|
final String str = out.toString();
|
||||||
|
assertTrue("Permission denied printed",
|
||||||
|
str.indexOf("Permission denied") != -1);
|
||||||
out.reset();
|
out.reset();
|
||||||
// Remove
|
return null;
|
||||||
ret = ToolRunner.run(fshell, new String[]{
|
}
|
||||||
"-setfattr", "-x", "user.a1", "/foo"});
|
});
|
||||||
|
|
||||||
|
// 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);
|
assertEquals("Returned should be 0", 0, ret);
|
||||||
out.reset();
|
out.reset();
|
||||||
return null;
|
return null;
|
||||||
|
|
Loading…
Reference in New Issue