From 63619b6dba17f90514355706cea0f825d131d5e4 Mon Sep 17 00:00:00 2001
From: Kyle Evans <kevans@FreeBSD.org>
Date: Thu, 4 Jun 2020 18:17:25 +0000
Subject: [PATCH] vfs: add restrictions to read(2) of a directory [2/2]

This commit adds the priv(9) that waters down the sysctl to make it only
allow read(2) of a dirfd by the system root. Jailed root is not allowed, but
jail policy and superuser policy will abstain from allowing/denying it so
that a MAC module can fully control the policy.

Such a MAC module has been written, and can be found at:
https://people.freebsd.org/~kevans/mac_read_dir-0.1.0.tar.gz

It is expected that the MAC module won't be needed by many, as most only
need to do such diagnostics that require this behavior as system root
anyways. Interested parties are welcome to grab the MAC module above and
create a port or locally integrate it, and with enough support it could see
introduction to base. As noted in mac_read_dir.c, it is released under the
BSD 2 clause license and allows the restrictions to be lifted for only
jailed root or for all unprivileged users.

PR:		246412
Reviewed by:	mckusick, kib, emaste, jilles, cy, phk, imp (all previous)
Reviewed by:	rgrimes (latest version)
Differential Revision:	https://reviews.freebsd.org/D24596
---
 lib/libc/sys/read.2  | 2 +-
 sys/kern/kern_jail.c | 8 ++++++++
 sys/kern/kern_priv.c | 8 ++++++++
 sys/kern/vfs_vnops.c | 6 +++++-
 sys/sys/priv.h       | 1 +
 5 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/lib/libc/sys/read.2 b/lib/libc/sys/read.2
index ad08c5a448e8..8b58debd3fbb 100644
--- a/lib/libc/sys/read.2
+++ b/lib/libc/sys/read.2
@@ -200,7 +200,7 @@ The file was marked for non-blocking I/O,
 and no data were ready to be read.
 .It Bq Er EISDIR
 The file descriptor is associated with a directory.
-Directories may only be read directly if the filesystem supports it and
+Directories may only be read directly by root if the filesystem supports it and
 the
 .Dv security.bsd.allow_read_dir
 sysctl MIB is set to a non-zero value.
diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c
index b71bcd7dd1ba..e09a2b66c64d 100644
--- a/sys/kern/kern_jail.c
+++ b/sys/kern/kern_jail.c
@@ -3323,6 +3323,14 @@ prison_priv_check(struct ucred *cred, int priv)
 		else
 			return (EPERM);
 
+		/*
+		 * Jails should hold no disposition on the PRIV_VFS_READ_DIR
+		 * policy.  priv_check_cred will not specifically allow it, and
+		 * we may want a MAC policy to allow it.
+		 */
+	case PRIV_VFS_READ_DIR:
+		return (0);
+
 		/*
 		 * Conditionnaly allow locking (unlocking) physical pages
 		 * in memory.
diff --git a/sys/kern/kern_priv.c b/sys/kern/kern_priv.c
index 58ce6cf342a5..a912336cbbca 100644
--- a/sys/kern/kern_priv.c
+++ b/sys/kern/kern_priv.c
@@ -194,6 +194,14 @@ priv_check_cred(struct ucred *cred, int priv)
 				goto out;
 			}
 			break;
+		case PRIV_VFS_READ_DIR:
+			/*
+			 * Allow PRIV_VFS_READ_DIR for root if we're not in a
+			 * jail, otherwise deny unless a MAC policy grants it.
+			 */
+			if (jailed(cred))
+				break;
+			/* FALLTHROUGH */
 		default:
 			if (cred->cr_uid == 0) {
 				error = 0;
diff --git a/sys/kern/vfs_vnops.c b/sys/kern/vfs_vnops.c
index 84c34501de4b..95a8e76af6c1 100644
--- a/sys/kern/vfs_vnops.c
+++ b/sys/kern/vfs_vnops.c
@@ -1226,13 +1226,17 @@ vn_io_fault(struct file *fp, struct uio *uio, struct ucred *active_cred,
 	 * The ability to read(2) on a directory has historically been
 	 * allowed for all users, but this can and has been the source of
 	 * at least one security issue in the past.  As such, it is now hidden
-	 * away behind a sysctl for those that actually need it to use it.
+	 * away behind a sysctl for those that actually need it to use it, and
+	 * restricted to root when it's turned on to make it relatively safe to
+	 * leave on for longer sessions of need.
 	 */
 	if (vp->v_type == VDIR) {
 		KASSERT(uio->uio_rw == UIO_READ,
 		    ("illegal write attempted on a directory"));
 		if (!vfs_allow_read_dir)
 			return (EISDIR);
+		if ((error = priv_check(td, PRIV_VFS_READ_DIR)) != 0)
+			return (EISDIR);
 	}
 
 	foffset_lock_uio(fp, uio, flags);
diff --git a/sys/sys/priv.h b/sys/sys/priv.h
index 7529231e3e2b..26c4e85f83b8 100644
--- a/sys/sys/priv.h
+++ b/sys/sys/priv.h
@@ -283,6 +283,7 @@
 #define	PRIV_VFS_SYSFLAGS	342	/* Can modify system flags. */
 #define	PRIV_VFS_UNMOUNT	343	/* Can unmount(). */
 #define	PRIV_VFS_STAT		344	/* Override vnode MAC stat perm. */
+#define	PRIV_VFS_READ_DIR	345	/* Can read(2) a dirfd, needs sysctl. */
 
 /*
  * Virtual memory privileges.