[jira] [Commented] (HDFS-13194) CachePool permissions incorrectly checked
[ https://issues.apache.org/jira/browse/HDFS-13194?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16379691#comment-16379691 ] Hudson commented on HDFS-13194: --- SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #13733 (See [https://builds.apache.org/job/Hadoop-trunk-Commit/13733/]) HDFS-13194. CachePool permissions incorrectly checked. Contributed by (yqlin: rev a9c14b11193adeaa31389578f4cb90fa79cad8c3) * (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java * (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCacheDirectives.java > CachePool permissions incorrectly checked > - > > Key: HDFS-13194 > URL: https://issues.apache.org/jira/browse/HDFS-13194 > Project: Hadoop HDFS > Issue Type: Bug >Affects Versions: 3.0.0 >Reporter: Yiqun Lin >Assignee: Jianfei Jiang >Priority: Major > Fix For: 3.1.0, 2.10.0, 3.2.0 > > Attachments: HDFS-13194.001.patch, HDFS-13194.002.patch > > > The permissions of CachePool incorrectly checked. The checking logic: > {code:java} > public void checkPermission(CachePool pool, FsAction access) > throws AccessControlException { > FsPermission mode = pool.getMode(); > if (isSuperUser()) { > return; > } > if (getUser().equals(pool.getOwnerName()) > && mode.getUserAction().implies(access)) { > return; > } > if (isMemberOfGroup(pool.getGroupName()) > && mode.getGroupAction().implies(access)) { > return; > } > // Following line seems incorrect, > // we should ensure current user is not belong the pool's owner or pool's > group. > if (mode.getOtherAction().implies(access)) { > return; > } > throw new AccessControlException("Permission denied while accessing pool " > + pool.getPoolName() + ": user " + getUser() + " does not have " > + access.toString() + " permissions."); > } > {code} > For example one corner case, a cachepool (owner: test, group,test-group, > permission mode:--rwx(007)), then one user which named "test" or whose > group is "test-group" can both access this pool. But actually this is not > allowed since permission for its owner or group is none. > The behavior of checking other user should be updated like this: > {code:java} > if (!getUser().equals(pool.getOwnerName()) > && !isMemberOfGroup(pool.getGroupName()) > && mode.getOtherAction().implies(access)) { > return; > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-13194) CachePool permissions incorrectly checked
[ https://issues.apache.org/jira/browse/HDFS-13194?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16378461#comment-16378461 ] Yiqun Lin commented on HDFS-13194: -- Thanks for the updating the patch, [~jiangjianfei]. LGTM, +1. Will commit this tomorrow in case there are some other comments. > CachePool permissions incorrectly checked > - > > Key: HDFS-13194 > URL: https://issues.apache.org/jira/browse/HDFS-13194 > Project: Hadoop HDFS > Issue Type: Bug >Affects Versions: 3.0.0 >Reporter: Yiqun Lin >Assignee: Jianfei Jiang >Priority: Major > Attachments: HDFS-13194.001.patch, HDFS-13194.002.patch > > > The permissions of CachePool incorrectly checked. The checking logic: > {code:java} > public void checkPermission(CachePool pool, FsAction access) > throws AccessControlException { > FsPermission mode = pool.getMode(); > if (isSuperUser()) { > return; > } > if (getUser().equals(pool.getOwnerName()) > && mode.getUserAction().implies(access)) { > return; > } > if (isMemberOfGroup(pool.getGroupName()) > && mode.getGroupAction().implies(access)) { > return; > } > // Following line seems incorrect, > // we should ensure current user is not belong the pool's owner or pool's > group. > if (mode.getOtherAction().implies(access)) { > return; > } > throw new AccessControlException("Permission denied while accessing pool " > + pool.getPoolName() + ": user " + getUser() + " does not have " > + access.toString() + " permissions."); > } > {code} > For example one corner case, a cachepool (owner: test, group,test-group, > permission mode:--rwx(007)), then one user which named "test" or whose > group is "test-group" can both access this pool. But actually this is not > allowed since permission for its owner or group is none. > The behavior of checking other user should be updated like this: > {code:java} > if (!getUser().equals(pool.getOwnerName()) > && !isMemberOfGroup(pool.getGroupName()) > && mode.getOtherAction().implies(access)) { > return; > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-13194) CachePool permissions incorrectly checked
[ https://issues.apache.org/jira/browse/HDFS-13194?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16378314#comment-16378314 ] genericqa commented on HDFS-13194: -- | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 12s{color} | {color:blue} Docker mode activated. {color} | || || || || {color:brown} Prechecks {color} || | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s{color} | {color:green} The patch does not contain any @author tags. {color} | | {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m 0s{color} | {color:green} The patch appears to include 1 new or modified test files. {color} | || || || || {color:brown} trunk Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 16m 55s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 53s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 40s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 59s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 11m 32s{color} | {color:green} branch has no errors when building and testing our client artifacts. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 54s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 53s{color} | {color:green} trunk passed {color} | || || || || {color:brown} Patch Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 56s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 51s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 51s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 36s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 55s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 10m 56s{color} | {color:green} patch has no errors when building and testing our client artifacts. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 59s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 52s{color} | {color:green} the patch passed {color} | || || || || {color:brown} Other Tests {color} || | {color:red}-1{color} | {color:red} unit {color} | {color:red} 86m 29s{color} | {color:red} hadoop-hdfs in the patch failed. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 23s{color} | {color:green} The patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black}137m 42s{color} | {color:black} {color} | \\ \\ || Reason || Tests || | Failed junit tests | hadoop.hdfs.server.federation.metrics.TestFederationMetrics | \\ \\ || Subsystem || Report/Notes || | Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639 | | JIRA Issue | HDFS-13194 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12912221/HDFS-13194.002.patch | | Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle | | uname | Linux cdca019dae9d 3.13.0-135-generic #184-Ubuntu SMP Wed Oct 18 11:55:51 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | /testptch/patchprocess/precommit/personality/provided.sh | | git revision | trunk / 1e85a99 | | maven | version: Apache Maven 3.3.9 | | Default Java | 1.8.0_151 | | findbugs | v3.1.0-RC1 | | unit | https://builds.apache.org/job/PreCommit-HDFS-Build/23218/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt | | Test Results | https://builds.apache.org/job/PreCommit-HDFS-Build/23218/testReport/ | | Max. process+thread count | 4594 (vs. ulimit of 1) | | modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs | | Console output | https://builds.apache.org/job/PreCommit-HDFS-Build/23218/console | | Powered by | Apache Yetus 0.8.0-SNAPSHOT
[jira] [Commented] (HDFS-13194) CachePool permissions incorrectly checked
[ https://issues.apache.org/jira/browse/HDFS-13194?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16378148#comment-16378148 ] genericqa commented on HDFS-13194: -- | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 23s{color} | {color:blue} Docker mode activated. {color} | || || || || {color:brown} Prechecks {color} || | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s{color} | {color:green} The patch does not contain any @author tags. {color} | | {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m 0s{color} | {color:green} The patch appears to include 1 new or modified test files. {color} | || || || || {color:brown} trunk Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 17m 35s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m 5s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 45s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 15s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 13m 10s{color} | {color:green} branch has no errors when building and testing our client artifacts. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 2m 14s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 1s{color} | {color:green} trunk passed {color} | || || || || {color:brown} Patch Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m 10s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m 1s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m 1s{color} | {color:green} the patch passed {color} | | {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange} 0m 41s{color} | {color:orange} hadoop-hdfs-project/hadoop-hdfs: The patch generated 2 new + 46 unchanged - 1 fixed = 48 total (was 47) {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 8s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 12m 14s{color} | {color:green} patch has no errors when building and testing our client artifacts. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 2m 16s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 57s{color} | {color:green} the patch passed {color} | || || || || {color:brown} Other Tests {color} || | {color:red}-1{color} | {color:red} unit {color} | {color:red}125m 12s{color} | {color:red} hadoop-hdfs in the patch failed. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 24s{color} | {color:green} The patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black}182m 9s{color} | {color:black} {color} | \\ \\ || Reason || Tests || | Failed junit tests | hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure | | | hadoop.hdfs.server.blockmanagement.TestBlockStatsMXBean | | | hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting | \\ \\ || Subsystem || Report/Notes || | Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:5b98639 | | JIRA Issue | HDFS-13194 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12912196/HDFS-13194.001.patch | | Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle | | uname | Linux a5e6fd2a1096 3.13.0-135-generic #184-Ubuntu SMP Wed Oct 18 11:55:51 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | /testptch/patchprocess/precommit/personality/provided.sh | | git revision | trunk / 1e85a99 | | maven | version: Apache Maven 3.3.9 | | Default Java | 1.8.0_151 | | findbugs | v3.1.0-RC1 | | checkstyle | https://builds.apache.org/job/PreCommit-HDFS-Build/23214/artifact/out/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt | | unit | https://builds.apache.org/job/PreCommit-HDFS-Build/23214/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt | | Test Results |
[jira] [Commented] (HDFS-13194) CachePool permissions incorrectly checked
[ https://issues.apache.org/jira/browse/HDFS-13194?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16378135#comment-16378135 ] Yiqun Lin commented on HDFS-13194: -- Thanks for providing the patch. Almost looks good, but the test case can be simplified a lot. *TestCacheDirectives.java* Would you reuse the {{unprivilegedUser}} info to construct the new pool5, like this {code:java} proto.addCachePool(new CachePoolInfo("pool5"). setMode(new FsPermission((short)0007)) .setOwnerName(unprivilegedUser.getShortUserName()) .setGroupName(unprivilegedUser.getPrimaryGroupName()); {code} And then we can just reuse {{addAsUnprivileged}} to verify the exception in following code: {code:java} try { addAsUnprivileged(new CacheDirectiveInfo.Builder(). setPath(new Path("/epsilon")). setPool("pool5"). build()); fail("expected an error when adding to a pool with " + "mode 007 (no permissions for pool owner)."); } catch (AccessControlException e) { GenericTestUtils. assertExceptionContains("Permission denied while accessing pool", e); } {code} Actually the cache directive won't be added in pool5, other change in this patch can all be removed. > CachePool permissions incorrectly checked > - > > Key: HDFS-13194 > URL: https://issues.apache.org/jira/browse/HDFS-13194 > Project: Hadoop HDFS > Issue Type: Bug >Affects Versions: 3.0.0 >Reporter: Yiqun Lin >Assignee: Jianfei Jiang >Priority: Major > Attachments: HDFS-13194.001.patch > > > The permissions of CachePool incorrectly checked. The checking logic: > {code:java} > public void checkPermission(CachePool pool, FsAction access) > throws AccessControlException { > FsPermission mode = pool.getMode(); > if (isSuperUser()) { > return; > } > if (getUser().equals(pool.getOwnerName()) > && mode.getUserAction().implies(access)) { > return; > } > if (isMemberOfGroup(pool.getGroupName()) > && mode.getGroupAction().implies(access)) { > return; > } > // Following line seems incorrect, > // we should ensure current user is not belong the pool's owner or pool's > group. > if (mode.getOtherAction().implies(access)) { > return; > } > throw new AccessControlException("Permission denied while accessing pool " > + pool.getPoolName() + ": user " + getUser() + " does not have " > + access.toString() + " permissions."); > } > {code} > For example one corner case, a cachepool (owner: test, group,test-group, > permission mode:--rwx(007)), then one user which named "test" or whose > group is "test-group" can both access this pool. But actually this is not > allowed since permission for its owner or group is none. > The behavior of checking other user should be updated like this: > {code:java} > if (!getUser().equals(pool.getOwnerName()) > && !isMemberOfGroup(pool.getGroupName()) > && mode.getOtherAction().implies(access)) { > return; > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-13194) CachePool permissions incorrectly checked
[ https://issues.apache.org/jira/browse/HDFS-13194?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16377994#comment-16377994 ] Yiqun Lin commented on HDFS-13194: -- [~jiangjianfei], you have already done the fix for this, it's okay to assign to yourself, :). I will help the review very soon. > CachePool permissions incorrectly checked > - > > Key: HDFS-13194 > URL: https://issues.apache.org/jira/browse/HDFS-13194 > Project: Hadoop HDFS > Issue Type: Bug >Affects Versions: 3.0.0 >Reporter: Yiqun Lin >Priority: Major > Attachments: HDFS-13194.001.patch > > > The permissions of CachePool incorrectly checked. The checking logic: > {code:java} > public void checkPermission(CachePool pool, FsAction access) > throws AccessControlException { > FsPermission mode = pool.getMode(); > if (isSuperUser()) { > return; > } > if (getUser().equals(pool.getOwnerName()) > && mode.getUserAction().implies(access)) { > return; > } > if (isMemberOfGroup(pool.getGroupName()) > && mode.getGroupAction().implies(access)) { > return; > } > // Following line seems incorrect, > // we should ensure current user is not belong the pool's owner or pool's > group. > if (mode.getOtherAction().implies(access)) { > return; > } > throw new AccessControlException("Permission denied while accessing pool " > + pool.getPoolName() + ": user " + getUser() + " does not have " > + access.toString() + " permissions."); > } > {code} > For example one corner case, a cachepool (owner: test, group,test-group, > permission mode:--rwx(007)), then one user which named "test" or whose > group is "test-group" can both access this pool. But actually this is not > allowed since permission for its owner or group is none. > The behavior of checking other user should be updated like this: > {code:java} > if (!getUser().equals(pool.getOwnerName()) > && !isMemberOfGroup(pool.getGroupName()) > && mode.getOtherAction().implies(access)) { > return; > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-13194) CachePool permissions incorrectly checked
[ https://issues.apache.org/jira/browse/HDFS-13194?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16377991#comment-16377991 ] Jianfei Jiang commented on HDFS-13194: -- Sorry [~linyiqun], I missed your comment and add a patch, should I unassigned to you? Sorry a lot. > CachePool permissions incorrectly checked > - > > Key: HDFS-13194 > URL: https://issues.apache.org/jira/browse/HDFS-13194 > Project: Hadoop HDFS > Issue Type: Bug >Affects Versions: 3.0.0 >Reporter: Yiqun Lin >Assignee: Jianfei Jiang >Priority: Major > Attachments: HDFS-13194.001.patch > > > The permissions of CachePool incorrectly checked. The checking logic: > {code:java} > public void checkPermission(CachePool pool, FsAction access) > throws AccessControlException { > FsPermission mode = pool.getMode(); > if (isSuperUser()) { > return; > } > if (getUser().equals(pool.getOwnerName()) > && mode.getUserAction().implies(access)) { > return; > } > if (isMemberOfGroup(pool.getGroupName()) > && mode.getGroupAction().implies(access)) { > return; > } > // Following line seems incorrect, > // we should ensure current user is not belong the pool's owner or pool's > group. > if (mode.getOtherAction().implies(access)) { > return; > } > throw new AccessControlException("Permission denied while accessing pool " > + pool.getPoolName() + ": user " + getUser() + " does not have " > + access.toString() + " permissions."); > } > {code} > For example one corner case, a cachepool (owner: test, group,test-group, > permission mode:--rwx(007)), then one user which named "test" or whose > group is "test-group" can both access this pool. But actually this is not > allowed since permission for its owner or group is none. > The behavior of checking other user should be updated like this: > {code:java} > if (!getUser().equals(pool.getOwnerName()) > && !isMemberOfGroup(pool.getGroupName()) > && mode.getOtherAction().implies(access)) { > return; > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-13194) CachePool permissions incorrectly checked
[ https://issues.apache.org/jira/browse/HDFS-13194?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16377986#comment-16377986 ] Yiqun Lin commented on HDFS-13194: -- Thanks for the comments, [~hexiaoqiao] and [~jiangjianfei]. I will attach the patch today, just a very easy fix, :). > CachePool permissions incorrectly checked > - > > Key: HDFS-13194 > URL: https://issues.apache.org/jira/browse/HDFS-13194 > Project: Hadoop HDFS > Issue Type: Bug >Affects Versions: 3.0.0 >Reporter: Yiqun Lin >Assignee: Yiqun Lin >Priority: Major > > The permissions of CachePool incorrectly checked. The checking logic: > {code:java} > public void checkPermission(CachePool pool, FsAction access) > throws AccessControlException { > FsPermission mode = pool.getMode(); > if (isSuperUser()) { > return; > } > if (getUser().equals(pool.getOwnerName()) > && mode.getUserAction().implies(access)) { > return; > } > if (isMemberOfGroup(pool.getGroupName()) > && mode.getGroupAction().implies(access)) { > return; > } > // Following line seems incorrect, > // we should ensure current user is not belong the pool's owner or pool's > group. > if (mode.getOtherAction().implies(access)) { > return; > } > throw new AccessControlException("Permission denied while accessing pool " > + pool.getPoolName() + ": user " + getUser() + " does not have " > + access.toString() + " permissions."); > } > {code} > For example one corner case, a cachepool (owner: test, group,test-group, > permission mode:--rwx(007)), then one user which named "test" or whose > group is "test-group" can both access this pool. But actually this is not > allowed since permission for its owner or group is none. > The behavior of checking other user should be updated like this: > {code:java} > if (!getUser().equals(pool.getOwnerName()) > && !isMemberOfGroup(pool.getGroupName()) > && mode.getOtherAction().implies(access)) { > return; > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-13194) CachePool permissions incorrectly checked
[ https://issues.apache.org/jira/browse/HDFS-13194?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16376935#comment-16376935 ] Jianfei Jiang commented on HDFS-13194: -- Disgree with [~hexiaoqiao], the fix still return without exception under the condition given by [~linyiqun]. What you mentions may be like following. Separate the determine statements to two \{{if}}. {code:java} public void checkPermission(CachePool pool, FsAction access) throws AccessControlException { FsPermission mode = pool.getMode(); if (isSuperUser()) { return; } else if (getUser().equals(pool.getOwnerName())) { if (mode.getUserAction().implies(access)) { return; } } else if (isMemberOfGroup(pool.getGroupName())) { if (mode.getGroupAction().implies(access)) { return; } } else if (mode.getOtherAction().implies(access)) { return; } throw new AccessControlException("Permission denied while accessing pool " + pool.getPoolName() + ": user " + getUser() + " does not have " + access.toString() + " permissions."); } {code} > CachePool permissions incorrectly checked > - > > Key: HDFS-13194 > URL: https://issues.apache.org/jira/browse/HDFS-13194 > Project: Hadoop HDFS > Issue Type: Bug >Affects Versions: 3.0.0 >Reporter: Yiqun Lin >Priority: Major > > The permissions of CachePool incorrectly checked. The checking logic: > {code:java} > public void checkPermission(CachePool pool, FsAction access) > throws AccessControlException { > FsPermission mode = pool.getMode(); > if (isSuperUser()) { > return; > } > if (getUser().equals(pool.getOwnerName()) > && mode.getUserAction().implies(access)) { > return; > } > if (isMemberOfGroup(pool.getGroupName()) > && mode.getGroupAction().implies(access)) { > return; > } > // Following line seems incorrect, > // we should ensure current user is not belong the pool's owner or pool's > group. > if (mode.getOtherAction().implies(access)) { > return; > } > throw new AccessControlException("Permission denied while accessing pool " > + pool.getPoolName() + ": user " + getUser() + " does not have " > + access.toString() + " permissions."); > } > {code} > For example one corner case, a cachepool (owner: test, group,test-group, > permission mode:--rwx(007)), then one user which named "test" or whose > group is "test-group" can both access this pool. But actually this is not > allowed since permission for its owner or group is none. > The behavior of checking other user should be updated like this: > {code:java} > if (!getUser().equals(pool.getOwnerName()) > && !isMemberOfGroup(pool.getGroupName()) > && mode.getOtherAction().implies(access)) { > return; > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-13194) CachePool permissions incorrectly checked
[ https://issues.apache.org/jira/browse/HDFS-13194?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16376839#comment-16376839 ] He Xiaoqiao commented on HDFS-13194: +1, maybe the following demonstrates approach is more POSIX-like permission check FYI. [~linyiqun]: {code:java} public void checkPermission(CachePool pool, FsAction access) throws AccessControlException { FsPermission mode = pool.getMode(); if (isSuperUser()) { return; } if (getUser().equals(pool.getOwnerName()) && mode.getUserAction().implies(access)) { return; } else if (isMemberOfGroup(pool.getGroupName()) && mode.getGroupAction().implies(access)) { return; } else if (mode.getOtherAction().implies(access)) { return; } throw new AccessControlException("Permission denied while accessing pool " + pool.getPoolName() + ": user " + getUser() + " does not have " + access.toString() + " permissions."); } {code} > CachePool permissions incorrectly checked > - > > Key: HDFS-13194 > URL: https://issues.apache.org/jira/browse/HDFS-13194 > Project: Hadoop HDFS > Issue Type: Bug >Affects Versions: 3.0.0 >Reporter: Yiqun Lin >Priority: Major > > The permissions of CachePool incorrectly checked. The checking logic: > {code:java} > public void checkPermission(CachePool pool, FsAction access) > throws AccessControlException { > FsPermission mode = pool.getMode(); > if (isSuperUser()) { > return; > } > if (getUser().equals(pool.getOwnerName()) > && mode.getUserAction().implies(access)) { > return; > } > if (isMemberOfGroup(pool.getGroupName()) > && mode.getGroupAction().implies(access)) { > return; > } > // Following line seems incorrect, > // we should ensure current user is not belong the pool's owner or pool's > group. > if (mode.getOtherAction().implies(access)) { > return; > } > throw new AccessControlException("Permission denied while accessing pool " > + pool.getPoolName() + ": user " + getUser() + " does not have " > + access.toString() + " permissions."); > } > {code} > For example one corner case, a cachepool (owner: test, group,test-group, > permission mode:--rwx(007)), then one user which named "test" or whose > group is "test-group" can both access this pool. But actually this is not > allowed since permission for its owner or group is none. > The behavior of checking other user should be updated like this: > {code:java} > if (!getUser().equals(pool.getOwnerName()) > && !isMemberOfGroup(pool.getGroupName()) > && mode.getOtherAction().implies(access)) { > return; > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org