svn commit: r360207 - stable/12/sys/fs/nfsserver

[Rick Macklem]

Author: rmacklem
Date: Wed Apr 22 21:08:08 2020
New Revision: 360207
URL: https://svnweb.freebsd.org/changeset/base/360207

Log:
  MFC: r359720
  Fix an interoperability issue w.r.t. the Linux client and the NFSv4 server.
  
  Luoqi Chen reported a problem on freebsd-fs@ where a Linux NFSv4 client
  was able to open and write to a file when the file's permissions were
  not set to allow the owner write access.
  
  Since NFS servers check file permissions on every write RPC, it is standard
  practice to allow the owner of the file to do writes, regardless of
  file permissions. This provides POSIX like behaviour, since POSIX only
  checks permissions upon open(2).
  The traditional way NFS clients handle this is to check access via the
  Access operation/RPC and use that to determine if an open(2) on the
  client is allowed.
  
  It appears that, for NFSv4, the Linux client expects the NFSv4 Open (not a
  POSIX open) operation to fail with NFSERR_ACCES if the file is not being
  created and file permissions do not allow owner access, unlike NFSv3.
  Since both the Linux and OpenSolaris NFSv4 servers seem to exhibit this
  behaviour, this patch changes the FreeBSD NFSv4 server to do the same.
  A sysctl called vfs.nfsd.v4openaccess can be set to 0 to return the
  NFSv4 server to its previous behaviour.
  
  Since both the Linux and FreeBSD NFSv4 clients seem to exhibit correct
  behaviour with the access check for file owner in Open enabled, it is enabled
  by default.
  
  Reported by:	luoqi.chen at gmail.com

Modified:
  stable/12/sys/fs/nfsserver/nfs_nfsdserv.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/sys/fs/nfsserver/nfs_nfsdserv.c
==============================================================================
--- stable/12/sys/fs/nfsserver/nfs_nfsdserv.c	Wed Apr 22 21:03:24 2020	(r360206)
+++ stable/12/sys/fs/nfsserver/nfs_nfsdserv.c	Wed Apr 22 21:08:08 2020	(r360207)
@@ -74,6 +74,10 @@ SYSCTL_INT(_vfs_nfsd, OID_AUTO, async, CTLFLAG_RW, &nf
 extern int	nfsrv_doflexfile;
 SYSCTL_INT(_vfs_nfsd, OID_AUTO, default_flexfile, CTLFLAG_RW,
     &nfsrv_doflexfile, 0, "Make Flex File Layout the default for pNFS");
+static bool	nfsrv_openaccess = true;
+SYSCTL_BOOL(_vfs_nfsd, OID_AUTO, v4openaccess, CTLFLAG_RW,
+    &nfsrv_openaccess, 0,
+    "Enable Linux style NFSv4 Open access check");
 
 /*
  * This list defines the GSS mechanisms supported.
@@ -2727,7 +2731,7 @@ nfsrvd_open(struct nfsrv_descript *nd, __unused int is
 	u_int32_t *tl;
 	int i, retext;
 	struct nfsstate *stp = NULL;
-	int error = 0, create, claim, exclusive_flag = 0;
+	int error = 0, create, claim, exclusive_flag = 0, override;
 	u_int32_t rflags = NFSV4OPEN_LOCKTYPEPOSIX, acemask;
 	int how = NFSCREATE_UNCHECKED;
 	int32_t cverf[2], tverf[2] = { 0, 0 };
@@ -3030,15 +3034,38 @@ nfsrvd_open(struct nfsrv_descript *nd, __unused int is
 		 */
 		nd->nd_repstat = (vp->v_type == VDIR) ? NFSERR_ISDIR : NFSERR_SYMLINK;
 	}
+
+	/*
+	 * If the Open is being done for a file that already exists, apply
+	 * normal permission checking including for the file owner, if
+	 * vfs.nfsd.v4openaccess is set.
+	 * Previously, the owner was always allowed to open the file to
+	 * be consistent with the NFS tradition of always allowing the
+	 * owner of the file to write to the file regardless of permissions.
+	 * It now appears that the Linux client expects the owner
+	 * permissions to be checked for opens that are not creating the
+	 * file.  I believe the correct approach is to use the Access
+	 * operation's results to be consistent with NFSv3, but that is
+	 * not what the current Linux client appears to be doing.
+	 * Since both the Linux and OpenSolaris NFSv4 servers do this check,
+	 * I have enabled it by default.
+	 * If this semantic change causes a problem, it can be disabled by
+	 * setting the sysctl vfs.nfsd.v4openaccess to 0 to re-enable the
+	 * previous semantics.
+	 */
+	if (nfsrv_openaccess && create == NFSV4OPEN_NOCREATE)
+		override = NFSACCCHK_NOOVERRIDE;
+	else
+		override = NFSACCCHK_ALLOWOWNER;
 	if (!nd->nd_repstat && (stp->ls_flags & NFSLCK_WRITEACCESS))
 	    nd->nd_repstat = nfsvno_accchk(vp, VWRITE, nd->nd_cred,
-	        exp, p, NFSACCCHK_ALLOWOWNER, NFSACCCHK_VPISLOCKED, NULL);
+	        exp, p, override, NFSACCCHK_VPISLOCKED, NULL);
 	if (!nd->nd_repstat && (stp->ls_flags & NFSLCK_READACCESS)) {
 	    nd->nd_repstat = nfsvno_accchk(vp, VREAD, nd->nd_cred,
-	        exp, p, NFSACCCHK_ALLOWOWNER, NFSACCCHK_VPISLOCKED, NULL);
+	        exp, p, override, NFSACCCHK_VPISLOCKED, NULL);
 	    if (nd->nd_repstat)
 		nd->nd_repstat = nfsvno_accchk(vp, VEXEC,
-		    nd->nd_cred, exp, p, NFSACCCHK_ALLOWOWNER,
+		    nd->nd_cred, exp, p, override,
 		    NFSACCCHK_VPISLOCKED, NULL);
 	}