jaildesc: remove file-mode-based access controls

Jail descriptors were given a file-like mode, user, and group, for the
purpose of controlling how the descriptor may be used.  This is too far
removed from the file paradigm to make sense.  Remove it in favor of a
better access control method to be added, such as Capsicum.

Also add missing code in jaildesc_fill_kinfo.

Reported by:	crest at rlwinm.de, kib
MFC after:	3 days

lib/libsys/jail.2

@@ -340,31 +340,6 @@ work the same as
 and
 .Fn jail_remove ,
 except that they operate on the jail referred to by the passed descriptor.
-.Pp
-Jail operations via descriptors can be done by processes that do not
-normally have permission to see or affect the jail,
-as long as they are allowed by the file permissions of the jail
-descriptor itself.
-These permissions can be changed by the descriptor owner via
-.Xr fchmod 2
-and
-.Xr fchown 2 .
-.Fn jail_get
-requires read permission,
-.Fn jail_set
-and
-.Fn jail_remove
-require write permission,
-and
-.Fn jail_attach
-requires execute permission.
-Also, use of a descriptor with the
-.Dv JAIL_AT_DESC
-flag requires execute permission.
-An owning descriptor is identified by the
-.Em sticky bit ,
-which may also be changed via
-.Xr fchmod 2 .
 .Sh RETURN VALUES
 If successful,
 .Fn jail ,
@@ -402,22 +377,6 @@ The
 system call
 will fail if:
 .Bl -tag -width Er
-.It Bq Er EACCES
-Write permission is denied on the jail descriptor in the
-.Va desc
-parameter,
-and the
-.Dv JAIL_USE_DESC
-flag was set.
-.It Bq Er EACCES
-Execute permission is denied on the jail descriptor in the
-.Va desc
-parameter,
-and either the
-.Dv JAIL_AT_DESC
-or
-.Dv JAIL_ATTACH
-flag was set.
 .It Bq Er EPERM
 This process is not allowed to create a jail, either because it is not
 the super-user, or because it would exceed the jail's
@@ -505,24 +464,6 @@ The
 system call
 will fail if:
 .Bl -tag -width Er
-.It Bq Er EACCES
-Read permission is denied on the jail descriptor in the
-.Va desc
-parameter,
-and the
-.Dv JAIL_USE_DESC
-flag was set.
-.It Bq Er EACCES
-Execute permission is denied on the jail descriptor in the
-.Va desc
-parameter,
-and the
-.Dv JAIL_AT_DESC
-flag was set.
-.It Bq Er EFAULT
-.Fa Iov ,
-or one of the addresses contained within it,
-points to an address outside the allocated address space of the process.
 .It Bq Er ENOENT
 The jail referred to by a
 .Va jid
@@ -597,14 +538,6 @@ will fail if:
 The
 .Fa fd
 argument is not a valid jail descriptor.
-.It Bq Er EACCES
-Permission is denied on the jail descriptor
-.Po
-execute permission for
-.Fn jail_attach_fd ,
-or write permission for
-.Fn jail_remove_fd
-.Pc .
 .It Bq Er EPERM
 The jail descriptor was created by a user other than the super-user.
 .It Bq Er EINVAL

sys/kern/kern_jail.c

@@ -991,7 +991,6 @@ int
 kern_jail_set(struct thread *td, struct uio *optuio, int flags)
 {
 	struct file *jfp_out;
-	struct jaildesc *desc_in;
 	struct nameidata nd;
 #ifdef INET
 	struct prison_ip *ip4;
@@ -1095,24 +1094,13 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
 			 * descriptor's prison.
 			 */
 			prison_free(mypr);
-			error = jaildesc_find(td, jfd_in, &desc_in, &mypr,
-			    NULL);
+			error = jaildesc_find(td, jfd_in, &mypr, NULL);
 			if (error != 0) {
 				vfs_opterror(opts, error == ENOENT ?
 				    "descriptor to dead jail" :
 				    "not a jail descriptor");
 				goto done_errmsg;
 			}
-			/*
-			 * Check file permissions using the current
-			 * credentials, and operation permissions
-			 * using the descriptor's credentials.
-			 */
-			error = vaccess(VREG, desc_in->jd_mode, desc_in->jd_uid,
-			    desc_in->jd_gid, VEXEC, td->td_ucred);
-			JAILDESC_UNLOCK(desc_in);
-			if (error != 0)
-				goto done_free;
 			if ((flags & JAIL_CREATE) && mypr->pr_childmax == 0) {
 				error = EPERM;
 				goto done_free;
@@ -1516,7 +1504,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
 	}
 	if (flags & JAIL_USE_DESC) {
 		/* Get the jail from its descriptor. */
-		error = jaildesc_find(td, jfd_in, &desc_in, &pr, &jdcred);
+		error = jaildesc_find(td, jfd_in, &pr, &jdcred);
 		if (error) {
 			vfs_opterror(opts, error == ENOENT ?
 			    "descriptor to dead jail" :
@@ -1524,19 +1512,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags)
 			goto done_deref;
 		}
 		drflags |= PD_DEREF;
-		/*
-		 * Check file permissions using the current credentials,
-		 * and operation permissions using the descriptor's
-		 * credentials.
-		 */
-		error = vaccess(VREG, desc_in->jd_mode, desc_in->jd_uid,
-		    desc_in->jd_gid, VWRITE, td->td_ucred);
-		if (error == 0 && (flags & JAIL_ATTACH))
-			error = vaccess(VREG, desc_in->jd_mode, desc_in->jd_uid,
-			    desc_in->jd_gid, VEXEC, td->td_ucred);
-		JAILDESC_UNLOCK(desc_in);
-		if (error == 0)
-			error = priv_check_cred(jdcred, PRIV_JAIL_SET);
+		error = priv_check_cred(jdcred, PRIV_JAIL_SET);
 		if (error == 0 && (flags & JAIL_ATTACH))
 			error = priv_check_cred(jdcred, PRIV_JAIL_ATTACH);
 		crfree(jdcred);
@@ -2500,7 +2476,6 @@ kern_jail_get(struct thread *td, struct uio *optuio, int flags)
 {
 	struct bool_flags *bf;
 	struct file *jfp_out;
-	struct jaildesc *desc_in;
 	struct jailsys_flags *jsf;
 	struct prison *pr, *mypr;
 	struct vfsopt *opt;
@@ -2547,7 +2522,7 @@ kern_jail_get(struct thread *td, struct uio *optuio, int flags)
 		}
 		if (flags & JAIL_USE_DESC) {
 			/* Get the jail from its descriptor. */
-			error = jaildesc_find(td, jfd_in, &desc_in, &pr, NULL);
+			error = jaildesc_find(td, jfd_in, &pr, NULL);
 			if (error) {
 				vfs_opterror(opts, error == ENOENT ?
 				    "descriptor to dead jail" :
@@ -2555,11 +2530,6 @@ kern_jail_get(struct thread *td, struct uio *optuio, int flags)
 				goto done;
 			}
 			drflags |= PD_DEREF;
-			error = vaccess(VREG, desc_in->jd_mode, desc_in->jd_uid,
-			    desc_in->jd_gid, VREAD, td->td_ucred);
-			JAILDESC_UNLOCK(desc_in);
-			if (error != 0)
-				goto done;
 			mtx_lock(&pr->pr_mtx);
 			drflags |= PD_LOCKED;
 			if (!(prison_isalive(pr) || (flags & JAIL_DYING))) {
@@ -2573,19 +2543,13 @@ kern_jail_get(struct thread *td, struct uio *optuio, int flags)
 		if (flags & JAIL_AT_DESC) {
 			/* Look up jails based on the descriptor's prison. */
 			prison_free(mypr);
-			error = jaildesc_find(td, jfd_in, &desc_in, &mypr,
-			    NULL);
+			error = jaildesc_find(td, jfd_in, &mypr, NULL);
 			if (error != 0) {
 				vfs_opterror(opts, error == ENOENT ?
 				    "descriptor to dead jail" :
 				    "not a jail descriptor");
 				goto done;
 			}
-			error = vaccess(VREG, desc_in->jd_mode, desc_in->jd_uid,
-			    desc_in->jd_gid, VEXEC, td->td_ucred);
-			JAILDESC_UNLOCK(desc_in);
-			if (error != 0)
-				goto done;
 		}
 		if (flags & (JAIL_GET_DESC | JAIL_OWN_DESC)) {
 			/* Allocate a jail descriptor to return later. */
@@ -2916,23 +2880,14 @@ sys_jail_remove(struct thread *td, struct jail_remove_args *uap)
 int
 sys_jail_remove_jd(struct thread *td, struct jail_remove_jd_args *uap)
 {
-	struct jaildesc *jd;
 	struct prison *pr;
 	struct ucred *jdcred;
 	int error;
 
-	error = jaildesc_find(td, uap->fd, &jd, &pr, &jdcred);
+	error = jaildesc_find(td, uap->fd, &pr, &jdcred);
 	if (error)
 		return (error);
-	/*
-	 * Check file permissions using the current credentials, and
-	 * operation permissions using the descriptor's credentials.
-	 */
-	error = vaccess(VREG, jd->jd_mode, jd->jd_uid, jd->jd_gid, VWRITE,
-	    td->td_ucred);
-	JAILDESC_UNLOCK(jd);
-	if (error == 0)
-	    error = priv_check_cred(jdcred, PRIV_JAIL_REMOVE);
+	error = priv_check_cred(jdcred, PRIV_JAIL_REMOVE);
 	crfree(jdcred);
 	if (error) {
 		prison_free(pr);
@@ -3002,26 +2957,17 @@ sys_jail_attach(struct thread *td, struct jail_attach_args *uap)
 int
 sys_jail_attach_jd(struct thread *td, struct jail_attach_jd_args *uap)
 {
-	struct jaildesc *jd;
 	struct prison *pr;
 	struct ucred *jdcred;
 	int drflags, error;
 
 	sx_slock(&allprison_lock);
 	drflags = PD_LIST_SLOCKED;
-	error = jaildesc_find(td, uap->fd, &jd, &pr, &jdcred);
+	error = jaildesc_find(td, uap->fd, &pr, &jdcred);
 	if (error)
 		goto fail;
 	drflags |= PD_DEREF;
-	/*
-	 * Check file permissions using the current credentials, and
-	 * operation permissions using the descriptor's credentials.
-	 */
-	error = vaccess(VREG, jd->jd_mode, jd->jd_uid, jd->jd_gid, VEXEC,
-	    td->td_ucred);
-	JAILDESC_UNLOCK(jd);
-	if (error == 0)
-		error = priv_check_cred(jdcred, PRIV_JAIL_ATTACH);
+	error = priv_check_cred(jdcred, PRIV_JAIL_ATTACH);
 	crfree(jdcred);
 	if (error)
 		goto fail;

sys/kern/kern_jaildesc.c

@@ -41,14 +41,13 @@
 #include <sys/sysproto.h>
 #include <sys/systm.h>
 #include <sys/ucred.h>
+#include <sys/user.h>
 #include <sys/vnode.h>
 
 MALLOC_DEFINE(M_JAILDESC, "jaildesc", "jail descriptors");
 
 static fo_stat_t	jaildesc_stat;
 static fo_close_t	jaildesc_close;
-static fo_chmod_t	jaildesc_chmod;
-static fo_chown_t	jaildesc_chown;
 static fo_fill_kinfo_t	jaildesc_fill_kinfo;
 static fo_cmp_t		jaildesc_cmp;
 
@@ -61,8 +60,8 @@ static struct fileops jaildesc_ops = {
 	.fo_kqfilter = invfo_kqfilter,
 	.fo_stat = jaildesc_stat,
 	.fo_close = jaildesc_close,
-	.fo_chmod = jaildesc_chmod,
-	.fo_chown = jaildesc_chown,
+	.fo_chmod = invfo_chmod,
+	.fo_chown = invfo_chown,
 	.fo_sendfile = invfo_sendfile,
 	.fo_fill_kinfo = jaildesc_fill_kinfo,
 	.fo_cmp = jaildesc_cmp,
@@ -70,13 +69,13 @@ static struct fileops jaildesc_ops = {
 };
 
 /*
- * Given a jail descriptor number, return the jaildesc, its prison,
- * and its credential.  The jaildesc will be returned locked, and
- * prison and the credential will be returned held.
+ * Given a jail descriptor number, return its prison and/or its
+ * credential.  They are returned held, and will need to be released
+ * by the caller.
  */
 int
-jaildesc_find(struct thread *td, int fd, struct jaildesc **jdp,
-    struct prison **prp, struct ucred **ucredp)
+jaildesc_find(struct thread *td, int fd, struct prison **prp,
+    struct ucred **ucredp)
 {
 	struct file *fp;
 	struct jaildesc *jd;
@@ -98,12 +97,11 @@ jaildesc_find(struct thread *td, int fd, struct jaildesc **jdp,
 		JAILDESC_UNLOCK(jd);
 		goto out;
 	}
-	prison_hold(pr);
-	*prp = pr;
-	if (jdp != NULL)
-		*jdp = jd;
-	else
-		JAILDESC_UNLOCK(jd);
+	if (prp != NULL) {
+		prison_hold(pr);
+		*prp = pr;
+	}
+	JAILDESC_UNLOCK(jd);
 	if (ucredp != NULL)
 		*ucredp = crhold(fp->f_cred);
  out:
@@ -122,15 +120,12 @@ jaildesc_alloc(struct thread *td, struct file **fpp, int *fdp, int owning)
 	struct file *fp;
 	struct jaildesc *jd;
 	int error;
-	mode_t mode;
 
 	if (owning) {
 		error = priv_check(td, PRIV_JAIL_REMOVE);
 		if (error != 0)
 			return (error);
-		mode = S_ISTXT;
-	} else
-		mode = 0;
+	}
 	jd = malloc(sizeof(*jd), M_JAILDESC, M_WAITOK | M_ZERO);
 	error = falloc_caps(td, &fp, fdp, 0, NULL);
 	if (error != 0) {
@@ -140,11 +135,8 @@ jaildesc_alloc(struct thread *td, struct file **fpp, int *fdp, int owning)
 	finit(fp, priv_check_cred(fp->f_cred, PRIV_JAIL_SET) == 0 ?
 	    FREAD | FWRITE : FREAD, DTYPE_JAILDESC, jd, &jaildesc_ops);
 	JAILDESC_LOCK_INIT(jd);
-	jd->jd_uid = fp->f_cred->cr_uid;
-	jd->jd_gid = fp->f_cred->cr_gid;
-	jd->jd_mode = S_IFREG | S_IRUSR | S_IRGRP | S_IROTH | mode |
-	    (priv_check(td, PRIV_JAIL_SET) == 0 ? S_IWUSR | S_IXUSR : 0) |
-	    (priv_check(td, PRIV_JAIL_ATTACH) == 0 ? S_IXUSR : 0);
+	if (owning)
+		jd->jd_flags |= JDF_OWNING;
 	*fpp = fp;
 	return (0);
 }
@@ -206,7 +198,7 @@ jaildesc_close(struct file *fp, struct thread *td)
 			 */
 			prison_hold(pr);
 			JAILDESC_UNLOCK(jd);
-			if (jd->jd_mode & S_ISTXT) {
+			if (jd->jd_flags & JDF_OWNING) {
 				sx_xlock(&allprison_lock);
 				prison_lock(pr);
 				if (jd->jd_prison != NULL) {
@@ -246,74 +238,24 @@ jaildesc_stat(struct file *fp, struct stat *sb, struct ucred *active_cred)
 	jd = fp->f_data;
 	JAILDESC_LOCK(jd);
 	if (jd->jd_prison != NULL) {
-		sb->st_ino = jd->jd_prison ? jd->jd_prison->pr_id : 0;
-		sb->st_uid = jd->jd_uid;
-		sb->st_gid = jd->jd_gid;
-		sb->st_mode = jd->jd_mode;
+		sb->st_ino = jd->jd_prison->pr_id;
+		sb->st_mode = S_IFREG | S_IRWXU;
 	} else
 		sb->st_mode = S_IFREG;
 	JAILDESC_UNLOCK(jd);
 	return (0);
 }
 
-static int
-jaildesc_chmod(struct file *fp, mode_t mode, struct ucred *active_cred,
-    struct thread *td)
-{
-	struct jaildesc *jd;
-	int error;
-
-	/* Reject permissions that the creator doesn't have. */
-	if (((mode & (S_IWUSR | S_IWGRP | S_IWOTH)) &&
-	    priv_check_cred(fp->f_cred, PRIV_JAIL_SET) != 0) ||
-	    ((mode & (S_IXUSR | S_IXGRP | S_IXOTH)) &&
-	    priv_check_cred(fp->f_cred, PRIV_JAIL_ATTACH) != 0 &&
-	    priv_check_cred(fp->f_cred, PRIV_JAIL_SET) != 0) ||
-	    ((mode & S_ISTXT) &&
-	    priv_check_cred(fp->f_cred, PRIV_JAIL_REMOVE) != 0))
-		return (EPERM);
-	if (mode & (S_ISUID | S_ISGID))
-		return (EINVAL);
-	jd = fp->f_data;
-	JAILDESC_LOCK(jd);
-	error = vaccess(VREG, jd->jd_mode, jd->jd_uid, jd->jd_gid, VADMIN,
-		active_cred);
-	if (error == 0)
-		jd->jd_mode = S_IFREG | (mode & ALLPERMS);
-	JAILDESC_UNLOCK(jd);
-	return (error);
-}
-
-static int
-jaildesc_chown(struct file *fp, uid_t uid, gid_t gid, struct ucred *active_cred,
-    struct thread *td)
-{
-	struct jaildesc *jd;
-	int error;
-
-	error = 0;
-	jd = fp->f_data;
-	JAILDESC_LOCK(jd);
-	if (uid == (uid_t)-1)
-		uid = jd->jd_uid;
-	if (gid == (gid_t)-1)
-		gid = jd->jd_gid;
-	if ((uid != jd->jd_uid && uid != active_cred->cr_uid) ||
-	    (gid != jd->jd_gid && !groupmember(gid, active_cred)))
-		error = priv_check_cred(active_cred, PRIV_VFS_CHOWN);
-	if (error == 0) {
-		jd->jd_uid = uid;
-		jd->jd_gid = gid;
-	}
-	JAILDESC_UNLOCK(jd);
-	return (error);
-}
-
 static int
 jaildesc_fill_kinfo(struct file *fp, struct kinfo_file *kif,
     struct filedesc *fdp)
 {
-	return (EINVAL);
+	struct jaildesc *jd;
+
+	jd = fp->f_data;
+	kif->kf_type = KF_TYPE_JAILDESC;
+	kif->kf_un.kf_jail.kf_jid = jd->jd_prison ? jd->jd_prison->pr_id : 0;
+	return (0);
 }
 
 static int

sys/sys/jaildesc.h

@@ -54,9 +54,6 @@ struct jaildesc {
 	LIST_ENTRY(jaildesc) jd_list;	/* (d,p) this prison's descs */
 	struct prison	*jd_prison;	/* (d) the prison */
 	struct mtx	 jd_lock;
-	uid_t		 jd_uid;	/* (d) nominal file owner */
-	gid_t		 jd_gid;	/* (d) nominal file group */
-	mode_t		 jd_mode;	/* (d) descriptor permissions */
 	unsigned	 jd_flags;	/* (d) JDF_* flags */
 };
 
@@ -73,9 +70,10 @@ struct jaildesc {
  * Flags for the jd_flags field
  */
 #define	JDF_REMOVED	0x00000002	/* jail was removed */
+#define	JDF_OWNING	0x00000004	/* closing descriptor removes jail */
 
-int jaildesc_find(struct thread *td, int fd, struct jaildesc **jdp,
-    struct prison **prp, struct ucred **ucredp);
+int jaildesc_find(struct thread *td, int fd, struct prison **prp,
+    struct ucred **ucredp);
 int jaildesc_alloc(struct thread *td, struct file **fpp, int *fdp, int owning);
 void jaildesc_set_prison(struct file *jd, struct prison *pr);
 void jaildesc_prison_cleanup(struct prison *pr);