From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3D475C952 for ; Tue, 21 Mar 2023 15:10:36 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6B1ACC433D2; Tue, 21 Mar 2023 15:10:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1679411435; bh=8K3gBcFOQuqNE2iVJnAM1ZPUBhJ/EmLCYk/eIeYLM2E=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=bBEWkBUSQLrbmt0UzXbxlU/5Xqh7uyBWaRDs4YU2wG3YSmGcypc6bGmdJofGpxw5A vCzeUSIPmLgyi6QGkwFh74MZVQslJv7Q5ojDWUlLxS3L4vGk0jD+ju9yNJFahjkFcA IJoSpEy9TIBxy77ZSMZM1wgg5L5LtVMFx7Cw9e+5mr4mzCynjm9k6FUceBDJb+e7aM mS1DqEWZnT95UQ84zgwfEaERPkLoDfXgdXPYFzCRO+Kz4KOrrH0S9f8gXRScBYAyQ2 tSF3Mr0NMGT3rmRxDmD/+Fnzrr1BdgTxFo42azayZ++XUr/28oeI47d/AkaeRVJ/4q lIpCPev+WxlUg== Message-ID: <00aa56c1944e5212fd0c64914aab9126cff5be06.camel@kernel.org> Subject: Re: [PATCH RFC 5/5] NFSD: Handle new xprtsec= export option From: Jeff Layton To: Chuck Lever III Cc: Chuck Lever , Linux NFS Mailing List , "kernel-tls-handshake@lists.linux.dev" Date: Tue, 21 Mar 2023 11:10:34 -0400 In-Reply-To: References: <167932094748.3131.11264549266195745851.stgit@manet.1015granger.net> <167932229302.3131.3108041458819604050.stgit@manet.1015granger.net> <3dec5ea0dd4efbf96767333d3de90457b9fd3ecd.camel@kernel.org> Content-Type: text/plain; charset="ISO-8859-15" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) Precedence: bulk X-Mailing-List: kernel-tls-handshake@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Tue, 2023-03-21 at 14:05 +0000, Chuck Lever III wrote: >=20 > > On Mar 21, 2023, at 7:50 AM, Jeff Layton wrote: > >=20 > > On Mon, 2023-03-20 at 10:24 -0400, Chuck Lever wrote: > > > From: Chuck Lever > > >=20 > > > Enable administrators to require clients to use transport layer > > > security when accessing particular exports. > >=20 > > > Signed-off-by: Chuck Lever > > > --- > > > fs/nfsd/export.c | 53 +++++++++++++++++++++++++++++++++++++++++++++= +++++--- > > > fs/nfsd/export.h | 11 +++++++++++ > > > 2 files changed, 61 insertions(+), 3 deletions(-) > > >=20 > > > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c > > > index 668c7527b17e..171ebc21bf07 100644 > > > --- a/fs/nfsd/export.c > > > +++ b/fs/nfsd/export.c > > > @@ -439,7 +439,6 @@ static int check_export(struct path *path, int *f= lags, unsigned char *uuid) > > > return -EINVAL; > > > } > > > return 0; > > > - > > > } > > >=20 > > > #ifdef CONFIG_NFSD_V4 > > > @@ -546,6 +545,31 @@ static inline int > > > secinfo_parse(char **mesg, char *buf, struct svc_export *exp) { retur= n 0; } > > > #endif > > >=20 > > > +static int xprtsec_parse(char **mesg, char *buf, struct svc_export *= exp) > > > +{ > > > + unsigned int i, mode, listsize; > > > + int err; > > > + > > > + err =3D get_uint(mesg, &listsize); > > > + if (err) > > > + return err; > > > + if (listsize > 3) > > > + return -EINVAL; > >=20 > > Might want to make a note that the limit of 3 here is arbitrary, and > > that it might need to be lifted in the future (if/when we grow other > > xprtsec options). >=20 > Well I can easily add a symbolic constant for that too. I > missed this one in the final clean-up before posting. >=20 > The bigger question is whether the new downcall parameter is > sensible. If there's a nicer way for mountd to get this > information to the kernel, I'm open to suggestion. >=20 I don't know of one. Export options seem fine here, since that's how we control all sorts of options in the nfs server. >=20 > > > + > > > + exp->ex_xprtsec_modes =3D 0; > > > + for (i =3D 0; i < listsize; i++) { > > > + err =3D get_uint(mesg, &mode); > > > + if (err) > > > + return err; > > > + mode--; > > > + if (mode > 2) > > > + return -EINVAL; > > > + /* Ad hoc */ > > > + exp->ex_xprtsec_modes |=3D 1 << mode; > > > + } > > > + return 0; > > > +} > > > + > > > static inline int > > > nfsd_uuid_parse(char **mesg, char *buf, unsigned char **puuid) > > > { > > > @@ -608,6 +632,7 @@ static int svc_export_parse(struct cache_detail *= cd, char *mesg, int mlen) > > > exp.ex_client =3D dom; > > > exp.cd =3D cd; > > > exp.ex_devid_map =3D NULL; > > > + exp.ex_xprtsec_modes =3D NFSEXP_XPRTSEC_ALL; > > >=20 > > > /* expiry */ > > > err =3D -EINVAL; > > > @@ -650,6 +675,8 @@ static int svc_export_parse(struct cache_detail *= cd, char *mesg, int mlen) > > > err =3D nfsd_uuid_parse(&mesg, buf, &exp.ex_uuid); > > > else if (strcmp(buf, "secinfo") =3D=3D 0) > > > err =3D secinfo_parse(&mesg, buf, &exp); > > > + else if (strcmp(buf, "xprtsec") =3D=3D 0) > > > + err =3D xprtsec_parse(&mesg, buf, &exp); > > > else > > > /* quietly ignore unknown words and anything > > > * following. Newer user-space can try to set > > > @@ -663,6 +690,7 @@ static int svc_export_parse(struct cache_detail *= cd, char *mesg, int mlen) > > > err =3D check_export(&exp.ex_path, &exp.ex_flags, exp.ex_uuid); > > > if (err) > > > goto out4; > > > + > > > /* > > > * No point caching this if it would immediately expire. > > > * Also, this protects exportfs's dummy export from the > > > @@ -824,6 +852,7 @@ static void export_update(struct cache_head *cnew= , struct cache_head *citem) > > > for (i =3D 0; i < MAX_SECINFO_LIST; i++) { > > > new->ex_flavors[i] =3D item->ex_flavors[i]; > > > } > > > + new->ex_xprtsec_modes =3D item->ex_xprtsec_modes; > > > } > > >=20 > > > static struct cache_head *svc_export_alloc(void) > > > @@ -1035,9 +1064,26 @@ static struct svc_export *exp_find(struct cach= e_detail *cd, > > >=20 > > > __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqs= tp) > > > { > > > - struct exp_flavor_info *f; > > > - struct exp_flavor_info *end =3D exp->ex_flavors + exp->ex_nflavors; > > > + struct exp_flavor_info *f, *end =3D exp->ex_flavors + exp->ex_nflav= ors; > > > + struct svc_xprt *xprt =3D rqstp->rq_xprt; > > > + > > > + if (exp->ex_xprtsec_modes & NFSEXP_XPRTSEC_NONE) { > > > + if (!test_bit(XPT_TLS_SESSION, &xprt->xpt_flags)) > > > + goto ok; > > > + } > > > + if (exp->ex_xprtsec_modes & NFSEXP_XPRTSEC_TLS) { > > > + if (test_bit(XPT_TLS_SESSION, &xprt->xpt_flags) && > > > + !test_bit(XPT_PEER_AUTH, &xprt->xpt_flags)) > > > + goto ok; > > > + } > > > + if (exp->ex_xprtsec_modes & NFSEXP_XPRTSEC_MTLS) { > > > + if (test_bit(XPT_TLS_SESSION, &xprt->xpt_flags) && > > > + test_bit(XPT_PEER_AUTH, &xprt->xpt_flags)) > > > + goto ok; > > > + } > > > + goto denied; > > >=20 > > > +ok: > > > /* legacy gss-only clients are always OK: */ > > > if (exp->ex_client =3D=3D rqstp->rq_gssclient) > > > return 0; > > > @@ -1062,6 +1108,7 @@ __be32 check_nfsd_access(struct svc_export *exp= , struct svc_rqst *rqstp) > > > if (nfsd4_spo_must_allow(rqstp)) > > > return 0; > > >=20 > > > +denied: > > > return rqstp->rq_vers < 4 ? nfserr_acces : nfserr_wrongsec; > > > } > > >=20 > > > diff --git a/fs/nfsd/export.h b/fs/nfsd/export.h > > > index d03f7f6a8642..61e1e8383c3d 100644 > > > --- a/fs/nfsd/export.h > > > +++ b/fs/nfsd/export.h > > > @@ -77,8 +77,19 @@ struct svc_export { > > > struct cache_detail *cd; > > > struct rcu_head ex_rcu; > > > struct export_stats ex_stats; > > > + unsigned long ex_xprtsec_modes; > > > }; > > >=20 > > > +enum { > > > + NFSEXP_XPRTSEC_NONE =3D 0x01, > > > + NFSEXP_XPRTSEC_TLS =3D 0x02, > > > + NFSEXP_XPRTSEC_MTLS =3D 0x04, > > > +}; > > > + > > > +#define NFSEXP_XPRTSEC_ALL (NFSEXP_XPRTSEC_NONE | \ > > > + NFSEXP_XPRTSEC_TLS | \ > > > + NFSEXP_XPRTSEC_MTLS) > > > + > > > /* an "export key" (expkey) maps a filehandlefragement to an > > > * svc_export for a given client. There can be several per export, > > > * for the different fsid types. > > >=20 > > >=20 > >=20 > > --=20 > > Jeff Layton > >=20 >=20 > -- > Chuck Lever >=20 >=20 --=20 Jeff Layton