hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Colin Patrick McCabe (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-11321) copyToLocal cannot save a file to an SMB share unless the user has Full Control permissions.
Date Fri, 12 Dec 2014 23:14:14 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-11321?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14244957#comment-14244957

Colin Patrick McCabe commented on HADOOP-11321:

Great.  Let me take a look.

-    if (permission != null) {
+    // A local file system implementation may choose to create the file and set
+    // permissions immediately in a single syscall.  If so, then skip setting
+    // permissions here.
+    if (permission != null && !(fs instanceof RawLocalFileSystem &&
+        Path.WINDOWS && NativeIO.isAvailable())) {

Do we need this part?  Earlier in this function, we invoke the {{FileSystem#create}} method
that takes an {{FsPermission}}, right?  Then the FileSystem handles it atomically (hopefully)
or non-atomically (ew).  Surely we're not going to try to work around {{FileSystem}} instances
that don't honor the permission passed to {{create}}?  (And surely we don't have any of those...
do we?)

     * Create a directory with permissions set to the specified mode.  By setting
     * permissions at creation time, we avoid issues related to the user lacking
     * WRITE_DAC rights on subsequent chmod calls.  One example where this can
     * occur is writing to an SMB share where the user does not have Full Control
     * rights, and therefore WRITE_DAC is denied.  This method mimics the
     * contract of {@link java.io.File#mkdir()}.  All exceptions are caught and
     * reported by returning {@code false} to the caller.
     * @param path directory to create
     * @param mode permissions of new directory
     * @return boolean true if directory creation succeeded
    public static boolean createDirectoryWithMode(File path, int mode) {

I think we should just pass the exception through here.  We don't want to keep people guessing
if there is an I/O error.  Just like the new JDK7 File functions throw an IOException on error,
we should throw an IOE if the IO can't be done.  Then the caller can catch it and ignore it
if appropriate.  I realize we need the (bad) "return false" semantics in FileSystem, but by
putting this here, we encourage people to use it for other stuff.

#ifdef UNIX
  THROW(env, "java/io/IOException",
    "The function Windows.createDirectoryWithMode0() is not supported on Unix");

#ifdef WINDOWS
Would it make more sense to have an {{#else}} after the {{#ifdef WINDOWS}} clause?

in {{libwinutils.c}}:
  *ppSD = pSD;

  if (dwRtnCode != ERROR_SUCCESS) {
I guess this is a nitpick, but perhaps we should only set {{ppSD}} when the return code is
success?  Of course it won't matter for callers that check the return code, as they should.

> copyToLocal cannot save a file to an SMB share unless the user has Full Control permissions.
> --------------------------------------------------------------------------------------------
>                 Key: HADOOP-11321
>                 URL: https://issues.apache.org/jira/browse/HADOOP-11321
>             Project: Hadoop Common
>          Issue Type: Bug
>          Components: fs
>    Affects Versions: 2.6.0
>            Reporter: Chris Nauroth
>            Assignee: Chris Nauroth
>         Attachments: HADOOP-11321.003.patch, HADOOP-11321.004.patch, HADOOP-11321.005.patch,
HADOOP-11321.1.patch, HADOOP-11321.2.patch, winutils.tmp.patch
> In Hadoop 2, it is impossible to use {{copyToLocal}} to copy a file from HDFS to a destination
on an SMB share.  This is because in Hadoop 2, the {{copyToLocal}} maps to 2 underlying {{RawLocalFileSystem}}
operations: {{create}} and {{setPermission}}.  On an SMB share, the user may be authorized
for the {{create}} but denied for the {{setPermission}}.  Windows denies the {{WRITE_DAC}}
right required by {{setPermission}} unless the user has Full Control permissions.  Granting
Full Control isn't feasible for most deployments, because it's insecure.  This is a regression
from Hadoop 1, where {{copyToLocal}} only did a {{create}} and didn't do a separate {{setPermission}}.

This message was sent by Atlassian JIRA

View raw message