Received-SPF: pass (google.com: best guess record for domain of linux-nfs-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 4 Sep 2019 16:58:26 -0400
From:   Scott Mayhew <smayhew@redhat.com>
To:     Simo Sorce <simo@redhat.com>
Cc:     Chuck Lever <chuck.lever@oracle.com>,
        Bruce Fields <bfields@fieldses.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 0/2] nfsd: add principal to the data being tracked by
 nfsdcld
Message-ID: <20190904205826.GH11980@coeurl.usersys.redhat.com>
References: <20190830162631.13195-1-smayhew@redhat.com>
 <A732539C-837A-4764-8281-C26E4203DE25@oracle.com>
 <4598a6617fcb0123fb8c5c19e0ed2e489b242bcf.camel@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4598a6617fcb0123fb8c5c19e0ed2e489b242bcf.camel@redhat.com>
User-Agent: Mutt/1.11.3 (2019-02-01)
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

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
> 
> 
> 
>