Skip to content

Commit

Permalink
OpenZFS 6765 - zfs_zaccess_delete() comments do not accurately
Browse files Browse the repository at this point in the history
reflect delete permissions for ACLs

Authored by: Kevin Crowe <[email protected]>
Reviewed by: Gordon Ross <[email protected]>
Reviewed by: Yuri Pankov <[email protected]>
Reviewed by: Brian Behlendorf <[email protected]>
Approved by: Richard Lowe <[email protected]>
Ported-by: Paul B. Henson <[email protected]>

Porting Notes:
* Only comments are updated

OpenZFS-issue: https://www.illumos.org/issues/6765
OpenZFS-commit: openzfs/openzfs@da412744bc
Closes openzfs#10266
  • Loading branch information
pbhenson authored and jsai20 committed Mar 30, 2021
1 parent 8a637ac commit 8e96017
Showing 1 changed file with 21 additions and 22 deletions.
43 changes: 21 additions & 22 deletions module/os/linux/zfs/zfs_acl.c
Original file line number Diff line number Diff line change
Expand Up @@ -2687,8 +2687,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 |
Expand All @@ -2697,14 +2699,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 | | | |
* -------------------------------------------------------
Expand All @@ -2717,24 +2719,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.)
* 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.
*
* 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.
* 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.
Expand Down

0 comments on commit 8e96017

Please sign in to comment.