secinfo_no_name question

PATCH In review Bryan Schumaker 5 participants 12 messages Apr 4, 2011 – Apr 11, 2011

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Bryan Schumaker

author

2011-04-04 13:43 UTC #0 lore raw

Hi Bruce,

I'm looking at secinfo_no_name on the client. RFC 5661 says to says to send PUTROOTFH followed by SECINFO_NO_NAME in the same compound and to use SECINFO_STYLE4_CURRENT_FH. My compound is: SEQUENCE, PUTROOTFH, SECINFO_NO_NAME. The server processes up to the PUTROOTFH, and then returns with NFS4ERR_WRONGSEC.

Am I doing something wrong? Is this a server problem?

Thanks,
Bryan

J. Bruce Fields

2011-04-04 15:14 UTC #1 lore raw

Quoted 5 lines · Bryan Schumaker

Hi Bruce,
I'm looking at secinfo_no_name on the client. RFC 5661 says to says to send PUTROOTFH followed by SECINFO_NO_NAME in the same compound and to use SECINFO_STYLE4_CURRENT_FH. My compound is: SEQUENCE, PUTROOTFH, SECINFO_NO_NAME. The server processes up to the PUTROOTFH, and then returns with NFS4ERR_WRONGSEC.
Am I doing something wrong? Is this a server problem?

Could be; is the compound is being sent with a security flavor that *isn't* permitted on the root export?

If so I believe the compound should have succeeded--the server needs some special exception there that we may have left out....

--b.

Bryan Schumaker

author

2011-04-04 15:22 UTC #2 lore raw

Quoted 5 lines · J. Bruce Fields

Quoted 4 lines · Bryan Schumaker
Hi Bruce,
I'm looking at secinfo_no_name on the client. RFC 5661 says to says to send PUTROOTFH followed by SECINFO_NO_NAME in the same compound and to use SECINFO_STYLE4_CURRENT_FH. My compound is: SEQUENCE, PUTROOTFH, SECINFO_NO_NAME. The server processes up to the PUTROOTFH, and then returns with NFS4ERR_WRONGSEC.

Quoted 7 lines

Quoted 1 line
Am I doing something wrong? Is this a server problem?
Could be; is the compound is being sent with a security flavor that *isn't* permitted on the root export?
If so I believe the compound should have succeeded--the server needs some special exception there that we may have left out....

The root export is set up with sec=null and my compound is using auth_unix.

Quoted 6 lines

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

Tom Haynes

2011-04-05 16:12 UTC #3 lore raw

Quoted 10 lines · Bryan Schumaker

Quoted 9 lines · J. Bruce Fields
Quoted 5 lines · Bryan Schumaker
Hi Bruce,
I'm looking at secinfo_no_name on the client. RFC 5661 says to says to send PUTROOTFH followed by SECINFO_NO_NAME in the same compound and to use SECINFO_STYLE4_CURRENT_FH. My compound is: SEQUENCE, PUTROOTFH, SECINFO_NO_NAME. The server processes up to the PUTROOTFH, and then returns with NFS4ERR_WRONGSEC.
Am I doing something wrong? Is this a server problem?
Could be; is the compound is being sent with a security flavor that *isn't* permitted on the root export?

It should never fail:

2.6.3.1.1.5. Put Filehandle Operation + SECINFO/SECINFO_NO_NAME

...

The NFSv4.1 server MUST NOT return NFS4ERR_WRONGSEC to a put filehandle operation that is immediately followed by SECINFO or SECINFO_NO_NAME. The NFSv4.1 server MUST NOT return NFS4ERR_WRONGSEC from SECINFO or SECINFO_NO_NAME.

Quoted 17 lines

Quoted 3 lines
If so I believe the compound should have succeeded--the server needs some special exception there that we may have left out....
The root export is set up with sec=null and my compound is using auth_unix.
Quoted 6 lines
--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected]
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines

Tom Haynes
ex-cfb

J. Bruce Fields

2011-04-10 16:25 UTC #4 lore raw

Quoted 14 lines · Tom Haynes

Quoted 10 lines · Bryan Schumaker
Quoted 9 lines · J. Bruce Fields
Quoted 5 lines · Bryan Schumaker
Hi Bruce,
I'm looking at secinfo_no_name on the client. RFC 5661 says to says to send PUTROOTFH followed by SECINFO_NO_NAME in the same compound and to use SECINFO_STYLE4_CURRENT_FH. My compound is: SEQUENCE, PUTROOTFH, SECINFO_NO_NAME. The server processes up to the PUTROOTFH, and then returns with NFS4ERR_WRONGSEC.
Am I doing something wrong? Is this a server problem?
Could be; is the compound is being sent with a security flavor that *isn't* permitted on the root export?
It should never fail:

Quoted 8 lines

2.6.3.1.1.5. Put Filehandle Operation + SECINFO/SECINFO_NO_NAME
...
The NFSv4.1 server MUST NOT return NFS4ERR_WRONGSEC to a put filehandle operation that is immediately followed by SECINFO or SECINFO_NO_NAME. The NFSv4.1 server MUST NOT return NFS4ERR_WRONGSEC from SECINFO or SECINFO_NO_NAME.

And I thought I'd implemented those recommendations at some point--I guess not! My attempt follows.

--b.

J. Bruce Fields

2011-04-10 16:29 UTC #5 lore raw

PATCH 1/5 nfsd: distinguish functions of NFSD_MAY_* flags

Most of the NFSD_MAY_* flags actually request permissions, but over the years we've accreted a few that modify the behavior of the permission or open code in other ways.

Distinguish the two cases a little more. In particular, allow the shortcut at the start of nfsd_permission to ignore the non-permission-requesting bits.

Signed-off-by J. Bruce Fields <[email protected]>

---
 fs/nfsd/vfs.c |    2 +-
 fs/nfsd/vfs.h |    3 +++
 2 files changed, 4 insertions(+), 1 deletions(-)

fs/nfsd/vfs.c +1 -1

index 2e1cebd..ac4f0b4 100644
@@ -2027,7 +2027,7 @@ nfsd_permission(struct svc_rqst *rqstp, struct svc_export *exp,
 	struct inode	*inode = dentry->d_inode;
 	int		err;
 
-	if (acc == NFSD_MAY_NOP)
+	if (acc & NFSD_MAY_MASK == NFSD_MAY_NOP)
 		return 0;
 #if 0
 	dprintk("nfsd: permission 0x%x%s%s%s%s%s%s%s mode 0%o%s%s%s\n",

fs/nfsd/vfs.h +3 -0

index 9a370a5..1036913 100644
@@ -17,6 +17,9 @@
 #define NFSD_MAY_SATTR		8
 #define NFSD_MAY_TRUNC		16
 #define NFSD_MAY_LOCK		32
+#define NFSD_MAY_MASK		63
+
+/* extra hints to permission and open routines: */
 #define NFSD_MAY_OWNER_OVERRIDE	64
 #define NFSD_MAY_LOCAL_ACCESS	128 /* IRIX doing local access check on device special file*/
 #define NFSD_MAY_BYPASS_GSS_ON_ROOT 256

Signature · 1 line

1.7.1

J. Bruce Fields

2011-04-10 16:29 UTC #8 lore raw

PATCH 2/5 nfsd4: allow fh_verify caller to skip pseudoflavor checks

In reply to J. Bruce Fields

Signed-off-by J. Bruce Fields <[email protected]>

---
 fs/nfsd/nfsfh.c |    2 +-
 fs/nfsd/vfs.h   |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

fs/nfsd/nfsfh.c +1 -1

index 55c8e63..90c6aa6 100644
@@ -344,7 +344,7 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, int access)
 	 * which clients virtually always use auth_sys for,
 	 * even while using RPCSEC_GSS for NFS.
 	 */
-	if (access & NFSD_MAY_LOCK)
+	if (access & NFSD_MAY_LOCK || access & NFSD_MAY_BYPASS_GSS)
 		goto skip_pseudoflavor_check;
 	/*
 	 * Clients may expect to be able to use auth_sys during mount,

fs/nfsd/vfs.h +1 -0

index 1036913..4d2509f 100644
@@ -24,6 +24,7 @@
 #define NFSD_MAY_LOCAL_ACCESS	128 /* IRIX doing local access check on device special file*/
 #define NFSD_MAY_BYPASS_GSS_ON_ROOT 256
 #define NFSD_MAY_NOT_BREAK_LEASE 512
+#define NFSD_MAY_BYPASS_GSS	1024
 
 #define NFSD_MAY_CREATE		(NFSD_MAY_EXEC|NFSD_MAY_WRITE)
 #define NFSD_MAY_REMOVE		(NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC)

Signature · 1 line

1.7.1

J. Bruce Fields

2011-04-10 16:29 UTC #9 lore raw

PATCH 3/5 nfsd4: introduce OPDESC helper

In reply to J. Bruce Fields

Signed-off-by J. Bruce Fields <[email protected]>

---
 fs/nfsd/nfs4proc.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

fs/nfsd/nfs4proc.c +6 -1

index 5fcb139..126b8f7 100644
@@ -1031,6 +1031,11 @@ static __be32 nfs41_check_op_ordering(struct nfsd4_compoundargs *args)
 	return nfs_ok;
 }
 
+static inline struct nfsd4_operation *OPDESC(struct nfsd4_op *op)
+{
+	return &nfsd4_ops[op->opnum];
+}
+
 /*
  * COMPOUND call.
  */

@@ -1108,7 +1113,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
 			goto encode_op;
 		}
 
-		opdesc = &nfsd4_ops[op->opnum];
+		opdesc = OPDESC(op);
 
 		if (!cstate->current_fh.fh_dentry) {
 			if (!(opdesc->op_flags & ALLOWED_WITHOUT_FH)) {

Signature · 1 line

1.7.1

J. Bruce Fields

2011-04-10 16:29 UTC #10 lore raw

PATCH 4/5 nfsd4: make fh_verify responsibility of nfsd_lookup_dentry caller

In reply to J. Bruce Fields

The secinfo caller actually won't want this.

Signed-off-by J. Bruce Fields <[email protected]>

---
 fs/nfsd/nfs4proc.c |    3 +++
 fs/nfsd/vfs.c      |    9 +++------
 2 files changed, 6 insertions(+), 6 deletions(-)

fs/nfsd/nfs4proc.c +3 -0

index 126b8f7..8059ada 100644
@@ -762,6 +762,9 @@ nfsd4_secinfo(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	__be32 err;
 
 	fh_init(&resfh, NFS4_FHSIZE);
+	err = fh_verify(rqstp, &cstate->current_fh, S_IFDIR, NFSD_MAY_EXEC);
+	if (err)
+		return err;
 	err = nfsd_lookup_dentry(rqstp, &cstate->current_fh,
 				    secinfo->si_name, secinfo->si_namelen,
 				    &exp, &dentry);

fs/nfsd/vfs.c +3 -6

index ac4f0b4..7d75890 100644
@@ -181,16 +181,10 @@ nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	struct svc_export	*exp;
 	struct dentry		*dparent;
 	struct dentry		*dentry;
-	__be32			err;
 	int			host_err;
 
 	dprintk("nfsd: nfsd_lookup(fh %s, %.*s)\n", SVCFH_fmt(fhp), len,name);
 
-	/* Obtain dentry and export. */
-	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_EXEC);
-	if (err)
-		return err;
-
 	dparent = fhp->fh_dentry;
 	exp  = fhp->fh_export;
 	exp_get(exp);

@@ -254,6 +248,9 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
 	struct dentry		*dentry;
 	__be32 err;
 
+	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_EXEC);
+	if (err)
+		return err;
 	err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
 	if (err)
 		return err;

Signature · 1 line

1.7.1

J. Bruce Fields

2011-04-10 16:29 UTC #11 lore raw

PATCH 5/5 nfsd4: fix wrongsec handling for PUTFH + op cases

In reply to J. Bruce Fields

When PUTFH is followed by an operation that uses the filehandle, and when the current client is using a security flavor that is inconsistent with the given filehandle, we have a choice: we can return WRONGSEC either when the current filehandle is set using the PUTFH, or when the filehandle is first used by the following operation.

Follow the recommendations of RFC 5661 in making this choice.

(Our current behavior prevented the client from doing security negotiation by returning WRONGSEC on SECINFO.)

Signed-off-by J. Bruce Fields <[email protected]>

---
 fs/nfsd/export.c   |    6 -----
 fs/nfsd/nfs4proc.c |   57 ++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 49 insertions(+), 14 deletions(-)

fs/nfsd/export.c +0 -6

index ad000ae..b9566e4 100644
@@ -1354,12 +1354,6 @@ exp_pseudoroot(struct svc_rqst *rqstp, struct svc_fh *fhp)
 	if (IS_ERR(exp))
 		return nfserrno(PTR_ERR(exp));
 	rv = fh_compose(fhp, exp, exp->ex_path.dentry, NULL);
-	if (rv)
-		goto out;
-	rv = check_nfsd_access(exp, rqstp);
-	if (rv)
-		fh_put(fhp);
-out:
 	exp_put(exp);
 	return rv;
 }

fs/nfsd/nfs4proc.c +49 -8

137 lines not on this page · raw message

Signature · 1 line

1.7.1

Mi Jinlong

2011-04-11 03:05 UTC #6 lore raw

Re: [PATCH 1/5] nfsd: distinguish functions of NFSD_MAY_* flags

In reply to J. Bruce Fields

Quoted 19 lines · J. Bruce Fields

Most of the NFSD_MAY_* flags actually request permissions, but over the years we've accreted a few that modify the behavior of the permission or open code in other ways.
Distinguish the two cases a little more. In particular, allow the shortcut at the start of nfsd_permission to ignore the non-permission-requesting bits.
Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/vfs.c |    2 +-
 fs/nfsd/vfs.h |    3 +++
 2 files changed, 4 insertions(+), 1 deletions(-)
fs/nfsd/vfs.c +0 -0
index 2e1cebd..ac4f0b4 100644

fs/nfsd/vfs.c nfsd_permission(struct svc_rqst *rqstp, struct svc_export *exp, · line 2031

Quoted 6 lines

fs/nfsd/vfs.c +1 -1

@@ -2027,7 +2027,7 @@ nfsd_permission(struct svc_rqst *rqstp, struct svc_export *exp,
 	struct inode	*inode = dentry->d_inode;
 	int		err;
 
-	if (acc == NFSD_MAY_NOP)
+	if (acc & NFSD_MAY_MASK == NFSD_MAY_NOP)
 		return 0;

Maybe there is a problem, the priority of '==' is higher than '&', this line equal to "if (acc & (NFSD_MAY_MASK == NFSD_MAY_NOP))", "return 0" will appears every time, I think it's not we really want.

"if ((acc & NFSD_MAY_MASK) == NFSD_MAY_NOP)" is we need, do you think so?

Signature · 3 lines

----
thanks
Mi Jinlong

Quoted 16 lines

fs/nfsd/vfs.c +0 -0

 #if 0
 	dprintk("nfsd: permission 0x%x%s%s%s%s%s%s%s mode 0%o%s%s%s\n",

fs/nfsd/vfs.h +3 -0

index 9a370a5..1036913 100644
@@ -17,6 +17,9 @@
 #define NFSD_MAY_SATTR		8
 #define NFSD_MAY_TRUNC		16
 #define NFSD_MAY_LOCK		32
+#define NFSD_MAY_MASK		63
+
+/* extra hints to permission and open routines: */
 #define NFSD_MAY_OWNER_OVERRIDE	64
 #define NFSD_MAY_LOCAL_ACCESS	128 /* IRIX doing local access check on device special file*/
 #define NFSD_MAY_BYPASS_GSS_ON_ROOT 256

Signature · 3 lines

----
thanks
Mi Jinlong

J. Bruce Fields

2011-04-11 12:42 UTC #7 lore raw

Re: [PATCH 1/5] nfsd: distinguish functions of NFSD_MAY_* flags

Quoted 4 lines · Mi Jinlong

=20
=20
J. Bruce Fields =E5=86=99=E9=81=93:
Quoted 1 line
Most of the NFSD_MAY_* flags actually request permissions, but over=

the

Quoted 1 line

Quoted 1 line
years we've accreted a few that modify the behavior of the permissi=

on or

Quoted 9 lines

Quoted 9 lines
open code in other ways.
=20
Distinguish the two cases a little more. In particular, allow the shortcut at the start of nfsd_permission to ignore the non-permission-requesting bits.
=20
Signed-off-by J. Bruce Fields <[email protected]>
---
 fs/nfsd/vfs.c |    2 +-

Quoted 8 lines

fs/nfsd/vfs.c nfsd_permission(struct svc_rqst *rqstp, struc=
Quoted 8 lines
 fs/nfsd/vfs.h |    3 +++
 2 files changed, 4 insertions(+), 1 deletions(-)
=20
fs/nfsd/vfs.c +0 -0
index 2e1cebd..ac4f0b4 100644
@@ -2027,7 +2027,7 @@ nfsd_permission(struct svc_rqst *rqstp, struc=

t svc_export *exp,

Quoted 8 lines

fs/nfsd/vfs.c
Quoted 6 lines
fs/nfsd/vfs.c +0 -0
 	struct inode	*inode =3D dentry->d_inode;
 	int		err;
=20
fs/nfsd/vfs.c +1 -1
-	if (acc =3D=3D NFSD_MAY_NOP)
+	if (acc & NFSD_MAY_MASK =3D=3D NFSD_MAY_NOP)
 		return 0;
=20
Maybe there is a problem, the priority of '=3D=3D' is higher than '=

&',=20

Quoted 1 line

this line equal to "if (acc & (NFSD_MAY_MASK =3D=3D NFSD_MAY_NOP))"=

Quoted 1 line

"return 0" will appears every time, I think it's not we really want=

=2E

Quoted 3 lines

=20
"if ((acc & NFSD_MAY_MASK) =3D=3D NFSD_MAY_NOP)" is we need, do you think so?

Yes, thanks--fixed.--b.