Bug 1392772
| Summary: | [setxattr_cbk] "Permission denied" warning messages are seen in logs while running pjd-fstest suite | |||
|---|---|---|---|---|
| Product: | [Community] GlusterFS | Reporter: | Nithya Balachandran <nbalacha> | |
| Component: | distribute | Assignee: | Nithya Balachandran <nbalacha> | |
| Status: | CLOSED CURRENTRELEASE | QA Contact: | ||
| Severity: | medium | Docs Contact: | ||
| Priority: | unspecified | |||
| Version: | mainline | CC: | bugs, rgowdapp, rhs-bugs, srangana, storage-qa-internal, tdesala | |
| Target Milestone: | --- | |||
| Target Release: | --- | |||
| Hardware: | Unspecified | |||
| OS: | Unspecified | |||
| Whiteboard: | ||||
| Fixed In Version: | glusterfs-3.10.0 | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | ||
| Clone Of: | 1391808 | |||
| : | 1397252 (view as bug list) | Environment: | ||
| Last Closed: | 2017-03-06 17:32:36 UTC | Type: | Bug | |
| Regression: | --- | Mount Type: | --- | |
| Documentation: | --- | CRM: | ||
| Verified Versions: | Category: | --- | ||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | ||
| Cloudforms Team: | --- | Target Upstream Version: | ||
| Embargoed: | ||||
| Bug Depends On: | ||||
| Bug Blocks: | 1391808, 1397252 | |||
|
Description
Nithya Balachandran
2016-11-08 08:38:36 UTC
REVIEW: http://review.gluster.org/15794 (cluster/dht Set layout after mkdir as root) posted (#1) for review on master by N Balachandran (nbalacha) 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> Seems like more investigation is needed on the RCA. Posting the email conversation we had. > Sent: Tuesday, November 22, 2016 9:22:09 AM > Subject: Re: On patch #15794 > > On 22 November 2016 at 09:19, Raghavendra Gowdappa <rgowdapp> > wrote: > > > > > > > > Sent: Monday, November 21, 2016 8:36:49 PM > > > Subject: On patch #15794 > > > > > > Hi Nithya/Shyam, > > > > > > While thinking about patch [1], it occurred to me that if a user has > > > permission to create directory, same credentials should be good enough to > > > set layout xattrs too. However, I realized that layout xattrs start with > > > "trusted." and only superuser can read/set them. > > > > Going by the above logic, we should've superuser access for setting all > > other gluster metadata too (like gfid, afr changelog etc). But we don't > > sudo to superuser while doing so. Also, from man 7 xattr, > > > > <man 7 xattr> > > Trusted extended attributes > > Trusted extended attributes are visible and accessible only to > > processes that have the CAP_SYS_ADMIN capability. Attributes in > > this > > class are used to implement mechanisms in user space (i.e., outside > > the kernel) which keep information in extended attributes to which > > ordinary processes should not have access. > > </man> > > > > So, it seems that any process with CAP_SYS_ADMIN capability can set the > > xattrs. It _seems_ (I am not confirming) that brick process retains that > > capability even though we do setfsuid/gid (frame->root->uid/gid) (uid/gid > > of process doing fops like mkdir etc, not the uid/gid of brick process). If > > this indeed is true, we are missing something as to why mkdir failed with > > EPERM. > > > > > mkdir is created with the permissions specified in the command. So I guess > the user would no longer have permissions to modify it (say set an xattr) > if the op comes in as the same uid. If that is indeed the case, its puzzling that the test I posted in my first mail passed. I've a feeling that we are missing out something and more investigation is needed here. > > Ideally, all internal ops should be marked internal_fop and no security > checks should hold in that case. The security implications of this need to > be determined. > > > > > As to why lookup path in dht does SUDO but not ops like mkdir/rmdir, it > > might be because lookup can be executed by a process with just read > > permission or process with no permission to access the directory. However, > > that might not be the case with mkdir/rmdir codepath. > > > > > However on testing I was > > > able to create a directory as non-root user. Note that the exported > > > directory is owned by "raghu:users". > > > > > > > > > [raghu@unused glusterfs]$ mkdir dir > > > > > > [raghu@unused glusterfs]$ whoami > > > raghu > > > > > > [raghu@unused glusterfs]$ getfattr -e hex -d -m. /home/export/newptop > > > getfattr: Removing leading '/' from absolute path names > > > # file: home/export/newptop > > > security.selinux=0x756e636f6e66696e65645f753a6f > > 626a6563745f723a686f6d655f726f6f745f743a733000 > > > > > > [raghu@unused glusterfs]$ getfattr -e hex -d -m. > > /home/export/newptop/dir > > > getfattr: Removing leading '/' from absolute path names > > > # file: home/export/newptop/dir > > > security.selinux=0x756e636f6e66696e65645f753a6f > > 626a6563745f723a686f6d655f726f6f745f743a733000 > > > > > > [raghu@unused glusterfs]$ sudo getfattr -e hex -d -m. > > > /home/export/newptop/dir > > > [sudo] password for raghu: > > > getfattr: Removing leading '/' from absolute path names > > > # file: home/export/newptop/dir > > > security.selinux=0x756e636f6e66696e65645f753a6f > > 626a6563745f723a686f6d655f726f6f745f743a733000 > > > trusted.gfid=0x17f753292e4b46c58b736d3a975af0dc > > > trusted.glusterfs.dht=0x000000010000000000000000ffffffff > > > > > > [raghu@unused glusterfs]$ cd .. > > > > > > [raghu@unused mnt]$ cd /home/ > > > > > > [raghu@unused home]$ cd > > > > > > [raghu@unused ~]$ mkdir tmp > > > > > > [raghu@unused ~]$ setfattr -n trusted.glusterfs.dht -v > > > 0x000000010000000000000000ffffffff tmp > > > setfattr: tmp: Operation not permitted > > > > > > [raghu@unused ~]$ ls -ld tmp > > > drwxrwxr-x. 2 raghu raghu 4096 Nov 21 20:25 tmp > > > > > > Note that last setfattr on backend fs failed with EPERM. > > > > > > Now the puzzling thing is how mkdir by a non-root user succeeded on > > glusterfs > > > with appropriate xattrs set. Any explanations? > > > > > > [1] http://review.gluster.org/#/c/15794/ > > > > > > regards, > > > Raghavendra > > > Except for how mkdir by non-root user succeeded in my test above, other things can be answered. * gfid is never set as a separate setxattr call sent through posix-acl. All posix-acl sees is a create/mkdir/symlink/mknod operation. * xattrop is not implemented by posix-acl (rightly so, as it is an internal operation). So, doesn't affect afr changelog etc. * however it will affect any metadata setting done through setxattr (dht layouts, marker dirty flags etc). Based on Nithya's analysis, I agree this patch is needed and solves the issue present here (though there are some open questions which are not directly related to this bz). (In reply to Raghavendra G from comment #4) > Except for how mkdir by non-root user succeeded in my test above, other > things can be answered. <raghug> I think the bug was a special case <raghug> look at this comment <raghug> 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. <nbalacha> the bug was using the mode to create the dir <nbalacha> yes <nbalacha> the user creates a dir with read perms <raghug> that explains <raghug> thanks <nbalacha> which is why I asked if you had used the mode :) <raghug> I didn't get that. Sorry :) <nbalacha> yesterday - I asked if you had used -m in the mkdir <nbalacha> because the test on which the bug was reported does <raghug> no.. I meant I didn't realize the importance of mode :) <nbalacha> so the mkdir ops in posix uses that and the perms are changed immediately <nbalacha> ah ok <nbalacha> so posix acl check fails because the layout set is a write <nbalacha> and there is no write perm <nbalacha> ideally we need to relook at the whole thing <nbalacha> this is a hacky way to fix things <raghug> the whole root of confusion was, I was wondering if the user can create a directory he'll have write permissions <nbalacha> we need special internal ops and a way to bypass checks for those <raghug> didn't realize that mode alters that Patch has been merged in master. Moving this to Modified. This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.10.0, please open a new bug report. glusterfs-3.10.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution. [1] http://lists.gluster.org/pipermail/gluster-users/2017-February/030119.html [2] https://www.gluster.org/pipermail/gluster-users/ |