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, 17 Apr 2023 10:25:37 +0800
From:   Wang Yugui <wangyugui@e16-tech.com>
To:     "NeilBrown" <neilb@suse.de>
Subject: Re: [PATCH nfs-utils] mountd: don't advertise krb5 for v4root when not configured.
Cc:     "Steve Dickson" <steved@redhat.com>, "Petr Vorel" <pvorel@suse.cz>,
        "linux-nfs" <linux-nfs@vger.kernel.org>,
        "Dave Jones" <davej@codemonkey.org.uk>, bfields@redhat.com
In-Reply-To: <168169801568.24821.12909751358635990715@noble.neil.brown.name>
References: <20230417100511.9131.409509F4@e16-tech.com> <168169801568.24821.12909751358635990715@noble.neil.brown.name>
Message-Id: <20230417102536.FAE6.409509F4@e16-tech.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

> On Mon, 17 Apr 2023, Wang Yugui wrote:
> > Hi,
> > 
> > > 
> > > If /etc/krb5.keytab does not exist, then krb5 cannot work, so
> > > advertising it as an option for v4root is pointless.
> > > Since linux commit 676e4ebd5f2c ("NFSD: SECINFO doesn't handle
> > > unsupported pseudoflavors correctly") this can result in an unhelpful
> > > warning if the krb5 code is not built, or built as a module which is not
> > > installed.
> > > 
> > > [  161.668635] NFS: SECINFO: security flavor 390003 is not supported
> > > [  161.668655] NFS: SECINFO: security flavor 390004 is not supported
> > > [  161.668670] NFS: SECINFO: security flavor 390005 is not supported
> > > 
> > > So avoid advertising krb5 security options when krb5.keytab cannot be
> > > found.
> > > 
> > > Link: https://lore.kernel.org/linux-nfs/20170104190327.v3wbpcbqtfa5jy7d@codemonkey.org.uk/
> > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > ---
> > >  support/export/v4root.c         |  2 ++
> > >  support/include/pseudoflavors.h |  1 +
> > >  support/nfs/exports.c           | 14 +++++++-------
> > >  3 files changed, 10 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/support/export/v4root.c b/support/export/v4root.c
> > > index fbb0ad5f5b81..3e049582d7c1 100644
> > > --- a/support/export/v4root.c
> > > +++ b/support/export/v4root.c
> > > @@ -66,6 +66,8 @@ set_pseudofs_security(struct exportent *pseudo)
> > >  
> > >  		if (!flav->fnum)
> > >  			continue;
> > > +		if (flav->need_krb5 && !access("/etc/krb5.keytab", F_OK))
> > > +			continue;
> > 
> > Could we replace "/etc/krb5.keytab" with krb5_kt_default_name()?
> 
> Maybe?  Why would we want to?
> 
> The presence of /etc/krb5.keytab is what we already use in a couple of
> systemd unit files to determine if krb5 is configured.  Why not just use
> the same here?

OK to just same as other files.

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2023/04/17