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

Eric Yang commented on YARN-7197:
---------------------------------

Hi [~jlowe]

{quote}
If black list contains misconfigured item, i.e, source doesn't exist, container 
creation fails.
What's the rationale behind this behavior? I could see admins configuring a 
path here that may only exist on some nodes but not all.
{quote}

This is to prevent mistake.  If a user misconfigure source path due to typing 
mistakes, it could make a empty directory all over the host OS by using -v.  
Hence, I took bind mount suggestion to fail fast instead.

{quote}
Seems like we could solve this limitation by either automatically determining 
whether the blacklisted path on the host is a file or a directory or having a 
way for the admin to give a directive whether the blacklisted path should be 
obscured with a file or a directory.
{quote}

Container-Executor has no prior knowledge of the object type in container 
image.  We depends on docker to determine if there are mismatches and fail 
fast.  It's more consistent that we let the system to fail fast instead of 
letting it slip through.  Docker will determine if the mounting source and 
target are the same type.  Directory is the only type that will slip through, 
and we replace the black list item with empty directory.  We have a universally 
empty directory for prevent jailbreak, and there is no universally empty 
file/socket for prevent jailbreak.  Therefore, I think it is reasonable to 
depend on docker to make this determination, and use empty directory as 
fallback on stuff that docker can not catch.

{quote}
Rather than normalizing the banned_mounts every time, would it make more sense 
to normalize them just once when we read them in from the config? That would 
also remove the need to skip null entries when checking since we could weed 
them out during the config read.
check_banned_mounts leaks normalized_path if banned_mounts is null.
check_banned_mounts needs to use strncmp for the length of the banned path to 
catch users trying to mount beneath a blacklisted path. For example, /home is 
blacklisted by the admin and the user tries to mount /home/yarn.
How do we know mount_src is big enough to do the strcpy of /var/empty/sshd 
without overrunning the buffer and scribbling on the heap?
Nit: Checking for a magic value from check_banned_mounts is odd for what is 
essentially a boolean predicate function. I'd expect callers to use if (banned) 
rather than knowing it's going to return -1 instead of the more natural value, 
1.
{quote}

Good catch on the coding style issues.  My C skill is a bit rusty, I will fix 
those issues.

{quote}
I think we should just fail the container launch if the user tries to mount at 
or under a blacklisted path. They clearly are trying to do something the admin 
does not want. We should only try to blot out blacklisted paths with an empty 
file/dir if and only if the user is trying to mount above a blacklisted path 
and could be legitimate behavior. Thoughts?
{quote}

Agree on the path depth issue, black out all children paths from black list.
Thanks for the review.


> Add support for a volume blacklist for docker containers
> --------------------------------------------------------
>
>                 Key: YARN-7197
>                 URL: https://issues.apache.org/jira/browse/YARN-7197
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: yarn
>            Reporter: Shane Kumpf
>            Assignee: Eric Yang
>            Priority: Major
>         Attachments: YARN-7197.001.patch, YARN-7197.002.patch, 
> YARN-7197.003.patch
>
>
> Docker supports bind mounting host directories into containers. Work is 
> underway to allow admins to configure a whilelist of volume mounts. While 
> this is a much needed and useful feature, it opens the door for 
> misconfiguration that may lead to users being able to compromise or crash the 
> system. 
> One example would be allowing users to mount /run from a host running 
> systemd, and then running systemd in that container, rendering the host 
> mostly unusable.
> This issue is to add support for a default blacklist. The default blacklist 
> would be where we put files and directories that if mounted into a container, 
> are likely to have negative consequences. Users are encouraged not to remove 
> items from the default blacklist, but may do so if necessary.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

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