[
https://issues.apache.org/jira/browse/HDFS-6705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14125209#comment-14125209
]
Andrew Wang commented on HDFS-6705:
-----------------------------------
Hi Charles, thanks for working on this, a few review comments:
* Why did the exception messages change in TestDistributedFileSystem and
SymlinkBaseTest? This is mildly incompatible, so I'd like to understand why
it's necessary.
* We're still doing another path resolution to do checkUnreadableBySuperuser.
Can we try to reuse the inode from the IIP just below? This would also let us
avoid throwing IOException in the check method.
* Consider folding FSPermissionChecker#checkUnreadableBySuperuser into the FSN
method, it's pretty simple.
* FSN#checkXAttrChangeAccess has unrelated change?
* Indentation of FSN#checkUnreadableBySuperuser is off
Doc:
* Extra whitespace change
* Text is still kinda verbose and still mentions preventing read access to
other xattrs and writing to the file. I'd prefer something like:
{noformat}
The <<<security>>> namespace is reserved for internal HDFS use. This
namespace is generally not accessible through userspace methods. One particular
use of <<<security>>> is the <<<security.hdfs.unreadable.by.superuser>>>
extended attribute. This xattr can only be set on files, and it will prevent
the superuser from reading the file's contents. The superuser can still read
and modify file metadata, such as the owner, permissions, etc. This xattr can
be set and accessed by any user, assuming normal filesystem permissions. This
xattr is also write-once, and cannot be removed once set. This xattr does not
allow a value to be set.
{noformat}
* Unrelated changes in TestXAttrCLI, TestSymlinkHdfsFileSystem
FSXattrBaseTest
* High-level comment, I'd like to pare down the new tests to focus on this new
functionality
* I still see references to MAX_XATTR_SIZE which should be unrelated here. It
also involves an extra mini cluster stop and start.
* I'd like to avoid doing extra minicluster start/stops to test persistence
too. It'd be better to add some security xattrs to the existing restart tests
instead.
* The "vanilla xattrs" test, it doesn't have a matching call to {{fail}}. I
don't think this needs to be tested anyway, since UBS doesn't affect xattr
operations.
* verifyFileAccess also still has testing for append and create, which isn't
valid anymore.
* I see a hardcoded "security.hdfs.unreadable.by.superuser" still, sub in the
string constant instead?
* Is RemoteException is being thrown by DistributedFileSystem for the new
AccessControlException? I see it being unwrapped in DFSClient, so I would
expect to see an ACE here.
Tests:
* Mention of "special" xattr is non-specific, could we say "unreadable by
superuser" or "UBS" or something instead?
> 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, HDFS-6705.002.patch,
> HDFS-6705.003.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
(v6.3.4#6332)
