[ 
https://issues.apache.org/jira/browse/YARN-7221?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16426994#comment-16426994
 ] 

Jason Lowe commented on YARN-7221:
----------------------------------

{quote}WNOHANG flag was used to track all child processes in case interrupted 
child process has not been reported by the while loop.
{quote}
WNOHANG has nothing to do with the what the child processes are doing. It just 
tells wait/waitpid to not block until a child process state change, and that's 
why we don't want to specify it. It will cause the parent process to spin 
quickly in the while loop as long as the child process is still running. That 
wastes CPU resources for no benefit.
{quote}If exec failed WUNTRACED, the unreported child process termination may 
get caught and getting reported.
{quote}
I'm not quite sure what is being said here. WUNTRACED means that wait/waitpid 
will also return if the process is stopped (e.g.: SIGSTOP) outside of ptrace. 
We don't care about those state changes. We only care when the child process 
terminates because only then do we potentially have an exit code we can act 
upon.
{quote}Waitpid doesn't seem to set EINTR flag for errno when SIGINT is sent to 
the child process.
{quote}
The EINTR errno has nothing to do with the child process. This simply means the 
current process received a signal while in the system call. Most system calls 
that block will return EINTR if the process receives an unblocked signal. 
Signal handlers are rather limited in what they can do directly, so they tend 
to simply set a global flag. The main process code then can react to that flag 
after being kicked out of the system call by EINTR. In this case we aren't 
handling any special flags, so we just want to re-enter the system call if we 
happened to get kicked out via EINTR.
{quote}If you have code example of how to make this better, I am happy to 
integrate it into the patch.
{quote}
Here's some sample code that should handle it properly and gives a bit more 
feedback when the privilege check fails because we couldn't launch sudo 
properly or sudo crashed. NOTE: I haven't compiled/tested this, but it should 
be close enough to convey the approach.
{code:java}
    int child_pid = fork();
    if (child_pid == 0) {
      execl("/bin/sudo", "sudo", "-U", user, "-n", "-l", "docker", NULL);
      fprintf(ERRORFILE, "sudo exec failed: %s\n", strerror(errno));
      exit(INITIALIZE_USER_FAILED);
    } else {
      while ((waitid = waitpid(child_pid, &statval, 0)) != child_pid) {
        if (waitid == -1 && errno != EINTR) {
          fprintf(ERRORFILE, "waitpid failed: %s\n", strerror(errno));
          break;
        }
      }
      if (waitid == child_pid) {
        if (WIFEXITED(statval)) {
          if (WEXITSTATUS(statval) == 0) {
            ret = 1;
          }
        } else if (WIFSIGNALED(statval)) {
          fprintf(ERRORFILE, "sudo terminated by signal %d\n", 
WTERMSIG(statval));
        }
      }
    }
{code}
 

> 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
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to