[ 
https://issues.apache.org/jira/browse/MAPREDUCE-899?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12806307#action_12806307
 ] 

Hemanth Yamijala commented on MAPREDUCE-899:
--------------------------------------------

A few minor comments:

- Remove unused variables - tt_uid and tt_gid and associated code.
- It will be good to check for user, group first and then permissions. It's a 
more natural code ordering than what exists currently, which is user, 
permissions, then group.
- Rather than check errno as return of getgrgid, it seems more conventional to 
check for return value being null. This is what man states anyway for return 
values of getgrgid. It was not clear to me that errno will be set to non-zero 
if the GID does not exist. However note that errno can still be captured and 
used for strerror.
- Should we rename mapreduce.tasktracker.taskcontroller.group to a more generic 
name and purpose like mapreduce.tasktracker.group. We could document that this 
is the group to which the tasktracker belongs and qualify that if 
LinuxTaskController is used, it takes more meaning - in that it should be the 
group owner of the binary etc.
- The wait introduced for in the ClusterWithLinuxTaskController does not seem 
required, because doesn't MiniMRCluster itself wait for clusters to join ? Can 
you please check if the timeout code you added actually helps ?

Thinking about tests, I think it is difficult to actually test all the paths 
verified by the C code without mocking the system calls or building different 
binary executables and setting them up with different permissions. While we 
work out details to improve the way we run LinuxTaskController tests - 
MAPREDUCE-1429, MAPREDUCE-1300 - I'd request you to document as a comment on 
this JIRA the various manual tests you have run with different binary 
permissions / ownership.

> When using LinuxTaskController, localized files may become accessible to 
> unintended users if permissions are misconfigured.
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-899
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-899
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: tasktracker
>            Reporter: Vinod K V
>            Assignee: Amareshwari Sriramadasu
>             Fix For: 0.22.0
>
>         Attachments: MAPREDUCE-899-20090828.txt, patch-899-1.txt, 
> patch-899-2.txt, patch-899-3.txt, patch-899-4.txt, patch-899-5.txt, 
> patch-899.txt
>
>
> To enforce the accessibility of job files to only the job-owner and the 
> TaskTracker, as per MAPREDUCE-842, it is _trusted_ that the  setuid/setgid 
> linux TaskController binary is group owned by a _special group_ to which only 
> TaskTracker belongs and not just any group to which TT belongs. If the trust 
> is broken, possibly due to misconfiguration by admins, the local files become 
> accessible to unintended users, yet giving false sense of security to the 
> admins.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to