Mercurial > illumos > fmac
changeset 7878:08be291fa804
[fmac-discuss] [PATCH] Move fmac_vnode_access() hook to zfs_zaccess()
Extend fmac_vnode_access() to understand ACE masks as well as
traditional modes, and move the calls to it from fop_access() and
zfs_create() into zfs_zaccess(). The access functions of other
filesystems would need to be similarly instrumented, although they can
pass conventional modes rather than ACE masks since fmac_vnode_access()
understands both and distinguishes them based on flags.
Note btw that extending fmac_vnode_access() to understand ACE masks is
required regardless of whether we call it from fop_access() or
zfs_zaccess() since callers of VOP_ACCESS() can pass ACE masks (as in
the nfs4 and smbsrv cases).
fmac_vnode_access() is only called if the normal zfs_zaccess() logic
would grant the access (i.e. either the base DAC logic or the secpolicy
hook authorized it). The secpolicy hook is not allowed to override a
FMAC denial.
One immediate benefit of taking the FMAC hook into zfs_zaccess is that
we get proper checking against the base file when a named attribute is
accessed, e.g. runat /etc/passwd cp /tmp/foo attr.1 fails as expected
with a setattr denial. We also gain more complete coverage of file
operations.
I looked at moving the fmac_vnode_access() hook inside of
zfs_zaccess_common(), but it seemed better to do it in zfs_zaccess()
because:
1) We can then place it after both the base DAC logic and the privilege
check.
2) We do not gain any benefit from checking all calls to
zfs_zaccess_common() since other callers like zfs_zaccess_delete() and
_rename() short-circuit their checking if access to the directory is
granted, whereas we want checking against the target file in all cases
and thus still need our separate hooks for those operations.
Some of the other FMAC checks may be obsoleted by taking
fmac_vnode_access() into zfs_zaccess(), but this requires a case-by-case
review and testing as callers of zfs_zaccess() do not always honor a
denial from it (can be overridden by other secpolicy calls), as in the
setattr case.
This is relative to the prior patch for the create hooks.
Webrev is available at:
http://cr.opensolaris.org/~sds/zaccess/
| author | Stephen Smalley <sds@tycho.nsa.gov> |
|---|---|
| date | Tue, 07 Oct 2008 11:13:46 -0400 |
| parents | 098ba4106c40 |
| children | 2b795f3baa58 |
| files | usr/src/uts/common/fmac/fmac.c usr/src/uts/common/fs/vnode.c usr/src/uts/common/fs/zfs/zfs_acl.c usr/src/uts/common/fs/zfs/zfs_vnops.c |
| diffstat | 4 files changed, 57 insertions(+), 23 deletions(-) [+] |
line wrap: on
line diff
--- a/usr/src/uts/common/fmac/fmac.c Tue Oct 07 11:13:13 2008 -0400 +++ b/usr/src/uts/common/fmac/fmac.c Tue Oct 07 11:13:46 2008 -0400 @@ -35,6 +35,7 @@ #include <sys/param.h> #include <sys/kobj.h> #include <sys/vfs.h> +#include <sys/acl.h> #include <sys/fmac/security.h> #include <sys/fmac/fmac.h> #include <sys/fmac/avc.h> @@ -549,6 +550,17 @@ return (0); } +#define fmac_ace_to_av(mask, perm) \ + if (mode & (mask)) { \ + mode &= ~(mask); \ + av |= (perm); \ + } \ + +#define ACE_GETATTR_MASK (ACE_READ_NAMED_ATTRS | ACE_READ_ATTRIBUTES | \ + ACE_READ_ACL) +#define ACE_SETATTR_MASK (ACE_WRITE_NAMED_ATTRS | ACE_WRITE_ATTRIBUTES | \ + ACE_WRITE_ACL | ACE_WRITE_OWNER) + int fmac_vnode_access(vnode_t *vp, int mode, int flags, cred_t *cr, boolean_t audit) @@ -568,17 +580,42 @@ return (0); av = 0; - if (mode & VREAD) - av |= FILE__READ; - if (flags & V_APPEND) - av |= FILE__APPEND; - else if (mode & VWRITE) - av |= FILE__WRITE; - if (mode & VEXEC) { - if (sclass == SECCLASS_DIR) - av |= DIR__SEARCH; - else - av |= FILE__EXECUTE; + + if (flags & V_ACE_MASK) { + mode &= ~ACE_SYNCHRONIZE; /* ignore synchronize bit */ + fmac_ace_to_av(ACE_READ_DATA, FILE__READ); + fmac_ace_to_av(ACE_GETATTR_MASK, FILE__GETATTR); + fmac_ace_to_av(ACE_SETATTR_MASK, FILE__SETATTR); + if (sclass == SECCLASS_DIR) { + fmac_ace_to_av((ACE_ADD_FILE | ACE_ADD_SUBDIRECTORY), + DIR__ADD_NAME); + fmac_ace_to_av(ACE_DELETE_CHILD, DIR__REMOVE_NAME); + fmac_ace_to_av(ACE_DELETE, DIR__RMDIR); + fmac_ace_to_av(ACE_EXECUTE, DIR__SEARCH); + } else { + fmac_ace_to_av(ACE_APPEND_DATA, FILE__APPEND); + fmac_ace_to_av(ACE_WRITE_DATA, FILE__WRITE); + fmac_ace_to_av(ACE_EXECUTE, FILE__EXECUTE); + fmac_ace_to_av(ACE_DELETE, FILE__UNLINK); + } + if (mode) { + cmn_err(CE_WARN, "FMAC: Unknown ACE mask 0x%x\n", + mode); + return (EACCES); + } + } else { + if (mode & VREAD) + av |= FILE__READ; + if (flags & V_APPEND) + av |= FILE__APPEND; + else if (mode & VWRITE) + av |= FILE__WRITE; + if (mode & VEXEC) { + if (sclass == SECCLASS_DIR) + av |= DIR__SEARCH; + else + av |= FILE__EXECUTE; + } } if (!av)
--- a/usr/src/uts/common/fs/vnode.c Tue Oct 07 11:13:13 2008 -0400 +++ b/usr/src/uts/common/fs/vnode.c Tue Oct 07 11:13:46 2008 -0400 @@ -3315,9 +3315,7 @@ err = (*(vp)->v_op->vop_access)(vp, mode, flags, cr, ct); VOPSTATS_UPDATE(vp, access); - if (err) - return (err); - return (fmac_vnode_access(vp, mode, flags, cr, B_TRUE)); + return (err); } int
--- a/usr/src/uts/common/fs/zfs/zfs_acl.c Tue Oct 07 11:13:13 2008 -0400 +++ b/usr/src/uts/common/fs/zfs/zfs_acl.c Tue Oct 07 11:13:46 2008 -0400 @@ -42,6 +42,7 @@ #include <sys/fs/zfs.h> #include <sys/mode.h> #include <sys/policy.h> +#include <sys/fmac/fmac.h> #include <sys/zfs_znode.h> #include <sys/zfs_fuid.h> #include <sys/zfs_acl.h> @@ -2353,11 +2354,8 @@ } if ((error = zfs_zaccess_common(check_zp, mode, &working_mode, - &check_privs, skipaclchk, cr)) == 0) { - if (is_attr) - VN_RELE(ZTOV(xzp)); - return (0); - } + &check_privs, skipaclchk, cr)) == 0) + goto out; if (error && !check_privs) { if (is_attr) @@ -2424,6 +2422,11 @@ } } +out: + if (!error) + error = fmac_vnode_access(ZTOV(check_zp), mode, V_ACE_MASK, cr, + B_TRUE); + if (is_attr) VN_RELE(ZTOV(xzp));
--- a/usr/src/uts/common/fs/zfs/zfs_vnops.c Tue Oct 07 11:13:13 2008 -0400 +++ b/usr/src/uts/common/fs/zfs/zfs_vnops.c Tue Oct 07 11:13:46 2008 -0400 @@ -1333,10 +1333,6 @@ goto out; } - error = fmac_vnode_access(ZTOV(zp), mode, aflags, cr, B_TRUE); - if (error) - goto out; - mutex_enter(&dzp->z_lock); dzp->z_seq++; mutex_exit(&dzp->z_lock);