[ 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