Comment 7 for bug 1534961

Revision history for this message
Seth Forshee (sforshee) wrote : Re: [Bug 1534961] Re: insecure overlayfs xattrs handling in copy_up

On Wed, Feb 17, 2016 at 06:20:57AM -0000, J. R. Okajima wrote:
> FYI
>
> The security bug hunter "halfdog" kindly told me that this problem can reproduce with AUFS.
> I've confirmed and fixed. Here is aufs's approach hoping with a little help for overlayfs.
>
> In copy-up, the internal sequence is
> - create an entry on the upper writable layer.
> - copy the all attributes from the inode on the lower readonly branch.
>
> The essential fix is inserting vfs_removexattr(XATTR_NAME_POSIX_ACL_ACCESS) between them.
> For dirs, XATTR_NAME_POSIX_ACL_DEFAULT should be removed too. And then copy the all attributes including XATTRs.
> But removing all ACL_ACCESS may cause another problem since some fs (for example, NFS) may want ACL which is equivalent to the permission bits. So just after removing XATTR, posix_acl_chmod() should be called.

Does your AUFS fix do this universally? I see no reason to remove the
xattrs if the mount was done by real root.

In Ubuntu an unprivileged user is allowed to mount overlayfs in a user
namespace, and that's where we run into problems as it allows a user to
create files with privileged xattrs or sxid to another user where they
could not have done so otherwise. These patches make it so that the copy
up will fail if the mounter could not have otherwise created the file in
upperdir with those attrs/xattrs.

Your approach is less extreme, but I have a few concerns. First, it
works for xattrs but not for sxid or seemingly for file capabilities
(though I assume it could be extended to do so). Second (and somewhat
related), there are more privileged xattrs than just ACLs (e.g.
trusted.* or security.* for some LSMs) and so it would appear that it
should be extended to filter those as well, but that seems to be
redundant with other checks in the kernel. And finally, this seems to be
inconsistent with standard behavior where setxid/setcap is cleared on
write and not on open.