Bug 1397252

+++ This bug was initially created as a clone of Bug #1392772 +++

+++ This bug was initially created as a clone of Bug #1391808 +++

Description of problem:
=======================
Using pjdfstest filesystem test suite when created a directory with its access and group permissions below warning messages are seen in FUSE and brick logs,

FUSE logs:
=========
[2016-11-03 10:27:10.948703] W [MSGID: 114031] [client-rpc-fops.c:1031:client3_3_setxattr_cbk] 2-distrep-client-7: remote operation failed [Permission denied]
Brick logs:
===========
[2016-11-03 10:26:56.494850] I [MSGID: 115060] [server-rpc-fops.c:865:_gf_server_log_setxattr_failure] 0-distrep-server: 2505: SETXATTR /dir/dir (ef02bc07-ba92-4e40-bd81-1661b0d41582) ==> trusted.glusterfs.dht
[2016-11-03 10:26:56.495084] I [MSGID: 115060] [server-rpc-fops.c:893:server_setxattr_cbk] 0-distrep-server: Permission denied

Version-Release number of selected component (if applicable):
3.8.4-3.el7rhgs.x86_64

How reproducible:
=================
Always

Steps to Reproduce:
===================
1) Create  a distributed replica volume and start it.
2) FUSE mount the volume on a client.
3) As a root user run the below command from mount point to clone pjd-fstest
	git://git.code.sf.net/p/ntfs-3g/pjd-fstest
4) Run the below commands
   a) ./pjd-fstest/fstest mkdir dir 0777
   b) ./pjd-fstest/fstest chown dir 65534 65534
   c) ./pjd-fstest/fstest -u 65534 -g 65534 create dir/file 0464
   d) ./pjd-fstest/fstest -u 65534 -g 65534 mkdir dir/dir 0464

When executed 4d the permission denied messages are seen in FUSE and brick logs.

Also, after issuing 'ls' on the parent directory created in 4a, we are seeing the below info message in FUSE logs,

[2016-11-03 10:30:54.284974] I [MSGID: 109063] [dht-layout.c:713:dht_layout_normalize] 2-distrep-dht: Found anomalies in /dir/dir (gfid = ef02bc07-ba92-4e40-bd81-1661b0d41582). Holes=1 overlaps=0

But, I did not see any holes on the directory created in 4d,

[root@server7 ~]# getfattr -d -e hex -m trusted.glusterfs.dht /bricks/brick*/*/dir/dir
getfattr: Removing leading '/' from absolute path names
# file: bricks/brick0/b0/dir/dir
trusted.glusterfs.dht=0x0000000100000000d5555552ffffffff

# file: bricks/brick1/b1/dir/dir
trusted.glusterfs.dht=0x00000001000000002aaaaaaa55555553

# file: bricks/brick2/b2/dir/dir
trusted.glusterfs.dht=0x00000001000000007ffffffeaaaaaaa7

[root@server141 ~]# getfattr -d -e hex -m trusted.glusterfs.dht /bricks/brick*/*/dir/dir
getfattr: Removing leading '/' from absolute path names
# file: bricks/brick0/b0/dir/dir
trusted.glusterfs.dht=0x0000000100000000000000002aaaaaa9

# file: bricks/brick1/b1/dir/dir
trusted.glusterfs.dht=0x0000000100000000555555547ffffffd

# file: bricks/brick2/b2/dir/dir
trusted.glusterfs.dht=0x0000000100000000aaaaaaa8d5555551


Actual results:
===============
[setxattr_cbk] "Permission denied" warning messages are seen in logs while running pjd-fstest suite

Expected results:
=================
Permission denied messages should not be seen.



--- Additional comment from Shyamsundar on 2016-11-07 13:44:52 EST ---

(In reply to Prasad Desala from comment #0)
> Also, after issuing 'ls' on the parent directory created in 4a, we are
> seeing the below info message in FUSE logs,
> 
> [2016-11-03 10:30:54.284974] I [MSGID: 109063]
> [dht-layout.c:713:dht_layout_normalize] 2-distrep-dht: Found anomalies in
> /dir/dir (gfid = ef02bc07-ba92-4e40-bd81-1661b0d41582). Holes=1 overlaps=0
> 
> But, I did not see any holes on the directory created in 4d,
> 
> [root@server7 ~]# getfattr -d -e hex -m trusted.glusterfs.dht
> /bricks/brick*/*/dir/dir

This is because the directory was created without the DHT xattrs and as the client detected an anomaly in the directory, it healed it as root, and so the layout *finally* on disk was not empty.

Before doing the ls on ./dir/ if you would take a look at the xattrs on the brick, you would find them missing.

--- Additional comment from Shyamsundar on 2016-11-07 14:07:13 EST ---

I was able to replicate the problem based on the steps given (thanks).

What occurs here is as follows,
- DHT sends the mkdir call to the bricks
- Post the mkdir, it sends the setxattr call to set the layout xattrs for these directories

The failure is noted in the bricks and on the FUSE client logs, as the setxattr fails.

The setxattr fails, due to the permissions on the directory being 0464 (set during the mkdir as requested by the call), where user has only READ permissions, and posix_acl xlator checks that the UID of the setxattr caller (internal DHT request above) is the same as the UID of the directory inode, and hence evaluates permissions based on this, which evaluates to READ ONLY and denies the setxattr from proceeding.

Code reference:
  - https://github.com/gluster/glusterfs/blob/master/xlators/system/posix-acl/src/posix-acl.c#L2088 posix_acl_setxattr -> calls setxattr_scrutiny
  - https://github.com/gluster/glusterfs/blob/master/xlators/system/posix-acl/src/posix-acl.c#L1920 setxattr_scrutiny -> acl_permits calls
  - acl_permits: Has the POSIX_ACL_USER_OBJ in its list first, and checks the UID equality of the caller to the inode (which matches) and hence uses permissions of the user to proceed doing a perm_check

The downside of all this, is that the DHT layout on this directory is not set, only on a subsequent layout anomaly detection by DHT is this corrected, as anomaly correction is an internal operation that uses UID/GID as root/root and the ACL xlator passes the request without the checks.

General discussion:
The manner in which gluster ACL xlator checks the permissions (user first and then group etc.) is correct as per standards, the reference would be, https://www.gnu.org/software/libc/manual/html_node/Access-Permission.html#Access-Permission

Further on checking the kernel code, the same rules apply there as well, and a directory created with READ for user, subsequently fails with EPERM for any setfattr calls. Kernel code reference: http://lxr.free-electrons.com/source/fs/posix_acl.c#L346

So basically, the implementation in out ACL xlator seems to be right. I.e just because group bits provide WRITE permissions, it is not allowed as the UID of the directory to perform any WRITE operations.

What this means:
So currently other than the logs, there is no real bug to start with. But, the finding is interesting, and we could make the setxattr of DHT xattrs post mkdir an internal operation to be done as root/root (just like healing an anomaly). That would fix the logging problem as well. Is it a security violation is something that is needed to be considered.

For example xattrop that is driven by the client (and is actually a setxattr call in the end) has no ACL xlator checks against that FOP (and as a result does not suffer this problem).

Regression notes:
This should not be a regression, and should be present in the prior releases as well, just a heads up!

--- Additional comment from Worker Ant on 2016-11-08 03:58:55 EST ---

REVIEW: http://review.gluster.org/15794 (cluster/dht Set layout after mkdir as root) posted (#1) for review on master by N Balachandran (nbalacha)

--- Additional comment from Worker Ant on 2016-11-21 09:40:19 EST ---

COMMIT: http://review.gluster.org/15794 committed in master by Jeff Darcy (jdarcy) 
------
commit 3e405b546e8b9fe15ae477613474e9cd2d2df4e7
Author: N Balachandran <nbalacha>
Date:   Tue Nov 8 14:10:49 2016 +0530

    cluster/dht Set layout after mkdir as root
    
    DHT does not set the layout for newly created
    directories as root. This causes EPERM failures
    when a non-root user with insufficient permissions
    creates directories.
    
    credit: srangana for RCA
    
    Change-Id: Ia646e41665ce172c43c5f01d2707455e8eb374ed
    BUG: 1392772
    Signed-off-by: N Balachandran <nbalacha>
    Reviewed-on: http://review.gluster.org/15794
    Reviewed-by: Susant Palai <spalai>
    NetBSD-regression: NetBSD Build System <jenkins.org>
    CentOS-regression: Gluster Build System <jenkins.org>
    Smoke: Gluster Build System <jenkins.org>
    Reviewed-by: Raghavendra G <rgowdapp>
    Reviewed-by: Jeff Darcy <jdarcy>