linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server
@ 2023-03-20 14:35 Chuck Lever
  2023-03-20 14:35 ` [PATCH v1 1/4] libexports: Fix whitespace damage in support/nfs/exports.c Chuck Lever
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Chuck Lever @ 2023-03-20 14:35 UTC (permalink / raw)
  To: SteveD; +Cc: linux-nfs

Hi Steve-

This is server-side support for RPC-with-TLS, to accompany similar
support in the Linux NFS client. This implementation can support
both the opportunistic use of transport layer security (it will be
used if the client cares to) and the required use of transport
layer security (the server requires the client to use it to access
a particular export).

Without any other user space componentry, this implementation will
be able to handle clients that request the use of RPC-with-TLS. To
support security policies that restrict access to exports based on
the client's use of TLS, modifications to exportfs and mountd are
needed. These can be found here:

git://git.linux-nfs.org/projects/cel/nfs-utils.git

They include an update to exports(5) explaining how to use the new
"xprtsec=" export option.

The kernel patches, along with the the handshake upcall, are carried
in the topic-rpc-with-tls-upcall branch available from:

https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git

This was posted under separate cover.

---

Chuck Lever (4):
      libexports: Fix whitespace damage in support/nfs/exports.c
      exports: Add an xprtsec= export option
      exportfs: Push xprtsec settings to the kernel
      exports.man: Add description of xprtsec= to exports(5)


 support/export/cache.c       |  15 ++++++
 support/include/nfs/export.h |   6 +++
 support/include/nfslib.h     |  14 +++++
 support/nfs/exports.c        | 100 ++++++++++++++++++++++++++++++++---
 utils/exportfs/exportfs.c    |   1 +
 utils/exportfs/exports.man   |  45 +++++++++++++++-
 6 files changed, 172 insertions(+), 9 deletions(-)

--
Chuck Lever


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v1 1/4] libexports: Fix whitespace damage in support/nfs/exports.c
  2023-03-20 14:35 [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server Chuck Lever
@ 2023-03-20 14:35 ` Chuck Lever
  2023-03-20 14:35 ` [PATCH v1 2/4] exports: Add an xprtsec= export option Chuck Lever
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Chuck Lever @ 2023-03-20 14:35 UTC (permalink / raw)
  To: SteveD; +Cc: linux-nfs

From: Chuck Lever <chuck.lever@oracle.com>

Clean up.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 support/nfs/exports.c |   15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/support/nfs/exports.c b/support/nfs/exports.c
index 2c8f0752ad9d..7f12383981c3 100644
--- a/support/nfs/exports.c
+++ b/support/nfs/exports.c
@@ -122,7 +122,7 @@ getexportent(int fromkernel, int fromexports)
 	if (first || (ok = getexport(exp, sizeof(exp))) == 0) {
 		has_default_opts = 0;
 		has_default_subtree_opts = 0;
-	
+
 		init_exportent(&def_ee, fromkernel);
 
 		ok = getpath(def_ee.e_path, sizeof(def_ee.e_path));
@@ -146,7 +146,7 @@ getexportent(int fromkernel, int fromexports)
 	if (exp[0] == '-' && !fromkernel) {
 		if (parseopts(exp + 1, &def_ee, 0, &has_default_subtree_opts) < 0)
 			return NULL;
-		
+
 		has_default_opts = 1;
 
 		ok = getexport(exp, sizeof(exp));
@@ -239,7 +239,6 @@ void secinfo_show(FILE *fp, struct exportent *ep)
 	if (ep->e_secinfo[0].flav == NULL)
 		secinfo_addflavor(find_flavor("sys"), ep);
 	for (p1=ep->e_secinfo; p1->flav; p1=p2) {
-
 		fprintf(fp, ",sec=%s", p1->flav->flavour);
 		for (p2=p1+1; (p2->flav != NULL) && (p1->flags == p2->flags);
 								p2++) {
@@ -621,7 +620,7 @@ parseopts(char *cp, struct exportent *ep, int warn, int *had_subtree_opt_ptr)
 			ep->e_anonuid = strtol(opt+8, &oe, 10);
 			if (opt[8]=='\0' || *oe != '\0') {
 				xlog(L_ERROR, "%s: %d: bad anonuid \"%s\"\n",
-				     flname, flline, opt);	
+				     flname, flline, opt);
 bad_option:
 				free(opt);
 				return -1;
@@ -631,7 +630,7 @@ bad_option:
 			ep->e_anongid = strtol(opt+8, &oe, 10);
 			if (opt[8]=='\0' || *oe != '\0') {
 				xlog(L_ERROR, "%s: %d: bad anongid \"%s\"\n",
-				     flname, flline, opt);	
+				     flname, flline, opt);
 				goto bad_option;
 			}
 		} else if (strncmp(opt, "squash_uids=", 12) == 0) {
@@ -649,13 +648,13 @@ bad_option:
 				setflags(NFSEXP_FSID, active, ep);
 			} else {
 				ep->e_fsid = strtoul(opt+5, &oe, 0);
-				if (opt[5]!='\0' && *oe == '\0') 
+				if (opt[5]!='\0' && *oe == '\0')
 					setflags(NFSEXP_FSID, active, ep);
 				else if (valid_uuid(opt+5))
 					ep->e_uuid = strdup(opt+5);
 				else {
 					xlog(L_ERROR, "%s: %d: bad fsid \"%s\"\n",
-					     flname, flline, opt);	
+					     flname, flline, opt);
 					goto bad_option;
 				}
 			}
@@ -709,7 +708,7 @@ out:
 	if (warn && !had_subtree_opt)
 		xlog(L_WARNING, "%s [%d]: Neither 'subtree_check' or 'no_subtree_check' specified for export \"%s:%s\".\n"
 				"  Assuming default behaviour ('no_subtree_check').\n"
-		     		"  NOTE: this default has changed since nfs-utils version 1.0.x\n",
+				"  NOTE: this default has changed since nfs-utils version 1.0.x\n",
 
 				flname, flline,
 				ep->e_hostname, ep->e_path);



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 2/4] exports: Add an xprtsec= export option
  2023-03-20 14:35 [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server Chuck Lever
  2023-03-20 14:35 ` [PATCH v1 1/4] libexports: Fix whitespace damage in support/nfs/exports.c Chuck Lever
@ 2023-03-20 14:35 ` Chuck Lever
  2023-03-21 11:55   ` Jeff Layton
  2023-03-20 14:35 ` [PATCH v1 3/4] exportfs: Push xprtsec settings to the kernel Chuck Lever
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Chuck Lever @ 2023-03-20 14:35 UTC (permalink / raw)
  To: SteveD; +Cc: linux-nfs

From: Chuck Lever <chuck.lever@oracle.com>

The overall goal is to enable administrators to require the use of
transport layer security when clients access particular exports.

This patch adds support to exportfs to parse and display a new
xprtsec= export option. The setting is not yet passed to the kernel.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 support/include/nfs/export.h |    6 +++
 support/include/nfslib.h     |   14 +++++++
 support/nfs/exports.c        |   85 ++++++++++++++++++++++++++++++++++++++++++
 utils/exportfs/exportfs.c    |    1 
 4 files changed, 106 insertions(+)

diff --git a/support/include/nfs/export.h b/support/include/nfs/export.h
index 0eca828ee3ad..b29c6fa4f554 100644
--- a/support/include/nfs/export.h
+++ b/support/include/nfs/export.h
@@ -40,4 +40,10 @@
 #define NFSEXP_OLD_SECINFO_FLAGS (NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
 					| NFSEXP_ALLSQUASH)
 
+enum {
+	NFSEXP_XPRTSEC_NONE = 1,
+	NFSEXP_XPRTSEC_TLS = 2,
+	NFSEXP_XPRTSEC_MTLS = 3,
+};
+
 #endif /* _NSF_EXPORT_H */
diff --git a/support/include/nfslib.h b/support/include/nfslib.h
index 6faba71bf0cd..9a188fb84790 100644
--- a/support/include/nfslib.h
+++ b/support/include/nfslib.h
@@ -62,6 +62,18 @@ struct sec_entry {
 	int flags;
 };
 
+#define XPRTSECMODE_COUNT 4
+
+struct xprtsec_info {
+	const char		*name;
+	int			number;
+};
+
+struct xprtsec_entry {
+	const struct xprtsec_info *info;
+	int			flags;
+};
+
 /*
  * Data related to a single exports entry as returned by getexportent.
  * FIXME: export options should probably be parsed at a later time to
@@ -83,6 +95,7 @@ struct exportent {
 	char *          e_fslocdata;
 	char *		e_uuid;
 	struct sec_entry e_secinfo[SECFLAVOR_COUNT+1];
+	struct xprtsec_entry e_xprtsec[XPRTSECMODE_COUNT + 1];
 	unsigned int	e_ttl;
 	char *		e_realpath;
 };
@@ -99,6 +112,7 @@ struct rmtabent {
 void			setexportent(char *fname, char *type);
 struct exportent *	getexportent(int,int);
 void 			secinfo_show(FILE *fp, struct exportent *ep);
+void			xprtsecinfo_show(FILE *fp, struct exportent *ep);
 void			putexportent(struct exportent *xep);
 void			endexportent(void);
 struct exportent *	mkexportent(char *hname, char *path, char *opts);
diff --git a/support/nfs/exports.c b/support/nfs/exports.c
index 7f12383981c3..da8ace3a65fd 100644
--- a/support/nfs/exports.c
+++ b/support/nfs/exports.c
@@ -99,6 +99,7 @@ static void init_exportent (struct exportent *ee, int fromkernel)
 	ee->e_fslocmethod = FSLOC_NONE;
 	ee->e_fslocdata = NULL;
 	ee->e_secinfo[0].flav = NULL;
+	ee->e_xprtsec[0].info = NULL;
 	ee->e_nsquids = 0;
 	ee->e_nsqgids = 0;
 	ee->e_uuid = NULL;
@@ -248,6 +249,17 @@ void secinfo_show(FILE *fp, struct exportent *ep)
 	}
 }
 
+void xprtsecinfo_show(FILE *fp, struct exportent *ep)
+{
+	struct xprtsec_entry *p1, *p2;
+
+	for (p1 = ep->e_xprtsec; p1->info; p1 = p2) {
+		fprintf(fp, ",xprtsec=%s", p1->info->name);
+		for (p2 = p1 + 1; p2->info && (p1->flags == p2->flags); p2++)
+			fprintf(fp, ":%s", p2->info->name);
+	}
+}
+
 static void
 fprintpath(FILE *fp, const char *path)
 {
@@ -344,6 +356,7 @@ putexportent(struct exportent *ep)
 	}
 	fprintf(fp, "anonuid=%d,anongid=%d", ep->e_anonuid, ep->e_anongid);
 	secinfo_show(fp, ep);
+	xprtsecinfo_show(fp, ep);
 	fprintf(fp, ")\n");
 }
 
@@ -482,6 +495,75 @@ static unsigned int parse_flavors(char *str, struct exportent *ep)
 	return out;
 }
 
+static const struct xprtsec_info xprtsec_name2info[] = {
+	{ "none",	NFSEXP_XPRTSEC_NONE },
+	{ "tls",	NFSEXP_XPRTSEC_TLS },
+	{ "mtls",	NFSEXP_XPRTSEC_MTLS },
+	{ NULL,		0 }
+};
+
+static const struct xprtsec_info *find_xprtsec_info(const char *name)
+{
+	const struct xprtsec_info *info;
+
+	for (info = xprtsec_name2info; info->name; info++)
+		if (strcmp(info->name, name) == 0)
+			return info;
+	return NULL;
+}
+
+/*
+ * Append the given xprtsec mode to the exportent's e_xprtsec array,
+ * or do nothing if it's already there. Returns the index of flavor in
+ * the resulting array in any case.
+ */
+static int xprtsec_addmode(const struct xprtsec_info *info, struct exportent *ep)
+{
+	struct xprtsec_entry *p;
+
+	for (p = ep->e_xprtsec; p->info; p++)
+		if (p->info == info || p->info->number == info->number)
+			return p - ep->e_xprtsec;
+
+	if (p - ep->e_xprtsec >= XPRTSECMODE_COUNT) {
+		xlog(L_ERROR, "more than %d xprtsec modes on an export\n",
+			XPRTSECMODE_COUNT);
+		return -1;
+	}
+	p->info = info;
+	p->flags = ep->e_flags;
+	(p + 1)->info = NULL;
+	return p - ep->e_xprtsec;
+}
+
+/*
+ * @str is a colon seperated list of transport layer security modes.
+ * Their order is recorded in @ep, and a bitmap corresponding to the
+ * list is returned.
+ *
+ * A zero return indicates an error.
+ */
+static unsigned int parse_xprtsec(char *str, struct exportent *ep)
+{
+	unsigned int out = 0;
+	char *name;
+
+	while ((name = strsep(&str, ":"))) {
+		const struct xprtsec_info *info = find_xprtsec_info(name);
+		int bit;
+
+		if (!info) {
+			xlog(L_ERROR, "unknown xprtsec mode %s\n", name);
+			return 0;
+		}
+		bit = xprtsec_addmode(info, ep);
+		if (bit < 0)
+			return 0;
+		out |= 1 << bit;
+	}
+	return out;
+}
+
 /* Sets the bits in @mask for the appropriate security flavor flags. */
 static void setflags(int mask, unsigned int active, struct exportent *ep)
 {
@@ -687,6 +769,9 @@ bad_option:
 			active = parse_flavors(opt+4, ep);
 			if (!active)
 				goto bad_option;
+		} else if (strncmp(opt, "xprtsec=", 8) == 0) {
+			if (!parse_xprtsec(opt + 8, ep))
+				goto bad_option;
 		} else {
 			xlog(L_ERROR, "%s:%d: unknown keyword \"%s\"\n",
 					flname, flline, opt);
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index 6d79a5b3480d..37b9e4b3612d 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -743,6 +743,7 @@ dump(int verbose, int export_format)
 #endif
 			}
 			secinfo_show(stdout, ep);
+			xprtsecinfo_show(stdout, ep);
 			printf("%c\n", (c != '(')? ')' : ' ');
 		}
 	}



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 3/4] exportfs: Push xprtsec settings to the kernel
  2023-03-20 14:35 [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server Chuck Lever
  2023-03-20 14:35 ` [PATCH v1 1/4] libexports: Fix whitespace damage in support/nfs/exports.c Chuck Lever
  2023-03-20 14:35 ` [PATCH v1 2/4] exports: Add an xprtsec= export option Chuck Lever
@ 2023-03-20 14:35 ` Chuck Lever
  2023-03-20 14:35 ` [PATCH v1 4/4] exports.man: Add description of xprtsec= to exports(5) Chuck Lever
  2023-03-21 11:52 ` [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server Jeff Layton
  4 siblings, 0 replies; 18+ messages in thread
From: Chuck Lever @ 2023-03-20 14:35 UTC (permalink / raw)
  To: SteveD; +Cc: linux-nfs

From: Chuck Lever <chuck.lever@oracle.com>

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 support/export/cache.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/support/export/cache.c b/support/export/cache.c
index 2497d4f48df3..9354f71db894 100644
--- a/support/export/cache.c
+++ b/support/export/cache.c
@@ -932,6 +932,7 @@ static void write_fsloc(char **bp, int *blen, struct exportent *ep)
 	release_replicas(servers);
 }
 #endif
+
 static void write_secinfo(char **bp, int *blen, struct exportent *ep, int flag_mask)
 {
 	struct sec_entry *p;
@@ -949,7 +950,20 @@ static void write_secinfo(char **bp, int *blen, struct exportent *ep, int flag_m
 		qword_addint(bp, blen, p->flav->fnum);
 		qword_addint(bp, blen, p->flags & flag_mask);
 	}
+}
+
+static void write_xprtsec(char **bp, int *blen, struct exportent *ep)
+{
+	struct xprtsec_entry *p;
+
+	for (p = ep->e_xprtsec; p->info; p++);
+	if (p == ep->e_xprtsec)
+		return;
 
+	qword_add(bp, blen, "xprtsec");
+	qword_addint(bp, blen, p - ep->e_xprtsec);
+	for (p = ep->e_xprtsec; p->info; p++)
+		qword_addint(bp, blen, p->info->number);
 }
 
 static int dump_to_cache(int f, char *buf, int blen, char *domain,
@@ -992,6 +1006,7 @@ static int dump_to_cache(int f, char *buf, int blen, char *domain,
 			qword_add(&bp, &blen, "uuid");
 			qword_addhex(&bp, &blen, u, 16);
 		}
+		write_xprtsec(&bp, &blen, exp);
 		xlog(D_AUTH, "granted access to %s for %s",
 		     path, *domain == '$' ? domain+1 : domain);
 	} else {



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 4/4] exports.man: Add description of xprtsec= to exports(5)
  2023-03-20 14:35 [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server Chuck Lever
                   ` (2 preceding siblings ...)
  2023-03-20 14:35 ` [PATCH v1 3/4] exportfs: Push xprtsec settings to the kernel Chuck Lever
@ 2023-03-20 14:35 ` Chuck Lever
  2023-03-21 12:06   ` Jeff Layton
  2023-03-21 11:52 ` [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server Jeff Layton
  4 siblings, 1 reply; 18+ messages in thread
From: Chuck Lever @ 2023-03-20 14:35 UTC (permalink / raw)
  To: SteveD; +Cc: linux-nfs

From: Chuck Lever <chuck.lever@oracle.com>

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 utils/exportfs/exports.man |   45 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/utils/exportfs/exports.man b/utils/exportfs/exports.man
index 54b3f8776ea6..cca9bb7b4aeb 100644
--- a/utils/exportfs/exports.man
+++ b/utils/exportfs/exports.man
@@ -125,7 +125,49 @@ In that case you may include multiple sec= options, and following options
 will be enforced only for access using flavors listed in the immediately
 preceding sec= option.  The only options that are permitted to vary in
 this way are ro, rw, no_root_squash, root_squash, and all_squash.
+.SS Transport layer security
+The Linux NFS server supports the use of transport layer security to
+protect RPC traffic between itself and its clients.
+This can be in the form of a VPN, an ssh tunnel, or via RPC-with-TLS,
+which uses TLSv1.3.
 .PP
+Administrators may choose to require the use of
+RPC-with-TLS to protect access to individual exports.
+This is particularly useful when using non-cryptographic security
+flavors such as
+.IR sec=sys .
+The
+.I xprtsec=
+option, followed by a colon-delimited list of security policies,
+can restrict access to the export to only clients that have negotiated
+transport-layer security.
+Currently supported transport layer security policies include:
+.TP
+.IR none
+The server permits clients to access the export
+without the use of transport layer security.
+.TP
+.IR tls
+The server permits clients that have negotiated an RPC-with-TLS session
+without peer authentication (confidentiality only) to access the export.
+Clients are not required to offer an x.509 certificate
+when establishing a transport layer security session.
+.TP
+.IR mtls
+The server permits clients that have negotiated an RPC-with-TLS session
+with peer authentication to access the export.
+The server requires clients to offer an x.509 certificate
+when establishing a transport layer security session.
+.PP
+The default setting is
+.IR xprtsec=none:tls:mtls .
+This is also known as "opportunistic mode".
+The server permits clients to use any transport layer security mechanism
+to access the export.
+.PP
+The server administrator must install and configure
+.BR tlshd
+to handle transport layer security handshake requests from the local kernel.
 .SS General Options
 .BR exportfs
 understands the following export options:
@@ -581,7 +623,8 @@ a character class wildcard match.
 .BR netgroup (5),
 .BR mountd (8),
 .BR nfsd (8),
-.BR showmount (8).
+.BR showmount (8),
+.BR tlshd (8).
 .\".SH DIAGNOSTICS
 .\"An error parsing the file is reported using syslogd(8) as level NOTICE from
 .\"a DAEMON whenever



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server
  2023-03-20 14:35 [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server Chuck Lever
                   ` (3 preceding siblings ...)
  2023-03-20 14:35 ` [PATCH v1 4/4] exports.man: Add description of xprtsec= to exports(5) Chuck Lever
@ 2023-03-21 11:52 ` Jeff Layton
  2023-03-23 17:57   ` Steve Dickson
  4 siblings, 1 reply; 18+ messages in thread
From: Jeff Layton @ 2023-03-21 11:52 UTC (permalink / raw)
  To: Chuck Lever, SteveD; +Cc: linux-nfs

On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
> Hi Steve-
> 
> This is server-side support for RPC-with-TLS, to accompany similar
> support in the Linux NFS client. This implementation can support
> both the opportunistic use of transport layer security (it will be
> used if the client cares to) and the required use of transport
> layer security (the server requires the client to use it to access
> a particular export).
> 
> Without any other user space componentry, this implementation will
> be able to handle clients that request the use of RPC-with-TLS. To
> support security policies that restrict access to exports based on
> the client's use of TLS, modifications to exportfs and mountd are
> needed. These can be found here:
> 
> git://git.linux-nfs.org/projects/cel/nfs-utils.git
> 
> They include an update to exports(5) explaining how to use the new
> "xprtsec=" export option.
> 
> The kernel patches, along with the the handshake upcall, are carried
> in the topic-rpc-with-tls-upcall branch available from:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
> 
> This was posted under separate cover.
> 
> ---
> 
> Chuck Lever (4):
>       libexports: Fix whitespace damage in support/nfs/exports.c
>       exports: Add an xprtsec= export option
>       exportfs: Push xprtsec settings to the kernel
>       exports.man: Add description of xprtsec= to exports(5)
> 
> 
>  support/export/cache.c       |  15 ++++++
>  support/include/nfs/export.h |   6 +++
>  support/include/nfslib.h     |  14 +++++
>  support/nfs/exports.c        | 100 ++++++++++++++++++++++++++++++++---
>  utils/exportfs/exportfs.c    |   1 +
>  utils/exportfs/exports.man   |  45 +++++++++++++++-
>  6 files changed, 172 insertions(+), 9 deletions(-)
> 

Nice work, Chuck! This all looks pretty straightforward. I have a
(minor) concern about potentially blocking nfsd threads for up to 20s in
a handshake, but this seems like a good place to start.
-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/4] exports: Add an xprtsec= export option
  2023-03-20 14:35 ` [PATCH v1 2/4] exports: Add an xprtsec= export option Chuck Lever
@ 2023-03-21 11:55   ` Jeff Layton
  2023-03-21 18:08     ` Chuck Lever III
  0 siblings, 1 reply; 18+ messages in thread
From: Jeff Layton @ 2023-03-21 11:55 UTC (permalink / raw)
  To: Chuck Lever, SteveD; +Cc: linux-nfs

On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> The overall goal is to enable administrators to require the use of
> transport layer security when clients access particular exports.
> 
> This patch adds support to exportfs to parse and display a new
> xprtsec= export option. The setting is not yet passed to the kernel.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  support/include/nfs/export.h |    6 +++
>  support/include/nfslib.h     |   14 +++++++
>  support/nfs/exports.c        |   85 ++++++++++++++++++++++++++++++++++++++++++
>  utils/exportfs/exportfs.c    |    1 
>  4 files changed, 106 insertions(+)
> 
> diff --git a/support/include/nfs/export.h b/support/include/nfs/export.h
> index 0eca828ee3ad..b29c6fa4f554 100644
> --- a/support/include/nfs/export.h
> +++ b/support/include/nfs/export.h
> @@ -40,4 +40,10 @@
>  #define NFSEXP_OLD_SECINFO_FLAGS (NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
>  					| NFSEXP_ALLSQUASH)
>  
> +enum {
> +	NFSEXP_XPRTSEC_NONE = 1,
> +	NFSEXP_XPRTSEC_TLS = 2,
> +	NFSEXP_XPRTSEC_MTLS = 3,
> +};
> +

Can we put these into a uapi header somewhere and then just have
nfs-utils use those if they're defined?

>  #endif /* _NSF_EXPORT_H */
> diff --git a/support/include/nfslib.h b/support/include/nfslib.h
> index 6faba71bf0cd..9a188fb84790 100644
> --- a/support/include/nfslib.h
> +++ b/support/include/nfslib.h
> @@ -62,6 +62,18 @@ struct sec_entry {
>  	int flags;
>  };
>  
> +#define XPRTSECMODE_COUNT 4
> +
> +struct xprtsec_info {
> +	const char		*name;
> +	int			number;
> +};
> +
> +struct xprtsec_entry {
> +	const struct xprtsec_info *info;
> +	int			flags;
> +};
> +
>  /*
>   * Data related to a single exports entry as returned by getexportent.
>   * FIXME: export options should probably be parsed at a later time to
> @@ -83,6 +95,7 @@ struct exportent {
>  	char *          e_fslocdata;
>  	char *		e_uuid;
>  	struct sec_entry e_secinfo[SECFLAVOR_COUNT+1];
> +	struct xprtsec_entry e_xprtsec[XPRTSECMODE_COUNT + 1];
>  	unsigned int	e_ttl;
>  	char *		e_realpath;
>  };
> @@ -99,6 +112,7 @@ struct rmtabent {
>  void			setexportent(char *fname, char *type);
>  struct exportent *	getexportent(int,int);
>  void 			secinfo_show(FILE *fp, struct exportent *ep);
> +void			xprtsecinfo_show(FILE *fp, struct exportent *ep);
>  void			putexportent(struct exportent *xep);
>  void			endexportent(void);
>  struct exportent *	mkexportent(char *hname, char *path, char *opts);
> diff --git a/support/nfs/exports.c b/support/nfs/exports.c
> index 7f12383981c3..da8ace3a65fd 100644
> --- a/support/nfs/exports.c
> +++ b/support/nfs/exports.c
> @@ -99,6 +99,7 @@ static void init_exportent (struct exportent *ee, int fromkernel)
>  	ee->e_fslocmethod = FSLOC_NONE;
>  	ee->e_fslocdata = NULL;
>  	ee->e_secinfo[0].flav = NULL;
> +	ee->e_xprtsec[0].info = NULL;
>  	ee->e_nsquids = 0;
>  	ee->e_nsqgids = 0;
>  	ee->e_uuid = NULL;
> @@ -248,6 +249,17 @@ void secinfo_show(FILE *fp, struct exportent *ep)
>  	}
>  }
>  
> +void xprtsecinfo_show(FILE *fp, struct exportent *ep)
> +{
> +	struct xprtsec_entry *p1, *p2;
> +
> +	for (p1 = ep->e_xprtsec; p1->info; p1 = p2) {
> +		fprintf(fp, ",xprtsec=%s", p1->info->name);
> +		for (p2 = p1 + 1; p2->info && (p1->flags == p2->flags); p2++)
> +			fprintf(fp, ":%s", p2->info->name);
> +	}
> +}
> +
>  static void
>  fprintpath(FILE *fp, const char *path)
>  {
> @@ -344,6 +356,7 @@ putexportent(struct exportent *ep)
>  	}
>  	fprintf(fp, "anonuid=%d,anongid=%d", ep->e_anonuid, ep->e_anongid);
>  	secinfo_show(fp, ep);
> +	xprtsecinfo_show(fp, ep);
>  	fprintf(fp, ")\n");
>  }
>  
> @@ -482,6 +495,75 @@ static unsigned int parse_flavors(char *str, struct exportent *ep)
>  	return out;
>  }
>  
> +static const struct xprtsec_info xprtsec_name2info[] = {
> +	{ "none",	NFSEXP_XPRTSEC_NONE },
> +	{ "tls",	NFSEXP_XPRTSEC_TLS },
> +	{ "mtls",	NFSEXP_XPRTSEC_MTLS },
> +	{ NULL,		0 }
> +};
> +
> +static const struct xprtsec_info *find_xprtsec_info(const char *name)
> +{
> +	const struct xprtsec_info *info;
> +
> +	for (info = xprtsec_name2info; info->name; info++)
> +		if (strcmp(info->name, name) == 0)
> +			return info;
> +	return NULL;
> +}
> +
> +/*
> + * Append the given xprtsec mode to the exportent's e_xprtsec array,
> + * or do nothing if it's already there. Returns the index of flavor in
> + * the resulting array in any case.
> + */
> +static int xprtsec_addmode(const struct xprtsec_info *info, struct exportent *ep)
> +{
> +	struct xprtsec_entry *p;
> +
> +	for (p = ep->e_xprtsec; p->info; p++)
> +		if (p->info == info || p->info->number == info->number)
> +			return p - ep->e_xprtsec;
> +
> +	if (p - ep->e_xprtsec >= XPRTSECMODE_COUNT) {
> +		xlog(L_ERROR, "more than %d xprtsec modes on an export\n",
> +			XPRTSECMODE_COUNT);
> +		return -1;
> +	}
> +	p->info = info;
> +	p->flags = ep->e_flags;
> +	(p + 1)->info = NULL;
> +	return p - ep->e_xprtsec;
> +}
> +
> +/*
> + * @str is a colon seperated list of transport layer security modes.
> + * Their order is recorded in @ep, and a bitmap corresponding to the
> + * list is returned.
> + *
> + * A zero return indicates an error.
> + */
> +static unsigned int parse_xprtsec(char *str, struct exportent *ep)
> +{
> +	unsigned int out = 0;
> +	char *name;
> +
> +	while ((name = strsep(&str, ":"))) {
> +		const struct xprtsec_info *info = find_xprtsec_info(name);
> +		int bit;
> +
> +		if (!info) {
> +			xlog(L_ERROR, "unknown xprtsec mode %s\n", name);
> +			return 0;
> +		}
> +		bit = xprtsec_addmode(info, ep);
> +		if (bit < 0)
> +			return 0;
> +		out |= 1 << bit;
> +	}
> +	return out;
> +}
> +
>  /* Sets the bits in @mask for the appropriate security flavor flags. */
>  static void setflags(int mask, unsigned int active, struct exportent *ep)
>  {
> @@ -687,6 +769,9 @@ bad_option:
>  			active = parse_flavors(opt+4, ep);
>  			if (!active)
>  				goto bad_option;
> +		} else if (strncmp(opt, "xprtsec=", 8) == 0) {
> +			if (!parse_xprtsec(opt + 8, ep))
> +				goto bad_option;
>  		} else {
>  			xlog(L_ERROR, "%s:%d: unknown keyword \"%s\"\n",
>  					flname, flline, opt);
> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> index 6d79a5b3480d..37b9e4b3612d 100644
> --- a/utils/exportfs/exportfs.c
> +++ b/utils/exportfs/exportfs.c
> @@ -743,6 +743,7 @@ dump(int verbose, int export_format)
>  #endif
>  			}
>  			secinfo_show(stdout, ep);
> +			xprtsecinfo_show(stdout, ep);
>  			printf("%c\n", (c != '(')? ')' : ' ');
>  		}
>  	}
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 4/4] exports.man: Add description of xprtsec= to exports(5)
  2023-03-20 14:35 ` [PATCH v1 4/4] exports.man: Add description of xprtsec= to exports(5) Chuck Lever
@ 2023-03-21 12:06   ` Jeff Layton
  2023-03-21 14:08     ` Chuck Lever III
  0 siblings, 1 reply; 18+ messages in thread
From: Jeff Layton @ 2023-03-21 12:06 UTC (permalink / raw)
  To: Chuck Lever, SteveD; +Cc: linux-nfs

On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  utils/exportfs/exports.man |   45 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/utils/exportfs/exports.man b/utils/exportfs/exports.man
> index 54b3f8776ea6..cca9bb7b4aeb 100644
> --- a/utils/exportfs/exports.man
> +++ b/utils/exportfs/exports.man
> @@ -125,7 +125,49 @@ In that case you may include multiple sec= options, and following options
>  will be enforced only for access using flavors listed in the immediately
>  preceding sec= option.  The only options that are permitted to vary in
>  this way are ro, rw, no_root_squash, root_squash, and all_squash.
> +.SS Transport layer security
> +The Linux NFS server supports the use of transport layer security to
> +protect RPC traffic between itself and its clients.
> +This can be in the form of a VPN, an ssh tunnel, or via RPC-with-TLS,
> +which uses TLSv1.3.

This is a little awkward, as the NFS server really isn't involved at all
at that level in the case of a VPN or ssh tunnel. How about:

The Linux NFS server supports transport layer security (TLS) to protect
RPC traffic between itself and its clients via RPC-with-TLS which uses
TLSv1.3. Alternately, RPC traffic can be secured via a VPN, ssh tunnel
or similar mechanism that encrypts traffic in a way that is transparent
to the server.

>  .PP
> +Administrators may choose to require the use of
> +RPC-with-TLS to protect access to individual exports.
> +This is particularly useful when using non-cryptographic security
> +flavors such as
> +.IR sec=sys .
> +The
> +.I xprtsec=
> +option, followed by a colon-delimited list of security policies,
> +can restrict access to the export to only clients that have negotiated
> +transport-layer security.
> +Currently supported transport layer security policies include:
> +.TP
> +.IR none
> +The server permits clients to access the export
> +without the use of transport layer security.
> +.TP
> +.IR tls
> +The server permits clients that have negotiated an RPC-with-TLS session
> +without peer authentication (confidentiality only) to access the export.
> +Clients are not required to offer an x.509 certificate
> +when establishing a transport layer security session.
> +.TP
> +.IR mtls
> +The server permits clients that have negotiated an RPC-with-TLS session
> +with peer authentication to access the export.
> +The server requires clients to offer an x.509 certificate
> +when establishing a transport layer security session.
> +.PP
> +The default setting is
> +.IR xprtsec=none:tls:mtls .

Shouldn't that list order be reversed? IOW, shouldn't we default to mtls
first since it's more secure?

It might also be good to spell out what the kernel does with an ordered
list here. In what order are these methods attempted and at what point
does the server give up?


> +This is also known as "opportunistic mode".
> +The server permits clients to use any transport layer security mechanism
> +to access the export.
> +.PP
> +The server administrator must install and configure
> +.BR tlshd
> +to handle transport layer security handshake requests from the local kernel.

In the event that tlshd isn't running, what happens? I assume we give up
on TLS at that point, but how long does it take for the kernel to
realize that it's not there?

>  .SS General Options
>  .BR exportfs
>  understands the following export options:
> @@ -581,7 +623,8 @@ a character class wildcard match.
>  .BR netgroup (5),
>  .BR mountd (8),
>  .BR nfsd (8),
> -.BR showmount (8).
> +.BR showmount (8),
> +.BR tlshd (8).
>  .\".SH DIAGNOSTICS
>  .\"An error parsing the file is reported using syslogd(8) as level NOTICE from
>  .\"a DAEMON whenever
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 4/4] exports.man: Add description of xprtsec= to exports(5)
  2023-03-21 12:06   ` Jeff Layton
@ 2023-03-21 14:08     ` Chuck Lever III
  2023-03-21 15:11       ` Jeff Layton
  0 siblings, 1 reply; 18+ messages in thread
From: Chuck Lever III @ 2023-03-21 14:08 UTC (permalink / raw)
  To: Jeff Layton; +Cc: Chuck Lever, Steve Dickson, Linux NFS Mailing List



> On Mar 21, 2023, at 8:06 AM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
>> From: Chuck Lever <chuck.lever@oracle.com>
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> utils/exportfs/exports.man |   45 +++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 44 insertions(+), 1 deletion(-)
>> 
>> diff --git a/utils/exportfs/exports.man b/utils/exportfs/exports.man
>> index 54b3f8776ea6..cca9bb7b4aeb 100644
>> --- a/utils/exportfs/exports.man
>> +++ b/utils/exportfs/exports.man
>> @@ -125,7 +125,49 @@ In that case you may include multiple sec= options, and following options
>> will be enforced only for access using flavors listed in the immediately
>> preceding sec= option.  The only options that are permitted to vary in
>> this way are ro, rw, no_root_squash, root_squash, and all_squash.
>> +.SS Transport layer security
>> +The Linux NFS server supports the use of transport layer security to
>> +protect RPC traffic between itself and its clients.
>> +This can be in the form of a VPN, an ssh tunnel, or via RPC-with-TLS,
>> +which uses TLSv1.3.
> 
> This is a little awkward, as the NFS server really isn't involved at all
> at that level in the case of a VPN or ssh tunnel. How about:
> 
> The Linux NFS server supports transport layer security (TLS) to protect
> RPC traffic between itself and its clients via RPC-with-TLS which uses
> TLSv1.3. Alternately, RPC traffic can be secured via a VPN, ssh tunnel
> or similar mechanism that encrypts traffic in a way that is transparent
> to the server.

Sure, that expresses what I was after.


>> .PP
>> +Administrators may choose to require the use of
>> +RPC-with-TLS to protect access to individual exports.
>> +This is particularly useful when using non-cryptographic security
>> +flavors such as
>> +.IR sec=sys .
>> +The
>> +.I xprtsec=
>> +option, followed by a colon-delimited list of security policies,
>> +can restrict access to the export to only clients that have negotiated
>> +transport-layer security.
>> +Currently supported transport layer security policies include:
>> +.TP
>> +.IR none
>> +The server permits clients to access the export
>> +without the use of transport layer security.
>> +.TP
>> +.IR tls
>> +The server permits clients that have negotiated an RPC-with-TLS session
>> +without peer authentication (confidentiality only) to access the export.
>> +Clients are not required to offer an x.509 certificate
>> +when establishing a transport layer security session.
>> +.TP
>> +.IR mtls
>> +The server permits clients that have negotiated an RPC-with-TLS session
>> +with peer authentication to access the export.
>> +The server requires clients to offer an x.509 certificate
>> +when establishing a transport layer security session.
>> +.PP
>> +The default setting is
>> +.IR xprtsec=none:tls:mtls .
> 
> Shouldn't that list order be reversed? IOW, shouldn't we default to mtls
> first since it's more secure?
> 
> It might also be good to spell out what the kernel does with an ordered
> list here. In what order are these methods attempted and at what point
> does the server give up?

There's no order to this list. It's simply a list of
transport layer security settings that the server will
permit clients to use on this transport.

The "ordered list" concept is from the MNT protocol.
For xprtsec, there's no communication or negotiation
of preferences with clients.


>> +This is also known as "opportunistic mode".
>> +The server permits clients to use any transport layer security mechanism
>> +to access the export.
>> +.PP
>> +The server administrator must install and configure
>> +.BR tlshd
>> +to handle transport layer security handshake requests from the local kernel.
> 
> In the event that tlshd isn't running, what happens? I assume we give up
> on TLS at that point, but how long does it take for the kernel to
> realize that it's not there?

If tlshd is not running, the handshake request fails immediately.
There's no timeout needed thanks to netlink multi-cast.


>> .SS General Options
>> .BR exportfs
>> understands the following export options:
>> @@ -581,7 +623,8 @@ a character class wildcard match.
>> .BR netgroup (5),
>> .BR mountd (8),
>> .BR nfsd (8),
>> -.BR showmount (8).
>> +.BR showmount (8),
>> +.BR tlshd (8).
>> .\".SH DIAGNOSTICS
>> .\"An error parsing the file is reported using syslogd(8) as level NOTICE from
>> .\"a DAEMON whenever
>> 
>> 
> 
> -- 
> Jeff Layton <jlayton@kernel.org>

--
Chuck Lever



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 4/4] exports.man: Add description of xprtsec= to exports(5)
  2023-03-21 14:08     ` Chuck Lever III
@ 2023-03-21 15:11       ` Jeff Layton
  0 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2023-03-21 15:11 UTC (permalink / raw)
  To: Chuck Lever III; +Cc: Chuck Lever, Steve Dickson, Linux NFS Mailing List

On Tue, 2023-03-21 at 14:08 +0000, Chuck Lever III wrote:
> 
> > On Mar 21, 2023, at 8:06 AM, Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
> > > From: Chuck Lever <chuck.lever@oracle.com>
> > > 
> > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > > ---
> > > utils/exportfs/exports.man |   45 +++++++++++++++++++++++++++++++++++++++++++-
> > > 1 file changed, 44 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/utils/exportfs/exports.man b/utils/exportfs/exports.man
> > > index 54b3f8776ea6..cca9bb7b4aeb 100644
> > > --- a/utils/exportfs/exports.man
> > > +++ b/utils/exportfs/exports.man
> > > @@ -125,7 +125,49 @@ In that case you may include multiple sec= options, and following options
> > > will be enforced only for access using flavors listed in the immediately
> > > preceding sec= option.  The only options that are permitted to vary in
> > > this way are ro, rw, no_root_squash, root_squash, and all_squash.
> > > +.SS Transport layer security
> > > +The Linux NFS server supports the use of transport layer security to
> > > +protect RPC traffic between itself and its clients.
> > > +This can be in the form of a VPN, an ssh tunnel, or via RPC-with-TLS,
> > > +which uses TLSv1.3.
> > 
> > This is a little awkward, as the NFS server really isn't involved at all
> > at that level in the case of a VPN or ssh tunnel. How about:
> > 
> > The Linux NFS server supports transport layer security (TLS) to protect
> > RPC traffic between itself and its clients via RPC-with-TLS which uses
> > TLSv1.3. Alternately, RPC traffic can be secured via a VPN, ssh tunnel
> > or similar mechanism that encrypts traffic in a way that is transparent
> > to the server.
> 
> Sure, that expresses what I was after.
> 
> 
> > > .PP
> > > +Administrators may choose to require the use of
> > > +RPC-with-TLS to protect access to individual exports.
> > > +This is particularly useful when using non-cryptographic security
> > > +flavors such as
> > > +.IR sec=sys .
> > > +The
> > > +.I xprtsec=
> > > +option, followed by a colon-delimited list of security policies,
> > > +can restrict access to the export to only clients that have negotiated
> > > +transport-layer security.
> > > +Currently supported transport layer security policies include:
> > > +.TP
> > > +.IR none
> > > +The server permits clients to access the export
> > > +without the use of transport layer security.
> > > +.TP
> > > +.IR tls
> > > +The server permits clients that have negotiated an RPC-with-TLS session
> > > +without peer authentication (confidentiality only) to access the export.
> > > +Clients are not required to offer an x.509 certificate
> > > +when establishing a transport layer security session.
> > > +.TP
> > > +.IR mtls
> > > +The server permits clients that have negotiated an RPC-with-TLS session
> > > +with peer authentication to access the export.
> > > +The server requires clients to offer an x.509 certificate
> > > +when establishing a transport layer security session.
> > > +.PP
> > > +The default setting is
> > > +.IR xprtsec=none:tls:mtls .
> > 
> > Shouldn't that list order be reversed? IOW, shouldn't we default to mtls
> > first since it's more secure?
> > 
> > It might also be good to spell out what the kernel does with an ordered
> > list here. In what order are these methods attempted and at what point
> > does the server give up?
> 
> There's no order to this list. It's simply a list of
> transport layer security settings that the server will
> permit clients to use on this transport.
> 
> The "ordered list" concept is from the MNT protocol.
> For xprtsec, there's no communication or negotiation
> of preferences with clients.
> 

Duh, of course. That makes sense.

> 
> > > +This is also known as "opportunistic mode".
> > > +The server permits clients to use any transport layer security mechanism
> > > +to access the export.
> > > +.PP
> > > +The server administrator must install and configure
> > > +.BR tlshd
> > > +to handle transport layer security handshake requests from the local kernel.
> > 
> > In the event that tlshd isn't running, what happens? I assume we give up
> > on TLS at that point, but how long does it take for the kernel to
> > realize that it's not there?
> 
> If tlshd is not running, the handshake request fails immediately.
> There's no timeout needed thanks to netlink multi-cast.
> 

Good, thanks!

> 
> > > .SS General Options
> > > .BR exportfs
> > > understands the following export options:
> > > @@ -581,7 +623,8 @@ a character class wildcard match.
> > > .BR netgroup (5),
> > > .BR mountd (8),
> > > .BR nfsd (8),
> > > -.BR showmount (8).
> > > +.BR showmount (8),
> > > +.BR tlshd (8).
> > > .\".SH DIAGNOSTICS
> > > .\"An error parsing the file is reported using syslogd(8) as level NOTICE from
> > > .\"a DAEMON whenever
> > > 
> > > 
> > 
> > -- 
> > Jeff Layton <jlayton@kernel.org>
> 
> --
> Chuck Lever
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/4] exports: Add an xprtsec= export option
  2023-03-21 11:55   ` Jeff Layton
@ 2023-03-21 18:08     ` Chuck Lever III
  2023-03-21 18:58       ` Jeff Layton
  0 siblings, 1 reply; 18+ messages in thread
From: Chuck Lever III @ 2023-03-21 18:08 UTC (permalink / raw)
  To: Jeff Layton; +Cc: Chuck Lever, Steve Dickson, Linux NFS Mailing List



> On Mar 21, 2023, at 7:55 AM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
>> From: Chuck Lever <chuck.lever@oracle.com>
>> 
>> The overall goal is to enable administrators to require the use of
>> transport layer security when clients access particular exports.
>> 
>> This patch adds support to exportfs to parse and display a new
>> xprtsec= export option. The setting is not yet passed to the kernel.
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> support/include/nfs/export.h |    6 +++
>> support/include/nfslib.h     |   14 +++++++
>> support/nfs/exports.c        |   85 ++++++++++++++++++++++++++++++++++++++++++
>> utils/exportfs/exportfs.c    |    1 
>> 4 files changed, 106 insertions(+)
>> 
>> diff --git a/support/include/nfs/export.h b/support/include/nfs/export.h
>> index 0eca828ee3ad..b29c6fa4f554 100644
>> --- a/support/include/nfs/export.h
>> +++ b/support/include/nfs/export.h
>> @@ -40,4 +40,10 @@
>> #define NFSEXP_OLD_SECINFO_FLAGS (NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
>> 					| NFSEXP_ALLSQUASH)
>> 
>> +enum {
>> +	NFSEXP_XPRTSEC_NONE = 1,
>> +	NFSEXP_XPRTSEC_TLS = 2,
>> +	NFSEXP_XPRTSEC_MTLS = 3,
>> +};
>> +
> 
> Can we put these into a uapi header somewhere and then just have
> nfs-utils use those if they're defined?

I moved these to include/uapi/linux/nfsd/export.h in the
kernel patches, and adjust the nfs-utils patches to use the
same numeric values in exportfs as the kernel.

But it's not clear how a uAPI header would become visible
during, say, an RPM build of nfs-utils. Does anyone know
how that works? The kernel docs I've read suggest uAPI is
for user space tools that actually live in the kernel source
tree.

I think the cases where only user space or only the kernel
support xprtsec should work OK: the kernel has a default
transport layer security policy of "all ok" and old kernels
ignore export options from user space they don't recognize.


>> #endif /* _NSF_EXPORT_H */
>> diff --git a/support/include/nfslib.h b/support/include/nfslib.h
>> index 6faba71bf0cd..9a188fb84790 100644
>> --- a/support/include/nfslib.h
>> +++ b/support/include/nfslib.h
>> @@ -62,6 +62,18 @@ struct sec_entry {
>> 	int flags;
>> };
>> 
>> +#define XPRTSECMODE_COUNT 4
>> +
>> +struct xprtsec_info {
>> +	const char		*name;
>> +	int			number;
>> +};
>> +
>> +struct xprtsec_entry {
>> +	const struct xprtsec_info *info;
>> +	int			flags;
>> +};
>> +
>> /*
>>  * Data related to a single exports entry as returned by getexportent.
>>  * FIXME: export options should probably be parsed at a later time to
>> @@ -83,6 +95,7 @@ struct exportent {
>> 	char *          e_fslocdata;
>> 	char *		e_uuid;
>> 	struct sec_entry e_secinfo[SECFLAVOR_COUNT+1];
>> +	struct xprtsec_entry e_xprtsec[XPRTSECMODE_COUNT + 1];
>> 	unsigned int	e_ttl;
>> 	char *		e_realpath;
>> };
>> @@ -99,6 +112,7 @@ struct rmtabent {
>> void			setexportent(char *fname, char *type);
>> struct exportent *	getexportent(int,int);
>> void 			secinfo_show(FILE *fp, struct exportent *ep);
>> +void			xprtsecinfo_show(FILE *fp, struct exportent *ep);
>> void			putexportent(struct exportent *xep);
>> void			endexportent(void);
>> struct exportent *	mkexportent(char *hname, char *path, char *opts);
>> diff --git a/support/nfs/exports.c b/support/nfs/exports.c
>> index 7f12383981c3..da8ace3a65fd 100644
>> --- a/support/nfs/exports.c
>> +++ b/support/nfs/exports.c
>> @@ -99,6 +99,7 @@ static void init_exportent (struct exportent *ee, int fromkernel)
>> 	ee->e_fslocmethod = FSLOC_NONE;
>> 	ee->e_fslocdata = NULL;
>> 	ee->e_secinfo[0].flav = NULL;
>> +	ee->e_xprtsec[0].info = NULL;
>> 	ee->e_nsquids = 0;
>> 	ee->e_nsqgids = 0;
>> 	ee->e_uuid = NULL;
>> @@ -248,6 +249,17 @@ void secinfo_show(FILE *fp, struct exportent *ep)
>> 	}
>> }
>> 
>> +void xprtsecinfo_show(FILE *fp, struct exportent *ep)
>> +{
>> +	struct xprtsec_entry *p1, *p2;
>> +
>> +	for (p1 = ep->e_xprtsec; p1->info; p1 = p2) {
>> +		fprintf(fp, ",xprtsec=%s", p1->info->name);
>> +		for (p2 = p1 + 1; p2->info && (p1->flags == p2->flags); p2++)
>> +			fprintf(fp, ":%s", p2->info->name);
>> +	}
>> +}
>> +
>> static void
>> fprintpath(FILE *fp, const char *path)
>> {
>> @@ -344,6 +356,7 @@ putexportent(struct exportent *ep)
>> 	}
>> 	fprintf(fp, "anonuid=%d,anongid=%d", ep->e_anonuid, ep->e_anongid);
>> 	secinfo_show(fp, ep);
>> +	xprtsecinfo_show(fp, ep);
>> 	fprintf(fp, ")\n");
>> }
>> 
>> @@ -482,6 +495,75 @@ static unsigned int parse_flavors(char *str, struct exportent *ep)
>> 	return out;
>> }
>> 
>> +static const struct xprtsec_info xprtsec_name2info[] = {
>> +	{ "none",	NFSEXP_XPRTSEC_NONE },
>> +	{ "tls",	NFSEXP_XPRTSEC_TLS },
>> +	{ "mtls",	NFSEXP_XPRTSEC_MTLS },
>> +	{ NULL,		0 }
>> +};
>> +
>> +static const struct xprtsec_info *find_xprtsec_info(const char *name)
>> +{
>> +	const struct xprtsec_info *info;
>> +
>> +	for (info = xprtsec_name2info; info->name; info++)
>> +		if (strcmp(info->name, name) == 0)
>> +			return info;
>> +	return NULL;
>> +}
>> +
>> +/*
>> + * Append the given xprtsec mode to the exportent's e_xprtsec array,
>> + * or do nothing if it's already there. Returns the index of flavor in
>> + * the resulting array in any case.
>> + */
>> +static int xprtsec_addmode(const struct xprtsec_info *info, struct exportent *ep)
>> +{
>> +	struct xprtsec_entry *p;
>> +
>> +	for (p = ep->e_xprtsec; p->info; p++)
>> +		if (p->info == info || p->info->number == info->number)
>> +			return p - ep->e_xprtsec;
>> +
>> +	if (p - ep->e_xprtsec >= XPRTSECMODE_COUNT) {
>> +		xlog(L_ERROR, "more than %d xprtsec modes on an export\n",
>> +			XPRTSECMODE_COUNT);
>> +		return -1;
>> +	}
>> +	p->info = info;
>> +	p->flags = ep->e_flags;
>> +	(p + 1)->info = NULL;
>> +	return p - ep->e_xprtsec;
>> +}
>> +
>> +/*
>> + * @str is a colon seperated list of transport layer security modes.
>> + * Their order is recorded in @ep, and a bitmap corresponding to the
>> + * list is returned.
>> + *
>> + * A zero return indicates an error.
>> + */
>> +static unsigned int parse_xprtsec(char *str, struct exportent *ep)
>> +{
>> +	unsigned int out = 0;
>> +	char *name;
>> +
>> +	while ((name = strsep(&str, ":"))) {
>> +		const struct xprtsec_info *info = find_xprtsec_info(name);
>> +		int bit;
>> +
>> +		if (!info) {
>> +			xlog(L_ERROR, "unknown xprtsec mode %s\n", name);
>> +			return 0;
>> +		}
>> +		bit = xprtsec_addmode(info, ep);
>> +		if (bit < 0)
>> +			return 0;
>> +		out |= 1 << bit;
>> +	}
>> +	return out;
>> +}
>> +
>> /* Sets the bits in @mask for the appropriate security flavor flags. */
>> static void setflags(int mask, unsigned int active, struct exportent *ep)
>> {
>> @@ -687,6 +769,9 @@ bad_option:
>> 			active = parse_flavors(opt+4, ep);
>> 			if (!active)
>> 				goto bad_option;
>> +		} else if (strncmp(opt, "xprtsec=", 8) == 0) {
>> +			if (!parse_xprtsec(opt + 8, ep))
>> +				goto bad_option;
>> 		} else {
>> 			xlog(L_ERROR, "%s:%d: unknown keyword \"%s\"\n",
>> 					flname, flline, opt);
>> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
>> index 6d79a5b3480d..37b9e4b3612d 100644
>> --- a/utils/exportfs/exportfs.c
>> +++ b/utils/exportfs/exportfs.c
>> @@ -743,6 +743,7 @@ dump(int verbose, int export_format)
>> #endif
>> 			}
>> 			secinfo_show(stdout, ep);
>> +			xprtsecinfo_show(stdout, ep);
>> 			printf("%c\n", (c != '(')? ')' : ' ');
>> 		}
>> 	}
>> 
>> 
> 
> -- 
> Jeff Layton <jlayton@kernel.org>

--
Chuck Lever



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/4] exports: Add an xprtsec= export option
  2023-03-21 18:08     ` Chuck Lever III
@ 2023-03-21 18:58       ` Jeff Layton
  2023-03-23 17:53         ` Steve Dickson
  0 siblings, 1 reply; 18+ messages in thread
From: Jeff Layton @ 2023-03-21 18:58 UTC (permalink / raw)
  To: Chuck Lever III; +Cc: Chuck Lever, Steve Dickson, Linux NFS Mailing List

On Tue, 2023-03-21 at 18:08 +0000, Chuck Lever III wrote:
> 
> > On Mar 21, 2023, at 7:55 AM, Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
> > > From: Chuck Lever <chuck.lever@oracle.com>
> > > 
> > > The overall goal is to enable administrators to require the use of
> > > transport layer security when clients access particular exports.
> > > 
> > > This patch adds support to exportfs to parse and display a new
> > > xprtsec= export option. The setting is not yet passed to the kernel.
> > > 
> > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > > ---
> > > support/include/nfs/export.h |    6 +++
> > > support/include/nfslib.h     |   14 +++++++
> > > support/nfs/exports.c        |   85 ++++++++++++++++++++++++++++++++++++++++++
> > > utils/exportfs/exportfs.c    |    1 
> > > 4 files changed, 106 insertions(+)
> > > 
> > > diff --git a/support/include/nfs/export.h b/support/include/nfs/export.h
> > > index 0eca828ee3ad..b29c6fa4f554 100644
> > > --- a/support/include/nfs/export.h
> > > +++ b/support/include/nfs/export.h
> > > @@ -40,4 +40,10 @@
> > > #define NFSEXP_OLD_SECINFO_FLAGS (NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
> > > 					| NFSEXP_ALLSQUASH)
> > > 
> > > +enum {
> > > +	NFSEXP_XPRTSEC_NONE = 1,
> > > +	NFSEXP_XPRTSEC_TLS = 2,
> > > +	NFSEXP_XPRTSEC_MTLS = 3,
> > > +};
> > > +
> > 
> > Can we put these into a uapi header somewhere and then just have
> > nfs-utils use those if they're defined?
> 
> I moved these to include/uapi/linux/nfsd/export.h in the
> kernel patches, and adjust the nfs-utils patches to use the
> same numeric values in exportfs as the kernel.
>
> But it's not clear how a uAPI header would become visible
> during, say, an RPM build of nfs-utils. Does anyone know
> how that works? The kernel docs I've read suggest uAPI is
> for user space tools that actually live in the kernel source
> tree.
> 

Unfortunately, you need to build on a box that has kernel headers from
an updated kernel.

The usual way to deal with this is to have a copy in the userland
sources but only define them if one of the relevant constants isn't
already defined.

So you'll probably want to keep something like this in the userland
tree:

#ifndef NFSEXP_XPRTSEC_NONE
# define NFSEXP_XPRTSEC_NONE 1
# define NFSEXP_XPRTSEC_TLS  2
# define NFSEXP_XPRTSEC_MTLS 3
#endif

There may be some way to do that and keep it as an enum too. I'm not
sure.

> I think the cases where only user space or only the kernel
> support xprtsec should work OK: the kernel has a default
> transport layer security policy of "all ok" and old kernels
> ignore export options from user space they don't recognize.
> 

Great!

> > > #endif /* _NSF_EXPORT_H */
> > > diff --git a/support/include/nfslib.h b/support/include/nfslib.h
> > > index 6faba71bf0cd..9a188fb84790 100644
> > > --- a/support/include/nfslib.h
> > > +++ b/support/include/nfslib.h
> > > @@ -62,6 +62,18 @@ struct sec_entry {
> > > 	int flags;
> > > };
> > > 
> > > +#define XPRTSECMODE_COUNT 4
> > > +
> > > +struct xprtsec_info {
> > > +	const char		*name;
> > > +	int			number;
> > > +};
> > > +
> > > +struct xprtsec_entry {
> > > +	const struct xprtsec_info *info;
> > > +	int			flags;
> > > +};
> > > +
> > > /*
> > >  * Data related to a single exports entry as returned by getexportent.
> > >  * FIXME: export options should probably be parsed at a later time to
> > > @@ -83,6 +95,7 @@ struct exportent {
> > > 	char *          e_fslocdata;
> > > 	char *		e_uuid;
> > > 	struct sec_entry e_secinfo[SECFLAVOR_COUNT+1];
> > > +	struct xprtsec_entry e_xprtsec[XPRTSECMODE_COUNT + 1];
> > > 	unsigned int	e_ttl;
> > > 	char *		e_realpath;
> > > };
> > > @@ -99,6 +112,7 @@ struct rmtabent {
> > > void			setexportent(char *fname, char *type);
> > > struct exportent *	getexportent(int,int);
> > > void 			secinfo_show(FILE *fp, struct exportent *ep);
> > > +void			xprtsecinfo_show(FILE *fp, struct exportent *ep);
> > > void			putexportent(struct exportent *xep);
> > > void			endexportent(void);
> > > struct exportent *	mkexportent(char *hname, char *path, char *opts);
> > > diff --git a/support/nfs/exports.c b/support/nfs/exports.c
> > > index 7f12383981c3..da8ace3a65fd 100644
> > > --- a/support/nfs/exports.c
> > > +++ b/support/nfs/exports.c
> > > @@ -99,6 +99,7 @@ static void init_exportent (struct exportent *ee, int fromkernel)
> > > 	ee->e_fslocmethod = FSLOC_NONE;
> > > 	ee->e_fslocdata = NULL;
> > > 	ee->e_secinfo[0].flav = NULL;
> > > +	ee->e_xprtsec[0].info = NULL;
> > > 	ee->e_nsquids = 0;
> > > 	ee->e_nsqgids = 0;
> > > 	ee->e_uuid = NULL;
> > > @@ -248,6 +249,17 @@ void secinfo_show(FILE *fp, struct exportent *ep)
> > > 	}
> > > }
> > > 
> > > +void xprtsecinfo_show(FILE *fp, struct exportent *ep)
> > > +{
> > > +	struct xprtsec_entry *p1, *p2;
> > > +
> > > +	for (p1 = ep->e_xprtsec; p1->info; p1 = p2) {
> > > +		fprintf(fp, ",xprtsec=%s", p1->info->name);
> > > +		for (p2 = p1 + 1; p2->info && (p1->flags == p2->flags); p2++)
> > > +			fprintf(fp, ":%s", p2->info->name);
> > > +	}
> > > +}
> > > +
> > > static void
> > > fprintpath(FILE *fp, const char *path)
> > > {
> > > @@ -344,6 +356,7 @@ putexportent(struct exportent *ep)
> > > 	}
> > > 	fprintf(fp, "anonuid=%d,anongid=%d", ep->e_anonuid, ep->e_anongid);
> > > 	secinfo_show(fp, ep);
> > > +	xprtsecinfo_show(fp, ep);
> > > 	fprintf(fp, ")\n");
> > > }
> > > 
> > > @@ -482,6 +495,75 @@ static unsigned int parse_flavors(char *str, struct exportent *ep)
> > > 	return out;
> > > }
> > > 
> > > +static const struct xprtsec_info xprtsec_name2info[] = {
> > > +	{ "none",	NFSEXP_XPRTSEC_NONE },
> > > +	{ "tls",	NFSEXP_XPRTSEC_TLS },
> > > +	{ "mtls",	NFSEXP_XPRTSEC_MTLS },
> > > +	{ NULL,		0 }
> > > +};
> > > +
> > > +static const struct xprtsec_info *find_xprtsec_info(const char *name)
> > > +{
> > > +	const struct xprtsec_info *info;
> > > +
> > > +	for (info = xprtsec_name2info; info->name; info++)
> > > +		if (strcmp(info->name, name) == 0)
> > > +			return info;
> > > +	return NULL;
> > > +}
> > > +
> > > +/*
> > > + * Append the given xprtsec mode to the exportent's e_xprtsec array,
> > > + * or do nothing if it's already there. Returns the index of flavor in
> > > + * the resulting array in any case.
> > > + */
> > > +static int xprtsec_addmode(const struct xprtsec_info *info, struct exportent *ep)
> > > +{
> > > +	struct xprtsec_entry *p;
> > > +
> > > +	for (p = ep->e_xprtsec; p->info; p++)
> > > +		if (p->info == info || p->info->number == info->number)
> > > +			return p - ep->e_xprtsec;
> > > +
> > > +	if (p - ep->e_xprtsec >= XPRTSECMODE_COUNT) {
> > > +		xlog(L_ERROR, "more than %d xprtsec modes on an export\n",
> > > +			XPRTSECMODE_COUNT);
> > > +		return -1;
> > > +	}
> > > +	p->info = info;
> > > +	p->flags = ep->e_flags;
> > > +	(p + 1)->info = NULL;
> > > +	return p - ep->e_xprtsec;
> > > +}
> > > +
> > > +/*
> > > + * @str is a colon seperated list of transport layer security modes.
> > > + * Their order is recorded in @ep, and a bitmap corresponding to the
> > > + * list is returned.
> > > + *
> > > + * A zero return indicates an error.
> > > + */
> > > +static unsigned int parse_xprtsec(char *str, struct exportent *ep)
> > > +{
> > > +	unsigned int out = 0;
> > > +	char *name;
> > > +
> > > +	while ((name = strsep(&str, ":"))) {
> > > +		const struct xprtsec_info *info = find_xprtsec_info(name);
> > > +		int bit;
> > > +
> > > +		if (!info) {
> > > +			xlog(L_ERROR, "unknown xprtsec mode %s\n", name);
> > > +			return 0;
> > > +		}
> > > +		bit = xprtsec_addmode(info, ep);
> > > +		if (bit < 0)
> > > +			return 0;
> > > +		out |= 1 << bit;
> > > +	}
> > > +	return out;
> > > +}
> > > +
> > > /* Sets the bits in @mask for the appropriate security flavor flags. */
> > > static void setflags(int mask, unsigned int active, struct exportent *ep)
> > > {
> > > @@ -687,6 +769,9 @@ bad_option:
> > > 			active = parse_flavors(opt+4, ep);
> > > 			if (!active)
> > > 				goto bad_option;
> > > +		} else if (strncmp(opt, "xprtsec=", 8) == 0) {
> > > +			if (!parse_xprtsec(opt + 8, ep))
> > > +				goto bad_option;
> > > 		} else {
> > > 			xlog(L_ERROR, "%s:%d: unknown keyword \"%s\"\n",
> > > 					flname, flline, opt);
> > > diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> > > index 6d79a5b3480d..37b9e4b3612d 100644
> > > --- a/utils/exportfs/exportfs.c
> > > +++ b/utils/exportfs/exportfs.c
> > > @@ -743,6 +743,7 @@ dump(int verbose, int export_format)
> > > #endif
> > > 			}
> > > 			secinfo_show(stdout, ep);
> > > +			xprtsecinfo_show(stdout, ep);
> > > 			printf("%c\n", (c != '(')? ')' : ' ');
> > > 		}
> > > 	}
> > > 
> > > 
> > 
> > -- 
> > Jeff Layton <jlayton@kernel.org>
> 
> --
> Chuck Lever
> 
> 

-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/4] exports: Add an xprtsec= export option
  2023-03-21 18:58       ` Jeff Layton
@ 2023-03-23 17:53         ` Steve Dickson
  2023-03-23 17:55           ` Chuck Lever III
  0 siblings, 1 reply; 18+ messages in thread
From: Steve Dickson @ 2023-03-23 17:53 UTC (permalink / raw)
  To: Jeff Layton, Chuck Lever III; +Cc: Chuck Lever, Linux NFS Mailing List


Sorry for the delayed response....

On 3/21/23 2:58 PM, Jeff Layton wrote:
> On Tue, 2023-03-21 at 18:08 +0000, Chuck Lever III wrote:
>>
>>> On Mar 21, 2023, at 7:55 AM, Jeff Layton <jlayton@kernel.org> wrote:
>>>
>>> On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
>>>> From: Chuck Lever <chuck.lever@oracle.com>
>>>>
>>>> The overall goal is to enable administrators to require the use of
>>>> transport layer security when clients access particular exports.
>>>>
>>>> This patch adds support to exportfs to parse and display a new
>>>> xprtsec= export option. The setting is not yet passed to the kernel.
>>>>
>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>>> ---
>>>> support/include/nfs/export.h |    6 +++
>>>> support/include/nfslib.h     |   14 +++++++
>>>> support/nfs/exports.c        |   85 ++++++++++++++++++++++++++++++++++++++++++
>>>> utils/exportfs/exportfs.c    |    1
>>>> 4 files changed, 106 insertions(+)
>>>>
>>>> diff --git a/support/include/nfs/export.h b/support/include/nfs/export.h
>>>> index 0eca828ee3ad..b29c6fa4f554 100644
>>>> --- a/support/include/nfs/export.h
>>>> +++ b/support/include/nfs/export.h
>>>> @@ -40,4 +40,10 @@
>>>> #define NFSEXP_OLD_SECINFO_FLAGS (NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
>>>> 					| NFSEXP_ALLSQUASH)
>>>>
>>>> +enum {
>>>> +	NFSEXP_XPRTSEC_NONE = 1,
>>>> +	NFSEXP_XPRTSEC_TLS = 2,
>>>> +	NFSEXP_XPRTSEC_MTLS = 3,
>>>> +};
>>>> +
>>>
>>> Can we put these into a uapi header somewhere and then just have
>>> nfs-utils use those if they're defined?
>>
>> I moved these to include/uapi/linux/nfsd/export.h in the
>> kernel patches, and adjust the nfs-utils patches to use the
>> same numeric values in exportfs as the kernel.
>>
>> But it's not clear how a uAPI header would become visible
>> during, say, an RPM build of nfs-utils. Does anyone know
>> how that works? The kernel docs I've read suggest uAPI is
>> for user space tools that actually live in the kernel source
>> tree.
>>
> 
> Unfortunately, you need to build on a box that has kernel headers from
> an updated kernel.
I would not like to add this dependency to nfs-utils or sub-packages.

> 
> The usual way to deal with this is to have a copy in the userland
> sources but only define them if one of the relevant constants isn't
> already defined.
> 
> So you'll probably want to keep something like this in the userland
> tree:
> 
> #ifndef NFSEXP_XPRTSEC_NONE
> # define NFSEXP_XPRTSEC_NONE 1
> # define NFSEXP_XPRTSEC_TLS  2
> # define NFSEXP_XPRTSEC_MTLS 3
> #endif
> 
> There may be some way to do that and keep it as an enum too. I'm not
> sure.
Is there a reason these could not be in export.h?

steved.

> 
>> I think the cases where only user space or only the kernel
>> support xprtsec should work OK: the kernel has a default
>> transport layer security policy of "all ok" and old kernels
>> ignore export options from user space they don't recognize.
>>
> 
> Great!
> 
>>>> #endif /* _NSF_EXPORT_H */
>>>> diff --git a/support/include/nfslib.h b/support/include/nfslib.h
>>>> index 6faba71bf0cd..9a188fb84790 100644
>>>> --- a/support/include/nfslib.h
>>>> +++ b/support/include/nfslib.h
>>>> @@ -62,6 +62,18 @@ struct sec_entry {
>>>> 	int flags;
>>>> };
>>>>
>>>> +#define XPRTSECMODE_COUNT 4
>>>> +
>>>> +struct xprtsec_info {
>>>> +	const char		*name;
>>>> +	int			number;
>>>> +};
>>>> +
>>>> +struct xprtsec_entry {
>>>> +	const struct xprtsec_info *info;
>>>> +	int			flags;
>>>> +};
>>>> +
>>>> /*
>>>>   * Data related to a single exports entry as returned by getexportent.
>>>>   * FIXME: export options should probably be parsed at a later time to
>>>> @@ -83,6 +95,7 @@ struct exportent {
>>>> 	char *          e_fslocdata;
>>>> 	char *		e_uuid;
>>>> 	struct sec_entry e_secinfo[SECFLAVOR_COUNT+1];
>>>> +	struct xprtsec_entry e_xprtsec[XPRTSECMODE_COUNT + 1];
>>>> 	unsigned int	e_ttl;
>>>> 	char *		e_realpath;
>>>> };
>>>> @@ -99,6 +112,7 @@ struct rmtabent {
>>>> void			setexportent(char *fname, char *type);
>>>> struct exportent *	getexportent(int,int);
>>>> void 			secinfo_show(FILE *fp, struct exportent *ep);
>>>> +void			xprtsecinfo_show(FILE *fp, struct exportent *ep);
>>>> void			putexportent(struct exportent *xep);
>>>> void			endexportent(void);
>>>> struct exportent *	mkexportent(char *hname, char *path, char *opts);
>>>> diff --git a/support/nfs/exports.c b/support/nfs/exports.c
>>>> index 7f12383981c3..da8ace3a65fd 100644
>>>> --- a/support/nfs/exports.c
>>>> +++ b/support/nfs/exports.c
>>>> @@ -99,6 +99,7 @@ static void init_exportent (struct exportent *ee, int fromkernel)
>>>> 	ee->e_fslocmethod = FSLOC_NONE;
>>>> 	ee->e_fslocdata = NULL;
>>>> 	ee->e_secinfo[0].flav = NULL;
>>>> +	ee->e_xprtsec[0].info = NULL;
>>>> 	ee->e_nsquids = 0;
>>>> 	ee->e_nsqgids = 0;
>>>> 	ee->e_uuid = NULL;
>>>> @@ -248,6 +249,17 @@ void secinfo_show(FILE *fp, struct exportent *ep)
>>>> 	}
>>>> }
>>>>
>>>> +void xprtsecinfo_show(FILE *fp, struct exportent *ep)
>>>> +{
>>>> +	struct xprtsec_entry *p1, *p2;
>>>> +
>>>> +	for (p1 = ep->e_xprtsec; p1->info; p1 = p2) {
>>>> +		fprintf(fp, ",xprtsec=%s", p1->info->name);
>>>> +		for (p2 = p1 + 1; p2->info && (p1->flags == p2->flags); p2++)
>>>> +			fprintf(fp, ":%s", p2->info->name);
>>>> +	}
>>>> +}
>>>> +
>>>> static void
>>>> fprintpath(FILE *fp, const char *path)
>>>> {
>>>> @@ -344,6 +356,7 @@ putexportent(struct exportent *ep)
>>>> 	}
>>>> 	fprintf(fp, "anonuid=%d,anongid=%d", ep->e_anonuid, ep->e_anongid);
>>>> 	secinfo_show(fp, ep);
>>>> +	xprtsecinfo_show(fp, ep);
>>>> 	fprintf(fp, ")\n");
>>>> }
>>>>
>>>> @@ -482,6 +495,75 @@ static unsigned int parse_flavors(char *str, struct exportent *ep)
>>>> 	return out;
>>>> }
>>>>
>>>> +static const struct xprtsec_info xprtsec_name2info[] = {
>>>> +	{ "none",	NFSEXP_XPRTSEC_NONE },
>>>> +	{ "tls",	NFSEXP_XPRTSEC_TLS },
>>>> +	{ "mtls",	NFSEXP_XPRTSEC_MTLS },
>>>> +	{ NULL,		0 }
>>>> +};
>>>> +
>>>> +static const struct xprtsec_info *find_xprtsec_info(const char *name)
>>>> +{
>>>> +	const struct xprtsec_info *info;
>>>> +
>>>> +	for (info = xprtsec_name2info; info->name; info++)
>>>> +		if (strcmp(info->name, name) == 0)
>>>> +			return info;
>>>> +	return NULL;
>>>> +}
>>>> +
>>>> +/*
>>>> + * Append the given xprtsec mode to the exportent's e_xprtsec array,
>>>> + * or do nothing if it's already there. Returns the index of flavor in
>>>> + * the resulting array in any case.
>>>> + */
>>>> +static int xprtsec_addmode(const struct xprtsec_info *info, struct exportent *ep)
>>>> +{
>>>> +	struct xprtsec_entry *p;
>>>> +
>>>> +	for (p = ep->e_xprtsec; p->info; p++)
>>>> +		if (p->info == info || p->info->number == info->number)
>>>> +			return p - ep->e_xprtsec;
>>>> +
>>>> +	if (p - ep->e_xprtsec >= XPRTSECMODE_COUNT) {
>>>> +		xlog(L_ERROR, "more than %d xprtsec modes on an export\n",
>>>> +			XPRTSECMODE_COUNT);
>>>> +		return -1;
>>>> +	}
>>>> +	p->info = info;
>>>> +	p->flags = ep->e_flags;
>>>> +	(p + 1)->info = NULL;
>>>> +	return p - ep->e_xprtsec;
>>>> +}
>>>> +
>>>> +/*
>>>> + * @str is a colon seperated list of transport layer security modes.
>>>> + * Their order is recorded in @ep, and a bitmap corresponding to the
>>>> + * list is returned.
>>>> + *
>>>> + * A zero return indicates an error.
>>>> + */
>>>> +static unsigned int parse_xprtsec(char *str, struct exportent *ep)
>>>> +{
>>>> +	unsigned int out = 0;
>>>> +	char *name;
>>>> +
>>>> +	while ((name = strsep(&str, ":"))) {
>>>> +		const struct xprtsec_info *info = find_xprtsec_info(name);
>>>> +		int bit;
>>>> +
>>>> +		if (!info) {
>>>> +			xlog(L_ERROR, "unknown xprtsec mode %s\n", name);
>>>> +			return 0;
>>>> +		}
>>>> +		bit = xprtsec_addmode(info, ep);
>>>> +		if (bit < 0)
>>>> +			return 0;
>>>> +		out |= 1 << bit;
>>>> +	}
>>>> +	return out;
>>>> +}
>>>> +
>>>> /* Sets the bits in @mask for the appropriate security flavor flags. */
>>>> static void setflags(int mask, unsigned int active, struct exportent *ep)
>>>> {
>>>> @@ -687,6 +769,9 @@ bad_option:
>>>> 			active = parse_flavors(opt+4, ep);
>>>> 			if (!active)
>>>> 				goto bad_option;
>>>> +		} else if (strncmp(opt, "xprtsec=", 8) == 0) {
>>>> +			if (!parse_xprtsec(opt + 8, ep))
>>>> +				goto bad_option;
>>>> 		} else {
>>>> 			xlog(L_ERROR, "%s:%d: unknown keyword \"%s\"\n",
>>>> 					flname, flline, opt);
>>>> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
>>>> index 6d79a5b3480d..37b9e4b3612d 100644
>>>> --- a/utils/exportfs/exportfs.c
>>>> +++ b/utils/exportfs/exportfs.c
>>>> @@ -743,6 +743,7 @@ dump(int verbose, int export_format)
>>>> #endif
>>>> 			}
>>>> 			secinfo_show(stdout, ep);
>>>> +			xprtsecinfo_show(stdout, ep);
>>>> 			printf("%c\n", (c != '(')? ')' : ' ');
>>>> 		}
>>>> 	}
>>>>
>>>>
>>>
>>> -- 
>>> Jeff Layton <jlayton@kernel.org>
>>
>> --
>> Chuck Lever
>>
>>
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/4] exports: Add an xprtsec= export option
  2023-03-23 17:53         ` Steve Dickson
@ 2023-03-23 17:55           ` Chuck Lever III
  0 siblings, 0 replies; 18+ messages in thread
From: Chuck Lever III @ 2023-03-23 17:55 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Jeff Layton, Chuck Lever, Linux NFS Mailing List



> On Mar 23, 2023, at 1:53 PM, Steve Dickson <steved@redhat.com> wrote:
> 
> 
> Sorry for the delayed response....
> 
> On 3/21/23 2:58 PM, Jeff Layton wrote:
>> On Tue, 2023-03-21 at 18:08 +0000, Chuck Lever III wrote:
>>> 
>>>> On Mar 21, 2023, at 7:55 AM, Jeff Layton <jlayton@kernel.org> wrote:
>>>> 
>>>> On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
>>>>> From: Chuck Lever <chuck.lever@oracle.com>
>>>>> 
>>>>> The overall goal is to enable administrators to require the use of
>>>>> transport layer security when clients access particular exports.
>>>>> 
>>>>> This patch adds support to exportfs to parse and display a new
>>>>> xprtsec= export option. The setting is not yet passed to the kernel.
>>>>> 
>>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>>>> ---
>>>>> support/include/nfs/export.h |    6 +++
>>>>> support/include/nfslib.h     |   14 +++++++
>>>>> support/nfs/exports.c        |   85 ++++++++++++++++++++++++++++++++++++++++++
>>>>> utils/exportfs/exportfs.c    |    1
>>>>> 4 files changed, 106 insertions(+)
>>>>> 
>>>>> diff --git a/support/include/nfs/export.h b/support/include/nfs/export.h
>>>>> index 0eca828ee3ad..b29c6fa4f554 100644
>>>>> --- a/support/include/nfs/export.h
>>>>> +++ b/support/include/nfs/export.h
>>>>> @@ -40,4 +40,10 @@
>>>>> #define NFSEXP_OLD_SECINFO_FLAGS (NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
>>>>> 					| NFSEXP_ALLSQUASH)
>>>>> 
>>>>> +enum {
>>>>> +	NFSEXP_XPRTSEC_NONE = 1,
>>>>> +	NFSEXP_XPRTSEC_TLS = 2,
>>>>> +	NFSEXP_XPRTSEC_MTLS = 3,
>>>>> +};
>>>>> +
>>>> 
>>>> Can we put these into a uapi header somewhere and then just have
>>>> nfs-utils use those if they're defined?
>>> 
>>> I moved these to include/uapi/linux/nfsd/export.h in the
>>> kernel patches, and adjust the nfs-utils patches to use the
>>> same numeric values in exportfs as the kernel.
>>> 
>>> But it's not clear how a uAPI header would become visible
>>> during, say, an RPM build of nfs-utils. Does anyone know
>>> how that works? The kernel docs I've read suggest uAPI is
>>> for user space tools that actually live in the kernel source
>>> tree.
>>> 
>> Unfortunately, you need to build on a box that has kernel headers from
>> an updated kernel.
> I would not like to add this dependency to nfs-utils or sub-packages.
> 
>> The usual way to deal with this is to have a copy in the userland
>> sources but only define them if one of the relevant constants isn't
>> already defined.
>> So you'll probably want to keep something like this in the userland
>> tree:
>> #ifndef NFSEXP_XPRTSEC_NONE
>> # define NFSEXP_XPRTSEC_NONE 1
>> # define NFSEXP_XPRTSEC_TLS  2
>> # define NFSEXP_XPRTSEC_MTLS 3
>> #endif
>> There may be some way to do that and keep it as an enum too. I'm not
>> sure.
> Is there a reason these could not be in export.h?

That's where I put them.

Jeff's thought was to use uapi, but that doesn't sound workable
at the moment.


> steved.
> 
>>> I think the cases where only user space or only the kernel
>>> support xprtsec should work OK: the kernel has a default
>>> transport layer security policy of "all ok" and old kernels
>>> ignore export options from user space they don't recognize.
>>> 
>> Great!
>>>>> #endif /* _NSF_EXPORT_H */
>>>>> diff --git a/support/include/nfslib.h b/support/include/nfslib.h
>>>>> index 6faba71bf0cd..9a188fb84790 100644
>>>>> --- a/support/include/nfslib.h
>>>>> +++ b/support/include/nfslib.h
>>>>> @@ -62,6 +62,18 @@ struct sec_entry {
>>>>> 	int flags;
>>>>> };
>>>>> 
>>>>> +#define XPRTSECMODE_COUNT 4
>>>>> +
>>>>> +struct xprtsec_info {
>>>>> +	const char		*name;
>>>>> +	int			number;
>>>>> +};
>>>>> +
>>>>> +struct xprtsec_entry {
>>>>> +	const struct xprtsec_info *info;
>>>>> +	int			flags;
>>>>> +};
>>>>> +
>>>>> /*
>>>>>  * Data related to a single exports entry as returned by getexportent.
>>>>>  * FIXME: export options should probably be parsed at a later time to
>>>>> @@ -83,6 +95,7 @@ struct exportent {
>>>>> 	char *          e_fslocdata;
>>>>> 	char *		e_uuid;
>>>>> 	struct sec_entry e_secinfo[SECFLAVOR_COUNT+1];
>>>>> +	struct xprtsec_entry e_xprtsec[XPRTSECMODE_COUNT + 1];
>>>>> 	unsigned int	e_ttl;
>>>>> 	char *		e_realpath;
>>>>> };
>>>>> @@ -99,6 +112,7 @@ struct rmtabent {
>>>>> void			setexportent(char *fname, char *type);
>>>>> struct exportent *	getexportent(int,int);
>>>>> void 			secinfo_show(FILE *fp, struct exportent *ep);
>>>>> +void			xprtsecinfo_show(FILE *fp, struct exportent *ep);
>>>>> void			putexportent(struct exportent *xep);
>>>>> void			endexportent(void);
>>>>> struct exportent *	mkexportent(char *hname, char *path, char *opts);
>>>>> diff --git a/support/nfs/exports.c b/support/nfs/exports.c
>>>>> index 7f12383981c3..da8ace3a65fd 100644
>>>>> --- a/support/nfs/exports.c
>>>>> +++ b/support/nfs/exports.c
>>>>> @@ -99,6 +99,7 @@ static void init_exportent (struct exportent *ee, int fromkernel)
>>>>> 	ee->e_fslocmethod = FSLOC_NONE;
>>>>> 	ee->e_fslocdata = NULL;
>>>>> 	ee->e_secinfo[0].flav = NULL;
>>>>> +	ee->e_xprtsec[0].info = NULL;
>>>>> 	ee->e_nsquids = 0;
>>>>> 	ee->e_nsqgids = 0;
>>>>> 	ee->e_uuid = NULL;
>>>>> @@ -248,6 +249,17 @@ void secinfo_show(FILE *fp, struct exportent *ep)
>>>>> 	}
>>>>> }
>>>>> 
>>>>> +void xprtsecinfo_show(FILE *fp, struct exportent *ep)
>>>>> +{
>>>>> +	struct xprtsec_entry *p1, *p2;
>>>>> +
>>>>> +	for (p1 = ep->e_xprtsec; p1->info; p1 = p2) {
>>>>> +		fprintf(fp, ",xprtsec=%s", p1->info->name);
>>>>> +		for (p2 = p1 + 1; p2->info && (p1->flags == p2->flags); p2++)
>>>>> +			fprintf(fp, ":%s", p2->info->name);
>>>>> +	}
>>>>> +}
>>>>> +
>>>>> static void
>>>>> fprintpath(FILE *fp, const char *path)
>>>>> {
>>>>> @@ -344,6 +356,7 @@ putexportent(struct exportent *ep)
>>>>> 	}
>>>>> 	fprintf(fp, "anonuid=%d,anongid=%d", ep->e_anonuid, ep->e_anongid);
>>>>> 	secinfo_show(fp, ep);
>>>>> +	xprtsecinfo_show(fp, ep);
>>>>> 	fprintf(fp, ")\n");
>>>>> }
>>>>> 
>>>>> @@ -482,6 +495,75 @@ static unsigned int parse_flavors(char *str, struct exportent *ep)
>>>>> 	return out;
>>>>> }
>>>>> 
>>>>> +static const struct xprtsec_info xprtsec_name2info[] = {
>>>>> +	{ "none",	NFSEXP_XPRTSEC_NONE },
>>>>> +	{ "tls",	NFSEXP_XPRTSEC_TLS },
>>>>> +	{ "mtls",	NFSEXP_XPRTSEC_MTLS },
>>>>> +	{ NULL,		0 }
>>>>> +};
>>>>> +
>>>>> +static const struct xprtsec_info *find_xprtsec_info(const char *name)
>>>>> +{
>>>>> +	const struct xprtsec_info *info;
>>>>> +
>>>>> +	for (info = xprtsec_name2info; info->name; info++)
>>>>> +		if (strcmp(info->name, name) == 0)
>>>>> +			return info;
>>>>> +	return NULL;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Append the given xprtsec mode to the exportent's e_xprtsec array,
>>>>> + * or do nothing if it's already there. Returns the index of flavor in
>>>>> + * the resulting array in any case.
>>>>> + */
>>>>> +static int xprtsec_addmode(const struct xprtsec_info *info, struct exportent *ep)
>>>>> +{
>>>>> +	struct xprtsec_entry *p;
>>>>> +
>>>>> +	for (p = ep->e_xprtsec; p->info; p++)
>>>>> +		if (p->info == info || p->info->number == info->number)
>>>>> +			return p - ep->e_xprtsec;
>>>>> +
>>>>> +	if (p - ep->e_xprtsec >= XPRTSECMODE_COUNT) {
>>>>> +		xlog(L_ERROR, "more than %d xprtsec modes on an export\n",
>>>>> +			XPRTSECMODE_COUNT);
>>>>> +		return -1;
>>>>> +	}
>>>>> +	p->info = info;
>>>>> +	p->flags = ep->e_flags;
>>>>> +	(p + 1)->info = NULL;
>>>>> +	return p - ep->e_xprtsec;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * @str is a colon seperated list of transport layer security modes.
>>>>> + * Their order is recorded in @ep, and a bitmap corresponding to the
>>>>> + * list is returned.
>>>>> + *
>>>>> + * A zero return indicates an error.
>>>>> + */
>>>>> +static unsigned int parse_xprtsec(char *str, struct exportent *ep)
>>>>> +{
>>>>> +	unsigned int out = 0;
>>>>> +	char *name;
>>>>> +
>>>>> +	while ((name = strsep(&str, ":"))) {
>>>>> +		const struct xprtsec_info *info = find_xprtsec_info(name);
>>>>> +		int bit;
>>>>> +
>>>>> +		if (!info) {
>>>>> +			xlog(L_ERROR, "unknown xprtsec mode %s\n", name);
>>>>> +			return 0;
>>>>> +		}
>>>>> +		bit = xprtsec_addmode(info, ep);
>>>>> +		if (bit < 0)
>>>>> +			return 0;
>>>>> +		out |= 1 << bit;
>>>>> +	}
>>>>> +	return out;
>>>>> +}
>>>>> +
>>>>> /* Sets the bits in @mask for the appropriate security flavor flags. */
>>>>> static void setflags(int mask, unsigned int active, struct exportent *ep)
>>>>> {
>>>>> @@ -687,6 +769,9 @@ bad_option:
>>>>> 			active = parse_flavors(opt+4, ep);
>>>>> 			if (!active)
>>>>> 				goto bad_option;
>>>>> +		} else if (strncmp(opt, "xprtsec=", 8) == 0) {
>>>>> +			if (!parse_xprtsec(opt + 8, ep))
>>>>> +				goto bad_option;
>>>>> 		} else {
>>>>> 			xlog(L_ERROR, "%s:%d: unknown keyword \"%s\"\n",
>>>>> 					flname, flline, opt);
>>>>> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
>>>>> index 6d79a5b3480d..37b9e4b3612d 100644
>>>>> --- a/utils/exportfs/exportfs.c
>>>>> +++ b/utils/exportfs/exportfs.c
>>>>> @@ -743,6 +743,7 @@ dump(int verbose, int export_format)
>>>>> #endif
>>>>> 			}
>>>>> 			secinfo_show(stdout, ep);
>>>>> +			xprtsecinfo_show(stdout, ep);
>>>>> 			printf("%c\n", (c != '(')? ')' : ' ');
>>>>> 		}
>>>>> 	}
>>>>> 
>>>>> 
>>>> 
>>>> -- 
>>>> Jeff Layton <jlayton@kernel.org>
>>> 
>>> --
>>> Chuck Lever

--
Chuck Lever



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server
  2023-03-21 11:52 ` [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server Jeff Layton
@ 2023-03-23 17:57   ` Steve Dickson
  2023-03-23 18:01     ` Chuck Lever III
  0 siblings, 1 reply; 18+ messages in thread
From: Steve Dickson @ 2023-03-23 17:57 UTC (permalink / raw)
  To: Jeff Layton, Chuck Lever; +Cc: linux-nfs



On 3/21/23 7:52 AM, Jeff Layton wrote:
> On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
>> Hi Steve-
>>
>> This is server-side support for RPC-with-TLS, to accompany similar
>> support in the Linux NFS client. This implementation can support
>> both the opportunistic use of transport layer security (it will be
>> used if the client cares to) and the required use of transport
>> layer security (the server requires the client to use it to access
>> a particular export).
>>
>> Without any other user space componentry, this implementation will
>> be able to handle clients that request the use of RPC-with-TLS. To
>> support security policies that restrict access to exports based on
>> the client's use of TLS, modifications to exportfs and mountd are
>> needed. These can be found here:
>>
>> git://git.linux-nfs.org/projects/cel/nfs-utils.git
>>
>> They include an update to exports(5) explaining how to use the new
>> "xprtsec=" export option.
>>
>> The kernel patches, along with the the handshake upcall, are carried
>> in the topic-rpc-with-tls-upcall branch available from:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
>>
>> This was posted under separate cover.
>>
>> ---
>>
>> Chuck Lever (4):
>>        libexports: Fix whitespace damage in support/nfs/exports.c
>>        exports: Add an xprtsec= export option
>>        exportfs: Push xprtsec settings to the kernel
>>        exports.man: Add description of xprtsec= to exports(5)
>>
>>
>>   support/export/cache.c       |  15 ++++++
>>   support/include/nfs/export.h |   6 +++
>>   support/include/nfslib.h     |  14 +++++
>>   support/nfs/exports.c        | 100 ++++++++++++++++++++++++++++++++---
>>   utils/exportfs/exportfs.c    |   1 +
>>   utils/exportfs/exports.man   |  45 +++++++++++++++-
>>   6 files changed, 172 insertions(+), 9 deletions(-)
>>
> 
> Nice work, Chuck! This all looks pretty straightforward. I have a
> (minor) concern about potentially blocking nfsd threads for up to 20s in
> a handshake, but this seems like a good place to start.
Yes! But Will here be a V2 with the suggested changes?

It would be good to get a new release with these patches
in before the upcoming Bakeathon.

steved.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server
  2023-03-23 17:57   ` Steve Dickson
@ 2023-03-23 18:01     ` Chuck Lever III
  2023-03-24 18:35       ` Steve Dickson
  0 siblings, 1 reply; 18+ messages in thread
From: Chuck Lever III @ 2023-03-23 18:01 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Jeff Layton, Chuck Lever, Linux NFS Mailing List



> On Mar 23, 2023, at 1:57 PM, Steve Dickson <steved@redhat.com> wrote:
> 
> 
> 
> On 3/21/23 7:52 AM, Jeff Layton wrote:
>> On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
>>> Hi Steve-
>>> 
>>> This is server-side support for RPC-with-TLS, to accompany similar
>>> support in the Linux NFS client. This implementation can support
>>> both the opportunistic use of transport layer security (it will be
>>> used if the client cares to) and the required use of transport
>>> layer security (the server requires the client to use it to access
>>> a particular export).
>>> 
>>> Without any other user space componentry, this implementation will
>>> be able to handle clients that request the use of RPC-with-TLS. To
>>> support security policies that restrict access to exports based on
>>> the client's use of TLS, modifications to exportfs and mountd are
>>> needed. These can be found here:
>>> 
>>> git://git.linux-nfs.org/projects/cel/nfs-utils.git
>>> 
>>> They include an update to exports(5) explaining how to use the new
>>> "xprtsec=" export option.
>>> 
>>> The kernel patches, along with the the handshake upcall, are carried
>>> in the topic-rpc-with-tls-upcall branch available from:
>>> 
>>> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
>>> 
>>> This was posted under separate cover.
>>> 
>>> ---
>>> 
>>> Chuck Lever (4):
>>>       libexports: Fix whitespace damage in support/nfs/exports.c
>>>       exports: Add an xprtsec= export option
>>>       exportfs: Push xprtsec settings to the kernel
>>>       exports.man: Add description of xprtsec= to exports(5)
>>> 
>>> 
>>>  support/export/cache.c       |  15 ++++++
>>>  support/include/nfs/export.h |   6 +++
>>>  support/include/nfslib.h     |  14 +++++
>>>  support/nfs/exports.c        | 100 ++++++++++++++++++++++++++++++++---
>>>  utils/exportfs/exportfs.c    |   1 +
>>>  utils/exportfs/exports.man   |  45 +++++++++++++++-
>>>  6 files changed, 172 insertions(+), 9 deletions(-)
>>> 
>> Nice work, Chuck! This all looks pretty straightforward. I have a
>> (minor) concern about potentially blocking nfsd threads for up to 20s in
>> a handshake, but this seems like a good place to start.
> Yes! But Will here be a V2 with the suggested changes?

I've done the suggested updates in my private tree already, so I can post a refresh soon.


> It would be good to get a new release with these patches
> in before the upcoming Bakeathon.

The concept and operation of xprtsec= is pretty new... would there be a problem if all this were to change significantly after community review?

--
Chuck Lever



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server
  2023-03-23 18:01     ` Chuck Lever III
@ 2023-03-24 18:35       ` Steve Dickson
  2023-03-24 19:50         ` Chuck Lever III
  0 siblings, 1 reply; 18+ messages in thread
From: Steve Dickson @ 2023-03-24 18:35 UTC (permalink / raw)
  To: Chuck Lever III; +Cc: Jeff Layton, Chuck Lever, Linux NFS Mailing List



On 3/23/23 2:01 PM, Chuck Lever III wrote:
> 
> 
>> On Mar 23, 2023, at 1:57 PM, Steve Dickson <steved@redhat.com> wrote:
>>
>>
>>
>> On 3/21/23 7:52 AM, Jeff Layton wrote:
>>> On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
>>>> Hi Steve-
>>>>
>>>> This is server-side support for RPC-with-TLS, to accompany similar
>>>> support in the Linux NFS client. This implementation can support
>>>> both the opportunistic use of transport layer security (it will be
>>>> used if the client cares to) and the required use of transport
>>>> layer security (the server requires the client to use it to access
>>>> a particular export).
>>>>
>>>> Without any other user space componentry, this implementation will
>>>> be able to handle clients that request the use of RPC-with-TLS. To
>>>> support security policies that restrict access to exports based on
>>>> the client's use of TLS, modifications to exportfs and mountd are
>>>> needed. These can be found here:
>>>>
>>>> git://git.linux-nfs.org/projects/cel/nfs-utils.git
>>>>
>>>> They include an update to exports(5) explaining how to use the new
>>>> "xprtsec=" export option.
>>>>
>>>> The kernel patches, along with the the handshake upcall, are carried
>>>> in the topic-rpc-with-tls-upcall branch available from:
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
>>>>
>>>> This was posted under separate cover.
>>>>
>>>> ---
>>>>
>>>> Chuck Lever (4):
>>>>        libexports: Fix whitespace damage in support/nfs/exports.c
>>>>        exports: Add an xprtsec= export option
>>>>        exportfs: Push xprtsec settings to the kernel
>>>>        exports.man: Add description of xprtsec= to exports(5)
>>>>
>>>>
>>>>   support/export/cache.c       |  15 ++++++
>>>>   support/include/nfs/export.h |   6 +++
>>>>   support/include/nfslib.h     |  14 +++++
>>>>   support/nfs/exports.c        | 100 ++++++++++++++++++++++++++++++++---
>>>>   utils/exportfs/exportfs.c    |   1 +
>>>>   utils/exportfs/exports.man   |  45 +++++++++++++++-
>>>>   6 files changed, 172 insertions(+), 9 deletions(-)
>>>>
>>> Nice work, Chuck! This all looks pretty straightforward. I have a
>>> (minor) concern about potentially blocking nfsd threads for up to 20s in
>>> a handshake, but this seems like a good place to start.
>> Yes! But Will here be a V2 with the suggested changes?
> 
> I've done the suggested updates in my private tree already, so I can post a refresh soon.
Fair enough.
> 
> 
>> It would be good to get a new release with these patches
>> in before the upcoming Bakeathon.
> 
> The concept and operation of xprtsec= is pretty new... would there be a problem if all this were to change significantly after community review?
Not a problem at all! I was just thinking about get the patches into
Fedora rawhide to get some testing...

steved.

> 
> --
> Chuck Lever
> 
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server
  2023-03-24 18:35       ` Steve Dickson
@ 2023-03-24 19:50         ` Chuck Lever III
  0 siblings, 0 replies; 18+ messages in thread
From: Chuck Lever III @ 2023-03-24 19:50 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Jeff Layton, Chuck Lever, Linux NFS Mailing List



> On Mar 24, 2023, at 2:35 PM, Steve Dickson <steved@redhat.com> wrote:
> 
> 
> 
> On 3/23/23 2:01 PM, Chuck Lever III wrote:
>>> On Mar 23, 2023, at 1:57 PM, Steve Dickson <steved@redhat.com> wrote:
>>> 
>>> 
>>> 
>>> On 3/21/23 7:52 AM, Jeff Layton wrote:
>>>> On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
>>>>> Hi Steve-
>>>>> 
>>>>> This is server-side support for RPC-with-TLS, to accompany similar
>>>>> support in the Linux NFS client. This implementation can support
>>>>> both the opportunistic use of transport layer security (it will be
>>>>> used if the client cares to) and the required use of transport
>>>>> layer security (the server requires the client to use it to access
>>>>> a particular export).
>>>>> 
>>>>> Without any other user space componentry, this implementation will
>>>>> be able to handle clients that request the use of RPC-with-TLS. To
>>>>> support security policies that restrict access to exports based on
>>>>> the client's use of TLS, modifications to exportfs and mountd are
>>>>> needed. These can be found here:
>>>>> 
>>>>> git://git.linux-nfs.org/projects/cel/nfs-utils.git
>>>>> 
>>>>> They include an update to exports(5) explaining how to use the new
>>>>> "xprtsec=" export option.
>>>>> 
>>>>> The kernel patches, along with the the handshake upcall, are carried
>>>>> in the topic-rpc-with-tls-upcall branch available from:
>>>>> 
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
>>>>> 
>>>>> This was posted under separate cover.
>>>>> 
>>>>> ---
>>>>> 
>>>>> Chuck Lever (4):
>>>>>       libexports: Fix whitespace damage in support/nfs/exports.c
>>>>>       exports: Add an xprtsec= export option
>>>>>       exportfs: Push xprtsec settings to the kernel
>>>>>       exports.man: Add description of xprtsec= to exports(5)
>>>>> 
>>>>> 
>>>>>  support/export/cache.c       |  15 ++++++
>>>>>  support/include/nfs/export.h |   6 +++
>>>>>  support/include/nfslib.h     |  14 +++++
>>>>>  support/nfs/exports.c        | 100 ++++++++++++++++++++++++++++++++---
>>>>>  utils/exportfs/exportfs.c    |   1 +
>>>>>  utils/exportfs/exports.man   |  45 +++++++++++++++-
>>>>>  6 files changed, 172 insertions(+), 9 deletions(-)
>>>>> 
>>>> Nice work, Chuck! This all looks pretty straightforward. I have a
>>>> (minor) concern about potentially blocking nfsd threads for up to 20s in
>>>> a handshake, but this seems like a good place to start.
>>> Yes! But Will here be a V2 with the suggested changes?
>> I've done the suggested updates in my private tree already, so I can post a refresh soon.
> Fair enough.
>>> It would be good to get a new release with these patches
>>> in before the upcoming Bakeathon.
>> The concept and operation of xprtsec= is pretty new... would there be a problem if all this were to change significantly after community review?
> Not a problem at all! I was just thinking about get the patches into
> Fedora rawhide to get some testing...

To test, rawhide would need to have a ktls-utils package. Jeff and I
have discussed this ... he's volunteered to pull it in when netdev
has finally blessed the kernel/netlink pieces of this stack.

--
Chuck Lever



^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-03-24 19:51 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-20 14:35 [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server Chuck Lever
2023-03-20 14:35 ` [PATCH v1 1/4] libexports: Fix whitespace damage in support/nfs/exports.c Chuck Lever
2023-03-20 14:35 ` [PATCH v1 2/4] exports: Add an xprtsec= export option Chuck Lever
2023-03-21 11:55   ` Jeff Layton
2023-03-21 18:08     ` Chuck Lever III
2023-03-21 18:58       ` Jeff Layton
2023-03-23 17:53         ` Steve Dickson
2023-03-23 17:55           ` Chuck Lever III
2023-03-20 14:35 ` [PATCH v1 3/4] exportfs: Push xprtsec settings to the kernel Chuck Lever
2023-03-20 14:35 ` [PATCH v1 4/4] exports.man: Add description of xprtsec= to exports(5) Chuck Lever
2023-03-21 12:06   ` Jeff Layton
2023-03-21 14:08     ` Chuck Lever III
2023-03-21 15:11       ` Jeff Layton
2023-03-21 11:52 ` [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server Jeff Layton
2023-03-23 17:57   ` Steve Dickson
2023-03-23 18:01     ` Chuck Lever III
2023-03-24 18:35       ` Steve Dickson
2023-03-24 19:50         ` Chuck Lever III

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).