diff mbox

RFC: make labeled NFS opt-in

Message ID 20170104165636.GA17649@fieldses.org (mailing list archive)
State New, archived
Headers show

Commit Message

J. Bruce Fields Jan. 4, 2017, 4:56 p.m. UTC
This is something I should have noticed before merging labeled NFS:
labeled NFS shouldn't be on by default, among other reasons because it
assumes everybody's using consistent security policies.  As 4.2 is
getting turned on by default, this is starting to generate bug reports.

The patch below turns it off by default, and provides a new option to
turn it on per export.  The drawback is a regression on kernel upgrade
for anyone depending on labeled NFS, until they find the new option.  My
impression is that's a specialized use case that still has a small
number of users at this point, so I think we can get away with that.

The below is untested.  I have a (small) nfs-utils patch as well.

--b.

commit ea84969006fa
Author: J. Bruce Fields <bfields@redhat.com>
Date:   Tue Jan 3 12:30:11 2017 -0500

    nfsd: opt in to labeled nfs per export
    
    Currently turning on NFSv4.2 results in 4.2 clients suddenly seeing the
    individual file labels as they're set on the server.  This is not what
    they've previously seen, and not appropriate in may cases.  (In
    particular, if clients have heterogenous security policies then one
    client's labels may not even make sense to another.)  Labeled NFS should
    be opted in only in those cases when the administrator knows it makes
    sense.
    
    It's helpful to be able to turn 4.2 on by default, and otherwise the
    protocol upgrade seems free of regressions.  So, default labeled NFS to
    off and provide an export flag to reenable it.
    
    Users wanting labeled NFS support on an export will henceforth need to:
    
            - make sure 4.2 support is enabled on client and server (as
              before), and
            - upgrade the server nfs-utils to a version supporting the new
              "security_label" export flag.
            - set that "security_label" flag on the export.
    
    This is commit may be seen as a regression to anyone currently depending
    on security labels.  We believe those cases are currently rare.
    
    Reported-by: tibbs@math.uh.edu
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

J. Bruce Fields Jan. 12, 2017, 2:17 a.m. UTC | #1
On Wed, Jan 04, 2017 at 11:56:36AM -0500, bfields wrote:
> This is something I should have noticed before merging labeled NFS:
> labeled NFS shouldn't be on by default, among other reasons because it
> assumes everybody's using consistent security policies.  As 4.2 is
> getting turned on by default, this is starting to generate bug reports.
> 
> The patch below turns it off by default, and provides a new option to
> turn it on per export.  The drawback is a regression on kernel upgrade
> for anyone depending on labeled NFS, until they find the new option.  My
> impression is that's a specialized use case that still has a small
> number of users at this point, so I think we can get away with that.
> 
> The below is untested.  I have a (small) nfs-utils patch as well.

OK, I've tested now, the only problem was actually a preexisting
bug--I'll post the patches, plus the companion nfs-utils patch.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
William A. (Andy) Adamson Jan. 12, 2017, 3:29 p.m. UTC | #2
HI Bruce

NFSv4.2 labeled NFS provides 'guest mode' Mandatory Access Control
(MAC) where the client can enforce labeling and store a label on the
server, but the server itself does not enforce the same MAC as the
client because the client request thread label is unknown to the
server. RPCSEC_GSS version 3 label assertions asserts the client
thread label on the NFSD thread handling the request, and so along
with LNFS provides Full Mode MAC.

AFAICS the only time we want GSS3 label assertions is if LNFS is
enabled.  Does this sound right to you? If so, I will use this new per
export LNFS option to determine when GSS3 label assertions are
enabled.

-->Andy

On Wed, Jan 11, 2017 at 9:17 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> On Wed, Jan 04, 2017 at 11:56:36AM -0500, bfields wrote:
>> This is something I should have noticed before merging labeled NFS:
>> labeled NFS shouldn't be on by default, among other reasons because it
>> assumes everybody's using consistent security policies.  As 4.2 is
>> getting turned on by default, this is starting to generate bug reports.
>>
>> The patch below turns it off by default, and provides a new option to
>> turn it on per export.  The drawback is a regression on kernel upgrade
>> for anyone depending on labeled NFS, until they find the new option.  My
>> impression is that's a specialized use case that still has a small
>> number of users at this point, so I think we can get away with that.
>>
>> The below is untested.  I have a (small) nfs-utils patch as well.
>
> OK, I've tested now, the only problem was actually a preexisting
> bug--I'll post the patches, plus the companion nfs-utils patch.
>
> --b.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields Jan. 12, 2017, 4:10 p.m. UTC | #3
On Thu, Jan 12, 2017 at 10:29:14AM -0500, Andy Adamson wrote:
> NFSv4.2 labeled NFS provides 'guest mode' Mandatory Access Control
> (MAC) where the client can enforce labeling and store a label on the
> server, but the server itself does not enforce the same MAC as the
> client because the client request thread label is unknown to the
> server. RPCSEC_GSS version 3 label assertions asserts the client
> thread label on the NFSD thread handling the request, and so along
> with LNFS provides Full Mode MAC.
> 
> AFAICS the only time we want GSS3 label assertions is if LNFS is
> enabled.  Does this sound right to you? If so, I will use this new per
> export LNFS option to determine when GSS3 label assertions are
> enabled.

How do you disable or enable this on the server side?

I haven't been following the GSS3 development well, apologies.  So I
guess you must do something like:

	1. decode the GSS3 stuff in the RPC layer and store the
	resulting subject label somewhere like rqstp->rq_cred.
	2. in nfsd_setuser, set the nfsd thread's label to the label
	stored in rq_cred.

You probably need to have the processing in step 1 enabled all the time,
because you don't know which export you're going to be dealing with yet
at that point.

By step 2 you have the export.  So I guess you'd use the export option
to decide whether to silently ignore the label, or apply it to the nfsd
thread?

That might make sense.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
William A. (Andy) Adamson Jan. 12, 2017, 4:51 p.m. UTC | #4
On Thu, Jan 12, 2017 at 11:10 AM, J. Bruce Fields <bfields@fieldses.org> wrote:
> On Thu, Jan 12, 2017 at 10:29:14AM -0500, Andy Adamson wrote:
>> NFSv4.2 labeled NFS provides 'guest mode' Mandatory Access Control
>> (MAC) where the client can enforce labeling and store a label on the
>> server, but the server itself does not enforce the same MAC as the
>> client because the client request thread label is unknown to the
>> server. RPCSEC_GSS version 3 label assertions asserts the client
>> thread label on the NFSD thread handling the request, and so along
>> with LNFS provides Full Mode MAC.
>>
>> AFAICS the only time we want GSS3 label assertions is if LNFS is
>> enabled.  Does this sound right to you? If so, I will use this new per
>> export LNFS option to determine when GSS3 label assertions are
>> enabled.
>
> How do you disable or enable this on the server side?

In nfsd_setuser, if the rq_authop has the new set_label operation, and
selinux_is_enabled() then the label is asserted.

>
> I haven't been following the GSS3 development well, apologies.  So I
> guess you must do something like:
>
>         1. decode the GSS3 stuff in the RPC layer and store the
>         resulting subject label somewhere like rqstp->rq_cred.
>         2. in nfsd_setuser, set the nfsd thread's label to the label
>         stored in rq_cred.


Yes, I do both of those tasks.

>
> You probably need to have the processing in step 1 enabled all the time,
> because you don't know which export you're going to be dealing with yet
> at that point.

Yes.

>
> By step 2 you have the export.  So I guess you'd use the export option
> to decide whether to silently ignore the label, or apply it to the nfsd
> thread?

yes.

>
> That might make sense.

Good! That is easy to add and is what I am thinking.

I'm building the gss prototype against your latest nfsd-next. Once I
test it, i'll send in version 4.

-->Andy

>
> --b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 43e109cc0ccc..03600b2ea345 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1102,6 +1102,7 @@  static struct flags {
 	{ NFSEXP_NOAUTHNLM, {"insecure_locks", ""}},
 	{ NFSEXP_V4ROOT, {"v4root", ""}},
 	{ NFSEXP_PNFS, {"pnfs", ""}},
+	{ NFSEXP_SECURITY_LABEL, {"security_label"}},
 	{ 0, {"", ""}}
 };
 
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 74a6e573e061..6f0b89d1c09b 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -95,11 +95,15 @@  check_attr_support(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		   u32 *bmval, u32 *writable)
 {
 	struct dentry *dentry = cstate->current_fh.fh_dentry;
+	struct svc_export *exp = cstate->current_fh.fh_export;
 
 	if (!nfsd_attrs_supported(cstate->minorversion, bmval))
 		return nfserr_attrnotsupp;
 	if ((bmval[0] & FATTR4_WORD0_ACL) && !IS_POSIXACL(d_inode(dentry)))
 		return nfserr_attrnotsupp;
+	if ((bmval[2] & FATTR4_WORD2_SECURITY_LABEL) &&
+			!(exp->ex_flags & NFSEXP_SECURITY_LABEL))
+		return nfserr_attrnotsupp;
 	if (writable && !bmval_is_subset(bmval, writable))
 		return nfserr_inval;
 	if (writable && (bmval[2] & FATTR4_WORD2_MODE_UMASK) &&
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 64c262aef633..8475c4df9aa2 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2416,8 +2416,11 @@  nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
 	if ((bmval2 & FATTR4_WORD2_SECURITY_LABEL) ||
 	     bmval0 & FATTR4_WORD0_SUPPORTED_ATTRS) {
-		err = security_inode_getsecctx(d_inode(dentry),
+		if (exp->ex_flags & NFSEXP_SECURITY_LABEL)
+			err = security_inode_getsecctx(d_inode(dentry),
 						&context, &contextlen);
+		else
+			err = -EOPNOTSUPP;
 		contextsupport = (err == 0);
 		if (bmval2 & FATTR4_WORD2_SECURITY_LABEL) {
 			if (err == -EOPNOTSUPP)
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 26c6fdb4bf67..9e11ee1ec3a9 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -486,12 +486,14 @@  __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	__be32 error;
 	int host_error;
 	struct dentry *dentry;
+	struct svc_export *exp;
 
 	error = fh_verify(rqstp, fhp, 0 /* S_IFREG */, NFSD_MAY_SATTR);
 	if (error)
 		return error;
 
 	dentry = fhp->fh_dentry;
+	exp = fhp->fh_export;
 
 	inode_lock(d_inode(dentry));
 	host_error = security_inode_setsecctx(dentry, label->data, label->len);
diff --git a/include/uapi/linux/nfsd/export.h b/include/uapi/linux/nfsd/export.h
index 0df7bd5d2fb1..c3be256107c6 100644
--- a/include/uapi/linux/nfsd/export.h
+++ b/include/uapi/linux/nfsd/export.h
@@ -32,7 +32,8 @@ 
 #define NFSEXP_ASYNC		0x0010
 #define NFSEXP_GATHERED_WRITES	0x0020
 #define NFSEXP_NOREADDIRPLUS    0x0040
-/* 80 100 currently unused */
+#define NFSEXP_SECURITY_LABEL	0x0080
+/* 0x100 currently unused */
 #define NFSEXP_NOHIDE		0x0200
 #define NFSEXP_NOSUBTREECHECK	0x0400
 #define	NFSEXP_NOAUTHNLM	0x0800		/* Don't authenticate NLM requests - just trust */
@@ -53,7 +54,7 @@ 
 #define NFSEXP_PNFS		0x20000
 
 /* All flags that we claim to support.  (Note we don't support NOACL.) */
-#define NFSEXP_ALLFLAGS		0x3FE7F
+#define NFSEXP_ALLFLAGS		0x3FEFF
 
 /* The flags that may vary depending on security flavor: */
 #define NFSEXP_SECINFO_FLAGS	(NFSEXP_READONLY | NFSEXP_ROOTSQUASH \