Re: [PATCH 1/1] cr: use a new capability to authorize c/r

["Serge E. Hallyn" <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>]

Quoting Alexey Dobriyan (adobriyan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org):
> On Tue, May 12, 2009 at 10:07:13AM -0500, Serge E. Hallyn wrote:
> > do you object to this patch?  The idea is to not give away any
> > privilege not otherwise needed.
> 
> > --- a/checkpoint/sys.c
> > +++ b/checkpoint/sys.c
> > @@ -281,7 +281,7 @@ asmlinkage long sys_checkpoint(pid_t pid, int fd, unsigned long flags)
> >  	if (flags & ~CKPT_USER_FLAGS)
> >  		return -EINVAL;
> >  
> > -	if (!ckpt_unpriv_allowed && !capable(CAP_SYS_ADMIN))
> > +	if (!ckpt_unpriv_allowed && !capable(CAP_CHECKPOINT_RESTART))
> >  		return -EPERM;
> >  
> >  	if (pid == 0)
> > @@ -318,7 +318,7 @@ asmlinkage long sys_restart(int crid, int fd, unsigned long flags)
> >  	if (flags)
> >  		return -EINVAL;
> >  
> > -	if (!ckpt_unpriv_allowed && !capable(CAP_SYS_ADMIN))
> > +	if (!ckpt_unpriv_allowed && !capable(CAP_CHECKPOINT_RESTART))
> >  		return -EPERM;
> >  
> >  	/* FIXME: for now, we use 'crid' as a pid */
> > diff --git a/include/linux/capability.h b/include/linux/capability.h
> > index 572b5a0..a593391 100644
> > --- a/include/linux/capability.h
> > +++ b/include/linux/capability.h
> > @@ -357,7 +357,9 @@ struct cpu_vfs_cap_data {
> >  
> >  #define CAP_MAC_ADMIN        33
> >  
> > -#define CAP_LAST_CAP         CAP_MAC_ADMIN
> > +#define CAP_CHECKPOINT_RESTART      34
> 
> I don't know if this is really needed.
> 
> If you allow restart(2) for everyone, you can during struct cred
> restoration check if, say, capabilities coming from image are more
> strict than capabilities of restorer, that aux groups are a subset of
> aux groups of restorer and so on.

Yes, I do all of that in the task credentials restore patchset I sent
yesterday.

> You still need these checks, otherwise CAP_CHECKPOINT_RESTART is much
> more powerful than it suggests.

Absolutely.  This patch I sent here is for Oren's checkpoint/restart
tree, which already makes sure (well aims to make sure) that at every
step of the way the task doing sys_restart() has the appopriate
privilege.

> I'm going to try and see how hard will it be.

-serge