| Message ID | 20230823223630.GG11263@frogsfrogsfrogs (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC] xfsprogs: don't allow udisks to automount XFS filesystems with no prompt | expand |
On Wed, Aug 23, 2023 at 03:36:30PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > The unending stream of syzbot bug reports and overwrought filing of CVEs > for corner case handling (i.e. things that distract from actual user > complaints) in XFS has generated all sorts of of overheated rhetoric > about how every bug is a Serious Security Issue(tm) because anyone can > craft a malicious filesystem on a USB stick, insert the stick into a > victim machine, and mount will trigger a bug in the kernel driver that > leads to some compromise or DoS or something. > > I thought that nobody would be foolish enough to automount an XFS > filesystem. What a fool I was! It turns out that udisks can be told > that it's okay to automount things, and then it will. Including mangled > XFS filesystems! *nod* > <delete angry rant about poor decisionmaking and armchair fs developers > blasting us on X while not actually doing any of the work> If only I had a dollar for every time I've deleted a similar rant... > Turn off /this/ idiocy by adding a udev rule to tell udisks not to > automount XFS filesystems. > > This will not stop a logged in user from unwittingly inserting a > malicious storage device and pressing [mount] and getting breached. > This is not a substitute for a thorough audit. This does not solve the > general problem of in-kernel fs drivers being a huge attack surface. > I just want a vacation from the shitstorm of bad ideas and threat > models that I never agreed to support. Yup, this seems like a right thing to do given the lack of action from the userspace side of the fence. [ The argument that "prompting the user to ask if they trust the device teaches them to ignore security prompts" is just stupid. We have persistent identifiers in filesystems - keep a database of known trusted identifiers and only prompt for "is this a trusted device" when an unknown device is inserted. Other desktop OS's have been doing this for years.... ] > [Does this actually stop udisks? I turned off all automounting at the > DE level years ago because I'm not that stupid.] Yeah, I turned off all the DE level automount stuff years ago, too. It's the first thing I do when setting up a new machine for anyone, too. ..... > diff --git a/scrub/64-xfs.rules b/scrub/64-xfs.rules > new file mode 100644 > index 00000000000..39f17850097 > --- /dev/null > +++ b/scrub/64-xfs.rules > @@ -0,0 +1,10 @@ > +# SPDX-License-Identifier: GPL-2.0-or-later > +# > +# Copyright (C) 2023 Oracle. All rights reserved. > +# > +# Author: Darrick J. Wong <djwong@kernel.org> > +# > +# Don't let udisks automount XFS filesystems without even asking a user. > +# This doesn't eliminate filesystems as an attack surface; it only prevents > +# evil maid attacks when all sessions are locked. > +SUBSYSTEM=="block", ENV{ID_FS_TYPE}=="xfs", ENV{UDISKS_AUTO}="0" I think this is correct, but the lack of documentation on how udev rules and overrides are supposed to work doesn't help me one bit. Ok, just tracked it through - only gvfs and clevis actually look at udisks_block_get_hint_auto() from udisks, so at least the gnome and lxde/lxqt desktop environments will no longer automount XFS filesystems. Who knows what magic is needed for other DEs, but this is a good start. Reviewed-by: Dave Chinner <dchinner@redhat.com> -Dave.
On Thu, Aug 24, 2023 at 09:49:41AM +1000, Dave Chinner wrote: > On Wed, Aug 23, 2023 at 03:36:30PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > The unending stream of syzbot bug reports and overwrought filing of CVEs > > for corner case handling (i.e. things that distract from actual user > > complaints) in XFS has generated all sorts of of overheated rhetoric > > about how every bug is a Serious Security Issue(tm) because anyone can > > craft a malicious filesystem on a USB stick, insert the stick into a > > victim machine, and mount will trigger a bug in the kernel driver that > > leads to some compromise or DoS or something. > > > > I thought that nobody would be foolish enough to automount an XFS > > filesystem. What a fool I was! It turns out that udisks can be told > > that it's okay to automount things, and then it will. Including mangled > > XFS filesystems! > > *nod* > > > <delete angry rant about poor decisionmaking and armchair fs developers > > blasting us on X while not actually doing any of the work> > > If only I had a dollar for every time I've deleted a similar rant... I do, and I'm raking in the Benjamins! https://www.youtube.com/watch?v=qpMvS1Q1sos > > Turn off /this/ idiocy by adding a udev rule to tell udisks not to > > automount XFS filesystems. > > > > This will not stop a logged in user from unwittingly inserting a > > malicious storage device and pressing [mount] and getting breached. > > This is not a substitute for a thorough audit. This does not solve the > > general problem of in-kernel fs drivers being a huge attack surface. > > I just want a vacation from the shitstorm of bad ideas and threat > > models that I never agreed to support. > > Yup, this seems like a right thing to do given the lack of action > from the userspace side of the fence. > > [ The argument that "prompting the user to ask if they trust the > device teaches them to ignore security prompts" is just stupid. We > have persistent identifiers in filesystems - keep a database of > known trusted identifiers and only prompt for "is this a trusted > device" when an unknown device is inserted. Other desktop OS's have > been doing this for years.... ] Not to mention that having a prompt at least stops the evil maid from waking up your laptop, plugging in a usb stick, and being able to mount crap in your system without even having to unlock it. But. I'm preaching to the choir here. > > [Does this actually stop udisks? I turned off all automounting at the > > DE level years ago because I'm not that stupid.] > > Yeah, I turned off all the DE level automount stuff years ago, too. > It's the first thing I do when setting up a new machine for anyone, > too. > > ..... > > > diff --git a/scrub/64-xfs.rules b/scrub/64-xfs.rules > > new file mode 100644 > > index 00000000000..39f17850097 > > --- /dev/null > > +++ b/scrub/64-xfs.rules > > @@ -0,0 +1,10 @@ > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +# > > +# Copyright (C) 2023 Oracle. All rights reserved. > > +# > > +# Author: Darrick J. Wong <djwong@kernel.org> > > +# > > +# Don't let udisks automount XFS filesystems without even asking a user. > > +# This doesn't eliminate filesystems as an attack surface; it only prevents > > +# evil maid attacks when all sessions are locked. > > +SUBSYSTEM=="block", ENV{ID_FS_TYPE}=="xfs", ENV{UDISKS_AUTO}="0" > > I think this is correct, but the lack of documentation on how > udev rules and overrides are supposed to work doesn't help > me one bit. > > Ok, just tracked it through - only gvfs and clevis actually look at > udisks_block_get_hint_auto() from udisks, so at least the gnome and > lxde/lxqt desktop environments will no longer automount XFS > filesystems. Who knows what magic is needed for other DEs, but this > is a good start. Hah, we could encode that too? kde5: [General] AutomountEnabled=false AutomountOnPlugin=false AutomountOnLogin=false AutomountUnknownDevices=false gnome3: [org.gnome.desktop.media-handling] automount=false automount-open=false autorun-never=true gnome2: /apps/nautilus/preferences/media_automount false /apps/nautilus/preferences/media_autorun_never true /apps/nautilus/preferences/media_autorun_never true /apps/nautilus/preferences/media_autorun_x_content_open_folder [] > > Reviewed-by: Dave Chinner <dchinner@redhat.com> Thanks! --D > -Dave. > -- > Dave Chinner > david@fromorbit.com
diff --git a/configure.ac b/configure.ac index 58f3b8e2e90..e447121a344 100644 --- a/configure.ac +++ b/configure.ac @@ -209,6 +209,7 @@ AC_HAVE_SG_IO AC_HAVE_HDIO_GETGEO AC_CONFIG_SYSTEMD_SYSTEM_UNIT_DIR AC_CONFIG_CROND_DIR +AC_CONFIG_UDEV_RULE_DIR if test "$enable_blkid" = yes; then AC_HAVE_BLKID_TOPO diff --git a/include/builddefs.in b/include/builddefs.in index fb8e239cab2..3318e00316c 100644 --- a/include/builddefs.in +++ b/include/builddefs.in @@ -184,6 +184,8 @@ HAVE_SYSTEMD = @have_systemd@ SYSTEMD_SYSTEM_UNIT_DIR = @systemd_system_unit_dir@ HAVE_CROND = @have_crond@ CROND_DIR = @crond_dir@ +HAVE_UDEV = @have_udev@ +UDEV_RULE_DIR = @udev_rule_dir@ HAVE_LIBURCU_ATOMIC64 = @have_liburcu_atomic64@ HAVE_MEMFD_CLOEXEC = @have_memfd_cloexec@ HAVE_MEMFD_NOEXEC_SEAL = @have_memfd_noexec_seal@ diff --git a/m4/package_services.m4 b/m4/package_services.m4 index f2d888a099a..a683ddb93e0 100644 --- a/m4/package_services.m4 +++ b/m4/package_services.m4 @@ -75,3 +75,45 @@ AC_DEFUN([AC_CONFIG_CROND_DIR], AC_SUBST(have_crond) AC_SUBST(crond_dir) ]) + +# +# Figure out where to put udev rule files +# +AC_DEFUN([AC_CONFIG_UDEV_RULE_DIR], +[ + AC_REQUIRE([PKG_PROG_PKG_CONFIG]) + AC_ARG_WITH([udev_rule_dir], + [AS_HELP_STRING([--with-udev-rule-dir@<:@=DIR@:>@], + [Install udev rules into DIR.])], + [], + [with_udev_rule_dir=yes]) + AS_IF([test "x${with_udev_rule_dir}" != "xno"], + [ + AS_IF([test "x${with_udev_rule_dir}" = "xyes"], + [ + PKG_CHECK_MODULES([udev], [udev], + [ + with_udev_rule_dir="$($PKG_CONFIG --variable=udev_dir udev)/rules.d" + ], [ + with_udev_rule_dir="" + ]) + m4_pattern_allow([^PKG_(MAJOR|MINOR|BUILD|REVISION)$]) + ]) + AC_MSG_CHECKING([for udev rule dir]) + udev_rule_dir="${with_udev_rule_dir}" + AS_IF([test -n "${udev_rule_dir}"], + [ + AC_MSG_RESULT(${udev_rule_dir}) + have_udev="yes" + ], + [ + AC_MSG_RESULT(no) + have_udev="no" + ]) + ], + [ + have_udev="disabled" + ]) + AC_SUBST(have_udev) + AC_SUBST(udev_rule_dir) +]) diff --git a/scrub/64-xfs.rules b/scrub/64-xfs.rules new file mode 100644 index 00000000000..39f17850097 --- /dev/null +++ b/scrub/64-xfs.rules @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +# +# Copyright (C) 2023 Oracle. All rights reserved. +# +# Author: Darrick J. Wong <djwong@kernel.org> +# +# Don't let udisks automount XFS filesystems without even asking a user. +# This doesn't eliminate filesystems as an attack surface; it only prevents +# evil maid attacks when all sessions are locked. +SUBSYSTEM=="block", ENV{ID_FS_TYPE}=="xfs", ENV{UDISKS_AUTO}="0" diff --git a/scrub/Makefile b/scrub/Makefile index ab9c2d14832..74193ed270b 100644 --- a/scrub/Makefile +++ b/scrub/Makefile @@ -41,6 +41,11 @@ endif endif # scrub_prereqs +UDEV_RULES = 64-xfs.rules +ifeq ($(HAVE_UDEV),yes) + INSTALL_SCRUB += install-udev +endif + HFILES = \ common.h \ counter.h \ @@ -180,6 +185,10 @@ install-scrub: default $(INSTALL) -m 755 $(XFS_SCRUB_ALL_PROG) $(PKG_SBIN_DIR) $(INSTALL) -m 755 -d $(PKG_STATE_DIR) +install-udev: $(UDEV_RULES) + $(INSTALL) -m 755 -d $(UDEV_RULE_DIR) + $(INSTALL) -m 644 $(UDEV_RULES) $(UDEV_RULE_DIR) + install-dev: -include .dep