Re: [PATCH 0/2] nfsd: add principal to the data being tracked by nfsdcld

[Scott Mayhew <smayhew@redhat.com>]

On Fri, 30 Aug 2019, Simo Sorce wrote:

> On Fri, 2019-08-30 at 12:32 -0400, Chuck Lever wrote:
> > Simo, any comments or questions?
> 
> Although it is unlikely, in most scenarios to have a principal name
> longer than 1024 characters, it is definitely not impossible, given the
> principal name for hosts is generally compose of 3 components:
> - a short service name
> - a fully qualified hostname
> - a realm name

Right now I'm using the svc_cred.cr_principal, which doesn't include
the realm.  The reason I chose that was because it's set by both
gssproxy and rpc.svcgssd.  I suppose I can check
svc_cred.cr_raw_princpal first and then fall back to
svc_cred.cr_principal.
> 
> The service name is generally "nfs" or "host" in the NFSv4 case,
> however the realm name can be arbitrarily large and usually is the
> capitalized domain name where the realm resides.
> 
> While thinking about this I wondered, why not simply hash (SHA-256 for
> example) the principal name and store the hash instead?
> 
> It will make the length fixed and uniform and probably often shorter
> than the real principal names, so saving space in the general case.
> 
> I am not against truncating to 1024, but a hash would be more elegant
> and correct.

I can do that.  Is there any reason I would want to convert the hash to
to a human-readable format (i.e. something that would match the
sha256sum command-line tool's output) or can I just use the raw buffer?
Note that if we wanted to print the hash in an error message or
something, I can just use printk's %*phN format specifier...

-Scott
> 
> Simo.
> 
> 
> > Patches can be found here:
> > 
> > https://marc.info/?l=linux-nfs&m=156718239314526&w=2
> > 
> > https://marc.info/?l=linux-nfs&m=156718239414527&w=2
> > 
> > 
> > > On Aug 30, 2019, at 12:26 PM, Scott Mayhew <smayhew@redhat.com> wrote:
> > > 
> > > At the spring bakeathon, Chuck suggested that we should store the
> > > kerberos principal in addition to the client id string in nfsdcld.  The
> > > idea is to prevent an illegitimate client from reclaiming another
> > > client's opens by supplying that client's id string.
> > > 
> > > The first patch lays some groundwork for supporting multiple message
> > > versions for the nfsdcld upcalls, adding fields for version and message
> > > length to the nfsd4_client_tracking_ops (these fields are only used for
> > > the nfsdcld upcalls and ignored for the other tracking methods), as well
> > > as an upcall to get the maximum version supported by the userspace
> > > daemon.
> > > 
> > > The second patch actually adds the v2 message, which adds the principal
> > > (actually just the first 1024 bytes) to the Cld_Create upcall and to the
> > > Cld_GraceStart downcall (which is what loads the data in the
> > > reclaim_str_hashtbl). I couldn't really figure out what the maximum length
> > > of a kerberos principal actually is (looking at krb5.h the length field in
> > > the struct krb5_data is an unsigned int, so I guess it can be pretty big).
> > > I don't think the principal will be that large in practice, and even if
> > > it is the first 1024 bytes should be sufficient for our purposes.
> > > 
> > > Scott Mayhew (2):
> > >  nfsd: add a "GetVersion" upcall for nfsdcld
> > >  nfsd: add support for upcall version 2
> > > 
> > > fs/nfsd/nfs4recover.c         | 332 +++++++++++++++++++++++++++-------
> > > fs/nfsd/nfs4state.c           |   6 +-
> > > fs/nfsd/state.h               |   3 +-
> > > include/uapi/linux/nfsd/cld.h |  37 +++-
> > > 4 files changed, 311 insertions(+), 67 deletions(-)
> > > 
> > > -- 
> > > 2.17.2
> > > 
> > 
> > --
> > Chuck Lever
> > 
> > 
> > 
> 
> -- 
> Simo Sorce
> RHEL Crypto Team
> Red Hat, Inc
> 
> 
> 
>