Fix NFS credential

The commit f74b821 caused a regression where creating file through NFS will always create a file owned by root. This is because the patch enables the KSID code in zfs_acl_ids_create, which it would use euid and egid of the current process. However, on Linux, we should use fsuid and fsgid for file operations, which is the original behaviour. So we revert this part of code. The patch also enables secpolicy_vnode_*, since they are also used in file operations, we change them to use fsuid and fsgid. Signed-off-by: Chunwei Chen <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#4772 Closes openzfs#4758
sempervictus · Jun 25, 2016 · 7424626 · 7424626
1 parent f05e87b
commit 7424626
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 12 deletions.
diff --git a/module/zfs/policy.c b/module/zfs/policy.c
@@ -96,7 +96,7 @@ secpolicy_vnode_access2(const cred_t *cr, struct inode *ip, uid_t owner,
 int
 secpolicy_vnode_any_access(const cred_t *cr, struct inode *ip, uid_t owner)
 {
-	if (crgetuid(cr) == owner)
+	if (crgetfsuid(cr) == owner)
 		return (0);
 
 	if (zpl_inode_owner_or_capable(ip))
@@ -117,7 +117,7 @@ secpolicy_vnode_any_access(const cred_t *cr, struct inode *ip, uid_t owner)
 int
 secpolicy_vnode_chown(const cred_t *cr, uid_t owner)
 {
-	if (crgetuid(cr) == owner)
+	if (crgetfsuid(cr) == owner)
 		return (0);
 
 	return (priv_policy(cr, CAP_FOWNER, B_FALSE, EPERM));
@@ -149,7 +149,7 @@ secpolicy_vnode_remove(const cred_t *cr)
 int
 secpolicy_vnode_setdac(const cred_t *cr, uid_t owner)
 {
-	if (crgetuid(cr) == owner)
+	if (crgetfsuid(cr) == owner)
 		return (0);
 
 	return (priv_policy(cr, CAP_FOWNER, B_FALSE, EPERM));
@@ -175,7 +175,7 @@ secpolicy_vnode_setid_retain(const cred_t *cr, boolean_t issuidroot)
 int
 secpolicy_vnode_setids_setgids(const cred_t *cr, gid_t gid)
 {
-	if (!groupmember(gid, cr))
+	if (crgetfsgid(cr) != gid && !groupmember(gid, cr))
 		return (priv_policy(cr, CAP_FSETID, B_FALSE, EPERM));
 
 	return (0);
@@ -219,7 +219,7 @@ secpolicy_setid_clear(vattr_t *vap, cred_t *cr)
 static int
 secpolicy_vnode_setid_modify(const cred_t *cr, uid_t owner)
 {
-	if (crgetuid(cr) == owner)
+	if (crgetfsuid(cr) == owner)
 		return (0);
 
 	return (priv_policy(cr, CAP_FSETID, B_FALSE, EPERM));

diff --git a/module/zfs/zfs_acl.c b/module/zfs/zfs_acl.c
@@ -1744,7 +1744,7 @@ zfs_acl_ids_create(znode_t *dzp, int flag, vattr_t *vap, cred_t *cr,
 	int		error;
 	zfs_sb_t	*zsb = ZTOZSB(dzp);
 	zfs_acl_t	*paclp;
-	gid_t		gid;
+	gid_t		gid = vap->va_gid;
 	boolean_t	need_chmod = B_TRUE;
 	boolean_t	inherited = B_FALSE;
 
@@ -1758,6 +1758,7 @@ zfs_acl_ids_create(znode_t *dzp, int flag, vattr_t *vap, cred_t *cr,
 
 	acl_ids->z_fuid = vap->va_uid;
 	acl_ids->z_fgid = vap->va_gid;
+#ifdef HAVE_KSID
 	/*
 	 * Determine uid and gid.
 	 */
@@ -1809,6 +1810,7 @@ zfs_acl_ids_create(znode_t *dzp, int flag, vattr_t *vap, cred_t *cr,
 			}
 		}
 	}
+#endif /* HAVE_KSID */
 
 	/*
 	 * If we're creating a directory, and the parent directory has the

diff --git a/module/zfs/zfs_fuid.c b/module/zfs/zfs_fuid.c
@@ -490,6 +490,7 @@ zfs_fuid_node_add(zfs_fuid_info_t **fuidpp, const char *domain, uint32_t rid,
 	}
 }
 
+#ifdef HAVE_KSID
 /*
  * Create a file system FUID, based on information in the users cred
  *
@@ -502,7 +503,6 @@ uint64_t
 zfs_fuid_create_cred(zfs_sb_t *zsb, zfs_fuid_type_t type,
     cred_t *cr, zfs_fuid_info_t **fuidp)
 {
-#ifdef HAVE_KSID
 	uint64_t	idx;
 	ksid_t		*ksid;
 	uint32_t	rid;
@@ -542,12 +542,8 @@ zfs_fuid_create_cred(zfs_sb_t *zsb, zfs_fuid_type_t type,
 	zfs_fuid_node_add(fuidp, kdomain, rid, idx, id, type);
 
 	return (FUID_ENCODE(idx, rid));
-#else
-	VERIFY(type == ZFS_OWNER || type == ZFS_GROUP);
-
-	return ((uint64_t)((type == ZFS_OWNER) ? crgetuid(cr) : crgetgid(cr)));
-#endif /* HAVE_KSID */
 }
+#endif /* HAVE_KSID */
 
 /*
  * Create a file system FUID for an ACL ace