2023-03-20 14:35:42

by Chuck Lever

[permalink] [raw]
Subject: [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server

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



2023-03-20 14:35:47

by Chuck Lever

[permalink] [raw]
Subject: [PATCH v1 1/4] libexports: Fix whitespace damage in support/nfs/exports.c

From: Chuck Lever <[email protected]>

Clean up.

Signed-off-by: Chuck Lever <[email protected]>
---
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);



2023-03-20 14:35:55

by Chuck Lever

[permalink] [raw]
Subject: [PATCH v1 2/4] exports: Add an xprtsec= export option

From: Chuck Lever <[email protected]>

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 <[email protected]>
---
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 != '(')? ')' : ' ');
}
}



2023-03-20 14:35:57

by Chuck Lever

[permalink] [raw]
Subject: [PATCH v1 3/4] exportfs: Push xprtsec settings to the kernel

From: Chuck Lever <[email protected]>

Signed-off-by: Chuck Lever <[email protected]>
---
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 {



2023-03-20 14:35:58

by Chuck Lever

[permalink] [raw]
Subject: [PATCH v1 4/4] exports.man: Add description of xprtsec= to exports(5)

From: Chuck Lever <[email protected]>

Signed-off-by: Chuck Lever <[email protected]>
---
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



2023-03-21 11:53:08

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server

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 <[email protected]>

2023-03-21 11:56:28

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH v1 2/4] exports: Add an xprtsec= export option

On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
> From: Chuck Lever <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>

2023-03-21 12:06:09

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH v1 4/4] exports.man: Add description of xprtsec= to exports(5)

On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
> From: Chuck Lever <[email protected]>
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> 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 <[email protected]>

2023-03-21 14:09:16

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH v1 4/4] exports.man: Add description of xprtsec= to exports(5)



> On Mar 21, 2023, at 8:06 AM, Jeff Layton <[email protected]> wrote:
>
> On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
>> From: Chuck Lever <[email protected]>
>>
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>> 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 <[email protected]>

--
Chuck Lever



2023-03-21 15:11:41

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH v1 4/4] exports.man: Add description of xprtsec= to exports(5)

On Tue, 2023-03-21 at 14:08 +0000, Chuck Lever III wrote:
>
> > On Mar 21, 2023, at 8:06 AM, Jeff Layton <[email protected]> wrote:
> >
> > On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
> > > From: Chuck Lever <[email protected]>
> > >
> > > Signed-off-by: Chuck Lever <[email protected]>
> > > ---
> > > 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 <[email protected]>
>
> --
> Chuck Lever
>
>

--
Jeff Layton <[email protected]>

2023-03-21 18:19:45

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH v1 2/4] exports: Add an xprtsec= export option



> On Mar 21, 2023, at 7:55 AM, Jeff Layton <[email protected]> wrote:
>
> On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
>> From: Chuck Lever <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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 <[email protected]>

--
Chuck Lever


2023-03-21 19:00:11

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH v1 2/4] exports: Add an xprtsec= export option

On Tue, 2023-03-21 at 18:08 +0000, Chuck Lever III wrote:
>
> > On Mar 21, 2023, at 7:55 AM, Jeff Layton <[email protected]> wrote:
> >
> > On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
> > > From: Chuck Lever <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> > > 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 <[email protected]>
>
> --
> Chuck Lever
>
>

--
Jeff Layton <[email protected]>

2023-03-23 17:59:08

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH v1 2/4] exports: Add an xprtsec= export option


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 <[email protected]> wrote:
>>>
>>> On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
>>>> From: Chuck Lever <[email protected]>
>>>>
>>>> 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 <[email protected]>
>>>> ---
>>>> 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 <[email protected]>
>>
>> --
>> Chuck Lever
>>
>>
>

2023-03-23 17:59:14

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH v1 2/4] exports: Add an xprtsec= export option



> On Mar 23, 2023, at 1:53 PM, Steve Dickson <[email protected]> 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 <[email protected]> wrote:
>>>>
>>>> On Mon, 2023-03-20 at 10:35 -0400, Chuck Lever wrote:
>>>>> From: Chuck Lever <[email protected]>
>>>>>
>>>>> 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 <[email protected]>
>>>>> ---
>>>>> 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 <[email protected]>
>>>
>>> --
>>> Chuck Lever

--
Chuck Lever


2023-03-23 17:59:26

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server



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.

2023-03-23 18:03:21

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server



> On Mar 23, 2023, at 1:57 PM, Steve Dickson <[email protected]> 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


2023-03-24 18:37:37

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server



On 3/23/23 2:01 PM, Chuck Lever III wrote:
>
>
>> On Mar 23, 2023, at 1:57 PM, Steve Dickson <[email protected]> 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
>
>

2023-03-24 19:58:15

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH v1 0/4] nfs-utils changes for RPC-with-TLS server



> On Mar 24, 2023, at 2:35 PM, Steve Dickson <[email protected]> wrote:
>
>
>
> On 3/23/23 2:01 PM, Chuck Lever III wrote:
>>> On Mar 23, 2023, at 1:57 PM, Steve Dickson <[email protected]> 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