Revert r299454 and r299448.

Those changes were found confusing FreeBSD libc ACL code, that doesn't
differentiate ACL for directories and files, and report ACLs for all
directories created after those patches as non-trivial.  On the other
side these changes were considered wrong from POSIX and NFSv4 points of
view.  Until further investigation done upstream, revert those changes
locally in preparation for FreeBSD 11.0 release.

Approved by:	re (hrs)

3 files changed, 111 insertions, 206 deletions

diff --git a/sys/cddl/contrib/opensolaris/common/acl/acl_common.c b/sys/cddl/contrib/opensolaris/common/acl/acl_common.c
index d729f36..a681905 100644
--- a/sys/cddl/contrib/opensolaris/common/acl/acl_common.c
+++ b/sys/cddl/contrib/opensolaris/common/acl/acl_common.c
@@ -20,7 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright 2014 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2011 Nexenta Systems, Inc.  All rights reserved.
  */
 
 #include <sys/types.h>
@@ -1580,8 +1580,7 @@ acl_trivial_access_masks(mode_t mode, boolean_t isdir, trivial_acl_t *masks)
 	uint32_t write_mask = ACE_WRITE_DATA|ACE_APPEND_DATA;
 	uint32_t execute_mask = ACE_EXECUTE;
 
-	if (isdir)
-		write_mask |= ACE_DELETE_CHILD;
+	(void) isdir;	/* will need this later */
 
 	masks->deny1 = 0;
 	if (!(mode & S_IRUSR) && (mode & (S_IRGRP|S_IROTH)))
@@ -1725,17 +1724,10 @@ ace_trivial_common(void *acep, int aclcnt,
 			return (1);
 
 		/*
-		 * Delete permission is never set by default
-		 */
-		if (mask & ACE_DELETE)
-			return (1);
-
-		/*
-		 * Child delete permission should be accompanied by write
+		 * Delete permissions are never set by default
 		 */
-		if ((mask & ACE_DELETE_CHILD) && !(mask & ACE_WRITE_DATA))
+		if (mask & (ACE_DELETE|ACE_DELETE_CHILD))
 			return (1);
-
 		/*
 		 * only allow owner@ to have
 		 * write_acl/write_owner/write_attributes/write_xattr/
diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c
index 9fc8d73..3e4c70d 100644
--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c
+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c
@@ -20,8 +20,8 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright 2011 Nexenta Systems, Inc.  All rights reserved.
  * Copyright (c) 2013 by Delphix. All rights reserved.
- * Copyright 2014 Nexenta Systems, Inc.  All rights reserved.
  */
 
 #include <sys/types.h>
@@ -2085,7 +2085,7 @@ zfs_zaccess_dataset_check(znode_t *zp, uint32_t v4_mode)
  * placed into the working_mode, giving the caller a mask of denied
  * accesses.  Returns:
  *	0		if all AoI granted
- *	EACCES		if the denied mask is non-zero
+ *	EACCESS 	if the denied mask is non-zero
  *	other error	if abnormal failure (e.g., IO error)
  *
  * A secondary usage of the function is to determine if any of the
@@ -2532,32 +2532,46 @@ zfs_zaccess_unix(znode_t *zp, mode_t mode, cred_t *cr)
 	return (zfs_zaccess(zp, v4_mode, 0, B_FALSE, cr));
 }
 
-/* See zfs_zaccess_delete() */
-int zfs_write_implies_delete_child = 1;
+static int
+zfs_delete_final_check(znode_t *zp, znode_t *dzp,
+    mode_t available_perms, cred_t *cr)
+{
+	int error;
+	uid_t downer;
+
+	downer = zfs_fuid_map_id(dzp->z_zfsvfs, dzp->z_uid, cr, ZFS_OWNER);
+
+	error = secpolicy_vnode_access2(cr, ZTOV(dzp),
+	    downer, available_perms, VWRITE|VEXEC);
+
+	if (error == 0)
+		error = zfs_sticky_remove_access(dzp, zp, cr);
+
+	return (error);
+}
 
 /*
- * Determine whether delete access should be granted.
+ * Determine whether Access should be granted/deny, without
+ * consulting least priv subsystem.
  *
- * The following chart outlines how we handle delete permissions which is
- * how recent versions of windows (Windows 2008) handles it.  The efficiency
- * comes from not having to check the parent ACL where the object itself grants
- * delete:
+ * The following chart is the recommended NFSv4 enforcement for
+ * ability to delete an object.
  *
  *      -------------------------------------------------------
- *      |   Parent Dir  |      Target Object Permissions      |
+ *      |   Parent Dir  |           Target Object Permissions |
  *      |  permissions  |                                     |
  *      -------------------------------------------------------
  *      |               | ACL Allows | ACL Denies| Delete     |
  *      |               |  Delete    |  Delete   | unspecified|
  *      -------------------------------------------------------
- *      | ACL Allows    | Permit     | Deny *    | Permit     |
- *      | DELETE_CHILD  |            |           |            |
+ *      |  ACL Allows   | Permit     | Permit    | Permit     |
+ *      |  DELETE_CHILD |                                     |
  *      -------------------------------------------------------
- *      | ACL Denies    | Permit     | Deny      | Deny       |
- *      | DELETE_CHILD  |            |           |            |
+ *      |  ACL Denies   | Permit     | Deny      | Deny       |
+ *      |  DELETE_CHILD |            |           |            |
  *      -------------------------------------------------------
  *      | ACL specifies |            |           |            |
- *      | only allow    | Permit     | Deny *    | Permit     |
+ *      | only allow    | Permit     | Permit    | Permit     |
  *      | write and     |            |           |            |
  *      | execute       |            |           |            |
  *      -------------------------------------------------------
@@ -2567,171 +2581,91 @@ int zfs_write_implies_delete_child = 1;
  *      -------------------------------------------------------
  *         ^
  *         |
- *         Re. execute permission on the directory:  if that's missing,
- *	   the vnode lookup of the target will fail before we get here.
- *
- * Re [*] in the table above:  NFSv4 would normally Permit delete for
- * these two cells of the matrix.
- * See acl.h for notes on which ACE_... flags should be checked for which
- * operations.  Specifically, the NFSv4 committee recommendation is in
- * conflict with the Windows interpretation of DENY ACEs, where DENY ACEs
- * should take precedence ahead of ALLOW ACEs.
- *
- * This implementation always consults the target object's ACL first.
- * If a DENY ACE is present on the target object that specifies ACE_DELETE,
- * delete access is denied.  If an ALLOW ACE with ACE_DELETE is present on
- * the target object, access is allowed.  If and only if no entries with
- * ACE_DELETE are present in the object's ACL, check the container's ACL
- * for entries with ACE_DELETE_CHILD.
- *
- * A summary of the logic implemented from the table above is as follows:
+ *         No search privilege, can't even look up file?
  *
- * First check for DENY ACEs that apply.
- * If either target or container has a deny, EACCES.
- *
- * Delete access can then be summarized as follows:
- * 1: The object to be deleted grants ACE_DELETE, or
- * 2: The containing directory grants ACE_DELETE_CHILD.
- * In a Windows system, that would be the end of the story.
- * In this system, (2) has some complications...
- * 2a: "sticky" bit on a directory adds restrictions, and
- * 2b: existing ACEs from previous versions of ZFS may
- * not carry ACE_DELETE_CHILD where they should, so we
- * also allow delete when ACE_WRITE_DATA is granted.
- *
- * Note: 2b is technically a work-around for a prior bug,
- * which hopefully can go away some day.  For those who
- * no longer need the work around, and for testing, this
- * work-around is made conditional via the tunable:
- * zfs_write_implies_delete_child
  */
 int
 zfs_zaccess_delete(znode_t *dzp, znode_t *zp, cred_t *cr)
 {
-	uint32_t wanted_dirperms;
 	uint32_t dzp_working_mode = 0;
 	uint32_t zp_working_mode = 0;
 	int dzp_error, zp_error;
-	boolean_t dzpcheck_privs;
-	boolean_t zpcheck_privs;
-
-	if (zp->z_pflags & (ZFS_IMMUTABLE | ZFS_NOUNLINK))
-		return (SET_ERROR(EPERM));
+	mode_t available_perms;
+	boolean_t dzpcheck_privs = B_TRUE;
+	boolean_t zpcheck_privs = B_TRUE;
 
 	/*
-	 * Case 1:
-	 * If target object grants ACE_DELETE then we are done.  This is
-	 * indicated by a return value of 0.  For this case we don't worry
-	 * about the sticky bit because sticky only applies to the parent
-	 * directory and this is the child access result.
+	 * We want specific DELETE permissions to
+	 * take precedence over WRITE/EXECUTE.  We don't
+	 * want an ACL such as this to mess us up.
+	 * user:joe:write_data:deny,user:joe:delete:allow
+	 *
+	 * However, deny permissions may ultimately be overridden
+	 * by secpolicy_vnode_access().
 	 *
-	 * If we encounter a DENY ACE here, we're also done (EACCES).
-	 * Note that if we hit a DENY ACE here (on the target) it should
-	 * take precedence over a DENY ACE on the container, so that when
-	 * we have more complete auditing support we will be able to
-	 * report an access failure against the specific target.
-	 * (This is part of why we're checking the target first.)
+	 * We will ask for all of the necessary permissions and then
+	 * look at the working modes from the directory and target object
+	 * to determine what was found.
 	 */
-	zp_error = zfs_zaccess_common(zp, ACE_DELETE, &zp_working_mode,
-	    &zpcheck_privs, B_FALSE, cr);
-	if (zp_error == EACCES) {
-		/* We hit a DENY ACE. */
-		if (!zpcheck_privs)
-			return (SET_ERROR(zp_error));
-		return (secpolicy_vnode_remove(ZTOV(dzp), cr)); /* XXXPJD: s/dzp/zp/ ? */
 
-	}
-	if (zp_error == 0)
-		return (0);
+	if (zp->z_pflags & (ZFS_IMMUTABLE | ZFS_NOUNLINK))
+		return (SET_ERROR(EPERM));
 
 	/*
-	 * Case 2:
-	 * If the containing directory grants ACE_DELETE_CHILD,
-	 * or we're in backward compatibility mode and the
-	 * containing directory has ACE_WRITE_DATA, allow.
-	 * Case 2b is handled with wanted_dirperms.
+	 * First row
+	 * If the directory permissions allow the delete, we are done.
 	 */
-	wanted_dirperms = ACE_DELETE_CHILD;
-	if (zfs_write_implies_delete_child)
-		wanted_dirperms |= ACE_WRITE_DATA;
-	dzp_error = zfs_zaccess_common(dzp, wanted_dirperms,
-	    &dzp_working_mode, &dzpcheck_privs, B_FALSE, cr);
-	if (dzp_error == EACCES) {
-		/* We hit a DENY ACE. */
-		if (!dzpcheck_privs)
-			return (SET_ERROR(dzp_error));
-		return (secpolicy_vnode_remove(ZTOV(dzp), cr));  /* XXXPJD: s/dzp/zp/ ? */
-	}
+	if ((dzp_error = zfs_zaccess_common(dzp, ACE_DELETE_CHILD,
+	    &dzp_working_mode, &dzpcheck_privs, B_FALSE, cr)) == 0)
+		return (0);
 
 	/*
-	 * Cases 2a, 2b (continued)
-	 *
-	 * Note: dzp_working_mode now contains any permissions
-	 * that were NOT granted.  Therefore, if any of the
-	 * wanted_dirperms WERE granted, we will have:
-	 *   dzp_working_mode != wanted_dirperms
-	 * We're really asking if ANY of those permissions
-	 * were granted, and if so, grant delete access.
+	 * If target object has delete permission then we are done
 	 */
-	if (dzp_working_mode != wanted_dirperms)
-		dzp_error = 0;
+	if ((zp_error = zfs_zaccess_common(zp, ACE_DELETE, &zp_working_mode,
+	    &zpcheck_privs, B_FALSE, cr)) == 0)
+		return (0);
+
+	ASSERT(dzp_error && zp_error);
+
+	if (!dzpcheck_privs)
+		return (dzp_error);
+	if (!zpcheck_privs)
+		return (zp_error);
 
 	/*
-	 * dzp_error is 0 if the container granted us permissions to "modify".
-	 * If we do not have permission via one or more ACEs, our current
-	 * privileges may still permit us to modify the container.
+	 * Second row
 	 *
-	 * dzpcheck_privs is false when i.e. the FS is read-only.
-	 * Otherwise, do privilege checks for the container.
+	 * If directory returns EACCES then delete_child was denied
+	 * due to deny delete_child.  In this case send the request through
+	 * secpolicy_vnode_remove().  We don't use zfs_delete_final_check()
+	 * since that *could* allow the delete based on write/execute permission
+	 * and we want delete permissions to override write/execute.
 	 */
-	if (dzp_error != 0 && dzpcheck_privs) {
-		uid_t owner;
 
-		/*
-		 * The secpolicy call needs the requested access and
-		 * the current access mode of the container, but it
-		 * only knows about Unix-style modes (VEXEC, VWRITE),
-		 * so this must condense the fine-grained ACE bits into
-		 * Unix modes.
-		 *
-		 * The VEXEC flag is easy, because we know that has
-		 * always been checked before we get here (during the
-		 * lookup of the target vnode).  The container has not
-		 * granted us permissions to "modify", so we do not set
-		 * the VWRITE flag in the current access mode.
-		 */
-		owner = zfs_fuid_map_id(dzp->z_zfsvfs, dzp->z_uid, cr,
-		    ZFS_OWNER);
-		dzp_error = secpolicy_vnode_access2(cr, ZTOV(dzp),
-		    owner, VEXEC, VWRITE|VEXEC);
-	}
-	if (dzp_error != 0) {
-		/*
-		 * Note: We may have dzp_error = -1 here (from
-		 * zfs_zacess_common).  Don't return that.
-		 */
-		return (SET_ERROR(EACCES));
-	}
+	if (dzp_error == EACCES)
+		return (secpolicy_vnode_remove(ZTOV(dzp), cr));	/* XXXPJD: s/dzp/zp/ ? */
 
 	/*
-	 * At this point, we know that the directory permissions allow
-	 * us to modify, but we still need to check for the additional
-	 * restrictions that apply when the "sticky bit" is set.
-	 *
-	 * Yes, zfs_sticky_remove_access() also checks this bit, but
-	 * checking it here and skipping the call below is nice when
-	 * you're watching all of this with dtrace.
+	 * Third Row
+	 * only need to see if we have write/execute on directory.
 	 */
-	if ((dzp->z_mode & S_ISVTX) == 0)
-		return (0);
+
+	dzp_error = zfs_zaccess_common(dzp, ACE_EXECUTE|ACE_WRITE_DATA,
+	    &dzp_working_mode, &dzpcheck_privs, B_FALSE, cr);
+
+	if (dzp_error != 0 && !dzpcheck_privs)
+		return (dzp_error);
 
 	/*
-	 * zfs_sticky_remove_access will succeed if:
-	 * 1. The sticky bit is absent.
-	 * 2. We pass the sticky bit restrictions.
-	 * 3. We have privileges that always allow file removal.
+	 * Fourth row
 	 */
-	return (zfs_sticky_remove_access(dzp, zp, cr));
+
+	available_perms = (dzp_working_mode & ACE_WRITE_DATA) ? 0 : VWRITE;
+	available_perms |= (dzp_working_mode & ACE_EXECUTE) ? 0 : VEXEC;
+
+	return (zfs_delete_final_check(zp, dzp, available_perms, cr));
+
 }
 
 int
diff --git a/sys/cddl/contrib/opensolaris/uts/common/sys/acl.h b/sys/cddl/contrib/opensolaris/uts/common/sys/acl.h
index 83c85ba..58ef69e 100644
--- a/sys/cddl/contrib/opensolaris/uts/common/sys/acl.h
+++ b/sys/cddl/contrib/opensolaris/uts/common/sys/acl.h
@@ -23,8 +23,6 @@
  *
  * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
- *
- * Copyright 2014 Nexenta Systems, Inc.  All rights reserved.
  */
 
 #ifndef _SYS_ACL_H
@@ -90,55 +88,37 @@ typedef struct acl_info acl_t;
 
 /*
  * The following are defined for ace_t.
- *
- * Note, these are intentionally the same as the Windows
- * "File Access Rights Constants" you can find on MSDN.
- * (See also: "Standard Access Rights" on MSDN).
- *
- * The equivalent Windows names for these are just like
- * those show below, with FILE_ in place of ACE_, except
- * as noted below.  Also note that Windows uses a special
- * privilege: BYPASS_TRAVERSE_CHECKING, normally granted
- * to everyone, that causes the absence of ACE_TRAVERSE
- * to be ignored.
- */
-#define	ACE_READ_DATA		0x00000001	/* file: read data */
-#define	ACE_LIST_DIRECTORY	0x00000001	/* dir: list files */
-#define	ACE_WRITE_DATA		0x00000002	/* file: write data */
-#define	ACE_ADD_FILE		0x00000002	/* dir: create file */
-#define	ACE_APPEND_DATA		0x00000004	/* file: append data */
-#define	ACE_ADD_SUBDIRECTORY	0x00000004	/* dir: create subdir */
-#define	ACE_READ_NAMED_ATTRS	0x00000008	/* FILE_READ_EA */
-#define	ACE_WRITE_NAMED_ATTRS	0x00000010	/* FILE_WRITE_EA */
-#define	ACE_EXECUTE		0x00000020	/* file: execute */
-#define	ACE_TRAVERSE		0x00000020	/* dir: lookup name */
-#define	ACE_DELETE_CHILD	0x00000040	/* dir: unlink child */
-#define	ACE_READ_ATTRIBUTES	0x00000080	/* (all) stat, etc. */
-#define	ACE_WRITE_ATTRIBUTES	0x00000100	/* (all) utimes, etc. */
-#define	ACE_DELETE		0x00010000	/* (all) unlink self */
-#define	ACE_READ_ACL		0x00020000	/* (all) getsecattr */
-#define	ACE_WRITE_ACL		0x00040000	/* (all) setsecattr */
-#define	ACE_WRITE_OWNER		0x00080000	/* (all) chown */
-#define	ACE_SYNCHRONIZE		0x00100000	/* (all) see MSDN */
-
-/*
- * Some of the following are the same as Windows uses. (but NOT ALL!)
- * See the "ACE_HEADER" structure description on MSDN for details.
- * Comments show relations to the MSDN names.
  */
-#define	ACE_FILE_INHERIT_ACE		0x0001	/* = OBJECT_INHERIT_ACE */
-#define	ACE_DIRECTORY_INHERIT_ACE	0x0002	/* = CONTAINER_INHERIT_ACE */
-#define	ACE_NO_PROPAGATE_INHERIT_ACE	0x0004	/* = NO_PROPAGATE_INHERIT_ACE */
-#define	ACE_INHERIT_ONLY_ACE		0x0008	/* = INHERIT_ONLY_ACE */
+#define	ACE_READ_DATA		0x00000001
+#define	ACE_LIST_DIRECTORY	0x00000001
+#define	ACE_WRITE_DATA		0x00000002
+#define	ACE_ADD_FILE		0x00000002
+#define	ACE_APPEND_DATA		0x00000004
+#define	ACE_ADD_SUBDIRECTORY	0x00000004
+#define	ACE_READ_NAMED_ATTRS	0x00000008
+#define	ACE_WRITE_NAMED_ATTRS	0x00000010
+#define	ACE_EXECUTE		0x00000020
+#define	ACE_DELETE_CHILD	0x00000040
+#define	ACE_READ_ATTRIBUTES	0x00000080
+#define	ACE_WRITE_ATTRIBUTES	0x00000100
+#define	ACE_DELETE		0x00010000
+#define	ACE_READ_ACL		0x00020000
+#define	ACE_WRITE_ACL		0x00040000
+#define	ACE_WRITE_OWNER		0x00080000
+#define	ACE_SYNCHRONIZE		0x00100000
+
+#define	ACE_FILE_INHERIT_ACE		0x0001
+#define	ACE_DIRECTORY_INHERIT_ACE	0x0002
+#define	ACE_NO_PROPAGATE_INHERIT_ACE	0x0004
+#define	ACE_INHERIT_ONLY_ACE		0x0008
 #define	ACE_SUCCESSFUL_ACCESS_ACE_FLAG	0x0010
 #define	ACE_FAILED_ACCESS_ACE_FLAG	0x0020
 #define	ACE_IDENTIFIER_GROUP		0x0040
-#define	ACE_INHERITED_ACE		0x0080	/* INHERITED_ACE, 0x10 on NT */
+#define	ACE_INHERITED_ACE		0x0080
 #define	ACE_OWNER			0x1000
 #define	ACE_GROUP			0x2000
 #define	ACE_EVERYONE			0x4000
 
-/* These four are the same as Windows, but with an ACE_ prefix added. */
 #define	ACE_ACCESS_ALLOWED_ACE_TYPE	0x0000
 #define	ACE_ACCESS_DENIED_ACE_TYPE	0x0001
 #define	ACE_SYSTEM_AUDIT_ACE_TYPE	0x0002
@@ -154,7 +134,6 @@ typedef struct acl_info acl_t;
 
 /*
  * These are only applicable in a CIFS context.
- * Here again, same as Windows, but with an ACE_ prefix added.
  */
 #define	ACE_ACCESS_ALLOWED_COMPOUND_ACE_TYPE		0x04
 #define	ACE_ACCESS_ALLOWED_OBJECT_ACE_TYPE		0x05