Seth Forshee <seth.forshee@xxxxxxxxxxxxx> writes:
> On Fri, Dec 16, 2016 at 07:06:09AM -0600, Seth Forshee wrote:
>> 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@xxxxxxxxxxxxxxx # v4.8+
>> > > CC: Eric W. Biederman <ebiederm@xxxxxxxxxxxx>
>> > > Fixes: aeaa4a79ff6a ("fs: Call d_automount with the filesystems
>> > > creds")
>> > > Signed-off-by: Seth Forshee <seth.forshee@xxxxxxxxxxxxx>
>> > > ---
>> > > 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?
>
> I haven't seen any responses, possibly just got lost in the shuffle
> during the holidays (I know it slipped my mind for a while).
>
> Eric, what do you think about the last option above? From what I can see
> looking up rpc credentials just isn't going to work with current_cred
> overridden as we're doing for automount.
I got as far as there wasn't a correct thing to apply, and I have been
bogged down in enough other things that I haven't gotten back to this
one.
My gut feel is that we propbably want to do a little more reworking on
the autmount path. But I don't exactly have a concrete proposal for
you at the moment. I just found another 10 year old bug in the mount
code...
Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
