[
https://issues.apache.org/jira/browse/HDFS-15051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17069926#comment-17069926
]
Íñigo Goiri commented on HDFS-15051:
------------------------------------
Thanks for the work, the functionality looks correct.
I would like to improve the readability a little.
Probably, we could have a function like:
{code}
/**
* Check parent path permission recursively. It needs WRITE permission
* of the nearest parent entry and other EXECUTE permission.
* @param Path to check.
* @throws AccessControlException if mount table cannot be accessed
*/
private void checkMountTablePermission(final String src) {
//
String parent = src.substring(0, src.lastIndexOf(Path.SEPARATOR));
checkMountTablePermission(parent, FsAction.WRITE);
parent = parent.substring(0, parent.lastIndexOf(Path.SEPARATOR));
while (!parent.isEmpty()) {
checkMountTablePermission(parent, FsAction.EXECUTE);
parent = parent.substring(0, parent.lastIndexOf(Path.SEPARATOR));
}
}
{code}
BTW, [^HDFS-15051.008.patch] also changes the code to return false when the
mount table entry does not exist.
I'm not sure this is covered by the test.
> RBF: Propose to revoke WRITE MountTableEntry privilege to super user only
> -------------------------------------------------------------------------
>
> Key: HDFS-15051
> URL: https://issues.apache.org/jira/browse/HDFS-15051
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: rbf
> Reporter: Xiaoqiao He
> Assignee: Xiaoqiao He
> Priority: Major
> Attachments: HDFS-15051.001.patch, HDFS-15051.002.patch,
> HDFS-15051.003.patch, HDFS-15051.004.patch, HDFS-15051.005.patch,
> HDFS-15051.006.patch, HDFS-15051.007.patch, HDFS-15051.008.patch
>
>
> The current permission checker of #MountTableStoreImpl is not very restrict.
> In some case, any user could add/update/remove MountTableEntry without the
> expected permission checking.
> The following code segment try to check permission when operate
> MountTableEntry, however mountTable object is from Client/RouterAdmin
> {{MountTable mountTable = request.getEntry();}}, and user could pass any mode
> which could bypass the permission checker.
> {code:java}
> public void checkPermission(MountTable mountTable, FsAction access)
> throws AccessControlException {
> if (isSuperUser()) {
> return;
> }
> FsPermission mode = mountTable.getMode();
> if (getUser().equals(mountTable.getOwnerName())
> && mode.getUserAction().implies(access)) {
> return;
> }
> if (isMemberOfGroup(mountTable.getGroupName())
> && mode.getGroupAction().implies(access)) {
> return;
> }
> if (!getUser().equals(mountTable.getOwnerName())
> && !isMemberOfGroup(mountTable.getGroupName())
> && mode.getOtherAction().implies(access)) {
> return;
> }
> throw new AccessControlException(
> "Permission denied while accessing mount table "
> + mountTable.getSourcePath()
> + ": user " + getUser() + " does not have " + access.toString()
> + " permissions.");
> }
> {code}
> I just propose revoke WRITE MountTableEntry privilege to super user only.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
