linux-kernel - Re: [PATCH v27 03/21] vfs: Add MAY_DELETE_SELF and MAY_DELETE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 2 Dec 2016 10:57:42 +0100
From:   Miklos Szeredi <miklos@...redi.hu>
To:     Andreas Gruenbacher <agruenba@...hat.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Christoph Hellwig <hch@...radead.org>,
        "Theodore Ts'o" <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        "J. Bruce Fields" <bfields@...ldses.org>,
        Jeff Layton <jlayton@...chiereds.net>,
        Trond Myklebust <trond.myklebust@...marydata.com>,
        Anna Schumaker <anna.schumaker@...app.com>,
        Dave Chinner <david@...morbit.com>, linux-ext4@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-nfs@...r.kernel.org, linux-cifs@...r.kernel.org,
        linux-api@...r.kernel.org
Subject: Re: [PATCH v27 03/21] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD
 permission flags

On Tue, Oct 11, 2016 at 2:50 PM, Andreas Gruenbacher
<agruenba@...hat.com> wrote:
> Normally, deleting a file requires MAY_WRITE access to the parent
> directory.  With richacls, a file may be deleted with MAY_DELETE_CHILD access
> to the parent directory or with MAY_DELETE_SELF access to the file.
>
> To support that, pass the MAY_DELETE_CHILD mask flag to inode_permission()
> when checking for delete access inside a directory, and MAY_DELETE_SELF
> when checking for delete access to a file itself.
>
> The MAY_DELETE_SELF permission overrides the sticky directory check.

And MAY_DELETE_SELF seems totally inappropriate to any kind of rename,
since from the point of view of the inode we are not doing anything at
all.  The modifications are all in the parent(s), and that's where the
permission checks need to be.

> @@ -2780,14 +2780,20 @@ static int may_delete_or_replace(struct inode *dir, struct dentry *victim,
>         BUG_ON(victim->d_parent->d_inode != dir);
>         audit_inode_child(dir, victim, AUDIT_TYPE_CHILD_DELETE);
>
> -       error = inode_permission(dir, mask);
> +       error = inode_permission(dir, mask | MAY_WRITE | MAY_DELETE_CHILD);
> +       if (!error && check_sticky(dir, inode))
> +               error = -EPERM;
> +       if (error && IS_RICHACL(inode) &&
> +           inode_permission(inode, MAY_DELETE_SELF) == 0 &&
> +           inode_permission(dir, mask) == 0)
> +               error = 0;

Why is MAY_WRITE missing here?  Everything not aware of
MAY_DELETE_SELF (e.g. LSMs) will still need MAY_WRITE otherwise this
is going to be a loophole.

Thanks,
Miklos