Eric Yang commented on YARN-7221:

[~jlowe] {quote}
I don't think WNOHANG is appropriate here. It will cause the parent process to 
spin continuously as long as the child is running. If we want to keep waiting 
for the child even when EINTR interrupts the wait then I think the loop should 
check for waitpid() == 0 && errno == EINTR.{quote}

WNOHANG flag was used to track all child processes in case interrupted child 
process has not been reported by the while loop.  If exec failed WUNTRACED, the 
unreported child process termination may get caught and getting reported.  The 
usage of -1, was making assumption there is only one child process for the sudo 
call.  I can see that assumption could easy be flawed when more fork exec call 
gets introduce to container-executor.  

Waitpid doesn't seem to set EINTR flag for errno when SIGINT is sent to the 
child process.  Base on testing wait and waitpid both produced the same result 
for EINTR flag.  I don't think we are more accurate on handling abnormal exit 
check for sudo command with waitpid.  SIGINT to the sudo check can only be 
issued by root user to interrupt the check, hence, the chance of someone trying 
to by pass sudo check using signal doesn't exist.  Sorry, I don't know how to 
make this better.  If you have code example of how to make this better, I am 
happy to integrate it into the patch.  At this time, Patch 16 is more correct 
than patch 17.

> Add security check for privileged docker container
> --------------------------------------------------
>                 Key: YARN-7221
>                 URL: https://issues.apache.org/jira/browse/YARN-7221
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: security
>    Affects Versions: 3.0.0, 3.1.0
>            Reporter: Eric Yang
>            Assignee: Eric Yang
>            Priority: Major
>         Attachments: YARN-7221.001.patch, YARN-7221.002.patch, 
> YARN-7221.003.patch, YARN-7221.004.patch, YARN-7221.005.patch, 
> YARN-7221.006.patch, YARN-7221.007.patch, YARN-7221.008.patch, 
> YARN-7221.009.patch, YARN-7221.010.patch, YARN-7221.011.patch, 
> YARN-7221.012.patch, YARN-7221.013.patch, YARN-7221.014.patch, 
> YARN-7221.015.patch, YARN-7221.016.patch, YARN-7221.017.patch
> When a docker is running with privileges, majority of the use case is to have 
> some program running with root then drop privileges to another user.  i.e. 
> httpd to start with privileged and bind to port 80, then drop privileges to 
> www user.  
> # We should add security check for submitting users, to verify they have 
> "sudo" access to run privileged container.  
> # We should remove --user=uid:gid for privileged containers.  
> Docker can be launched with --privileged=true, and --user=uid:gid flag.  With 
> this parameter combinations, user will not have access to become root user.  
> All docker exec command will be drop to uid:gid user to run instead of 
> granting privileges.  User can gain root privileges if container file system 
> contains files that give user extra power, but this type of image is 
> considered as dangerous.  Non-privileged user can launch container with 
> special bits to acquire same level of root power.  Hence, we lose control of 
> which image should be run with --privileges, and who have sudo rights to use 
> privileged container images.  As the result, we should check for sudo access 
> then decide to parameterize --privileged=true OR --user=uid:gid.  This will 
> avoid leading developer down the wrong path.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to