Module Name:    src
Committed By:   riastradh
Date:           Thu Oct 18 14:22:58 UTC 2012

Modified Files:
        src/external/cddl/osnet/dist/uts/common/fs/zfs: zfs_vnops.c
        src/external/cddl/osnet/sys/kern: policy.c
        src/external/cddl/osnet/sys/sys: policy.h

Log Message:
Take a first whack at making zfs permissions work.

zfs_access uses secpolicy_vnode_access, so it makes no sense for the
latter to call VOP_ACCESS!

Everything seems to return EACCES instead of EPERM, probably because
that's what kauth returns.  This should be fixed, but that may
require some nontrivial surgery to zfs's calls to secpolicy_*, which
is where kauth gets involved.

This commit imports some code from illumos to implement the routine
secpolicy_vnode_setattr.  This shouldn't be outside dist/, but for
now it is expedient to do so.  We ought to fix that, along with all
the other CDDL code outside dist/, when we next import a newer
version of zfs.


To generate a diff of this commit:
cvs rdiff -u -r1.13 -r1.14 \
    src/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c
cvs rdiff -u -r1.3 -r1.4 src/external/cddl/osnet/sys/kern/policy.c
cvs rdiff -u -r1.5 -r1.6 src/external/cddl/osnet/sys/sys/policy.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c
diff -u src/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c:1.13 src/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c:1.14
--- src/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c:1.13	Tue Oct 16 00:04:15 2012
+++ src/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c	Thu Oct 18 14:22:57 2012
@@ -1422,7 +1422,8 @@ top:
 		 * Create a new file object and update the directory
 		 * to reference it.
 		 */
-		if (error = zfs_zaccess(dzp, ACE_ADD_FILE, 0, B_FALSE, cr)) {
+		error = zfs_zaccess(dzp, ACE_ADD_FILE, 0, B_FALSE, cr);
+		if (error) {
 			goto out;
 		}
 
@@ -4779,22 +4780,34 @@ zfs_netbsd_write(void *v)
 static int
 zfs_netbsd_access(void *v)
 {
-	struct vop_access_args *ap = v;
+	struct vop_access_args /* {
+		struct vnode *a_vp;
+		int a_mode;
+		kauth_cred_t a_cred;
+	} */ *ap = v;
+	struct vnode *vp = ap->a_vp;
+	int mode = ap->a_mode;
+	mode_t zfs_mode = 0;
+	kauth_cred_t cred = ap->a_cred;
+	int error;
 
 	/*
-	 * ZFS itself only knowns about VREAD, VWRITE and VEXEC, the rest
-	 * we have to handle by calling vaccess().
-	 */
-	if ((ap->a_mode & ~(VREAD|VWRITE|VEXEC)) != 0) {
-		vnode_t *vp = ap->a_vp;
-		znode_t *zp = VTOZ(vp);
-		znode_phys_t *zphys = zp->z_phys;
+	 * XXX This is really random, especially the left shift by six,
+	 * and it exists only because of randomness in zfs_unix_to_v4
+	 * and zfs_zaccess_rwx in zfs_acl.c.
+	 */
+	if (mode & VREAD)
+		zfs_mode |= S_IROTH;
+	if (mode & VWRITE)
+		zfs_mode |= S_IWOTH;
+	if (mode & VEXEC)
+		zfs_mode |= S_IXOTH;
+	zfs_mode <<= 6;
 
-		return (vaccess(vp->v_type, zphys->zp_mode, zphys->zp_uid,
-		    zphys->zp_gid, ap->a_mode, ap->a_cred));
-	}
+	KASSERT(VOP_ISLOCKED(vp));
+	error = zfs_access(vp, zfs_mode, 0, cred, NULL);
 
-	return (zfs_access(ap->a_vp, ap->a_mode, 0, ap->a_cred, NULL));
+	return (error);
 }
 
 static int
@@ -4849,13 +4862,19 @@ zfs_netbsd_lookup(void *v)
 	    NULL, NULL);
 
 	/*
-	 * Translate errors to match our namei insanity.
+	 * Translate errors to match our namei insanity.  Also, if the
+	 * caller wants to create an entry here, it's apparently our
+	 * responsibility as lookup to make sure that's permissible.
+	 * Go figure.
 	 */
 	if (cnp->cn_flags & ISLASTCN) {
 		switch (cnp->cn_nameiop) {
 		case CREATE:
 		case RENAME:
 			if (error == ENOENT) {
+				error = VOP_ACCESS(dvp, VWRITE, cnp->cn_cred);
+				if (error)
+					break;
 				error = EJUSTRETURN;
 				break;
 			}

Index: src/external/cddl/osnet/sys/kern/policy.c
diff -u src/external/cddl/osnet/sys/kern/policy.c:1.3 src/external/cddl/osnet/sys/kern/policy.c:1.4
--- src/external/cddl/osnet/sys/kern/policy.c:1.3	Mon Oct 15 22:50:25 2012
+++ src/external/cddl/osnet/sys/kern/policy.c	Thu Oct 18 14:22:58 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: policy.c,v 1.3 2012/10/15 22:50:25 riastradh Exp $	*/
+/*	$NetBSD: policy.c,v 1.4 2012/10/18 14:22:58 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2009 The NetBSD Foundation, Inc.
@@ -55,6 +55,31 @@
  * SUCH DAMAGE.
  */
 
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
+ *
+ * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+ * or http://www.opensolaris.org/os/licensing.
+ * See the License for the specific language governing permissions
+ * and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information: Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ */
+/*
+ * Copyright (c) 2003, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright 2012, Joyent, Inc. All rights reserved.
+ */
+
 #include <sys/param.h>
 #include <sys/priv.h>
 #include <sys/vnode.h>
@@ -84,7 +109,7 @@ secpolicy_zinject(kauth_cred_t cred)
 }
 
 int
-secpolicy_fs_mount(kauth_cred_t cred, vnode_t *mvp, struct mount *vfsp)
+secpolicy_fs_mount(kauth_cred_t cred, struct vnode *mvp, struct mount *vfsp)
 {
 
 	return kauth_authorize_system(cred, KAUTH_SYSTEM_MOUNT,
@@ -113,7 +138,7 @@ int
 secpolicy_vnode_stky_modify(kauth_cred_t cred)
 {
 
-	return (EPERM);
+	return kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL);
 }
 
 int
@@ -125,80 +150,69 @@ secpolicy_vnode_remove(kauth_cred_t cred
 
 
 int
-secpolicy_vnode_owner(cred_t *cred, uid_t owner)
+secpolicy_vnode_owner(kauth_cred_t cred, uid_t owner)
 {
-	uid_t uid;
 
-	uid = crgetuid(cred);
-	
-	if (owner == uid)
-		 return (0);
+	if (owner == kauth_cred_getuid(cred))
+		return (0);
 
-	return 0;
-//	return kauth_authorize_system(cred, KAUTH_SYSTEM_MOUNT,
-//	    KAUTH_REQ_SYSTEM_MOUNT_NEW, vfsp, NULL, NULL);
+	return kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL);
 }
 
 int
-secpolicy_vnode_access(kauth_cred_t cred, struct vnode *vp, uint64_t owner,
+secpolicy_vnode_access(kauth_cred_t cred, struct vnode *vp, uid_t owner,
     int mode)
 {
 
-	(void)owner;		/* XXX ignore? */
-	KASSERT(VOP_ISLOCKED(vp));
-	return VOP_ACCESS(vp, mode, cred);
+	return kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL);
 }
 
-/*
- * Check privileges for setting xvattr attributes
- */
 int
-secpolicy_xvattr(xvattr_t *xvap, uid_t owner, cred_t *cr, vtype_t vtype)
+secpolicy_xvattr(xvattr_t *xvap, uid_t owner, kauth_cred_t cred, vtype_t vtype)
 {
-/*	return kauth_authorize_system(cred, KAUTH_SYSTEM_MOUNT,
-	KAUTH_REQ_SYSTEM_MOUNT_UPDATE, vfsp, NULL, NULL);*/
-	return 0;
+
+	return kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL);
 }
 
 int
 secpolicy_vnode_setid_retain(kauth_cred_t cred, boolean_t issuidroot __unused)
 {
 
-	return  (kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL));
+	return kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL);
 }
 
 int
 secpolicy_vnode_setids_setgids(kauth_cred_t cred, gid_t gid)
 {
 
-	if (!groupmember(gid, cred))
-		return  (kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER,
-			NULL));
-	return (0);
+	if (groupmember(gid, cred))
+		return (0);
+
+	return kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL);
 }
 
 int
-secpolicy_vnode_chown(struct kauth_cred *cred, boolean_t check_self)
+secpolicy_vnode_chown(kauth_cred_t cred, boolean_t check_self)
 {
 
-	return  (kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER,
-		NULL));
-	/* return (priv_check_cred(cred, PRIV_VFS_CHOWN, 0)); */
+	return kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL);
 }
 
 int
-secpolicy_vnode_create_gid(struct kauth_cred *cred)
+secpolicy_vnode_create_gid(kauth_cred_t cred)
 {
 
-	return (EPERM);
+	return kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL);
 }
 
 int
-secpolicy_vnode_setdac(struct kauth_cred *cred, uid_t owner)
+secpolicy_vnode_setdac(kauth_cred_t cred, uid_t owner)
 {
 
-	return 0;
-	/*return (priv_check_cred(cred, PRIV_VFS_ADMIN, 0));*/
+	if (owner == kauth_cred_getuid(cred))
+		return (0);
+
+	return kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL);
 }
 
 int
@@ -211,7 +225,7 @@ secpolicy_setid_setsticky_clear(struct v
 	 * is not a member of. Both of these are allowed in jail(8).
 	 */
 	if (vp->v_type != VDIR && (vap->va_mode & S_ISTXT)) {
-		if (kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL) != 0)
+		if (kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL))
 			return (EFTYPE);
 	}
 	/*
@@ -220,17 +234,131 @@ secpolicy_setid_setsticky_clear(struct v
 	 */
 	if ((vap->va_mode & S_ISGID) != 0)
 		return (secpolicy_vnode_setids_setgids(cred, ovap->va_gid));
-       
+
 	return (0);
 }
 
+/*
+ * XXX Copied from illumos.  Should not be here; should be under
+ * external/cddl/osnet/dist.  Not sure why it is even in illumos's
+ * policy.c rather than somewhere in vnode.c or something.
+ */
 int
 secpolicy_vnode_setattr(kauth_cred_t cred, struct vnode *vp, struct vattr *vap,
     const struct vattr *ovap, int flags,
-    int unlocked_access(void *, int, kauth_cred_t ), void *node)
+    int unlocked_access(void *, int, kauth_cred_t), void *node)
 {
+	int mask = vap->va_mask;
+	int error = 0;
+	boolean_t skipaclchk = (flags & ATTR_NOACLCHECK) ? B_TRUE : B_FALSE;
 
-	return 0;
+	if (mask & AT_SIZE) {
+		if (vp->v_type == VDIR) {
+			error = EISDIR;
+			goto out;
+		}
+
+		/*
+		 * If ATTR_NOACLCHECK is set in the flags, then we don't
+		 * perform the secondary unlocked_access() call since the
+		 * ACL (if any) is being checked there.
+		 */
+		if (skipaclchk == B_FALSE) {
+			error = unlocked_access(node, VWRITE, cred);
+			if (error)
+				goto out;
+		}
+	}
+	if (mask & AT_MODE) {
+		/*
+		 * If not the owner of the file then check privilege
+		 * for two things: the privilege to set the mode at all
+		 * and, if we're setting setuid, we also need permissions
+		 * to add the set-uid bit, if we're not the owner.
+		 * In the specific case of creating a set-uid root
+		 * file, we need even more permissions.
+		 */
+		if ((error = secpolicy_vnode_setdac(cred, ovap->va_uid)) != 0)
+			goto out;
+
+		if ((error = secpolicy_setid_setsticky_clear(vp, vap,
+		    ovap, cred)) != 0)
+			goto out;
+	} else
+		vap->va_mode = ovap->va_mode;
+
+	if (mask & (AT_UID|AT_GID)) {
+		boolean_t checkpriv = B_FALSE;
+
+		/*
+		 * Chowning files.
+		 *
+		 * If you are the file owner:
+		 *	chown to other uid		FILE_CHOWN_SELF
+		 *	chown to gid (non-member) 	FILE_CHOWN_SELF
+		 *	chown to gid (member) 		<none>
+		 *
+		 * Instead of PRIV_FILE_CHOWN_SELF, FILE_CHOWN is also
+		 * acceptable but the first one is reported when debugging.
+		 *
+		 * If you are not the file owner:
+		 *	chown from root			PRIV_FILE_CHOWN + zone
+		 *	chown from other to any		PRIV_FILE_CHOWN
+		 *
+		 */
+		if (kauth_cred_getuid(cred) != ovap->va_uid) {
+			checkpriv = B_TRUE;
+		} else {
+			if (((mask & AT_UID) && vap->va_uid != ovap->va_uid) ||
+			    ((mask & AT_GID) && vap->va_gid != ovap->va_gid &&
+			    !groupmember(vap->va_gid, cred))) {
+				checkpriv = B_TRUE;
+			}
+		}
+		/*
+		 * If necessary, check privilege to see if update can be done.
+		 */
+		if (checkpriv &&
+		    (error = secpolicy_vnode_chown(cred, ovap->va_uid)) != 0) {
+			goto out;
+		}
+
+		/*
+		 * If the file has either the set UID or set GID bits
+		 * set and the caller can set the bits, then leave them.
+		 */
+		secpolicy_setid_clear(vap, cred);
+	}
+	if (mask & (AT_ATIME|AT_MTIME)) {
+		/*
+		 * If not the file owner and not otherwise privileged,
+		 * always return an error when setting the
+		 * time other than the current (ATTR_UTIME flag set).
+		 * If setting the current time (ATTR_UTIME not set) then
+		 * unlocked_access will check permissions according to policy.
+		 */
+		if (kauth_cred_getuid(cred) != ovap->va_uid) {
+			if (flags & ATTR_UTIME)
+				error = secpolicy_vnode_utime_modify(cred);
+			else if (skipaclchk == B_FALSE) {
+				error = unlocked_access(node, VWRITE, cred);
+				if (error == EACCES &&
+				    secpolicy_vnode_utime_modify(cred) == 0)
+					error = 0;
+			}
+			if (error)
+				goto out;
+		}
+	}
+
+	/*
+	 * Check for optional attributes here by checking the following:
+	 */
+	if (mask & AT_XVATTR)
+		error = secpolicy_xvattr((xvattr_t *)vap, ovap->va_uid, cred,
+		    vp->v_type);
+out:
+	return (error);
 }
 
 void
@@ -240,10 +368,10 @@ secpolicy_setid_clear(struct vattr *vap,
 		return;
 
 	if ((vap->va_mode & (S_ISUID | S_ISGID)) != 0) {
-			vap->va_mask |= AT_MODE;
-			vap->va_mode &= ~(S_ISUID|S_ISGID);
+		vap->va_mask |= AT_MODE;
+		vap->va_mode &= ~(S_ISUID|S_ISGID);
 	}
-	
+
 	return;
 }
 
@@ -260,7 +388,7 @@ secpolicy_vnode_setdac(kauth_cred_t cred
 int
 secpolicy_vnode_setattr(kauth_cred_t cred, struct vnode *vp, struct vattr *vap,
     const struct vattr *ovap, int flags,
-    int unlocked_access(void *, int, kauth_cred_t ), void *node)
+    int unlocked_access(void *, int, kauth_cred_t), void *node)
 {
 	int mask = vap->va_mask;
 	int error;
@@ -351,7 +479,7 @@ secpolicy_setid_clear(struct vattr *vap,
 
 	if (kauth_authorize_generic(cred, KAUTH_GENERIC_ISSUSER, NULL))
 		return;
-	
+
 	if ((vap->va_mode & (S_ISUID | S_ISGID)) != 0) {
 		if (priv_check_cred(cred, PRIV_VFS_RETAINSUGID, 0)) {
 			vap->va_mask |= AT_MODE;

Index: src/external/cddl/osnet/sys/sys/policy.h
diff -u src/external/cddl/osnet/sys/sys/policy.h:1.5 src/external/cddl/osnet/sys/sys/policy.h:1.6
--- src/external/cddl/osnet/sys/sys/policy.h:1.5	Mon Mar  1 11:19:40 2010
+++ src/external/cddl/osnet/sys/sys/policy.h	Thu Oct 18 14:22:58 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: policy.h,v 1.5 2010/03/01 11:19:40 darran Exp $	*/
+/*	$NetBSD: policy.h,v 1.6 2012/10/18 14:22:58 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2007 Pawel Jakub Dawidek <p...@freebsd.org>
@@ -38,34 +38,35 @@
 #include <sys/vnode.h>
 
 struct mount;
-struct ucred;
 struct vattr;
 struct vnode;
 
-int	secpolicy_zfs(struct kauth_cred  *cred);
-int	secpolicy_sys_config(struct kauth_cred  *cred, int checkonly);
-int	secpolicy_zinject(struct kauth_cred  *cred);
-int 	secpolicy_fs_mount(struct kauth_cred *cred, struct vnode *mvp, struct mount *vfsp);
-int	secpolicy_fs_unmount(struct kauth_cred  *cred, struct mount *vfsp);
-int	secpolicy_basic_link(struct kauth_cred  *cred);
-int	secpolicy_vnode_stky_modify(struct kauth_cred *cred);
-int 	secpolicy_vnode_owner(cred_t *cred, uid_t owner);
-int	secpolicy_vnode_remove(struct kauth_cred *cred);
-int	secpolicy_vnode_access(struct kauth_cred *cred, struct vnode *vp,
-	    uint64_t owner, int mode);
-int 	secpolicy_vnode_chown(struct kauth_cred *cred,
+int	secpolicy_zfs(kauth_cred_t cred);
+int	secpolicy_sys_config(kauth_cred_t cred, int checkonly);
+int	secpolicy_zinject(kauth_cred_t cred);
+int 	secpolicy_fs_mount(kauth_cred_t cred, struct vnode *mvp,
+	    struct mount *vfsp);
+int	secpolicy_fs_unmount(kauth_cred_t cred, struct mount *vfsp);
+int	secpolicy_basic_link(kauth_cred_t cred);
+int	secpolicy_vnode_stky_modify(kauth_cred_t cred);
+int 	secpolicy_vnode_owner(kauth_cred_t cred, uid_t owner);
+int	secpolicy_vnode_remove(kauth_cred_t cred);
+int	secpolicy_vnode_access(kauth_cred_t cred, struct vnode *vp,
+	    uid_t owner, int mode);
+int 	secpolicy_vnode_chown(kauth_cred_t cred,
             boolean_t check_self);
-int	secpolicy_vnode_setdac(struct kauth_cred *cred, uid_t owner);
-int	secpolicy_vnode_setattr(struct kauth_cred *cred, struct vnode *vp,
+int	secpolicy_vnode_setdac(kauth_cred_t cred, uid_t owner);
+int	secpolicy_vnode_setattr(kauth_cred_t cred, struct vnode *vp,
 	    struct vattr *vap, const struct vattr *ovap, int flags,
-	    int unlocked_access(void *, int, struct kauth_cred *), void *node);
-int	secpolicy_vnode_create_gid(struct kauth_cred *cred);
-int	secpolicy_vnode_setids_setgids(struct kauth_cred *cred, gid_t gid);
-int	secpolicy_vnode_setid_retain(struct kauth_cred *cred, boolean_t issuidroot);
-void	secpolicy_setid_clear(struct vattr *vap, struct kauth_cred *cred);
+	    int unlocked_access(void *, int, kauth_cred_t), void *node);
+int	secpolicy_vnode_create_gid(kauth_cred_t cred);
+int	secpolicy_vnode_setids_setgids(kauth_cred_t cred, gid_t gid);
+int	secpolicy_vnode_setid_retain(kauth_cred_t cred, boolean_t issuidroot);
+void	secpolicy_setid_clear(struct vattr *vap, kauth_cred_t cred);
 int	secpolicy_setid_setsticky_clear(struct vnode *vp, struct vattr *vap,
-	    const struct vattr *ovap, struct kauth_cred *cred);
-int     secpolicy_xvattr(xvattr_t *xvap, uid_t owner, cred_t *cr, vtype_t vtype);
+	    const struct vattr *ovap, kauth_cred_t cred);
+int     secpolicy_xvattr(xvattr_t *xvap, uid_t owner, kauth_cred_t cred,
+	    vtype_t vtype);
 
 #endif	/* _KERNEL */
 

Reply via email to