[ 
https://issues.apache.org/jira/browse/HDFS-8748?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Chris Nauroth updated HDFS-8748:
--------------------------------
    Assignee: Chris Nauroth

Hello [~scott_o].  Thank you for reporting this.  I can confirm that this is a 
bug.  (The design doc is correct, and the current code has a bug.)

To confirm this, I ran a test case on an ext4 file system with ACLs enabled.  
See below for a transcript of my test case.  Executing a file requires both 
read and execute permissions (r-x).  In my test case, I defined the read 
permission on one named group entry and the execute permission on a second 
named group entry.  My user was able to execute the file.  This proves that on 
ext4, permissions can be defined on separate named group ACL entries, and the 
permission checks will treat the union of those entries as the effective 
permissions.

Scott, are you interested in coding a patch?  If not, then I'll assign this to 
myself for the fix.

{code}
> whoami
cnauroth

> groups
cnauroth sudo testgroup1

> getfacl test_HDFS-8748 
# file: test_HDFS-8748
# owner: root
# group: root
user::rwx
group::---
group:sudo:r--
group:testgroup1:--x
mask::r-x
other::---

> ./test_HDFS-8748 

> echo $?
0

> sudo setfacl -m group:sudo:r--,group:testgroup1:--- test_HDFS-8748 

> ./test_HDFS-8748 
-bash: ./test_HDFS-8748: Permission denied

> echo $?
126

> sudo setfacl -m group:sudo:---,group:testgroup1:--x test_HDFS-8748 

> ./test_HDFS-8748 
bash: ./test_HDFS-8748: Permission denied

> echo $?
126

> sudo setfacl -m group:sudo:r--,group:testgroup1:--x test_HDFS-8748 

> ./test_HDFS-8748 

> echo $?
0
{code}


> ACL permission check does not union groups to determine effective permissions
> -----------------------------------------------------------------------------
>
>                 Key: HDFS-8748
>                 URL: https://issues.apache.org/jira/browse/HDFS-8748
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: HDFS
>    Affects Versions: 2.7.1
>            Reporter: Scott Opell
>            Assignee: Chris Nauroth
>              Labels: acl, permission
>
> In the ACL permission checking routine, the implemented named group section 
> does not match the design document.
> In the design document, its shown in the pseudo-code that if the requester is 
> not the owner or a named user, then the applicable groups are unioned 
> together to form effective permissions for the requester.
> Instead, the current implementation will search for the first group that 
> grants access and will use that. It will not union the permissions together.
> Here is the design document's description of the desired behavior
> {quote}
> If the user is a member of the file's group or at least one group for which 
> there is a
> named group entry in the ACL, then effective permissions are calculated from 
> groups.
> This is the union of the file group permissions (if the user is a member of 
> the file group)
> and all named group entries matching the user's groups. For example, consider 
> a user
> that is a member of 2 groups: sales and execs. The user is not the file 
> owner, and the
> ACL contains no named user entries. The ACL contains named group entries for 
> both
> groups as follows: group:sales:r­­\-\-, group:execs:\-­w\-­. In this case, 
> the user's effective
> permissions are rw­-.
> {quote}
>  
> ??https://issues.apache.org/jira/secure/attachment/12627729/HDFS-ACLs-Design-3.pdf
>  page 10??
> The design document's algorithm matches that description:
> *Design Document Algorithm*
> {code:title=DesignDocument}
> if (user == fileOwner) {
>     effectivePermissions = aclEntries.getOwnerPermissions()
> } else if (user ∈ aclEntries.getNamedUsers()) {
>     effectivePermissions = aclEntries.getNamedUserPermissions(user)
> } else if (userGroupsInAcl != ∅) {
>     effectivePermissions = ∅
>     if (fileGroup ∈ userGroupsInAcl) {
>         effectivePermissions = effectivePermissions ∪
>         aclEntries.getGroupPermissions()
>     }
>     for ({group | group ∈ userGroupsInAcl}) {
>         effectivePermissions = effectivePermissions ∪
>         aclEntries.getNamedGroupPermissions(group)
>     }
> } else {
>     effectivePermissions = aclEntries.getOthersPermissions()
> }
> {code}
> ??https://issues.apache.org/jira/secure/attachment/12627729/HDFS-ACLs-Design-3.pdf
>  page 9??
> The current implementation does NOT match the description.
> *Current Trunk*
> {code:title=FSPermissionChecker.java}
>     // Use owner entry from permission bits if user is owner.
>     if (getUser().equals(inode.getUserName())) {
>       if (mode.getUserAction().implies(access)) {
>         return;
>       }
>       foundMatch = true;
>     }
>     // Check named user and group entries if user was not denied by owner 
> entry.
>     if (!foundMatch) {
>       for (int pos = 0, entry; pos < aclFeature.getEntriesSize(); pos++) {
>         entry = aclFeature.getEntryAt(pos);
>         if (AclEntryStatusFormat.getScope(entry) == AclEntryScope.DEFAULT) {
>           break;
>         }
>         AclEntryType type = AclEntryStatusFormat.getType(entry);
>         String name = AclEntryStatusFormat.getName(entry);
>         if (type == AclEntryType.USER) {
>           // Use named user entry with mask from permission bits applied if 
> user
>           // matches name.
>           if (getUser().equals(name)) {
>             FsAction masked = AclEntryStatusFormat.getPermission(entry).and(
>                 mode.getGroupAction());
>             if (masked.implies(access)) {
>               return;
>             }
>             foundMatch = true;
>             break;
>           }
>         } else if (type == AclEntryType.GROUP) {
>           // Use group entry (unnamed or named) with mask from permission bits
>           // applied if user is a member and entry grants access.  If user is 
> a
>           // member of multiple groups that have entries that grant access, 
> then
>           // it doesn't matter which is chosen, so exit early after first 
> match.
>           String group = name == null ? inode.getGroupName() : name;
>           if (getGroups().contains(group)) {
>             FsAction masked = AclEntryStatusFormat.getPermission(entry).and(
>                 mode.getGroupAction());
>             if (masked.implies(access)) {
>               return;
>             }
>             foundMatch = true;
>           }
>         }
>       }
>     }
> {code}
> As seen in the GROUP section, the permissions check will succeed if and only 
> if a single group (either owning group or named group) has all of the 
> requested permissions. The permissions check should instead succeed if the 
> requested permissions can be obtained by unioning all of the groups 
> permissions.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to