From b0b7d978b6a886ae441c975b36002e43e222d271 Mon Sep 17 00:00:00 2001 From: Rick Macklem Date: Wed, 8 Apr 2020 01:12:54 +0000 Subject: [PATCH] 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@gmail.com MFC after: 2 weeks --- sys/fs/nfsserver/nfs_nfsdserv.c | 35 +++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/sys/fs/nfsserver/nfs_nfsdserv.c b/sys/fs/nfsserver/nfs_nfsdserv.c index 269ad8ce438e..602431486bdb 100644 --- a/sys/fs/nfsserver/nfs_nfsdserv.c +++ b/sys/fs/nfsserver/nfs_nfsdserv.c @@ -81,6 +81,10 @@ static int nfsrv_linux42server = 1; SYSCTL_INT(_vfs_nfsd, OID_AUTO, linux42server, CTLFLAG_RW, &nfsrv_linux42server, 0, "Enable Linux style NFSv4.2 server (non-RFC compliant)"); +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. @@ -2742,7 +2746,7 @@ nfsrvd_open(struct nfsrv_descript *nd, __unused int isdgram, 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 }; @@ -3046,15 +3050,38 @@ nfsrvd_open(struct nfsrv_descript *nd, __unused int isdgram, */ 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); }