[
https://issues.apache.org/jira/browse/YARN-5534?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15835291#comment-15835291
]
Daniel Templeton edited comment on YARN-5534 at 1/26/17 6:56 PM:
-----------------------------------------------------------------
Thanks for updating the patch, [~luhuichun]. The new patch matches more with
what I had in mind. There are still a couple of issues:
* In {{YarnConfiguration}}, you should add a Javadoc comment for the new
property
* In {{DLCR.isArbitraryMount()}}, instead of the _for_ loop, you should use a
_foreach_.
* {{DSCR.isArbitraryMount()}} always returns {{false}}. {code} File child =
new File(mount);
for (int i = 0; i < whiteList.length; i++){
File parent = new File(mount);
if (isSubDirectory(parent, child)){
return false;
}
}{code} It's always true that {{parent.equals(child)}}, so
{{isSubdirectory()}} will always return true.
* You should parse the white list property once and store it instead of doing
it every time the method is called
* Instead of using {{String.split()}}, you could use {{Pattern.split()}} to
allow for whitespace, making it a little more user friendly
* Look at
http://stackoverflow.com/questions/26530445/compare-directories-to-check-if-one-is-sub-directory-of-another
for ideas of how to do the subdirectory check more efficiently. I like the
{{Set}} idea.
* In {{DLCR.isSubdirectory()}}, the naming around _child_, _parent_, and
_parentFile_ is pretty confusing.
* In {{DLCR.isSubdirectory()}}, printing the stack trace is a bad idea. Do
something more useful. At a minimum, log the exception instead of printing the
stack trace to stderr.
* You should have a space before the curly brace throughout, e.g. {code} if
(parent.equals(parentFile)){ {code}
was (Author: templedf):
Thanks for updating the patch, [~luhuichun]. The new patch matches more with
what I had in mind. There are still a couple of issues:
* In {{YarnConfiguration}}, you should add a Javadoc comment for the new
property
* In {{DLCR.isArbitraryMount()}}, instead of the _for_ loop, you should use a
_foreach_.
* {{DSCR.isArbitraryMount()}} always returns {{false}}. {code} File child =
new File(mount);
for (int i = 0; i < whiteList.length; i++){
File parent = new File(mount);
if (isSubDirectory(parent, child)){
return false;
}
}{code} It's always true that {{parent.equals(child)}}, so
{{isSubdirectory()}} will always return true.
* You should parse the white list property once and store it instead of doing
it every time the method is called
* Instead of using {{String.split()}}, you could use {{Pattern.split()}} to
allow for whitespace, making it a little more user friendly
* Look at
http://stackoverflow.com/questions/26530445/compare-directories-to-check-if-one-is-sub-directory-of-another
for ideas of how to do the subdirectory check more efficiently. I like the
{{Set}} idea.
* In {{DLCR.isSubdirectory()}}, the naming around _child_, _parent_, and
_parentFile_ is pretty confusing.
* In {{DLCR.isSubdirectory()}}, printing the stack trace in a bad idea. Do
something more useful. At a minimum, log the exception instead of printing it
in stderr.
* You should have a space before the curly brace throughout, e.g. {code} if
(parent.equals(parentFile)){ {code}
> Allow whitelisted volume mounts
> --------------------------------
>
> Key: YARN-5534
> URL: https://issues.apache.org/jira/browse/YARN-5534
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: yarn
> Reporter: luhuichun
> Assignee: luhuichun
> Attachments: YARN-5534.001.patch, YARN-5534.002.patch
>
>
> Introduction
> Mounting files or directories from the host is one way of passing
> configuration and other information into a docker container.
> We could allow the user to set a list of mounts in the environment of
> ContainerLaunchContext (e.g. /dir1:/targetdir1,/dir2:/targetdir2).
> These would be mounted read-only to the specified target locations. This has
> been resolved in YARN-4595
> 2.Problem Definition
> Bug mounting arbitrary volumes into a Docker container can be a security risk.
> 3.Possible solutions
> one approach to provide safe mounts is to allow the cluster administrator to
> configure a set of parent directories as white list mounting directories.
> Add a property named yarn.nodemanager.volume-mounts.white-list, when
> container executor do mount checking, only the allowed directories or
> sub-directories can be mounted.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
