On Fri, Mar 08, 2024 at 08:02:13AM +0100, Arnd Bergmann wrote: > On Fri, Mar 8, 2024, at 00:09, Dave Chinner wrote: > > On Thu, Mar 07, 2024 at 03:40:44PM -0500, Paul Moore wrote: > >> On Thu, Mar 7, 2024 at 7:57 AM Günther Noack <gnoack@xxxxxxxxxx> wrote: > >> I need some more convincing as to why we need to introduce these new > >> hooks, or even the vfs_masked_device_ioctl() classifier as originally > >> proposed at the top of this thread. I believe I understand why > >> Landlock wants this, but I worry that we all might have different > >> definitions of a "safe" ioctl list, and encoding a definition into the > >> LSM hooks seems like a bad idea to me. > > > > I have no idea what a "safe" ioctl means here. Subsystems already > > restrict ioctls that can do damage if misused to CAP_SYS_ADMIN, so > > "safe" clearly means something different here. > > That was my problem with the first version as well, but I think > drawing the line between "implemented in fs/ioctl.c" and > "implemented in a random device driver fops->unlock_ioctl()" > seems like a more helpful definition. > > This won't just protect from calling into drivers that are lacking > a CAP_SYS_ADMIN check, but also from those that end up being > harmful regardless of the ioctl command code passed into them > because of stupid driver bugs. Indeed. "safe" is definitely not the right word, it is too broad, relative to use cases and threat models. There is no "safe" IOCTL. Let's replace "safe IOCTL" with "IOCTL always allowed in a Landlock sandbox". Our assumptions are (in the context of Landlock): 1. There are IOCTLs tied to file types (e.g. block device with major/minor) that can easily be identified from user space (e.g. with the path name and file's metadata). /dev/* files make sense for user space and they scope to a specific use case (with relative privileges). This category of IOCTLs is implemented in standalone device drivers (for most of them). 2. Most user space processes should not be denied access to IOCTLs that are managed by the VFS layer or the underlying filesystem implementations. For instance, the do_vfs_ioctl()'s ones (e.g. FIOCLEX, FIONREAD) should always be allowed because they may be required to legitimately use files, and for performance and security reasons (e.g. fs-crypt, fsverity implemented at the filesystem layer). Moreover, these IOCTLs should already check the read/write permission (on the related FD), which is not the case for most block/char device IOCTL. 3. IOCTLs to pipes and sockets are out of scope. They should always be allowed for now because they don't directly expose files' data but IPCs instead, and we are focusing on FS access rights for now. We want to add a new LANDLOCK_ACCESS_FS_IOCTL_DEV right that could match on char/block device's specific IOCTLs, but it would not have any impact on other IOCTLs which would then always be allowed (if the sandboxed process is allowed to open the file). Because IOCTLs are implemented in layers and all IOCTLs commands live in the same 32-bit namespace, we need a way to identify the layer implemented by block and character devices. The new LSM hook proposal enables us to cleanly and efficiently identify the char/block device IOCTL layer with an additional check on the file type.
