[
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: [email protected]
For additional commands, e-mail: [email protected]
