On Mon, Jun 1, 2020 at 9:42 PM Stephen Smalley
<stephen.smalley.work@xxxxxxxxx> wrote:
>
> On Mon, Jun 1, 2020 at 3:29 AM Chirantan Ekbote <chirantan@xxxxxxxxxxxx> wrote:
> >
> > Normally a process needs CAP_SYS_ADMIN in the namespace that mounted a
> > particular filesystem in order to set a security xattr. However, this
> > restriction is relaxed for the security.selinux xattr: the file owner
> > or a process with CAP_FOWNER in its namespace may set this attribute.
> >
> > Apply this relaxed restriction to the security.sehash xattr as well.
> > Since this xattr is mainly a performance optimization when labeling
> > files recursively it shouldn't have stricter requirements than setting
> > the selinux xattr in the first place.
>
> First, setting either security.<non-selinux> or security.selinux has
> an additional MAC check beyond the DAC/capability check; in the former
> case there is the FILE__SETATTR check and in the latter there are the
> FILE__RELABELFROM/TO checks. We need to preserve some kind of SELinux
> permission check here.
>
So I understand correctly, what you're asking for is to change this section:
if (is_sehash)
return 0;
to this instead:
if (is_sehash)
return dentry_has_perm(current_cred(), dentry, FILE__SETATTR);
Is that correct?
> Second, security.sehash logic in userspace was introduced by Android
> in its libselinux fork and then copied in upstream logic. I'm not
> sure Android wants to relax the current requirement for CAP_SYS_ADMIN
> - I have copied them above. A possible concern is that an
> unprivileged process could disable the relabeling of a part of the
> tree that it owns upon an upgrade, which could have unexpected
> consequences.
That's a good point. Is this not an issue for the selinux xattr
because the selinux check could prevent a process from changing the
label of a file it owns?
The background for this patch is that I have a fuse server that runs
in a user namespace. It runs as root in that namespace and keeps all
the file system caps so that it can set selinux xattrs. However, it
cannot set the sehash xattr as that needs CAP_SYS_ADMIN in the parent
namespace. Looking at the code I thought that might have just been an
oversight but if it's intentional then do you have any suggestions for
how to make this work? I'd rather not weaken the sandbox for this
process just so that it can set this one xattr.
Thanks,
Chirantan
