From owner-svn-src-vendor@freebsd.org Wed May 11 13:51:54 2016 Return-Path: Delivered-To: svn-src-vendor@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id D9B61B36A99; Wed, 11 May 2016 13:51:54 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 9A7CE197F; Wed, 11 May 2016 13:51:54 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u4BDprRI077407; Wed, 11 May 2016 13:51:53 GMT (envelope-from mav@FreeBSD.org) Received: (from mav@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u4BDproS077406; Wed, 11 May 2016 13:51:53 GMT (envelope-from mav@FreeBSD.org) Message-Id: <201605111351.u4BDproS077406@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mav set sender to mav@FreeBSD.org using -f From: Alexander Motin Date: Wed, 11 May 2016 13:51:53 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-vendor@freebsd.org Subject: svn commit: r299453 - vendor-sys/illumos/dist/uts/common/fs/zfs X-SVN-Group: vendor-sys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-vendor@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: SVN commit messages for the vendor work area tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 May 2016 13:51:55 -0000 Author: mav Date: Wed May 11 13:51:53 2016 New Revision: 299453 URL: https://svnweb.freebsd.org/changeset/base/299453 Log: 6765 zfs_zaccess_delete() comments do not accurately reflect delete permissions for ACLs Reviewed by: Gordon Ross Reviewed by: Yuri Pankov Author: Kevin Crowe openzfs/openzfs@a40149b935cbbe87bf95e2cc44b3bc99d400513a Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_acl.c Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_acl.c ============================================================================== --- vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_acl.c Wed May 11 13:50:34 2016 (r299452) +++ vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_acl.c Wed May 11 13:51:53 2016 (r299453) @@ -2516,8 +2516,10 @@ int zfs_write_implies_delete_child = 1; /* * Determine whether delete access should be granted. * - * The following chart is the recommended NFSv4 enforcement for - * ability to delete an object. + * 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: * * ------------------------------------------------------- * | Parent Dir | Target Object Permissions | @@ -2526,14 +2528,14 @@ int zfs_write_implies_delete_child = 1; * | | ACL Allows | ACL Denies| Delete | * | | Delete | Delete | unspecified| * ------------------------------------------------------- - * | ACL Allows | Permit | Permit * | Permit | - * | DELETE_CHILD | | | | + * | ACL Allows | Permit | Deny * | Permit | + * | DELETE_CHILD | | | | * ------------------------------------------------------- - * | ACL Denies | Permit * | Deny | Deny | - * | DELETE_CHILD | | | | + * | ACL Denies | Permit | Deny | Deny | + * | DELETE_CHILD | | | | * ------------------------------------------------------- * | ACL specifies | | | | - * | only allow | Permit | Permit * | Permit | + * | only allow | Permit | Deny * | Permit | * | write and | | | | * | execute | | | | * ------------------------------------------------------- @@ -2546,24 +2548,21 @@ 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: We are intentionally disregarding the - * NFSv4 committee recommendation for these three cells of the matrix - * because that recommendation conflicts with the behavior expected - * by Windows clients for ACL evaluation. 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 + * 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 takes a conservative approach by checking for - * DENY ACEs on both the target object and it's container; checking - * the ACE_DELETE on the target object, and ACE_DELETE_CHILD on the - * container. If a DENY ACE is found for either of those, delete - * access is denied. (Note that DENY ACEs are very rare.) - * - * Note that after these changes, entire the second row and the - * entire middle column of the table above change to Deny. - * Accordingly, the logic here is somewhat simplified. + * 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: * * First check for DENY ACEs that apply. * If either target or container has a deny, EACCES.