svn commit: r302297 - in head/sys/cddl/contrib/opensolaris: common/acl uts/common/fs/zfs uts/common/sys
Alexander Motin
mav at FreeBSD.org
Thu Jun 30 14:55:50 UTC 2016
Author: mav
Date: Thu Jun 30 14:55:49 2016
New Revision: 302297
URL: https://svnweb.freebsd.org/changeset/base/302297
Log:
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)
Modified:
head/sys/cddl/contrib/opensolaris/common/acl/acl_common.c
head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c
head/sys/cddl/contrib/opensolaris/uts/common/sys/acl.h
Modified: head/sys/cddl/contrib/opensolaris/common/acl/acl_common.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/common/acl/acl_common.c Thu Jun 30 14:53:46 2016 (r302296)
+++ head/sys/cddl/contrib/opensolaris/common/acl/acl_common.c Thu Jun 30 14:55:49 2016 (r302297)
@@ -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, bo
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 aclcn
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/
Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c Thu Jun 30 14:53:46 2016 (r302296)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c Thu Jun 30 14:55:49 2016 (r302297)
@@ -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, u
* 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 mod
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;
+ mode_t available_perms;
+ boolean_t dzpcheck_privs = B_TRUE;
+ boolean_t zpcheck_privs = B_TRUE;
+
+ /*
+ * 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().
+ *
+ * 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.
+ */
if (zp->z_pflags & (ZFS_IMMUTABLE | ZFS_NOUNLINK))
return (SET_ERROR(EPERM));
/*
- * 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.
- *
- * 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.)
- */
- 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)
+ * First row
+ * If the directory permissions allow the delete, we are done.
+ */
+ if ((dzp_error = zfs_zaccess_common(dzp, ACE_DELETE_CHILD,
+ &dzp_working_mode, &dzpcheck_privs, B_FALSE, cr)) == 0)
return (0);
/*
- * 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.
- */
- 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 target object has delete permission then we are done
+ */
+ if ((zp_error = zfs_zaccess_common(zp, ACE_DELETE, &zp_working_mode,
+ &zpcheck_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 (dzp_working_mode != wanted_dirperms)
- dzp_error = 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
Modified: head/sys/cddl/contrib/opensolaris/uts/common/sys/acl.h
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/sys/acl.h Thu Jun 30 14:53:46 2016 (r302296)
+++ head/sys/cddl/contrib/opensolaris/uts/common/sys/acl.h Thu Jun 30 14:55:49 2016 (r302297)
@@ -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
More information about the svn-src-head
mailing list