From e24ed47d9a19f34a4dd8d4bad9b5c78ca3dd1c2e Mon Sep 17 00:00:00 2001 From: Xiao Chen Date: Fri, 24 Feb 2017 16:49:16 -0800 Subject: [PATCH] HDFS-11421. Make WebHDFS' ACLs RegEx configurable. Contributed by Harsh J. --- .../hdfs/client/HdfsClientConfigKeys.java | 2 ++ .../hadoop/hdfs/web/WebHdfsFileSystem.java | 6 +++- .../web/resources/AclPermissionParam.java | 17 +++++++++- .../datanode/web/webhdfs/WebHdfsHandler.java | 10 ++++-- .../server/namenode/NameNodeHttpServer.java | 4 +++ .../src/main/resources/hdfs-default.xml | 8 +++++ .../apache/hadoop/hdfs/web/TestWebHDFS.java | 29 +++++++++++++++- .../hadoop/hdfs/web/resources/TestParam.java | 34 +++++++++++++++++++ 8 files changed, 104 insertions(+), 6 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java index 7ad79e001ba..6f8c661a5ba 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java @@ -35,6 +35,8 @@ public interface HdfsClientConfigKeys { String DFS_WEBHDFS_USER_PATTERN_KEY = "dfs.webhdfs.user.provider.user.pattern"; String DFS_WEBHDFS_USER_PATTERN_DEFAULT = "^[A-Za-z_][A-Za-z0-9._-]*[$]?$"; + String DFS_WEBHDFS_ACL_PERMISSION_PATTERN_KEY = + "dfs.webhdfs.acl.provider.permission.pattern"; String DFS_WEBHDFS_ACL_PERMISSION_PATTERN_DEFAULT = "^(default:)?(user|group|mask|other):[[A-Za-z_][A-Za-z0-9._-]]*:([rwx-]{3})?(,(default:)?(user|group|mask|other):[[A-Za-z_][A-Za-z0-9._-]]*:([rwx-]{3})?)*$"; diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java index 135eef7deda..a9bc79595e2 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java @@ -183,10 +183,14 @@ public class WebHdfsFileSystem extends FileSystem ) throws IOException { super.initialize(uri, conf); setConf(conf); - /** set user pattern based on configuration file */ + + // set user and acl patterns based on configuration file UserParam.setUserPattern(conf.get( HdfsClientConfigKeys.DFS_WEBHDFS_USER_PATTERN_KEY, HdfsClientConfigKeys.DFS_WEBHDFS_USER_PATTERN_DEFAULT)); + AclPermissionParam.setAclPermissionPattern(conf.get( + HdfsClientConfigKeys.DFS_WEBHDFS_ACL_PERMISSION_PATTERN_KEY, + HdfsClientConfigKeys.DFS_WEBHDFS_ACL_PERMISSION_PATTERN_DEFAULT)); boolean isOAuth = conf.getBoolean( HdfsClientConfigKeys.DFS_WEBHDFS_OAUTH_ENABLED_KEY, diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/resources/AclPermissionParam.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/resources/AclPermissionParam.java index 130c8fd6cf6..9ab3ad57fa8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/resources/AclPermissionParam.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/resources/AclPermissionParam.java @@ -24,6 +24,7 @@ import java.util.Iterator; import java.util.List; import java.util.regex.Pattern; +import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.fs.permission.AclEntry; /** AclPermission parameter. */ @@ -33,7 +34,7 @@ public class AclPermissionParam extends StringParam { /** Default parameter value. */ public static final String DEFAULT = ""; - private static final Domain DOMAIN = new Domain(NAME, + private static Domain DOMAIN = new Domain(NAME, Pattern.compile(DFS_WEBHDFS_ACL_PERMISSION_PATTERN_DEFAULT)); /** @@ -49,6 +50,20 @@ public class AclPermissionParam extends StringParam { super(DOMAIN,parseAclSpec(acl).equals(DEFAULT) ? null : parseAclSpec(acl)); } + @VisibleForTesting + public static Domain getAclPermissionPattern() { + return DOMAIN; + } + + @VisibleForTesting + public static void setAclPermissionPattern(Domain dm) { + DOMAIN = dm; + } + + public static void setAclPermissionPattern(String pattern) { + DOMAIN = new Domain(NAME, Pattern.compile(pattern)); + } + @Override public String getName() { return NAME; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/web/webhdfs/WebHdfsHandler.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/web/webhdfs/WebHdfsHandler.java index f8c15fc3040..d2b2ec22346 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/web/webhdfs/WebHdfsHandler.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/web/webhdfs/WebHdfsHandler.java @@ -48,11 +48,12 @@ import org.apache.hadoop.fs.MD5MD5CRC32FileChecksum; import org.apache.hadoop.fs.permission.FsCreateModes; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hdfs.DFSClient; -import org.apache.hadoop.hdfs.DFSConfigKeys; +import org.apache.hadoop.hdfs.client.HdfsClientConfigKeys; import org.apache.hadoop.hdfs.client.HdfsDataInputStream; import org.apache.hadoop.hdfs.security.token.delegation.DelegationTokenIdentifier; import org.apache.hadoop.hdfs.web.JsonUtil; import org.apache.hadoop.hdfs.web.WebHdfsFileSystem; +import org.apache.hadoop.hdfs.web.resources.AclPermissionParam; import org.apache.hadoop.hdfs.web.resources.GetOpParam; import org.apache.hadoop.hdfs.web.resources.PostOpParam; import org.apache.hadoop.hdfs.web.resources.PutOpParam; @@ -112,8 +113,11 @@ public class WebHdfsHandler extends SimpleChannelInboundHandler { this.confForCreate = confForCreate; /** set user pattern based on configuration file */ UserParam.setUserPattern( - conf.get(DFSConfigKeys.DFS_WEBHDFS_USER_PATTERN_KEY, - DFSConfigKeys.DFS_WEBHDFS_USER_PATTERN_DEFAULT)); + conf.get(HdfsClientConfigKeys.DFS_WEBHDFS_USER_PATTERN_KEY, + HdfsClientConfigKeys.DFS_WEBHDFS_USER_PATTERN_DEFAULT)); + AclPermissionParam.setAclPermissionPattern( + conf.get(HdfsClientConfigKeys.DFS_WEBHDFS_ACL_PERMISSION_PATTERN_KEY, + HdfsClientConfigKeys.DFS_WEBHDFS_ACL_PERMISSION_PATTERN_DEFAULT)); } @Override diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeHttpServer.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeHttpServer.java index a1959e457fb..e7e5f512868 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeHttpServer.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeHttpServer.java @@ -41,6 +41,7 @@ import org.apache.hadoop.hdfs.server.namenode.startupprogress.StartupProgress; import org.apache.hadoop.hdfs.server.namenode.web.resources.NamenodeWebHdfsMethods; import org.apache.hadoop.hdfs.web.AuthFilter; import org.apache.hadoop.hdfs.web.WebHdfsFileSystem; +import org.apache.hadoop.hdfs.web.resources.AclPermissionParam; import org.apache.hadoop.hdfs.web.resources.Param; import org.apache.hadoop.hdfs.web.resources.UserParam; import org.apache.hadoop.http.HttpConfig; @@ -80,6 +81,9 @@ public class NameNodeHttpServer { UserParam.setUserPattern(conf.get( HdfsClientConfigKeys.DFS_WEBHDFS_USER_PATTERN_KEY, HdfsClientConfigKeys.DFS_WEBHDFS_USER_PATTERN_DEFAULT)); + AclPermissionParam.setAclPermissionPattern(conf.get( + HdfsClientConfigKeys.DFS_WEBHDFS_ACL_PERMISSION_PATTERN_KEY, + HdfsClientConfigKeys.DFS_WEBHDFS_ACL_PERMISSION_PATTERN_DEFAULT)); // add authentication filter for webhdfs final String className = conf.get( diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml b/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml index 652b216732f..d23b9674ed1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml @@ -2477,6 +2477,14 @@ + + dfs.webhdfs.acl.provider.permission.pattern + ^(default:)?(user|group|mask|other):[[A-Za-z_][A-Za-z0-9._-]]*:([rwx-]{3})?(,(default:)?(user|group|mask|other):[[A-Za-z_][A-Za-z0-9._-]]*:([rwx-]{3})?)*$ + + Valid pattern for user and group names in webhdfs acl operations, it must be a valid java regex. + + + dfs.webhdfs.socket.connect-timeout 60s diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHDFS.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHDFS.java index 1ff04de9b63..7cfaa99294b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHDFS.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHDFS.java @@ -49,6 +49,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.ws.rs.core.MediaType; +import com.google.common.collect.ImmutableList; import org.apache.commons.io.IOUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -64,6 +65,9 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.RemoteIterator; import org.apache.hadoop.fs.StorageType; +import org.apache.hadoop.fs.permission.AclEntry; +import org.apache.hadoop.fs.permission.AclEntryScope; +import org.apache.hadoop.fs.permission.AclEntryType; import org.apache.hadoop.fs.permission.FsAction; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hdfs.DFSConfigKeys; @@ -379,10 +383,17 @@ public class TestWebHDFS { } @Test(timeout=300000) - public void testNumericalUserName() throws Exception { + public void testCustomizedUserAndGroupNames() throws Exception { final Configuration conf = WebHdfsTestUtil.createConf(); + conf.setBoolean(DFSConfigKeys.DFS_NAMENODE_ACLS_ENABLED_KEY, true); + // Modify username pattern to allow numeric usernames conf.set(HdfsClientConfigKeys.DFS_WEBHDFS_USER_PATTERN_KEY, "^[A-Za-z0-9_][A-Za-z0-9" + "._-]*[$]?$"); + // Modify acl pattern to allow numeric and "@" characters user/groups in ACL spec + conf.set(HdfsClientConfigKeys.DFS_WEBHDFS_ACL_PERMISSION_PATTERN_KEY, + "^(default:)?(user|group|mask|other):" + + "[[0-9A-Za-z_][@A-Za-z0-9._-]]*:([rwx-]{3})?(,(default:)?" + + "(user|group|mask|other):[[0-9A-Za-z_][@A-Za-z0-9._-]]*:([rwx-]{3})?)*$"); final MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).numDataNodes(1).build(); try { @@ -391,6 +402,7 @@ public class TestWebHDFS { .setPermission(new Path("/"), new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL)); + // Test a numeric username UserGroupInformation.createUserForTesting("123", new String[]{"my-group"}) .doAs(new PrivilegedExceptionAction() { @Override @@ -399,6 +411,21 @@ public class TestWebHDFS { WebHdfsConstants.WEBHDFS_SCHEME); Path d = new Path("/my-dir"); Assert.assertTrue(fs.mkdirs(d)); + // Test also specifying a default ACL with a numeric username + // and another of a groupname with '@' + fs.modifyAclEntries(d, ImmutableList.of( + new AclEntry.Builder() + .setPermission(FsAction.READ) + .setScope(AclEntryScope.DEFAULT) + .setType(AclEntryType.USER) + .setName("11010") + .build(), + new AclEntry.Builder() + .setPermission(FsAction.READ_WRITE) + .setType(AclEntryType.GROUP) + .setName("foo@bar") + .build() + )); return null; } }); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/resources/TestParam.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/resources/TestParam.java index 6449bf73c01..d444cb4360a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/resources/TestParam.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/resources/TestParam.java @@ -351,6 +351,40 @@ public class TestParam { LOG.info("EXPECTED: " + e); } } + + @Test + public void testUserGroupOkAfterAlteringAclPattern() { + // Preserve default pattern value + AclPermissionParam.Domain oldDomain = + AclPermissionParam.getAclPermissionPattern(); + + // Override the pattern with one that accepts '@' and numbers + // in the first character of usernames/groupnames + String newPattern = + "^(default:)?(user|group|mask|other):" + + "[[0-9A-Za-z_][@A-Za-z0-9._-]]*:([rwx-]{3})?" + + "(,(default:)?(user|group|mask|other):" + + "[[0-9A-Za-z_][@A-Za-z0-9._-]]*:([rwx-]{3})?)*$"; + + try { + AclPermissionParam.setAclPermissionPattern(newPattern); + + String numericUserSpec = "user:110201:rwx"; + AclPermissionParam aclNumericUserParam = + new AclPermissionParam(numericUserSpec); + Assert.assertEquals(numericUserSpec, aclNumericUserParam.getValue()); + + String oddGroupSpec = "group:foo@bar:rwx"; + AclPermissionParam aclGroupWithDomainParam = + new AclPermissionParam(oddGroupSpec); + Assert.assertEquals(oddGroupSpec, aclGroupWithDomainParam.getValue()); + + } finally { + // Revert back to the default rules for remainder of tests + AclPermissionParam.setAclPermissionPattern(oldDomain); + } + + } @Test public void testXAttrNameParam() {