VOP_ACCESS() and new VADMIN/VATTRIB?
Robert Watson
rwatson at FreeBSD.org
Mon Oct 9 16:49:39 GMT 2000
(Apologies again for wide-cross posting :-).
As described in my prior e-mail, I have updated my local tree to make use
of a new VADMIN right for testing access control requests requiring
"ownership" of a file system object to succeed. I've attached below the
patches required to support this change in UFS/FFS, although I have not
yet updated the other file systems. I'd like to go ahead and commit this
support, as it is required for mandatory access control (centralizing the
VADMIN decision means MAC policies can block VADMIN requests centrally in
VOP_ACCESS() for file system that support labeling, rather than having MAC
checks scattered through reems of file system code).
However, I'd like to get someone (or several someone's :-) to review the
code for correctness. In particular, I'm concerned about VFS locking
issues: VOP_ACCESS() requires an exclusive lock on its vp argument, which
is good since VOP_GETACL() and label retrieval functions will require a
lock in UFS, but the requirement for a lock to test ip->i_uid directly
wasn't explicit previously as part of the locking protocol. I believe
I've managed to demonstrate to myself that in locations where the VADMIN
VOP_ACCESS() test occurs, a lock will always be held on the pertinent
vnodes, but I'd like confirmation. If it wasn't require before, the code
was probably buggy anyway, but those bugs will become far more visible in
a world where VOP_ACCESS() could involve a blocking call to access ACLs or
label information.
Robert N M Watson
robert at fledge.watson.org http://www.watson.org/~robert/
PGP key fingerprint: AF B5 5F FF A6 4A 79 37 ED 5F 55 E9 58 04 6A B1
TIS Labs at Network Associates, Safeport Network Services
Index: sys/sys/vnode.h
===================================================================
RCS file: /home/ncvs/src/sys/sys/vnode.h,v
retrieving revision 1.130
diff -u -r1.130 vnode.h
--- sys/sys/vnode.h 2000/10/04 01:29:15 1.130
+++ sys/sys/vnode.h 2000/10/09 16:29:27
@@ -218,12 +218,13 @@
/*
* Modes. Some values same as Ixxx entries from inode.h for now.
*/
-#define VSUID 04000 /* set user id on execution */
-#define VSGID 02000 /* set group id on execution */
-#define VSVTX 01000 /* save swapped text even after use */
-#define VREAD 00400 /* read, write, execute permissions */
-#define VWRITE 00200
-#define VEXEC 00100
+#define VADMIN 010000 /* permission to administer vnode */
+#define VSUID 004000 /* set user id on execution */
+#define VSGID 002000 /* set group id on execution */
+#define VSVTX 001000 /* save swapped text even after use */
+#define VREAD 000400 /* read, write, execute permissions */
+#define VWRITE 000200
+#define VEXEC 000100
/*
* Token indicating no attribute value yet assigned.
Index: sys/kern/vfs_subr.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/vfs_subr.c,v
retrieving revision 1.285
diff -u -r1.285 vfs_subr.c
--- sys/kern/vfs_subr.c 2000/10/06 08:04:48 1.285
+++ sys/kern/vfs_subr.c 2000/10/09 16:29:33
@@ -3050,6 +3050,7 @@
/* Check the owner. */
if (cred->cr_uid == file_uid) {
+ dac_granted |= VADMIN;
if (file_mode & S_IXUSR)
dac_granted |= VEXEC;
if (file_mode & S_IRUSR)
@@ -3116,6 +3117,10 @@
if ((acc_mode & VWRITE) && ((dac_granted & VWRITE) == 0) &&
!cap_check_xxx(cred, NULL, CAP_DAC_WRITE, PRISON_ROOT))
cap_granted |= VWRITE;
+
+ if ((acc_mode & VADMIN) && ((dac_granted & VADMIN) == 0) &&
+ !cap_check_xxx(cred, NULL, CAP_FOWNER, PRISON_ROOT))
+ cap_granted |= VADMIN;
if ((acc_mode & (cap_granted | dac_granted)) == acc_mode) {
/* XXX audit: privilege used */
Index: sys/ufs/ufs/ufs_lookup.c
===================================================================
RCS file: /home/ncvs/src/sys/ufs/ufs/ufs_lookup.c,v
retrieving revision 1.40
diff -u -r1.40 ufs_lookup.c
--- sys/ufs/ufs/ufs_lookup.c 2000/09/18 16:13:01 1.40
+++ sys/ufs/ufs/ufs_lookup.c 2000/10/09 16:29:34
@@ -476,9 +476,8 @@
* implements append-only directories.
*/
if ((dp->i_mode & ISVTX) &&
- suser_xxx(cred, p, PRISON_ROOT) &&
- cred->cr_uid != dp->i_uid &&
- VTOI(tdp)->i_uid != cred->cr_uid) {
+ VOP_ACCESS(vdp, VADMIN, cred, cnp->cn_proc) &&
+ VOP_ACCESS(tdp, VADMIN, cred, cnp->cn_proc)) {
vput(tdp);
return (EPERM);
}
Index: sys/ufs/ufs/ufs_vnops.c
===================================================================
RCS file: /home/ncvs/src/sys/ufs/ufs/ufs_vnops.c,v
retrieving revision 1.150
diff -u -r1.150 ufs_vnops.c
--- sys/ufs/ufs/ufs_vnops.c 2000/10/04 01:29:17 1.150
+++ sys/ufs/ufs/ufs_vnops.c 2000/10/09 16:29:36
@@ -411,13 +411,17 @@
if (vp->v_mount->mnt_flag & MNT_RDONLY)
return (EROFS);
/*
- * Privileged processes in jail() are permitted to modify
- * arbitrary user flags on files, but are not permitted
- * to modify system flags.
+ * Callers may only modify the file flags on objects they
+ * have VADMIN rights for.
*/
- if (cred->cr_uid != ip->i_uid &&
- (error = suser_xxx(cred, p, PRISON_ROOT)))
+ if ((error = VOP_ACCESS(vp, VADMIN, cred, p)))
return (error);
+ /*
+ * Unprivileged processes and privileged processes in
+ * jail() are not permitted to set system flags.
+ * Privileged processes not in jail() may only set system
+ * flags if the securelevel <= 0.
+ */
if (!suser_xxx(cred, NULL, 0)) {
if ((ip->i_flags
& (SF_NOUNLINK | SF_IMMUTABLE | SF_APPEND)) &&
@@ -450,7 +454,8 @@
if (vap->va_uid != (uid_t)VNOVAL || vap->va_gid != (gid_t)VNOVAL) {
if (vp->v_mount->mnt_flag & MNT_RDONLY)
return (EROFS);
- if ((error = ufs_chown(vp, vap->va_uid, vap->va_gid, cred, p)) != 0)
+ if ((error = ufs_chown(vp, vap->va_uid, vap->va_gid, cred,
+ p)) != 0)
return (error);
}
if (vap->va_size != VNOVAL) {
@@ -480,8 +485,15 @@
return (EROFS);
if ((ip->i_flags & SF_SNAPSHOT) != 0)
return (EPERM);
- if (cred->cr_uid != ip->i_uid &&
- (error = suser_xxx(cred, p, PRISON_ROOT)) &&
+ /*
+ * From utimes(2):
+ * If times is NULL, ... The caller must be the owner of
+ * the file, have permission to write the file, or be the
+ * super-user.
+ * If times is non-NULL, ... The caller must be the owner of
+ * the file or be the super-user.
+ */
+ if ((error = VOP_ACCESS(vp, VADMIN, cred, p)) &&
((vap->va_vaflags & VA_UTIMES_NULL) == 0 ||
(error = VOP_ACCESS(vp, VWRITE, cred, p))))
return (error);
@@ -529,11 +541,17 @@
register struct inode *ip = VTOI(vp);
int error;
- if (cred->cr_uid != ip->i_uid) {
- error = suser_xxx(cred, p, PRISON_ROOT);
- if (error)
+ /*
+ * To modify the permissions on a file, must possess VADMIN
+ * for that file.
+ */
+ if ((error = VOP_ACCESS(vp, VADMIN, cred, p)))
return (error);
- }
+ /*
+ * Privileged processes may set the sticky bit on non-directories,
+ * as well as set the setgid bit on a file with a group that the
+ * process is not a member of.
+ */
if (suser_xxx(cred, NULL, PRISON_ROOT)) {
if (vp->v_type != VDIR && (mode & S_ISTXT))
return (EFTYPE);
@@ -571,12 +589,18 @@
uid = ip->i_uid;
if (gid == (gid_t)VNOVAL)
gid = ip->i_gid;
+ /*
+ * To modify the ownership of a file, must possess VADMIN
+ * for that file.
+ */
+ if ((error = VOP_ACCESS(vp, VADMIN, cred, p)))
+ return (error);
/*
- * If we don't own the file, are trying to change the owner
- * of the file, or are not a member of the target group,
- * the caller must be superuser or the call fails.
+ * To change the owner of a file, or change the group of a file
+ * to a group of which we are not a member, the caller must
+ * have privilege.
*/
- if ((cred->cr_uid != ip->i_uid || uid != ip->i_uid ||
+ if ((uid != ip->i_uid ||
(gid != ip->i_gid && !groupmember(gid, cred))) &&
(error = suser_xxx(cred, p, PRISON_ROOT)))
return (error);
@@ -1095,15 +1119,14 @@
if (xp->i_number == ip->i_number)
panic("ufs_rename: same file");
/*
- * If the parent directory is "sticky", then the user must
- * own the parent directory, or the destination of the rename,
- * otherwise the destination may not be changed (except by
- * root). This implements append-only directories.
+ * If the parent directory is "sticky", then the caller
+ * must possess VADMIN for the parent directory, or the
+ * destination of the rename. This implements append-only
+ * directories.
*/
if ((dp->i_mode & S_ISTXT) &&
- suser_xxx(tcnp->cn_cred, NULL, PRISON_ROOT) &&
- tcnp->cn_cred->cr_uid != dp->i_uid &&
- xp->i_uid != tcnp->cn_cred->cr_uid) {
+ VOP_ACCESS(tdvp, VADMIN, tcnp->cn_cred, p) &&
+ VOP_ACCESS(tvp, VADMIN, tcnp->cn_cred, p)) {
error = EPERM;
goto bad;
}
To Unsubscribe: send mail to majordomo at trustedbsd.org
with "unsubscribe trustedbsd-discuss" in the body of the message
More information about the trustedbsd-discuss
mailing list