svn commit: r363017 - in stable/12: . bin/csh lib/libc/sys sys/kern
Kyle Evans
kevans at FreeBSD.org
Wed Jul 8 18:29:08 UTC 2020
Author: kevans
Date: Wed Jul 8 18:29:06 2020
New Revision: 363017
URL: https://svnweb.freebsd.org/changeset/base/363017
Log:
MFC r361798, r361800: vfs: default disallow read(2) of a directory
This MFC is in accordance with the original MFC plan outlined in the commit
message for r361798, appearing in full (with exception to metadata) below.
To summarize: this MFC only merges back the sysctl with a default disallow
policy, as in head, to ensure we hit any issues quickly but in a fashion
that end users can easily revert. Interested parties can flip the
security.bsd.allow_read_dir sysctl back to 1 to fully honor the previous
behavior of allowing read(2) of any dir, filesystem permitting.
r361798:
vfs: add restrictions to read(2) of a directory [1/2]
Historically, we've allowed read() of a directory and some filesystems will
accommodate (e.g. ufs/ffs, msdosfs). From the history department staffed by
Warner: <<EOF
pdp-7 unix seemed to allow reading directories, but they were weird, special
things there so I'm unsure (my pdp-7 assembler sucks).
1st Edition's sources are lost, mostly. The kernel allows it. The
reconstructed sources from 2nd or 3rd edition read it though.
V6 to V7 changed the filesystem format, and should have been a warning, but
reading directories weren't materially changed.
4.1b BSD introduced readdir because of UFS. UFS broke all directory reading
programs in 1983. ls, du, find, etc all had to be rewritten. readdir() and
friends were introduced here.
SysVr3 picked up readdir() in 1987 for the AT&T fork of Unix. SysVr4 updated
all the directory reading programs in 1988 because different filesystem
types were introduced.
In the 90s, these interfaces became completely ubiquitous as PDP-11s running
V7 faded from view and all the folks that initially started on V7 upgraded
to SysV. Linux never supported this (though I've not done the software
archeology to check) because it has always had a pathological diversity of
filesystems.
EOF
Disallowing read(2) on a directory has the side-effect of masking
application bugs from relying on other implementation's behavior
(e.g. Linux) of rejecting these with EISDIR across the board, but allowing
it has been a vector for at least one stack disclosure bug in the past[0].
By POSIX, this is implementation-defined whether read() handles directories
or not. Popular implementations have chosen to reject them, and this seems
sensible: the data you're reading from a directory is not structured in some
unified way across filesystem implementations like with readdir(2), so it is
impossible for applications to portably rely on this.
With this patch, we will reject most read(2) of a dirfd with EISDIR. Users
that know what they're doing can conscientiously set
bsd.security.allow_read_dir=1 to allow read(2) of directories, as it has
proven useful for debugging or recovery. A future commit will further limit
the sysctl to allow only the system root to read(2) directories, to make it
at least relatively safe to leave on for longer periods of time.
While we're adding logic pertaining to directory vnodes to vn_io_fault, an
additional assertion has also been added to ensure that we're not reaching
vn_io_fault with any write request on a directory vnode. Such request would
be a logical error in the kernel, and must be debugged rather than allowing
it to potentially silently error out.
Commented out shell aliases have been placed in root's chsrc/shrc to promote
awareness that grep may become noisy after this change, depending on your
usage.
A tentative MFC plan has been put together to try and make it as trivial as
possible to identify issues and collect reports; note that this will be
strongly re-evaluated. Tentatively, I will MFC this knob with the default as
it is in HEAD to improve our odds of actually getting reports. The future
priv(9) to further restrict the sysctl WILL NOT BE MERGED BACK, so the knob
will be a faithful reversion on stable/12. We will go into the merge
acknowledging that the sysctl default may be flipped back to restore
historical behavior at *any* point if it's warranted.
[0] https://www.freebsd.org/security/advisories/FreeBSD-SA-19:10.ufs.asc
r361800:
RELNOTES and UPDATING: Document the new policy on read(2) of dirfd
These changes have been completely flushed as of r361799; note it.
PR: 246412
Relnotes: yes 100%
Modified:
stable/12/UPDATING
stable/12/bin/csh/dot.cshrc
stable/12/lib/libc/sys/read.2
stable/12/sys/kern/vfs_vnops.c
Directory Properties:
stable/12/ (props changed)
Modified: stable/12/UPDATING
==============================================================================
--- stable/12/UPDATING Wed Jul 8 17:59:00 2020 (r363016)
+++ stable/12/UPDATING Wed Jul 8 18:29:06 2020 (r363017)
@@ -16,6 +16,16 @@ from older versions of FreeBSD, try WITHOUT_CLANG and
the tip of head, and then rebuild without this option. The bootstrap process
from older version of current across the gcc/clang cutover is a bit fragile.
+20200708:
+ read(2) of a directory fd is now rejected by default. root may
+ re-enable it for the entire system with the
+ security.bsd.allow_read_dir sysctl(8) MIB.
+
+ It may be advised to setup aliases for grep to default to `-d skip` if
+ commonly non-recursively grepping a list that includes directories and
+ the potential for the resulting stderr output is not tolerable. Example
+ aliases are now installed, commented out, in /root/.cshrc.
+
20200414:
Upstream DTS from Linux 5.6 was merged and they now have the SID
and THS (Secure ID controller and THermal Sensor) node present.
Modified: stable/12/bin/csh/dot.cshrc
==============================================================================
--- stable/12/bin/csh/dot.cshrc Wed Jul 8 17:59:00 2020 (r363016)
+++ stable/12/bin/csh/dot.cshrc Wed Jul 8 18:29:06 2020 (r363017)
@@ -12,6 +12,10 @@ alias la ls -aF
alias lf ls -FA
alias ll ls -lAF
+# read(2) of directories may not be desirable by default, as this will provoke
+# EISDIR errors from each directory encountered.
+# alias grep grep -d skip
+
# A righteous umask
umask 22
Modified: stable/12/lib/libc/sys/read.2
==============================================================================
--- stable/12/lib/libc/sys/read.2 Wed Jul 8 17:59:00 2020 (r363016)
+++ stable/12/lib/libc/sys/read.2 Wed Jul 8 18:29:06 2020 (r363017)
@@ -28,7 +28,7 @@
.\" @(#)read.2 8.4 (Berkeley) 2/26/94
.\" $FreeBSD$
.\"
-.Dd July 6, 2019
+.Dd July 8, 2020
.Dt READ 2
.Os
.Sh NAME
@@ -197,9 +197,14 @@ was negative.
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 residing
-on a file system that does not allow regular read operations on
-directories (e.g.\& NFS).
+The file descriptor is associated with a directory.
+Directories may only be read directly if the filesystem supports it and
+the
+.Dv security.bsd.allow_read_dir
+sysctl MIB is set to a non-zero value.
+For most scenarios, the
+.Xr readdir 3
+function should be used instead.
.It Bq Er EOPNOTSUPP
The file descriptor is associated with a file system and file type that
do not allow regular read operations on it.
Modified: stable/12/sys/kern/vfs_vnops.c
==============================================================================
--- stable/12/sys/kern/vfs_vnops.c Wed Jul 8 17:59:00 2020 (r363016)
+++ stable/12/sys/kern/vfs_vnops.c Wed Jul 8 18:29:06 2020 (r363017)
@@ -132,6 +132,11 @@ static u_long vn_io_faults_cnt;
SYSCTL_ULONG(_debug, OID_AUTO, vn_io_faults, CTLFLAG_RD,
&vn_io_faults_cnt, 0, "Count of vn_io_fault lock avoidance triggers");
+static int vfs_allow_read_dir = 0;
+SYSCTL_INT(_security_bsd, OID_AUTO, allow_read_dir, CTLFLAG_RW,
+ &vfs_allow_read_dir, 0,
+ "Enable read(2) of directory for filesystems that support it");
+
/*
* Returns true if vn_io_fault mode of handling the i/o request should
* be used.
@@ -1153,6 +1158,20 @@ vn_io_fault(struct file *fp, struct uio *uio, struct u
doio = uio->uio_rw == UIO_READ ? vn_read : vn_write;
vp = fp->f_vnode;
+
+ /*
+ * 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.
+ */
+ if (vp->v_type == VDIR) {
+ KASSERT(uio->uio_rw == UIO_READ,
+ ("illegal write attempted on a directory"));
+ if (!vfs_allow_read_dir)
+ return (EISDIR);
+ }
+
foffset_lock_uio(fp, uio, flags);
if (do_vn_io_fault(vp, uio)) {
args.kind = VN_IO_FAULT_FOP;
More information about the svn-src-all
mailing list