Project

General

Profile

Actions

Feature #15338

closed

ZFS implicit owner rights should be configurable

Added by Gordon Ross over 1 year ago. Updated 7 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
zfs - Zettabyte File System
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
External Bug:
racktop:BSR-12324

Description

ZFS currently grants some implicit permissions to the owner of an object.
In particular, "Read_Control" and "Write_DAC" are granted to owners of
objects even if those rights are not in the owner ACE.  That was the
traditional behavior on ZFS, and Windows systems before 2008.
The old behavior (implicit rights for owner) is now considered a
security flaw, so customers want a way to turn that off.

On modern Windows systems (Win2008 and later), the implicit rights
for owner are conditional.  When an ACL contains an ACE with the
"Owner Rights" SID (S-1-3-4) then the rights from that ACE are used
instead of the implicit rights.

See the description of "S-1-3-4" (Owner Rights) in:
[understand-special-identities-groups](https://learn.microsoft.com/en-us/windows-server/identity/ad-ds/manage/understand-special-identities-groups)
We'd like similarly flexible behavior from ZFS re. owner rights.

Now one challenge (or opportunity) is that with ZFS we already have an
ACE type that serves the role that the "Owner Rights" SID serves, and
that's our "owner@" ACE type.  One simple approach here would be to
treat the rights in the "owner@" ACE as explicit/definitive, and not add
the implicit owner rights to those explicitly granted.

I had initially thought to make ZFS always behave the way Windows would
when the "S-1-3-4" is present, not granting implicit rights to file owners in the
code paths for write_owner and write_acl. After some discussion, I now think
It would be safer to make this configurable, in case there are environments that
require the traditional "implicit owner rights" behavior.

We could make this configurable the same way Windows does, but that's a
fairly complicated configuration method and we don't need granularity of
this setting at the per-object level. Instead, we can let this configuration
have granularity at the level of ZFS datasets, using a new ZFS property
"aclimplicit" (implicit owner rights, true/false). When true, the dataset will
keep the traditional behavior, and when false, owner gets only the rights
explicitly granted in the "owner@" ACEs.

For the new property "aclimplicit", we will let the default be "on", which
adheres to the principle of "least surprise" by maintaining the current
behavior as seen on systems before this change. One must set this
property "aclimplicit=off" to disable implicit owner rights.

Note that during testing, we found that "aclimplicit=off" really only makes sense
with "aclmode=passthrough" and "aclinherit=passthrough". With other
aclmode and aclinherit settings, one ends up with the "write_acl" flag in
the "owner@" ACE such that "aclimplicit" would have no impact.

Note that ZFS on Linux has completely different ACL code, and does not
fully implement fine-grained ACLs, so this change should not affect them.

Testing done: as suggested below:

root@beastie:/export# zfs create -o aclimplicit=off rpool/export/test
root@beastie:/export# chown tsoome:staff test
root@beastie:/export# exit
exit
tsoome@beastie:/code/illumos-gate$ cd /export/test/
tsoome@beastie:/export/test$ 
tsoome@beastie:/export/test$ mkdir testdir
tsoome@beastie:/export/test$ touch testfile.txt
tsoome@beastie:/export/test$ ls -la
total 27
drwxr-xr-x   3 tsoome   staff          4 Nov  2 22:26 .
drwxr-xr-x  13 root     root          13 Nov  2 22:25 ..
drwxr-xr-x   2 tsoome   staff          2 Nov  2 22:25 testdir
-rw-r--r--   1 tsoome   staff          0 Nov  2 22:26 testfile.txt
tsoome@beastie:/export/test$ ls -lV
total 2
drwxr-xr-x   2 tsoome   staff          2 Nov  2 22:25 testdir
                 owner@:rwxp-DaARWcCos:-------:allow
                 group@:r-x---a-R-c--s:-------:allow
              everyone@:r-x---a-R-c--s:-------:allow
-rw-r--r--   1 tsoome   staff          0 Nov  2 22:26 testfile.txt
                 owner@:rw-p--aARWcCos:-------:allow
                 group@:r-----a-R-c--s:-------:allow
              everyone@:r-----a-R-c--s:-------:allow
tsoome@beastie:/export/test$ mkdir tmp
tsoome@beastie:/export/test$ ls -l
total 3
drwxr-xr-x   2 tsoome   staff          2 Nov  2 22:25 testdir
-rw-r--r--   1 tsoome   staff          0 Nov  2 22:26 testfile.txt
drwxr-xr-x   2 tsoome   staff          2 Nov  2 22:26 tmp
tsoome@beastie:/export/test$
tsoome@beastie:/export/test$ sudo chown root:root testdir testfile.txt 
tsoome@beastie:/export/test$ ls -la
total 28
drwxr-xr-x   4 tsoome   staff          5 Nov  2 22:26 .
drwxr-xr-x  13 root     root          13 Nov  2 22:25 ..
drwxr-xr-x   2 root     root           2 Nov  2 22:25 testdir
-rw-r--r--   1 root     root           0 Nov  2 22:26 testfile.txt
drwxr-xr-x   2 tsoome   staff          2 Nov  2 22:26 tmp
tsoome@beastie:/export/test$ ls -lV
total 3
drwxr-xr-x   2 root     root           2 Nov  2 22:25 testdir
                 owner@:rwxp-DaARWcCos:-------:allow
                 group@:r-x---a-R-c--s:-------:allow
              everyone@:r-x---a-R-c--s:-------:allow
-rw-r--r--   1 root     root           0 Nov  2 22:26 testfile.txt
                 owner@:rw-p--aARWcCos:-------:allow
                 group@:r-----a-R-c--s:-------:allow
              everyone@:r-----a-R-c--s:-------:allow
drwxr-xr-x   2 tsoome   staff          2 Nov  2 22:26 tmp
                 owner@:rwxp-DaARWcCos:-------:allow
                 group@:r-x---a-R-c--s:-------:allow
              everyone@:r-x---a-R-c--s:-------:allow
tsoome@beastie:/export/test$
tsoome@beastie:/export/test$ chmod A0=owner@:rwxp-DaARWcCos:fd:allow tmp
tsoome@beastie:/export/test$ ls -lV
total 3
drwxr-xr-x   2 root     root           2 Nov  2 22:25 testdir
                 owner@:rwxp-DaARWcCos:-------:allow
                 group@:r-x---a-R-c--s:-------:allow
              everyone@:r-x---a-R-c--s:-------:allow
-rw-r--r--   1 root     root           0 Nov  2 22:26 testfile.txt
                 owner@:rw-p--aARWcCos:-------:allow
                 group@:r-----a-R-c--s:-------:allow
              everyone@:r-----a-R-c--s:-------:allow
drwxr-xr-x+  2 tsoome   staff          2 Nov  2 22:26 tmp
                 owner@:rwxp-DaARWcCos:fd-----:allow
                 group@:r-x---a-R-c--s:-------:allow
              everyone@:r-x---a-R-c--s:-------:allow
tsoome@beastie:/export/test$ chmod A0=owner@:rwxp-DaARWc--s:fd:allow tmp
tsoome@beastie:/export/test$ ls -lV
total 3
drwxr-xr-x   2 root     root           2 Nov  2 22:25 testdir
                 owner@:rwxp-DaARWcCos:-------:allow
                 group@:r-x---a-R-c--s:-------:allow
              everyone@:r-x---a-R-c--s:-------:allow
-rw-r--r--   1 root     root           0 Nov  2 22:26 testfile.txt
                 owner@:rw-p--aARWcCos:-------:allow
                 group@:r-----a-R-c--s:-------:allow
              everyone@:r-----a-R-c--s:-------:allow
drwxr-xr-x+  2 tsoome   staff          2 Nov  2 22:26 tmp
                 owner@:rwxp-DaARWc--s:fd-----:allow
                 group@:r-x---a-R-c--s:-------:allow
              everyone@:r-x---a-R-c--s:-------:allow
tsoome@beastie:/export/test$ chmod A0=owner@:read_set:fd:allow tmp
chmod: ERROR: Failed to set ACL: Not owner
tsoome@beastie:/export/test$ chown guest tmp
chown: tmp: Not owner
tsoome@beastie:/export/test$ chmod A0=owner@:rwxpDaARWcs:fd:allow tmp 
chmod: ERROR: Failed to set ACL: Not owner
tsoome@beastie:/export/test$ sudo chmod A0=owner@:rwxpDaARWcs:fd:allow tmp 
tsoome@beastie:/export/test$ ls -ldV tmp
drwxr-xr-x+  2 tsoome   staff          2 Nov  2 22:26 tmp
                 owner@:rwxp-DaARWc--s:fd-----:allow
                 group@:r-x---a-R-c--s:-------:allow
              everyone@:r-x---a-R-c--s:-------:allow
tsoome@beastie:/export/test$ chmod 750 tmp
chmod: WARNING: can't change tmp
tsoome@beastie:/export/test$ ls -ldV tmp
drwxr-xr-x+  2 tsoome   staff          2 Nov  2 22:26 tmp
                 owner@:rwxp-DaARWc--s:fd-----:allow
                 group@:r-x---a-R-c--s:-------:allow
              everyone@:r-x---a-R-c--s:-------:allow
tsoome@beastie:/export/test$ 


Related issues

Related to illumos gate - Bug #15964: ZFS test fixes to go with 15338ClosedToomas Soome

Actions
Actions #1

Updated by Gordon Ross over 1 year ago

  • External Bug set to racktop:BSR-12324
Actions #2

Updated by Gordon Ross over 1 year ago

  • Subject changed from ZFS Implicit owner rights should be configurable to ZFS implicit owner rights should be configurable
  • Status changed from New to In Progress
Actions #3

Updated by Gordon Ross over 1 year ago

  • Description updated (diff)
Actions #4

Updated by Toomas Soome 8 months ago

  • Assignee changed from Gordon Ross to Toomas Soome
  • % Done changed from 0 to 90
  • Gerrit CR set to 3071
Actions #5

Updated by Toomas Soome 8 months ago

  • Related to Bug #15964: ZFS test fixes to go with 15338 added
Actions #6

Updated by Gordon Ross 8 months ago

Tested primarily with the ZFS ACL tests shown in the related issue #15964
We've had this in the field for about six months.

Actions #7

Updated by Gordon Ross 8 months ago

Some simple "spot checks" for this change:
(with "aclimplicit=off")

gwr@gwr-test1:/storage/data/global/foo$ ls -lV                                  
total 4
drwxr-xr-x   2 root     root           2 May 12  2022 smart1
                 owner@:rwxp-DaARWcCos:-------:allow
                 group@:r-x---a-R-c--s:-------:allow
              everyone@:r-x---a-R-c--s:-------:allow
-rwxr-xr-x   1 root     root         179 Mar  9  2022 snaps
                 owner@:rwxp--aARWcCos:-------:allow
                 group@:r-x---a-R-c--s:-------:allow
              everyone@:r-x---a-R-c--s:-------:allow
drwxrwxrwx+  2 gwr      staff          2 Jan 14  2022 tmp
                 owner@:rwxpdDaARWc--s:fd-----:allow
                 group@:rwxp-Da-R-c--s:-------:allow
              everyone@:rwxp-Da-R-c--s:-------:allow
gwr@gwr-test1:/storage/data/global/foo$ chmod A0=owner@:read_set:fd:allow tmp   
chmod: ERROR: Failed to set ACL: Not owner
gwr@gwr-test1:/storage/data/global/foo$ chown test tmp                          
chown: tmp: Not owner
gwr@gwr-test1:/storage/data/global/foo$

Note that, even though "gwr" is the owner, because the ACL is not a "trivial" ACL (the explicit write_owner | write_dac bits are not in the "owner@" ACE for "tmp") the policy overrides for owner do not apply.

Another chmod test case:

gwr@gwr-test1:/storage/data/global/foo$ chmod A0=owner@:rwxpDaARWcs:fd:allow tmp                                        
gwr@gwr-test1:/storage/data/global/foo$ ls -ldV tmp                             
drwxr-xr-x+  2 gwr      staff          2 Dec  2 21:34 tmp
                 owner@:rwxp-DaARWc--s:fd-----:allow
                 group@:r-x---a-R-c--s:-------:allow
              everyone@:r-x---a-R-c--s:-------:allow
gwr@gwr-test1:/storage/data/global/foo$ chmod 750 tmp                           
chmod: WARNING: can't change tmp
gwr@gwr-test1:/storage/data/global/foo$ ls -ldV tmp                             
drwxr-xr-x+  2 gwr      staff          2 Dec  2 21:34 tmp
                 owner@:rwxp-DaARWc--s:fd-----:allow
                 group@:r-x---a-R-c--s:-------:allow
              everyone@:r-x---a-R-c--s:-------:allow
gwr@gwr-test1:/storage/data/global/foo$                                         

Actions #8

Updated by Toomas Soome 7 months ago

  • Description updated (diff)
Actions #9

Updated by Electric Monk 7 months ago

  • Status changed from In Progress to Closed
  • % Done changed from 90 to 100

git commit 705610572a32b33c0da7d60b05c6dc30597a9e5a

commit  705610572a32b33c0da7d60b05c6dc30597a9e5a
Author: Gordon Ross <gwr@racktopsystems.com>
Date:   2023-11-03T20:32:59.000Z

    15338 Implicit owner rights should be configurable
    Reviewed by: Matt Barden <mbarden@racktopsystems.com>
    Reviewed by: Paul Zuchowski <p.zuchowski98@gmail.com>
    Approved by: Joshua M. Clulow <josh@sysmgr.org>

Actions

Also available in: Atom PDF