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

Eric Badger commented on YARN-7221:
-----------------------------------

Hey [~eyang], thanks for the new patch! I tried it out and I think we're pretty 
close here. Comments below:

{noformat}
+    fprintf(ERRORFILE, "Out of memory.\n");
{noformat}

{noformat}
+    fprintf(ERRORFILE, "User does not exist.\n");
{noformat}

{noformat}
+    fprintf(ERRORFILE, "Fail to lookup groups.\n");
{noformat}
I think we can do better in all of these log messages. We can give information 
on what user/group failed or where we failed with OOM (i.e. during group buffer 
creation). 

{noformat}
+  fprintf(ERRORFILE, "check privileges for %s: %d\n", user, ret);
{noformat}
This log message could be a little more intuitive as well. In its current 
state, a user getting this message wouldn't really know what to do with this or 
even if it was an error or not. When I was testing it out, this message was 
hidden in the weeds and didn't really stick out as an intuitive log to why 
something would fail. 

I've tested the patch and it seems to work as designed. However, when I run 
with a user that does not have docker permissions, the error message that it 
gives is quite unintuitive because it is surrounded by a lot of docker and 
containermanager errors. I think it would be better to fail the launch if a 
container is trying to launch as privileged and not allowed. That way the error 
message will come out nice and clean. I'm not sure I see a use case where a 
user will ask for a privileged container and then be fine with it running as 
unprivileged in the even that it doesn't pass the ACL check, so I think we're 
safe here. 

> 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
>
>
> 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