hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Wang (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-6705) Create an XAttr that disallows the HDFS admin from accessing a file
Date Wed, 27 Aug 2014 20:46:59 GMT

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

Andrew Wang commented on HDFS-6705:

Hi Charles, thanks for working on this. Had some review feedback for you:

* Would be nice to include XATTR in the name of the constant, e.g. SECURITY_XATTR_*
* Where we throw the AccessControlException, could throw the constant instead since it's cheaper.
* checkNotSuperUserPrivilege, "superuser" is not a proper noun, no caps in exception text.
Also some mild inconsistency in javadoc, "the super user" -> "a superuser", "is superuser"
-> "is a superuser"

Checking access:
* getPermissionChecker is expensive, and we already have a pc passed in, so let's use the
passed one instead
* Checking for this xattr is fairly expensive, since it involves calling into FSD to do path
resolution. checkPathAccess is also called in a lot of places that aren't related to reading
file data, and we'll restrict those too.
* Particularly, I don't think we need to restrict being able to write data. The superuser
could always delete the file and recreate it. For an encrypted file, the superuser doesn't
have access to the encryption key, so won't be able to spoof any content. I think this also
breaks doing an fsck as the superuser since that requires getting block locations.
* Also not sure about restricting xattr access. xattrs are file metadata just like owner/group/times.
The encryption attrs are also safe for handling by the superuser, so not sure what we're hiding.
* One idea for doing this in a more targeted fashion is not providing a block token, since
that's required for a user to read data. FSNamesystem#getBlockLocationsUpdateTimes looks like
a good candidate, since we also already have the IIP, pc, and the needsBlockToken boolean.

* Naming: xattr is called UNREADABLE_BY_SUPERUSER, would be nice for all the new methods to
be named similarly.
* Is there a need for the additional brace blocks in many of these methods? They're nice if
you want to scope a new variable, but that's not happening here.
* Would like a shell test that tries deleting the xattr, tries opening the file
* For the calls to restart, I think we want to flip the booleans. We want to *not* checkpoint
the first time to test edit log loading, then checkpoint the second time to test fsimage loading.
* Comment about "just hope for no exception" ? It should return null for the value, which
we can also assert.
* The MAX_SIZE stuff and additional cluster restarts looks like copy paste, we shouldn't need
to do that here.
* weird formatting for doTestDenySuperUserAccessXAttr
* The assertExceptionContains, could use the string constant for "security.hdfs.unreadable.by.superuser"
rather than hardcoding it again

* Making a new first-level header is a bit much. Can we just put the new text up top with
the explanation of the security namespace? 
* Capitalization: "superuser" rather than "Superuser"
* Saying that "it exists in name only" makes it sound like it doesn't do anything. Could just
say "does not allow a value to be set."
* Could clarify that this can only be set on files and not on directories.
* Using the term "write-once" might make things more clear too.
* xattrs are metadata, so the statement about metadata still being accessible by the superuser
is confusing. This is all pending the above semantic discussion.

> Create an XAttr that disallows the HDFS admin from accessing a file
> -------------------------------------------------------------------
>                 Key: HDFS-6705
>                 URL: https://issues.apache.org/jira/browse/HDFS-6705
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: namenode, security
>    Affects Versions: 3.0.0
>            Reporter: Charles Lamb
>            Assignee: Charles Lamb
>         Attachments: HDFS-6705.001.patch
> There needs to be an xattr that specifies that the HDFS admin can not access a file.
This is needed for m/r delegation tokens and data at rest encryption.

This message was sent by Atlassian JIRA

View raw message