Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 24 Apr 2023 10:02:25 -0400
From:   Scott Mayhew <smayhew@redhat.com>
To:     Ben Boeckel <me@benboeckel.net>
Cc:     linux-nfs@vger.kernel.org, keyrings@vger.kernel.org
Subject: Re: [RFC PATCH 5/5] SUNRPC: store GSS creds in keyrings
Message-ID: <ZEaL8Wueo5/vOGTg@aion.usersys.redhat.com>
References: <20230420202004.239116-1-smayhew@redhat.com>
 <20230420202004.239116-6-smayhew@redhat.com>
 <20230422212710.GA813856@farprobe>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230422212710.GA813856@farprobe>
Precedence: bulk

On Sat, 22 Apr 2023, Ben Boeckel wrote:

> On Thu, Apr 20, 2023 at 16:20:04 -0400, Scott Mayhew wrote:
> > This patch adds the option to store GSS credentials in keyrings as an
> > alternative to the RPC credential cache, to give users the ability to
> > destroy their GSS credentials on demand via 'keyctl unlink'.
> 
> Can documentation please be added to `Documentation/security/keys` about
> this key type?

I'll work on that.

> 
> > Summary of the changes:
> > 
> > - Added key_type key_type_gss_cred and associated functions.  The
> >   request_key function makes use of the existing upcall mechanism to
> >   gssd.
> > 
> > - Added a keyring to the gss_auth struct to allow all of the assocated
> >   GSS credentials to be destroyed on RPC client shutdown (when the
> >   filesystem is unmounted).
> > 
> > - The key description contains the RPC client id, the user id, and the
> >   principal (for machine creds).
> 
> What is the format of this within the bytes?

The format is "clid: <client-id> id: <fsuid> princ:<princ>", where
client-id and fsuid are unsigned ints and princ is either "(none)" or
"*" if it's a machine cred:

crash> p ((struct key *) 0xffff8b4410197900)->description
$1 = 0xffff8b4446cbd740 "clid:1 id:1000 princ:(none)"

> 
> > - The key payload contains the address of the gss_cred.
> 
> What is the format of this within the bytes?

The payload is just a pointer:

crash> p ((struct key *) 0xffff8b4410197900)->payload.data[0]
$2 = (void *) 0xffff8b44381cd480

> 
> > - The key is linked to the user's user keyring (KEY_SPEC_USER_KEYRING)
> >   as well as to the keyring on the gss_auth struct.
> 
> Where is this documented? Can the key be moved later?

It's not - I can add that to the documentation for the new key type.
The key should not be moved.  I haven't tested if it's possible to move
it, but it's something that we'd want to disallow.

-Scott
> 
> > - gss_cred_init() now takes an optional pointer to an authkey, which is
> >   passed down to gss_create_upcall() and gss_setup_upcall(), where it is
> >   added to the gss_msg.  This is used for complete_request_key() after
> >   the upcall is done.
> > 
> > - put_rpccred() now returns a bool to indicate whether it called
> >   crdestroy(), and is used by gss_key_revoke() and gss_key_destroy() to
> >   determine whether to clear the key payload.
> > 
> > - gss_fill_context() now returns the GSS context's timeout via the tout
> >   parameter, which is used to set the timeout of the key.
> > 
> > - Added the module parameter 'use_keyring'.  When set to true, the GSS
> >   credentials are stored in the keyrings.  When false, the GSS
> >   credentials are stored in the RPC credential caches.
> > 
> > - Added a tracepoint to log the result of the key request, which prints
> >   either the key serial or an error return value.
> > 
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > ---
> >  include/linux/sunrpc/auth.h    |   4 +-
> >  include/trace/events/rpcgss.h  |  46 ++++-
> >  net/sunrpc/auth.c              |   9 +-
> >  net/sunrpc/auth_gss/auth_gss.c | 338 +++++++++++++++++++++++++++++++--
> >  4 files changed, 376 insertions(+), 21 deletions(-)
> 
> Thanks,
> 
> --Ben
>