Pawel Jakub Dawidek
2007-Jul-08 23:44 UTC
[zfs-code] PSARC/2006/465 ZFS Delegated Administration
On Tue, Jun 26, 2007 at 07:45:51AM -0700, Mark.Shellenbaum at Sun.COM wrote:> Author: marks > Repository: /hg/onnv/onnv-gate > Latest revision: 12bb2876a62ea4f4c1b28320f39a0d30334fdf21 > Total changesets: 1 > Log message: > PSARC/2006/465 ZFS Delegated Administration > PSARC/2006/577 zpool property to disable delegation > PSARC/2006/625 Enhancements to zpool history > PSARC/2007/228 ZFS delegation amendments > PSARC/2007/295 ZFS Delegated Administration Addendum > 6280676 restore "owner" property > 6349470 investigate non-root restore/backup > 6572465 ''zpool set bootfs=...'' records history as ''zfs set bootfs=...''In this commit you changed: { zfs_ioc_set_prop, zfs_secpolicy_write, dataset_name }, to: { zfs_ioc_set_prop, zfs_secpolicy_none, dataset_name, B_TRUE }, Was that intended? Is zfs_secpolicy_none() safe for zfs_ioc_set_prop()? -- Pawel Jakub Dawidek http://www.wheel.pl pjd at FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 187 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/zfs-code/attachments/20070709/48c30980/attachment.bin>
Mark Shellenbaum
2007-Jul-08 23:57 UTC
[zfs-code] PSARC/2006/465 ZFS Delegated Administration
Pawel Jakub Dawidek wrote:> On Tue, Jun 26, 2007 at 07:45:51AM -0700, Mark.Shellenbaum at Sun.COM wrote: >> Author: marks >> Repository: /hg/onnv/onnv-gate >> Latest revision: 12bb2876a62ea4f4c1b28320f39a0d30334fdf21 >> Total changesets: 1 >> Log message: >> PSARC/2006/465 ZFS Delegated Administration >> PSARC/2006/577 zpool property to disable delegation >> PSARC/2006/625 Enhancements to zpool history >> PSARC/2007/228 ZFS delegation amendments >> PSARC/2007/295 ZFS Delegated Administration Addendum >> 6280676 restore "owner" property >> 6349470 investigate non-root restore/backup >> 6572465 ''zpool set bootfs=...'' records history as ''zfs set bootfs=...'' > > In this commit you changed: > > { zfs_ioc_set_prop, zfs_secpolicy_write, dataset_name }, > > to: > > { zfs_ioc_set_prop, zfs_secpolicy_none, dataset_name, B_TRUE }, > > Was that intended? Is zfs_secpolicy_none() safe for zfs_ioc_set_prop()? >Yes, that was intended. The permission to set specific properties is checked in zfs_set_prop_nvlist() via zfs_secpolicy_write_perms(). -Mark
Pawel Jakub Dawidek
2007-Jul-09 00:03 UTC
[zfs-code] PSARC/2006/465 ZFS Delegated Administration
On Sun, Jul 08, 2007 at 05:57:25PM -0600, Mark Shellenbaum wrote:> Pawel Jakub Dawidek wrote: > >On Tue, Jun 26, 2007 at 07:45:51AM -0700, Mark.Shellenbaum at Sun.COM wrote: > >>Author: marks > >>Repository: /hg/onnv/onnv-gate > >>Latest revision: 12bb2876a62ea4f4c1b28320f39a0d30334fdf21 > >>Total changesets: 1 > >>Log message: > >>PSARC/2006/465 ZFS Delegated Administration > >>PSARC/2006/577 zpool property to disable delegation > >>PSARC/2006/625 Enhancements to zpool history > >>PSARC/2007/228 ZFS delegation amendments > >>PSARC/2007/295 ZFS Delegated Administration Addendum > >>6280676 restore "owner" property > >>6349470 investigate non-root restore/backup > >>6572465 ''zpool set bootfs=...'' records history as ''zfs set bootfs=...'' > >In this commit you changed: > >{ zfs_ioc_set_prop, zfs_secpolicy_write, dataset_name }, > >to: > >{ zfs_ioc_set_prop, zfs_secpolicy_none, dataset_name, B_TRUE }, > >Was that intended? Is zfs_secpolicy_none() safe for zfs_ioc_set_prop()? > > Yes, that was intended. The permission to set specific properties is checked in zfs_set_prop_nvlist() via zfs_secpolicy_write_perms().Thanks for explanation then and sorry for the noice:) -- Pawel Jakub Dawidek http://www.wheel.pl pjd at FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 187 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/zfs-code/attachments/20070709/26de3eaf/attachment.bin>
Pawel Jakub Dawidek
2007-Jul-09 10:51 UTC
[zfs-code] PSARC/2006/465 ZFS Delegated Administration
On Tue, Jun 26, 2007 at 07:45:51AM -0700, Mark.Shellenbaum at Sun.COM wrote:> Author: marks > Repository: /hg/onnv/onnv-gate > Latest revision: 12bb2876a62ea4f4c1b28320f39a0d30334fdf21 > Total changesets: 1 > Log message: > PSARC/2006/465 ZFS Delegated Administration > PSARC/2006/577 zpool property to disable delegation > PSARC/2006/625 Enhancements to zpool history > PSARC/2007/228 ZFS delegation amendments > PSARC/2007/295 ZFS Delegated Administration Addendum > 6280676 restore "owner" property > 6349470 investigate non-root restore/backup > 6572465 ''zpool set bootfs=...'' records history as ''zfs set bootfs=...''[...] One more very minor thing. In zfs_vfsops.c you added the code below: error = dsl_deleg_access(osname, ZFS_DELEG_PERM_MOUNT, cr); if (error == 0) { vattr_t vattr; /* * Make sure user is the owner of the mount * point * or has sufficient privileges. */ vattr.va_mask = AT_UID; if (VOP_GETATTR(mvp, &vattr, 0, cr)) { goto out; } if (error = secpolicy_vnode_owner(cr, vattr.va_uid)) { goto out; } if (error = VOP_ACCESS(mvp, VWRITE, 0, cr)) { goto out; } secpolicy_fs_mount_clearopts(cr, vfsp); } else { goto out; } In case of VOP_GETATTR() failure, error variable is not set, so we return success without mounting the file system. It''s quite hard to makr VOP_GETATTR() to fail, but just for consistency... -- Pawel Jakub Dawidek http://www.wheel.pl pjd at FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 187 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/zfs-code/attachments/20070709/ea05e87c/attachment.bin>
Mark Shellenbaum
2007-Jul-09 14:01 UTC
[zfs-code] PSARC/2006/465 ZFS Delegated Administration
Pawel Jakub Dawidek wrote:> On Tue, Jun 26, 2007 at 07:45:51AM -0700, Mark.Shellenbaum at Sun.COM wrote: >> Author: marks >> Repository: /hg/onnv/onnv-gate >> Latest revision: 12bb2876a62ea4f4c1b28320f39a0d30334fdf21 >> Total changesets: 1 >> Log message: >> PSARC/2006/465 ZFS Delegated Administration >> PSARC/2006/577 zpool property to disable delegation >> PSARC/2006/625 Enhancements to zpool history >> PSARC/2007/228 ZFS delegation amendments >> PSARC/2007/295 ZFS Delegated Administration Addendum >> 6280676 restore "owner" property >> 6349470 investigate non-root restore/backup >> 6572465 ''zpool set bootfs=...'' records history as ''zfs set bootfs=...'' > [...] > > One more very minor thing. In zfs_vfsops.c you added the code below: > > error = dsl_deleg_access(osname, ZFS_DELEG_PERM_MOUNT, cr); > if (error == 0) { > vattr_t vattr; > > /* > * Make sure user is the owner of the mount > * point > * or has sufficient privileges. > */ > > vattr.va_mask = AT_UID; > > if (VOP_GETATTR(mvp, &vattr, 0, cr)) { > goto out; > } > > if (error = secpolicy_vnode_owner(cr, vattr.va_uid)) { > goto out; > } > > if (error = VOP_ACCESS(mvp, VWRITE, 0, cr)) { > goto out; > } > > secpolicy_fs_mount_clearopts(cr, vfsp); > } else { > goto out; > } > > In case of VOP_GETATTR() failure, error variable is not set, so we > return success without mounting the file system. It''s quite hard to makr > VOP_GETATTR() to fail, but just for consistency... > >I opened the following bug for this. 6578215 zfs_mount()needs to handle GETATTR failures better. -Mark