Re: [PATCH] sunrpc: Use current_real_cred() when looking up rpc credentials

[Seth Forshee <seth.forshee@canonical.com>]

On Thu, Dec 15, 2016 at 11:01:41PM +0000, Trond Myklebust wrote:
> On Thu, 2016-12-15 at 11:13 -0600, Seth Forshee wrote:
> > Since 4.8 follow_automount() overrides the credentials with
> > &init_cred before calling d_automount(). When
> > rpcauth_lookupcred() is called in this context it is now using
> > fs[ug]id from the override creds instead of from the user's
> > creds, which can cause authentication to fail. To fix this, take
> > the ids from current_real_cred() instead.
> > 
> > Cc: stable@vger.kernel.org # v4.8+
> > CC: Eric W. Biederman <ebiederm@xmission.com>
> > Fixes: aeaa4a79ff6a ("fs: Call d_automount with the filesystems
> > creds")
> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> > ---
> >  net/sunrpc/auth.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
> > index 2bff63a73cf8..e6197b2bda86 100644
> > --- a/net/sunrpc/auth.c
> > +++ b/net/sunrpc/auth.c
> > @@ -622,7 +622,7 @@ rpcauth_lookupcred(struct rpc_auth *auth, int
> > flags)
> >  {
> >  	struct auth_cred acred;
> >  	struct rpc_cred *ret;
> > -	const struct cred *cred = current_cred();
> > +	const struct cred *cred = current_real_cred();
> >  
> >  	dprintk("RPC:       looking up %s cred\n",
> >  		auth->au_ops->au_name);
> 
> Among other things, this will break the access() syscall.

Okay, I see that now.

> It's completely the wrong level in which to override credentials.

The reason for it is that sget() now has a capability check which will
fail on automount if current doesn't have CAP_SYS_ADMIN. So what are the
alternatives? A few ideas:

 - Instead of using a completely differnet set of creds, we could copy
   the current creds and raise CAP_SYS_ADMIN. This won't work if
   curreent is in a different user ns however.

 - Filesystems could get around the capability check by using
   sget_userns() during automount.

 - We could add a mount flag, say MS_AUTOMOUNT, and skip the capability
   check if that is set.

Any opinions or other ideas?

Thanks,
Seth