[SRU][UNSTABLE/EOAN/FOCAL][PATCH] UBUNTU: SAUCE: shiftfs: let userns root destroy subvolumes from other users

[Seth Forshee]

On Mon, Jun 01, 2020 at 08:53:36AM -0500, Seth Forshee wrote:
> On Wed, May 20, 2020 at 01:44:27PM +0200, Christian Brauner wrote:
> > BugLink: https://bugs.launchpad.net/bugs/1879688
> > 
> > Stéphane reported a bug found during NorthSec that makes heavy use of
> > shiftfs. When a subvolume or snapshot is created as userns root in the
> > container and then chowned to another user a delete as the root user
> > will fail. The reason for this is that we drop all capabilities as a
> > safety measure before calling btrfs ioctls. The only workable fix I
> > could think of is to retain the CAP_DAC_OVERRIDE capability for the
> > BTRFS_IOC_SNAP_DESTROY ioctl. All other solutions would be way more
> > invasive.
> 
> I think this might allow more access than what you're intending. There
> doesn't seem to be anything which restricts it to the userns root, so
> wouldn't it allow non-root in the ns to delete snapshots belonging to
> other users in the userns?

Sorry ... yes it is restricted to userns root, it's right there. Let me
look at this a little deeper ...

> 
> Seth
> 
> > 
> > Cc: Seth Forshee <seth.forshee at canonical.com>
> > Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
> > ---
> >  fs/shiftfs.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/shiftfs.c b/fs/shiftfs.c
> > index 652fb67b0e0b..2636871f05f4 100644
> > --- a/fs/shiftfs.c
> > +++ b/fs/shiftfs.c
> > @@ -1343,7 +1343,7 @@ static int shiftfs_fadvise(struct file *file, loff_t offset, loff_t len,
> >  	return ret;
> >  }
> >  
> > -static int shiftfs_override_ioctl_creds(const struct super_block *sb,
> > +static int shiftfs_override_ioctl_creds(int cmd, const struct super_block *sb,
> >  					const struct cred **oldcred,
> >  					struct cred **newcred)
> >  {
> > @@ -1368,6 +1368,16 @@ static int shiftfs_override_ioctl_creds(const struct super_block *sb,
> >  	cap_clear((*newcred)->cap_inheritable);
> >  	cap_clear((*newcred)->cap_permitted);
> >  
> > +	if (cmd == BTRFS_IOC_SNAP_DESTROY) {
> > +		kuid_t kuid_root = make_kuid(sb->s_user_ns, 0);
> > +		/*
> > +		 * Allow the root user in the container to remove subvolumes
> > +		 * from other users.
> > +		 */
> > +		if (uid_valid(kuid_root) && uid_eq(fsuid, kuid_root))
> > +			cap_raise((*newcred)->cap_effective, CAP_DAC_OVERRIDE);
> > +	}
> > +
> >  	put_cred(override_creds(*newcred));
> >  	return 0;
> >  }
> > @@ -1500,7 +1510,7 @@ static long shiftfs_real_ioctl(struct file *file, unsigned int cmd,
> >  	if (ret)
> >  		goto out_restore;
> >  
> > -	ret = shiftfs_override_ioctl_creds(sb, &oldcred, &newcred);
> > +	ret = shiftfs_override_ioctl_creds(cmd, sb, &oldcred, &newcred);
> >  	if (ret)
> >  		goto out_fdput;
> >  
> > -- 
> > 2.26.2
> >