hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Daniel Templeton (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (YARN-5534) Allow whitelisted volume mounts
Date Thu, 26 Jan 2017 18:57:24 GMT

    [ 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: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org


Mime
View raw message